[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <03164cb32445f1054f8a3f04b846aa97b5cffa95.camel@mediatek.com>
Date: Mon, 24 Feb 2025 12:49:04 +0000
From: Jay Liu (刘博) <Jay.Liu@...iatek.com>
To: Yongqiang Niu (牛永强)
<yongqiang.niu@...iatek.com>, "chunkuang.hu@...nel.org"
<chunkuang.hu@...nel.org>, "simona@...ll.ch" <simona@...ll.ch>,
"tzimmermann@...e.de" <tzimmermann@...e.de>, "mripard@...nel.org"
<mripard@...nel.org>, "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
CK Hu (胡俊光) <ck.hu@...iatek.com>,
"maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
"conor+dt@...nel.org" <conor+dt@...nel.org>, "robh@...nel.org"
<robh@...nel.org>, "hsinyi@...omium.org" <hsinyi@...omium.org>,
"airlied@...il.com" <airlied@...il.com>, "matthias.bgg@...il.com"
<matthias.bgg@...il.com>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
CC: "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, Project_Global_Chrome_Upstream_Group
<Project_Global_Chrome_Upstream_Group@...iatek.com>
Subject: Re: [PATCH 1/7] drm/mediatek: Add CCORR component support for MT8196
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@...iatek.com>
>
> 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[]
> > = {
>
>
Powered by blists - more mailing lists