On Wed, 2025-02-19 at 13:49 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Il 19/02/25 10:20, Jay Liu ha scritto:
> > 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 hardware platform includes two CCORR (Color Correction)
> > units.
> > However, 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 by adding mandatory_ccorr flag in the
> > driver_data.
> >
> > Signed-off-by: Jay Liu <jay.liu@xxxxxxxxxxxx>
>
> This is yet another thing that can be resolved by using OF Graph for
> defining the
> display pipeline: by using that, I don't see how can CCORR1 be used
> instead of
> CCORR0, if the latter is in the pipeline, but not the former.
>
> NACK.
>
> Regards,
> Angelo
> hi Angelo, Thank you very much for your feedback.
> 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. In the current project, the ctm_set interface will
> traverse all CCORRs and set parameters, but in fact, only one needs
> to be set. Therefore, setting mandatory_ccorr ensures that only this
> CCORR is used.
> Regards,
> Jay
> > ---
> > drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 3 ++-
> > drivers/gpu/drm/mediatek/mtk_disp_ccorr.c | 16 ++++++++++++++++
> > 2 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > index edc6417639e6..d7e230bac53e 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > @@ -457,7 +457,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_disp_ccorr.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> > index 10d60d2c2a56..94e82b3fa2d8 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> > @@ -31,11 +31,13 @@
> >
> > struct mtk_disp_ccorr_data {
> > u32 matrix_bits;
> > + enum mtk_ddp_comp_id mandatory_ccorr;
> > };
> >
> > struct mtk_disp_ccorr {
> > struct clk *clk;
> > void __iomem *regs;
> > + enum mtk_ddp_comp_id comp_id;
> > struct cmdq_client_reg cmdq_reg;
> > const struct mtk_disp_ccorr_data *data;
> > };
> > @@ -115,6 +117,9 @@ void mtk_ccorr_ctm_set(struct device *dev,
> > struct drm_crtc_state *state)
> > if (!blob)
> > return;
> >
> > + if (ccorr->comp_id != ccorr->data->mandatory_ccorr)
> > + return;
> > +
> > ctm = (struct drm_color_ctm *)blob->data;
> > input = ctm->matrix;
> >
> > @@ -154,6 +159,7 @@ static int mtk_disp_ccorr_probe(struct
> > platform_device *pdev)
> > struct device *dev = &pdev->dev;
> > struct mtk_disp_ccorr *priv;
> > int ret;
> > + enum mtk_ddp_comp_id comp_id;
> >
> > priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > if (!priv)
> > @@ -169,6 +175,14 @@ static int mtk_disp_ccorr_probe(struct
> > platform_device *pdev)
> > return dev_err_probe(dev, PTR_ERR(priv->regs),
> > "failed to ioremap ccorr\n");
> >
> > + comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DISP_CCORR);
> > + if (comp_id < 0) {
> > + dev_err(dev, "Failed to identify by alias: %d\n",
> > comp_id);
> > + return comp_id;
> > + }
> > +
> > + priv->comp_id = comp_id;
> > +
> > #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > ret = cmdq_dev_get_client_reg(dev, &priv->cmdq_reg, 0);
> > if (ret)
> > @@ -192,10 +206,12 @@ static void mtk_disp_ccorr_remove(struct
> > platform_device *pdev)
> >
> > static const struct mtk_disp_ccorr_data mt8183_ccorr_driver_data
> > = {
> > .matrix_bits = 10,
> > + .mandatory_ccorr = DDP_COMPONENT_CCORR0,
> > };
> >
> > static const struct mtk_disp_ccorr_data mt8192_ccorr_driver_data
> > = {
> > .matrix_bits = 11,
> > + .mandatory_ccorr = DDP_COMPONENT_CCORR0,
> > };
> >
> > static const struct of_device_id mtk_disp_ccorr_driver_dt_match[]
> > = {
>
>
