Hi Laurent,
On Wed, 2022-09-28 at 03:58 +0300, Laurent Pinchart wrote:
> From: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
>
> The LCDIF includes a color space converter that supports YUV input. Use
> it to support YUV planes, either through the converter if the output
> format is RGB, or in conversion bypass mode otherwise.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> Changes since v1:
>
> - Support all YCbCr encodings and quantization ranges
> - Drop incorrect comment
> ---
> drivers/gpu/drm/mxsfb/lcdif_kms.c | 183 +++++++++++++++++++++++++----
> drivers/gpu/drm/mxsfb/lcdif_regs.h | 5 +-
> 2 files changed, 164 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> index c3622be0c587..b469a90fd50f 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> @@ -15,6 +15,7 @@
> #include <drm/drm_atomic.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_bridge.h>
> +#include <drm/drm_color_mgmt.h>
> #include <drm/drm_crtc.h>
> #include <drm/drm_encoder.h>
> #include <drm/drm_framebuffer.h>
> @@ -32,13 +33,77 @@
> /* -----------------------------------------------------------------------------
> * CRTC
> */
> +
> +/*
> + * Despite the reference manual stating the opposite, the D1, D2 and D3 offset
> + * values are added to Y, U and V, not subtracted. They must thus be programmed
> + * with negative values.
> + */
> +static const u32 lcdif_yuv2rgb_coeffs[3][2][6] = {
> + [DRM_COLOR_YCBCR_BT601] = {
> + [DRM_COLOR_YCBCR_LIMITED_RANGE] = {
Can you add conversion equations as comments in code for each encoding
and range, like imx-pxp.c does? Also for RGB to YCbCr conversion.
> + CSC0_COEF0_A1(0x012a) | CSC0_COEF0_A2(0x0000),
Looks like hex with 3 numbers is enough for each coefficient. Use
'0x12a' and '0x000'?
> + CSC0_COEF1_A3(0x01a2) | CSC0_COEF1_B1(0x0123),
> + CSC0_COEF2_B2(0x079c) | CSC0_COEF2_B3(0x0730),
> + CSC0_COEF3_C1(0x0124) | CSC0_COEF3_C2(0x0204),
> + CSC0_COEF4_C3(0x0000) | CSC0_COEF4_D1(0x01f0),
Shouldn't D1 be 0x110 since it's -16 and you set D2/D3 to 0x180 as they
are -128? The same for D1s with other encodings and limited ranges.
I didn't check other coefficients closely though.
[...]
> @@ -456,6 +574,12 @@ static const u32 lcdif_primary_plane_formats[] = {
> DRM_FORMAT_XRGB1555,
> DRM_FORMAT_XRGB4444,
> DRM_FORMAT_XRGB8888,
> +
> + /* packed YCbCr */
Nitpick: Add a similar comment for above RGB pixel formats?
Regards,
Liu Ying
> + DRM_FORMAT_YUYV,
> + DRM_FORMAT_YVYU,
> + DRM_FORMAT_UYVY,
> + DRM_FORMAT_VYUY,
> };
