On Wed, Sep 28, 2022 at 02:37:04AM +0200, Marek Vasut wrote: > On 9/28/22 02:21, Laurent Pinchart wrote: > > Hi, > > [...] > > >>> - /* CSC: BT.601 Full Range RGB to YCbCr coefficients. */ > >>> - writel(CSC0_COEF0_A2(0x096) | CSC0_COEF0_A1(0x04c), > >>> + /* CSC: BT.601 Limited Range RGB to YCbCr coefficients. */ > >>> + writel(CSC0_COEF0_A2(0x081) | CSC0_COEF0_A1(0x042), > >>> lcdif->base + LCDC_V8_CSC0_COEF0); > >>> - writel(CSC0_COEF1_B1(0x7d5) | CSC0_COEF1_A3(0x01d), > >>> + writel(CSC0_COEF1_B1(0x7da) | CSC0_COEF1_A3(0x019), > >>> lcdif->base + LCDC_V8_CSC0_COEF1); > >>> - writel(CSC0_COEF2_B3(0x080) | CSC0_COEF2_B2(0x7ac), > >>> + writel(CSC0_COEF2_B3(0x070) | CSC0_COEF2_B2(0x7b6), > >>> lcdif->base + LCDC_V8_CSC0_COEF2); > >>> - writel(CSC0_COEF3_C2(0x795) | CSC0_COEF3_C1(0x080), > >>> + writel(CSC0_COEF3_C2(0x7a2) | CSC0_COEF3_C1(0x070), > >>> lcdif->base + LCDC_V8_CSC0_COEF3); > >>> - writel(CSC0_COEF4_D1(0x000) | CSC0_COEF4_C3(0x7ec), > >>> + writel(CSC0_COEF4_D1(0x010) | CSC0_COEF4_C3(0x7ee), > >>> lcdif->base + LCDC_V8_CSC0_COEF4); > >>> writel(CSC0_COEF5_D3(0x080) | CSC0_COEF5_D2(0x080), > >>> lcdif->base + LCDC_V8_CSC0_COEF5); > >> > >> Would it make sense to use the same coeffs as csc2_coef_bt601_lim in > >> drivers/media/platform/nxp/imx-pxp.c , since the block is most likely > >> identical ? > > > > The coefficients in this patch have been computed to distribute the > > error in such a way that the sum of all lines stays the same. This > > avoids biases and overflow, but it likely makes very little difference > > in practice. I'm thus fine with the coefficients from imx-pxp.c. > > Would it then make sense to update the coeffs in the pxp driver instead? > > Either option works for me. It could, but I won't be able to easily test it. As the hardware clamps the calculated value, there's no risk of wraparound, and the difference in the +/-1 error distribution will not be noticeable, so I'll just copy the coefficients from imx-pxp.c to ensure coherency. -- Regards, Laurent Pinchart
