Re: [PATCH v6 13/14] drm/mediatek: Support CRC in OVL

[Shawn Sung (宋孝謙)]

Hi CK,

On Tue, 2024-03-26 at 06:11 +0000, CK Hu (胡俊光) wrote:
> > @@ -488,6 +567,83 @@ void mtk_ovl_layer_config(struct device *dev,
> > unsigned int idx,
> >  	    (state->base.fb && !state->base.fb->format->has_alpha))
> >  		ignore_pixel_alpha = OVL_CONST_BLEND;
> >  
> > +	/*
> > +	 * OVL only supports 8 bits data in CRC calculation, transform
> > 10-bit
> > +	 * RGB to 8-bit RGB by leveraging the ability of the Y2R (YUV-
> > to-RGB)
> > +	 * hardware to multiply coefficients, although there is nothing
> > to do
> > +	 * with the YUV format.
> > +	 */
> > +	if (ovl->data->supports_clrfmt_ext) {
> > +		u32 y2r_coef = 0, y2r_offset = 0, r2r_coef = 0, csc_en
> > = 0;
> > +
> > +		if (is_10bit_rgb(fmt)) {
> > +			con |= OVL_CON_MTX_AUTO_DIS | OVL_CON_MTX_EN |
> > OVL_CON_MTX_PROGRAMMABLE;
> > +
> > +			/*
> > +			 * Y2R coefficient setting
> > +			 * bit 13 is 2^1, bit 12 is 2^0, bit 11 is 2^-
> > 1,
> > +			 * bit 10 is 2^-2 = 0.25
> > +			 */
> > +			y2r_coef = BIT(10);
> > +
> > +			/* -1 in 10bit */
> > +			y2r_offset = GENMASK(10, 0) - 1;
> 
> I don't know why do this? If an input value is 0x100, then
> 
> 0x100 right shit 2 bit become 0x40.
> 0x40 - 1 = 0x3f.
> 0x3f left shift 2 bit become 0xfc.
> 
> So input 0x100 and output 0xfc. Why?
> 

There is no input here, all the settings are direct bit assignment, and
all the values are calculated by the designer. The main purpose of it
is to configure the Y2R module to be able to transform 10bit RGB format
into 8bit RGB, while this is not Y2R module is originally designed for.

> > +
> > +			/*
> > +			 * R2R coefficient setting
> > +			 * bit 19 is 2^1, bit 18 is 2^0, bit 17 is 2^-
> > 1,
> > +			 * bit 20 is 2^2 = 4
> > +			 */
> > +			r2r_coef = BIT(20);
> > +
> > +			/* CSC_EN is for R2R */
> > +			csc_en = OVL_CLRFMT_EXT1_CSC_EN(idx);
> > +
> > +			/*
> > +			 * 1. YUV input data - 1 and shift right for 2
> > bits to remove it
> > +			 * [R']   [0.25    0    0]   [Y in - 1]
> > +			 * [G'] = [   0 0.25    0] * [U in - 1]
> > +			 * [B']   [   0    0 0.25]   [V in - 1]
> > +			 *
> > +			 * 2. shift left for 2 bit letting the last 2
> > bits become 0
> 
> You truncate the last two bit, so some quality lost. I think the
> quality is main function and CRC is just for debug. So it's better
> that
> in normal case we keep quality and only for debug to lost the
> quality.

Got it. Will modify in the next version to enter this section only if
we need to calculate the CRC.

> I have another question. You just truncate the last two bit but it is
> still 10 bit value, so CRC could calculate this 10 bit value? I don't
> know why you say CRC just for 8 bit?
> 

Yes, for RGB format, OVL can only handle 8bit per channel for CRC
calculation, so I assume there may be similar issue handling 10bit YUV
formats (P010) in the future.

Thanks,
Shawn