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@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>
> > Reviewed-by: Marek Vasut <marex@xxxxxxx>
> > Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> > ---
> > 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
