Re: [PATCH 10/11] drm/mediatek: ovl: Enable support for R2R Color Space Conversion

["Nícolas F. R. A. Prado" <nfraprado@collabora.com>]

On Tue, 2026-03-03 at 07:16 +0000, CK Hu (胡俊光) wrote:
> On Tue, 2025-12-23 at 16:44 -0300, Nícolas F. R. A. Prado wrote:
> > External email : Please do not click links or open attachments
> > until you have verified the sender or the content.
> > 
> > 
> > The OVL hardware allows applying a 3x3 matrix transformation for
> > each
> > layer through the 'RGB to RGB Color Space Conversion' (R2R CSC)
> > setting.
> > Implement support for it and expose it as a colorop through the DRM
> > plane color pipeline uAPI.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 55
> > ++++++++++++++++++++++++++++++++-
> >  1 file changed, 54 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index a70092c792a9..c8a2b1b13035 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -4,6 +4,7 @@
> >   */
> > 
> >  #include <drm/drm_blend.h>
> > +#include <drm/drm_color_mgmt.h>
> >  #include <drm/drm_colorop.h>
> >  #include <drm/drm_fourcc.h>
> >  #include <drm/drm_framebuffer.h>
> > @@ -52,6 +53,7 @@
> >  #define OVL_CON_CLRFMT_10_BIT                          (1)
> >  #define DISP_REG_OVL_WCG_CFG1                  0x2d8
> >  #define IGAMMA_EN(layer)                               BIT(0 + 4 *
> > (layer))
> > +#define CSC_EN(layer)                                  BIT(1 + 4 *
> > (layer))
> >  #define GAMMA_EN(layer)                                       
> > BIT(2 + 4 * (layer))
> >  #define DISP_REG_OVL_WCG_CFG2                  0x2dc
> >  #define IGAMMA_MASK(layer)                            
> > GENMASK((layer) * 4 + 1, (layer) * 4)
> > @@ -62,6 +64,7 @@
> >  #define GAMMA_BT709                                    1
> >  #define GAMMA_BT2020                                   2
> >  #define GAMMA_HLG                                      3
> > +#define DISP_REG_OVL_R2R_PARA(layer)           (0x500 + (layer) *
> > 0x40)
> >  #define DISP_REG_OVL_ADDR_MT8173               0x0f40
> >  #define DISP_REG_OVL_ADDR(ovl, n)              ((ovl)->data->addr
> > + 0x20 * (n))
> >  #define DISP_REG_OVL_HDR_ADDR(ovl, n)          ((ovl)->data->addr
> > + 0x20 * (n) + 0x04)
> > @@ -579,11 +582,44 @@ static void mtk_ovl_apply_igamma(struct
> > mtk_disp_ovl *ovl, unsigned int idx,
> >                            IGAMMA_EN(idx));
> >  }
> > 
> > +static void mtk_ovl_write_r2r_para(struct mtk_disp_ovl *ovl,
> > unsigned int idx,
> > +                                  struct drm_color_ctm *ctm,
> > +                                  struct cmdq_pkt *cmdq_pkt)
> > +{
> > +       unsigned int i;
> > +       u64 val;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(ctm->matrix); i++) {
> > +               val = drm_color_ctm_s31_32_to_qm_n(ctm->matrix[i],
> > 5, 18);
> > +               mtk_ddp_write(cmdq_pkt, val, &ovl->cmdq_reg, ovl-
> > >regs,
> > +                             DISP_REG_OVL_R2R_PARA(idx) + i * 4);
> > +       }
> > +}
> > +
> > +static void mtk_ovl_apply_r2r_csc(struct mtk_disp_ovl *ovl,
> > unsigned int idx,
> > +                                 struct drm_colorop *colorop,
> > +                                 struct cmdq_pkt *cmdq_pkt)
> > +{
> > +       struct drm_color_ctm *ctm;
> > +
> > +       if (colorop->state->data && colorop->state->data->data) {
> > +               ctm = (struct drm_color_ctm *)colorop->state->data-
> > >data;
> > +               mtk_ovl_write_r2r_para(ovl, idx, ctm, cmdq_pkt);
> > +       }
> > +
> 
> If bypass imply colorop->state->data->data is NULL, nothing need to
> change.
> If bypass does not imply colorop->state->data->data is NULL, I would
> like to disable CSC first then clear r2r parameter.

bypass does not imply NULL data. But even when setting the CSC block to
be bypassed, we don't want to clear its parameters, we want to set the
parameters passed by userspace in this atomic commit, if any, and if
not we want to keep the current parameters, in case they have been set
in a previous atomic commit, so that a future commit setting bypass=0
will have the parameters previously set, without having to pass them
again.

Thanks,
Nícolas