Hi Laurent, On 21/12/2020 01:57, Laurent Pinchart wrote: > From: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > > The CMM module provides a three-dimensional cubic look up table that > converts three-color-component data into desired three color components > by use of a lookup table. > > While the 1D-LUT can only control each of three color components > separately, the 3D-CLU can be used for specific color adjustment. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> For the updates since I wrote the patch: Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/rcar-du/rcar_cmm.c | 52 ++++++++++++++++++++++++++++-- > drivers/gpu/drm/rcar-du/rcar_cmm.h | 11 ++++--- > 2 files changed, 57 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c b/drivers/gpu/drm/rcar-du/rcar_cmm.c > index ccc8c8b03bac..9a20728a3534 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_cmm.c > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c > @@ -17,9 +17,18 @@ > > #define CM2_LUT_CTRL 0x0000 > #define CM2_LUT_CTRL_LUT_EN BIT(0) > + > +#define CM2_CLU_CTRL 0x0100 > +#define CM2_CLU_CTRL_CLU_EN BIT(1) > +#define CM2_CLU_CTRL_MVS BIT(24) > +#define CM2_CLU_CTRL_AAI BIT(28) > + > #define CM2_LUT_TBL_BASE 0x0600 > #define CM2_LUT_TBL(__i) (CM2_LUT_TBL_BASE + (__i) * 4) > > +#define CM2_CLU_ADDR 0x0a00 > +#define CM2_CLU_DATA 0x0a04 > + > struct rcar_cmm { > void __iomem *base; > > @@ -30,6 +39,10 @@ struct rcar_cmm { > struct { > bool enabled; > } lut; > + > + struct { > + bool enabled; > + } clu; > }; > > static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg) > @@ -72,13 +85,44 @@ static void rcar_cmm_lut_configure(struct rcar_cmm *rcmm, > } > } > > +static void rcar_cmm_clu_configure(struct rcar_cmm *rcmm, > + const struct drm_color_lut *table) > +{ > + static const u32 cfg = CM2_CLU_CTRL_AAI > + | CM2_CLU_CTRL_MVS > + | CM2_CLU_CTRL_CLU_EN; > + bool enable = !!table; Good, not sure why I was calculating this outside of the call. > + > + if (rcmm->clu.enabled != enable) { > + rcar_cmm_write(rcmm, CM2_CLU_CTRL, enable ? cfg : 0); > + rcmm->clu.enabled = enable; > + } > + > + if (table) { > + unsigned int i; > + > + /* Utilise CM2_CLU_CTRL_AAI (auto-increment). */ > + rcar_cmm_write(rcmm, CM2_CLU_ADDR, 0); > + > + for (i = 0; i < CM2_CLU_SIZE; ++i) { > + const struct drm_color_lut *lut = &table[i]; > + > + u32 entry = drm_color_lut_extract(lut->red, 8) << 16 > + | drm_color_lut_extract(lut->green, 8) << 8 > + | drm_color_lut_extract(lut->blue, 8); > + > + rcar_cmm_write(rcmm, CM2_CLU_DATA, entry); > + } > + } > +} > + > /* > * rcar_cmm_setup() - Configure the CMM unit > * @pdev: The platform device associated with the CMM instance > * @config: The CMM unit configuration > * > - * Configure the CMM unit with the given configuration. Currently enabling, > - * disabling and programming of the 1-D LUT unit is supported. > + * Configure the CMM unit with the given configuration, handling both the > + * 1-D LUT and the 3-D CLU. > * > * As rcar_cmm_setup() accesses the CMM registers the unit should be powered > * and its functional clock enabled. To guarantee this, before any call to > @@ -96,6 +140,9 @@ int rcar_cmm_setup(struct platform_device *pdev, > if (config->lut.update) > rcar_cmm_lut_configure(rcmm, config->lut.table); > > + if (config->clu.update) > + rcar_cmm_clu_configure(rcmm, config->clu.table); > + > return 0; > } > EXPORT_SYMBOL_GPL(rcar_cmm_setup); > @@ -140,6 +187,7 @@ void rcar_cmm_disable(struct platform_device *pdev) > struct rcar_cmm *rcmm = platform_get_drvdata(pdev); > > rcar_cmm_lut_configure(rcmm, NULL); > + rcar_cmm_clu_configure(rcmm, NULL); > > pm_runtime_put(&pdev->dev); > } > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.h b/drivers/gpu/drm/rcar-du/rcar_cmm.h > index f4b16535ec16..35f901158cec 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_cmm.h > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.h > @@ -9,14 +9,15 @@ > #define __RCAR_CMM_H__ > > #define CM2_LUT_SIZE 256 > +#define CM2_CLU_SIZE (17 * 17 * 17) > > struct drm_color_lut; > struct platform_device; > > /** > - * struct rcar_cmm_table_config - CMM LUT configuration > - * @update: When true, update the LUT configuration. > - * @table: Table data. The LUT is enabled if non-NULL, and disabled > + * struct rcar_cmm_table_config - CMM LUT and CLU configuration > + * @update: When true, update the LUT or CLU configuration. > + * @table: Table data. The LUT or CLU is enabled if non-NULL, and disabled > * otherwise. The value is ignored if @update is false. > */ > struct rcar_cmm_table_config { > @@ -26,10 +27,12 @@ struct rcar_cmm_table_config { > > /** > * struct rcar_cmm_config - CMM configuration > - * @lut: 1D-LUT configuration > + * @lut: 1D LUT configuration > + * @clu: 3D (cubic) LUT configuration > */ > struct rcar_cmm_config { > struct rcar_cmm_table_config lut; > + struct rcar_cmm_table_config clu; > }; > > #if IS_ENABLED(CONFIG_DRM_RCAR_CMM) > -- Regards -- Kieran _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel