Messages in this thread Patch in this message | | | From | CK Hu (胡俊光) <> | Subject | Re: [PATCH v6 13/14] drm/mediatek: Support CRC in OVL | Date | Thu, 28 Mar 2024 05:21:32 +0000 |
| |
Hi, Shawn:
On Thu, 2024-03-28 at 03:22 +0000, Shawn Sung (宋孝謙) wrote: > 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.
The input I mean is the input of Y2R, the pixel value. If input a pixel value 0x100 into Y2R, then Y2R output pixel value 0xfc, why do this? I think it should not be minus 1. If remove the minus 1, then input 0x100 and output 0x100.
> > > > + > > > + /* > > > + * 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.
I think this truncation is not necessary. No matter you truncate in Y2R, OVL would finally truncate pixel value last 2 two bit when calculate CRC. So this truncation is not necessary.
Regards, CK
> > Thanks, > Shawn >
| |