[PATCH v3 4/4] drm: lcdif: Add support for YUV planes

[Laurent Pinchart]

Hi Liu,

On Fri, Sep 30, 2022 at 12:01:39PM +0800, Liu Ying wrote:
> On Thu, 2022-09-29 at 23:42 +0300, Laurent Pinchart wrote:
> > From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> > 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 at ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Reviewed-by: Marek Vasut <marex at denx.de>
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> > Changes since v2:
> > 
> > - Fix YUV to RGB coefficients
> > - List floating point coefficient values in comment
> > - Express CSC coefficients with three hex digits
> > - Move | to the end of the line
> > - Add comment header before RGB formats
> > 
> > Changes since v1:
> > 
> > - Support all YCbCr encodings and quantization ranges
> > - Drop incorrect comment
> > ---
> >  drivers/gpu/drm/mxsfb/lcdif_kms.c  | 260 ++++++++++++++++++++++++-----
> >  drivers/gpu/drm/mxsfb/lcdif_regs.h |   5 +-
> >  2 files changed, 224 insertions(+), 41 deletions(-)
> 
> Hmmm, 'git am' fails to apply this one to drm-misc-next but it's ok
> with v6.0-rc1.  I should have tried drm-misc-next with v2.

Oops. I'll rebase on top of drm-misc-next.

> > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> > index 08e4880ec6cf..81454b53936f 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,119 @@
> >  /* -----------------------------------------------------------------------------
> >   * 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.
> 
> Can you add the generic conversion equations with [A-D][1-3]
> coefficient symbols as comments in code, like imx-pxp.c does?  It will
> improve code readability better and tell which coefficient is which.
> Also for RGB to YCbCr conversion if you want to do that with this
> series.  Sorry for not pointing this out in v2 review cycle.

I'll fix that.

> > + */
> > +static const u32 lcdif_yuv2rgb_coeffs[3][2][6] = {
> 
> [...]
> 
> > +	/* YUYV Formats */
> 
> 'YUV Formats' would be better?  Again, failed to catch this in v2...

Will fix this too.

> > +	case DRM_FORMAT_YUYV:
> > +		writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_VY2UY1,
> > +		       lcdif->base + LCDC_V8_CTRLDESCL0_5);
> > +		in_yuv = true;
> > +		break;
> > +	case DRM_FORMAT_YVYU:
> > +		writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_UY2VY1,
> > +		       lcdif->base + LCDC_V8_CTRLDESCL0_5);
> > +		in_yuv = true;
> > +		break;
> > +	case DRM_FORMAT_UYVY:
> > +		writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_Y2VY1U,
> > +		       lcdif->base + LCDC_V8_CTRLDESCL0_5);
> > +		in_yuv = true;
> > +		break;
> > +	case DRM_FORMAT_VYUY:
> > +		writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_Y2UY1V,
> > +		       lcdif->base + LCDC_V8_CTRLDESCL0_5);
> > +		in_yuv = true;
> > +		break;
> > +
> > +	default:
> > +		dev_err(drm->dev, "Unknown pixel format 0x%x\n", format);
> > +		break;
> > +	}

-- 
Regards,

Laurent Pinchart