[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4b1b3614-6a76-1ce9-7274-b5bb4deb6b74@collabora.com>
Date: Wed, 20 Sep 2023 10:08:21 +0200
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: CK Hu (胡俊光) <ck.hu@...iatek.com>,
"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>
Cc: "linux-mediatek@...ts.infradead.org"
<linux-mediatek@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"wenst@...omium.org" <wenst@...omium.org>,
Jason-JH Lin (林睿祥)
<Jason-JH.Lin@...iatek.com>,
"kernel@...labora.com" <kernel@...labora.com>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"amergnat@...libre.com" <amergnat@...libre.com>,
"ehristev@...labora.com" <ehristev@...labora.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>
Subject: Re: [PATCH v10 03/16] drm/mediatek: gamma: Support SoC specific LUT
size
Il 18/09/23 04:30, CK Hu (胡俊光) ha scritto:
> Hi, Angelo:
>
> On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
>> Newer SoCs support a bigger Gamma LUT table: wire up a callback
>> to retrieve the correct LUT size for each different Gamma IP.
>>
>> Co-developed-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
>> [Angelo: Rewritten commit message/description + porting]
>> Signed-off-by: AngeloGioacchino Del Regno <
>> angelogioacchino.delregno@...labora.com>
>> Reviewed-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
>> Reviewed-by: Alexandre Mergnat <amergnat@...libre.com>
>> ---
>> drivers/gpu/drm/mediatek/mtk_disp_aal.c | 17 ++++++++++++++-
>> drivers/gpu/drm/mediatek/mtk_disp_drv.h | 2 ++
>> drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 23 ++++++++++++++++++-
>> --
>> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 8 +++++--
>> drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 1 -
>> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 ++
>> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 9 ++++++++
>> 7 files changed, 55 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
>> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
>> index 2f602f1f1c49..e2e4155faf01 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
>> @@ -19,7 +19,7 @@
>> #define AAL_EN BIT(0)
>> #define DISP_AAL_SIZE 0x0030
>> #define DISP_AAL_OUTPUT_SIZE 0x04d8
>> -
>> +#define DISP_AAL_LUT_SIZE 512
>>
>> struct mtk_disp_aal_data {
>> bool has_gamma;
>> @@ -61,6 +61,21 @@ void mtk_aal_config(struct device *dev, unsigned
>> int w,
>> mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs,
>> DISP_AAL_OUTPUT_SIZE);
>> }
>>
>> +/**
>> + * mtk_aal_gamma_get_lut_size() - Get gamma LUT size for AAL
>> + * @dev: Pointer to struct device
>> + *
>> + * Return: 0 if gamma control not supported in AAL or gamma LUT size
>> + */
>> +unsigned int mtk_aal_gamma_get_lut_size(struct device *dev)
>> +{
>> + struct mtk_disp_aal *aal = dev_get_drvdata(dev);
>> +
>> + if (aal->data && aal->data->has_gamma)
>> + return DISP_AAL_LUT_SIZE;
>> + return 0;
>> +}
>> +
>> void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state
>> *state)
>> {
>> struct mtk_disp_aal *aal = dev_get_drvdata(dev);
>> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>> index 75045932353e..ca377265e5eb 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>> @@ -17,6 +17,7 @@ void mtk_aal_clk_disable(struct device *dev);
>> void mtk_aal_config(struct device *dev, unsigned int w,
>> unsigned int h, unsigned int vrefresh,
>> unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
>> +unsigned int mtk_aal_gamma_get_lut_size(struct device *dev);
>> 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);
>> @@ -53,6 +54,7 @@ void mtk_gamma_clk_disable(struct device *dev);
>> void mtk_gamma_config(struct device *dev, unsigned int w,
>> unsigned int h, unsigned int vrefresh,
>> unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
>> +unsigned int mtk_gamma_get_lut_size(struct device *dev);
>> void mtk_gamma_set(struct device *dev, struct drm_crtc_state
>> *state);
>> void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
>> struct drm_crtc_state *state);
>> void mtk_gamma_start(struct device *dev);
>> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> index 47751864bd5c..7575237625d2 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> @@ -24,10 +24,12 @@
>> #define DISP_GAMMA_LUT 0x0700
>>
>> #define LUT_10BIT_MASK 0x03ff
>> +#define LUT_SIZE_DEFAULT 512
>>
>> struct mtk_disp_gamma_data {
>> bool has_dither;
>> bool lut_diff;
>> + u16 lut_size;
>> };
>>
>> /*
>> @@ -54,6 +56,15 @@ void mtk_gamma_clk_disable(struct device *dev)
>> clk_disable_unprepare(gamma->clk);
>> }
>>
>> +unsigned int mtk_gamma_get_lut_size(struct device *dev)
>> +{
>> + struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
>> +
>> + if (gamma && gamma->data)
>> + return gamma->data->lut_size;
>> + return LUT_SIZE_DEFAULT;
>> +}
>> +
>> void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
>> struct drm_crtc_state *state)
>> {
>> struct mtk_disp_gamma *gamma;
>> @@ -61,6 +72,7 @@ void mtk_gamma_set_common(struct device *dev, void
>> __iomem *regs, struct drm_crt
>> struct drm_color_lut *lut;
>> void __iomem *lut_base;
>> bool lut_diff;
>> + u16 lut_size;
>> u32 word;
>> u32 diff[3] = {0};
>>
>> @@ -71,17 +83,20 @@ void mtk_gamma_set_common(struct device *dev,
>> void __iomem *regs, struct drm_crt
>> /* If we're called from AAL, dev is NULL */
>> gamma = dev ? dev_get_drvdata(dev) : NULL;
>>
>> - if (gamma && gamma->data)
>> + if (gamma && gamma->data) {
>> lut_diff = gamma->data->lut_diff;
>> - else
>> + lut_size = gamma->data->lut_size;
>> + } else {
>> lut_diff = false;
>> + lut_size = LUT_SIZE_DEFAULT;
>
> Only AAL use this definiton. As previous discussion,
> mtk_gamm_set_common() would not be common and has gamma version and AAL
> version.
>
Hello CK,
Yes, that's true, but every patch has to work on its own: this means that
I forcefully have to add this here to avoid breaking functionality!
Please check patch [08/16] where I de-commonize the gamma_set function, as
there I am removing the AAL-only definition and adding a AAL-specific
gamma_set() callback.
I've done it this way for multiple reasons, one of which is to show why a
de-commonization of this function is required.
Regards,
Angelo
> Regards,
> CK
>
Powered by blists - more mailing lists