Re: [PATCH v5 5/6] drm/mediatek: Support multiple CCORR component

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

On Mon, 2026-04-27 at 19:20 +0800, Jay Liu wrote:
> Add CCORR component support for MT8196.
> CCORR is a hardware module that optimizes the visual effects of images
> by adjusting the color matrix, enabling features such as night light.
> 
> The 8196 SoC has two CCORR hardware units, which must be chained together
> in a fixed order in the display path to display the image correctly.
> the `mtk_ccorr_ctm_set` API only utilizes one of these units. To prevent
> the unused CCORR unit from inadvertently taking effect, we need to block
> it in the mtk_crtc.c.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Jay Liu <jay.liu@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_crtc.c       | 5 ++++-
>  drivers/gpu/drm/mediatek/mtk_ddp_comp.c   | 3 ++-
>  drivers/gpu/drm/mediatek/mtk_ddp_comp.h   | 7 ++++---
>  drivers/gpu/drm/mediatek/mtk_disp_ccorr.c | 6 ++++--
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h   | 2 +-
>  5 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
> index fcb16f3f7b23..09b260a9a4ee 100644
> --- a/drivers/gpu/drm/mediatek/mtk_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
> @@ -872,11 +872,14 @@ static void mtk_crtc_atomic_flush(struct drm_crtc *crtc,
>  {
>  	struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc);
>  	int i;
> +	bool ctm_set = false;
>  
>  	if (crtc->state->color_mgmt_changed)
>  		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
>  			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> -			mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
> +			/* only set ctm once for the pipeline with two CCORR components */
> +			if (!ctm_set)
> +				ctm_set = mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
>  		}
>  	mtk_crtc_update_config(mtk_crtc, !!mtk_crtc->event);
>  }
> diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> index 9672ea1f91a2..5cbc4b995d66 100644
> --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> @@ -458,7 +458,8 @@ static const struct mtk_ddp_comp_match mtk_ddp_matches[DDP_COMPONENT_DRM_ID_MAX]
>  	[DDP_COMPONENT_AAL0]		= { MTK_DISP_AAL,		0, &ddp_aal },
>  	[DDP_COMPONENT_AAL1]		= { MTK_DISP_AAL,		1, &ddp_aal },
>  	[DDP_COMPONENT_BLS]		= { MTK_DISP_BLS,		0, NULL },
> -	[DDP_COMPONENT_CCORR]		= { MTK_DISP_CCORR,		0, &ddp_ccorr },
> +	[DDP_COMPONENT_CCORR0]		= { MTK_DISP_CCORR,		0, &ddp_ccorr },
> +	[DDP_COMPONENT_CCORR1]		= { MTK_DISP_CCORR,		1, &ddp_ccorr },
>  	[DDP_COMPONENT_COLOR0]		= { MTK_DISP_COLOR,		0, &ddp_color },
>  	[DDP_COMPONENT_COLOR1]		= { MTK_DISP_COLOR,		1, &ddp_color },
>  	[DDP_COMPONENT_DITHER0]		= { MTK_DISP_DITHER,		0, &ddp_dither },
> diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> index 3f3d43f4330d..7244b55f6732 100644
> --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> @@ -77,7 +77,7 @@ struct mtk_ddp_comp_funcs {
>  			  struct drm_crtc_state *state);
>  	void (*bgclr_in_on)(struct device *dev);
>  	void (*bgclr_in_off)(struct device *dev);
> -	void (*ctm_set)(struct device *dev,
> +	bool (*ctm_set)(struct device *dev,
>  			struct drm_crtc_state *state);
>  	struct device * (*dma_dev_get)(struct device *dev);
>  	u32 (*get_blend_modes)(struct device *dev);
> @@ -254,11 +254,12 @@ static inline void mtk_ddp_comp_bgclr_in_off(struct mtk_ddp_comp *comp)
>  		comp->funcs->bgclr_in_off(comp->dev);
>  }
>  
> -static inline void mtk_ddp_ctm_set(struct mtk_ddp_comp *comp,
> +static inline bool mtk_ddp_ctm_set(struct mtk_ddp_comp *comp,
>  				   struct drm_crtc_state *state)
>  {
>  	if (comp->funcs && comp->funcs->ctm_set)
> -		comp->funcs->ctm_set(comp->dev, state);
> +		return comp->funcs->ctm_set(comp->dev, state);
> +	return false;
>  }
>  
>  static inline struct device *mtk_ddp_comp_dma_dev_get(struct mtk_ddp_comp *comp)
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> index 6d7bf4afa78d..ac59d81dbb26 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> @@ -80,7 +80,7 @@ void mtk_ccorr_stop(struct device *dev)
>  	writel_relaxed(0x0, ccorr->regs + DISP_CCORR_EN);
>  }
>  
> -void mtk_ccorr_ctm_set(struct device *dev, struct drm_crtc_state *state)
> +bool mtk_ccorr_ctm_set(struct device *dev, struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_ccorr *ccorr = dev_get_drvdata(dev);
>  	struct drm_property_blob *blob = state->ctm;
> @@ -92,7 +92,7 @@ void mtk_ccorr_ctm_set(struct device *dev, struct drm_crtc_state *state)
>  	u32 matrix_bits = ccorr->data->matrix_bits;
>  
>  	if (!blob)
> -		return;
> +		return false;
>  
>  	ctm = (struct drm_color_ctm *)blob->data;
>  	input = ctm->matrix;
> @@ -110,6 +110,8 @@ void mtk_ccorr_ctm_set(struct device *dev, struct drm_crtc_state *state)
>  		      &ccorr->cmdq_reg, ccorr->regs, DISP_CCORR_COEF_3);
>  	mtk_ddp_write(cmdq_pkt, coeffs[8] << 16,
>  		      &ccorr->cmdq_reg, ccorr->regs, DISP_CCORR_COEF_4);
> +
> +	return true;
>  }
>  
>  static int mtk_disp_ccorr_bind(struct device *dev, struct device *master,
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index 679d413bf10b..4203c28c38ce 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -22,7 +22,7 @@ void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state);
>  void mtk_aal_start(struct device *dev);
>  void mtk_aal_stop(struct device *dev);
>  
> -void mtk_ccorr_ctm_set(struct device *dev, struct drm_crtc_state *state);
> +bool mtk_ccorr_ctm_set(struct device *dev, struct drm_crtc_state *state);
>  int mtk_ccorr_clk_enable(struct device *dev);
>  void mtk_ccorr_clk_disable(struct device *dev);
>  void mtk_ccorr_config(struct device *dev, unsigned int w,