diff mbox series

[v1,1/1] drm/mediatek: add lut diff flag for new gamma hardware support

Message ID 20220420130617.814-2-yongqiang.niu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series drm/mediatek: add lut diff flag for new gamma hardware support | expand

Commit Message

Yongqiang Niu April 20, 2022, 1:06 p.m. UTC
From: Yongqiang Niu <yongqiang.niu@mediatek.corp-partner.google.com>

mt8183 gamma module usage is different with before soc,
gamma odd(index start from 0) lut value set to hardware
register should be
the difference of current lut value with last lut value.

gamma function support both increase and decrease lut.
chrome os app set increase lut normally.

for increase lut example, chrome os user space set lut
like this(only r chanel for example):
2 4 6 8 10 12.
1) mt8183 gamma driver should set the gamma lut to hardware
register like this:
2 [2] 6 [8] 10 [2]
the value with [] is the difference value
2) gamma hardware will restore the lut when apply gamma
function to display

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.corp-partner.google.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_aal.c   |  4 ++-
 drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  2 +-
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34 +++++++++++++++++++----
 3 files changed, 32 insertions(+), 8 deletions(-)

Comments

Rex-BC Chen (陳柏辰) April 21, 2022, 2:38 a.m. UTC | #1
On Wed, 2022-04-20 at 21:06 +0800, Yongqiang Niu wrote:
> From: Yongqiang Niu <yongqiang.niu@mediatek.corp-partner.google.com>
> 
> mt8183 gamma module usage is different with before soc,
> gamma odd(index start from 0) lut value set to hardware
> register should be
> the difference of current lut value with last lut value.
> 

Hello Yongqiang,

Do you mean for SoCs before 8183 (like 8173) are using original
method(setting without lut diff) and for 8183/8192/8195/8186, we
need to use the method of lut diff?

> gamma function support both increase and decrease lut.
> chrome os app set increase lut normally.
> 
> for increase lut example, chrome os user space set lut
> like this(only r chanel for example):
> 2 4 6 8 10 12.
> 1) mt8183 gamma driver should set the gamma lut to hardware
> register like this:
> 2 [2] 6 [8] 10 [2]

this should be something like this?
2 [2] 6 [2] 10 [2]

> the value with [] is the difference value
> 2) gamma hardware will restore the lut when apply gamma
> function to display
> 
> Signed-off-by: Yongqiang Niu <
> yongqiang.niu@mediatek.corp-partner.google.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c   |  4 ++-
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  2 +-
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34 +++++++++++++++++++
> ----
>  3 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index f46d4ab73d6a..e6378b074a17 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -23,6 +23,7 @@
>  
>  struct mtk_disp_aal_data {
>  	bool has_gamma;
> +	bool lut_diff;
>  };
>  
>  /**
> @@ -66,7 +67,7 @@ void mtk_aal_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
>  
>  	if (aal->data && aal->data->has_gamma)
> -		mtk_gamma_set_common(aal->regs, state);
> +		mtk_gamma_set_common(aal->regs, state, aal->data-
> >lut_diff);
>  }
>  
>  void mtk_aal_start(struct device *dev)
> @@ -148,6 +149,7 @@ static int mtk_disp_aal_remove(struct
> platform_device *pdev)
>  
>  static const struct mtk_disp_aal_data mt8173_aal_driver_data = {
>  	.has_gamma = true,
> +	.lut_diff = false,
>  };
>  
>  static const struct of_device_id mtk_disp_aal_driver_dt_match[] = {
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index 86c3068894b1..3380651c6707 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -51,7 +51,7 @@ void mtk_gamma_config(struct device *dev, unsigned
> int w,
>  		      unsigned int h, unsigned int vrefresh,
>  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> *state);
> -void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state
> *state);
> +void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state
> *state, bool lut_diff);
>  void mtk_gamma_start(struct device *dev);
>  void mtk_gamma_stop(struct device *dev);
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 3a5815ab4079..fec2e9a5b60d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -27,6 +27,7 @@
>  
>  struct mtk_disp_gamma_data {
>  	bool has_dither;
> +	bool lut_diff;
>  };
>  
>  /*
> @@ -53,12 +54,13 @@ void mtk_gamma_clk_disable(struct device *dev)
>  	clk_disable_unprepare(gamma->clk);
>  }
>  
> -void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state
> *state)
> +void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state
> *state, bool lut_diff)
>  {
>  	unsigned int i, reg;
>  	struct drm_color_lut *lut;
>  	void __iomem *lut_base;
>  	u32 word;
> +	u32 diff[3] = {0};
>  
>  	if (state->gamma_lut) {
>  		reg = readl(regs + DISP_GAMMA_CFG);
> @@ -67,9 +69,20 @@ void mtk_gamma_set_common(void __iomem *regs,
> struct drm_crtc_state *state)
>  		lut_base = regs + DISP_GAMMA_LUT;
>  		lut = (struct drm_color_lut *)state->gamma_lut->data;
>  		for (i = 0; i < MTK_LUT_SIZE; i++) {
> -			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) <<
> 20) +
> -				(((lut[i].green >> 6) & LUT_10BIT_MASK)
> << 10) +
> -				((lut[i].blue >> 6) & LUT_10BIT_MASK);
> +
> +			if (!lut_diff || (i % 2 == 0)) {

if it's not used lut_diff for the MT8173, it is strange for this
condition.

BRs,
Rex

> +				word = (((lut[i].red >> 6) &
> LUT_10BIT_MASK) << 20) +
> +					(((lut[i].green >> 6) &
> LUT_10BIT_MASK) << 10) +
> +					((lut[i].blue >> 6) &
> LUT_10BIT_MASK);
> +			} else {
> +				diff[0] = abs((lut[i].red >> 6) -
> (lut[i - 1].red >> 6));
> +				diff[1] = abs((lut[i].green >> 6) -
> (lut[i - 1].green >> 6));
> +				diff[2] = abs((lut[i].blue >> 6) -
> (lut[i - 1].blue >> 6));
> +
> +				word = ((diff[0] & LUT_10BIT_MASK) <<
> 20) +
> +					((diff[1] & LUT_10BIT_MASK) <<
> 10) +
> +					(diff[2] & LUT_10BIT_MASK);
> +			}
>  			writel(word, (lut_base + i * 4));
>  		}
>  	}
> @@ -78,8 +91,12 @@ void mtk_gamma_set_common(void __iomem *regs,
> struct drm_crtc_state *state)
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +	bool lut_diff = false;
> +
> +	if (gamma->data)
> +		lut_diff = gamma->data->lut_diff;
>  
> -	mtk_gamma_set_common(gamma->regs, state);
> +	mtk_gamma_set_common(gamma->regs, state, lut_diff);
>  }
>  
>  void mtk_gamma_config(struct device *dev, unsigned int w,
> @@ -176,10 +193,15 @@ static const struct mtk_disp_gamma_data
> mt8173_gamma_driver_data = {
>  	.has_dither = true,
>  };
>  
> +static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
> +	.lut_diff = true,
> +};
> +
>  static const struct of_device_id mtk_disp_gamma_driver_dt_match[] =
> {
>  	{ .compatible = "mediatek,mt8173-disp-gamma",
>  	  .data = &mt8173_gamma_driver_data},
> -	{ .compatible = "mediatek,mt8183-disp-gamma"},
> +	{ .compatible = "mediatek,mt8183-disp-gamma",
> +	  .data = &mt8183_gamma_driver_data},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
Yongqiang Niu April 21, 2022, 8:05 a.m. UTC | #2
On Thu, 2022-04-21 at 10:38 +0800, Rex-BC Chen wrote:
> On Wed, 2022-04-20 at 21:06 +0800, Yongqiang Niu wrote:
> > From: Yongqiang Niu <yongqiang.niu@mediatek.corp-partner.google.com
> > >
> > 
> > mt8183 gamma module usage is different with before soc,
> > gamma odd(index start from 0) lut value set to hardware
> > register should be
> > the difference of current lut value with last lut value.
> > 
> 
> Hello Yongqiang,
> 
> Do you mean for SoCs before 8183 (like 8173) are using original
> method(setting without lut diff) and for 8183/8192/8195/8186, we
> need to use the method of lut diff?
> 
as i know, 8173 use original lut
for other soc, need module maintainer confirm with hardware designer or
ic verifier, or module maintainer test gamma lut function byself.
i will confirm 8192 and 8186

> > gamma function support both increase and decrease lut.
> > chrome os app set increase lut normally.
> > 
> > for increase lut example, chrome os user space set lut
> > like this(only r chanel for example):
> > 2 4 6 8 10 12.
> > 1) mt8183 gamma driver should set the gamma lut to hardware
> > register like this:
> > 2 [2] 6 [8] 10 [2]
> 
> this should be something like this?
> 2 [2] 6 [2] 10 [2]
> 
this is only a sample, the value with [] means that is diff, but not
original lut value
> > the value with [] is the difference value
> > 2) gamma hardware will restore the lut when apply gamma
> > function to display
> > 
> > Signed-off-by: Yongqiang Niu <
> > yongqiang.niu@mediatek.corp-partner.google.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_aal.c   |  4 ++-
> >  drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  2 +-
> >  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34 +++++++++++++++++++
> > ----
> >  3 files changed, 32 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > index f46d4ab73d6a..e6378b074a17 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > @@ -23,6 +23,7 @@
> >  
> >  struct mtk_disp_aal_data {
> >  	bool has_gamma;
> > +	bool lut_diff;
> >  };
> >  
> >  /**
> > @@ -66,7 +67,7 @@ void mtk_aal_gamma_set(struct device *dev, struct
> > drm_crtc_state *state)
> >  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> >  
> >  	if (aal->data && aal->data->has_gamma)
> > -		mtk_gamma_set_common(aal->regs, state);
> > +		mtk_gamma_set_common(aal->regs, state, aal->data-
> > > lut_diff);
> > 
> >  }
> >  
> >  void mtk_aal_start(struct device *dev)
> > @@ -148,6 +149,7 @@ static int mtk_disp_aal_remove(struct
> > platform_device *pdev)
> >  
> >  static const struct mtk_disp_aal_data mt8173_aal_driver_data = {
> >  	.has_gamma = true,
> > +	.lut_diff = false,
> >  };
> >  
> >  static const struct of_device_id mtk_disp_aal_driver_dt_match[] =
> > {
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > index 86c3068894b1..3380651c6707 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -51,7 +51,7 @@ void mtk_gamma_config(struct device *dev,
> > unsigned
> > int w,
> >  		      unsigned int h, unsigned int vrefresh,
> >  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> >  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > *state);
> > -void mtk_gamma_set_common(void __iomem *regs, struct
> > drm_crtc_state
> > *state);
> > +void mtk_gamma_set_common(void __iomem *regs, struct
> > drm_crtc_state
> > *state, bool lut_diff);
> >  void mtk_gamma_start(struct device *dev);
> >  void mtk_gamma_stop(struct device *dev);
> >  
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > index 3a5815ab4079..fec2e9a5b60d 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > @@ -27,6 +27,7 @@
> >  
> >  struct mtk_disp_gamma_data {
> >  	bool has_dither;
> > +	bool lut_diff;
> >  };
> >  
> >  /*
> > @@ -53,12 +54,13 @@ void mtk_gamma_clk_disable(struct device *dev)
> >  	clk_disable_unprepare(gamma->clk);
> >  }
> >  
> > -void mtk_gamma_set_common(void __iomem *regs, struct
> > drm_crtc_state
> > *state)
> > +void mtk_gamma_set_common(void __iomem *regs, struct
> > drm_crtc_state
> > *state, bool lut_diff)
> >  {
> >  	unsigned int i, reg;
> >  	struct drm_color_lut *lut;
> >  	void __iomem *lut_base;
> >  	u32 word;
> > +	u32 diff[3] = {0};
> >  
> >  	if (state->gamma_lut) {
> >  		reg = readl(regs + DISP_GAMMA_CFG);
> > @@ -67,9 +69,20 @@ void mtk_gamma_set_common(void __iomem *regs,
> > struct drm_crtc_state *state)
> >  		lut_base = regs + DISP_GAMMA_LUT;
> >  		lut = (struct drm_color_lut *)state->gamma_lut->data;
> >  		for (i = 0; i < MTK_LUT_SIZE; i++) {
> > -			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) <<
> > 20) +
> > -				(((lut[i].green >> 6) & LUT_10BIT_MASK)
> > << 10) +
> > -				((lut[i].blue >> 6) & LUT_10BIT_MASK);
> > +
> > +			if (!lut_diff || (i % 2 == 0)) {
> 
> if it's not used lut_diff for the MT8173, it is strange for this
> condition.
> 
for the soc which user original lut, lut_diff is false, all original
lut will be set to hardware register
> BRs,
> Rex
> 
> > +				word = (((lut[i].red >> 6) &
> > LUT_10BIT_MASK) << 20) +
> > +					(((lut[i].green >> 6) &
> > LUT_10BIT_MASK) << 10) +
> > +					((lut[i].blue >> 6) &
> > LUT_10BIT_MASK);
> > +			} else {
> > +				diff[0] = abs((lut[i].red >> 6) -
> > (lut[i - 1].red >> 6));
> > +				diff[1] = abs((lut[i].green >> 6) -
> > (lut[i - 1].green >> 6));
> > +				diff[2] = abs((lut[i].blue >> 6) -
> > (lut[i - 1].blue >> 6));
> > +
> > +				word = ((diff[0] & LUT_10BIT_MASK) <<
> > 20) +
> > +					((diff[1] & LUT_10BIT_MASK) <<
> > 10) +
> > +					(diff[2] & LUT_10BIT_MASK);
> > +			}
> >  			writel(word, (lut_base + i * 4));
> >  		}
> >  	}
> > @@ -78,8 +91,12 @@ void mtk_gamma_set_common(void __iomem *regs,
> > struct drm_crtc_state *state)
> >  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > *state)
> >  {
> >  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> > +	bool lut_diff = false;
> > +
> > +	if (gamma->data)
> > +		lut_diff = gamma->data->lut_diff;
> >  
> > -	mtk_gamma_set_common(gamma->regs, state);
> > +	mtk_gamma_set_common(gamma->regs, state, lut_diff);
> >  }
> >  
> >  void mtk_gamma_config(struct device *dev, unsigned int w,
> > @@ -176,10 +193,15 @@ static const struct mtk_disp_gamma_data
> > mt8173_gamma_driver_data = {
> >  	.has_dither = true,
> >  };
> >  
> > +static const struct mtk_disp_gamma_data mt8183_gamma_driver_data =
> > {
> > +	.lut_diff = true,
> > +};
> > +
> >  static const struct of_device_id mtk_disp_gamma_driver_dt_match[]
> > =
> > {
> >  	{ .compatible = "mediatek,mt8173-disp-gamma",
> >  	  .data = &mt8173_gamma_driver_data},
> > -	{ .compatible = "mediatek,mt8183-disp-gamma"},
> > +	{ .compatible = "mediatek,mt8183-disp-gamma",
> > +	  .data = &mt8183_gamma_driver_data},
> >  	{},
> >  };
> >  MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
> 
>
CK Hu (胡俊光) April 26, 2022, 1:58 a.m. UTC | #3
Hi, Yongqiang:

On Wed, 2022-04-20 at 21:06 +0800, Yongqiang Niu wrote:
> From: Yongqiang Niu <yongqiang.niu@mediatek.corp-partner.google.com>
> 
> mt8183 gamma module usage is different with before soc,
> gamma odd(index start from 0) lut value set to hardware
> register should be
> the difference of current lut value with last lut value.
> 
> gamma function support both increase and decrease lut.

How to set decrease lut?

Original lut:
12 10 8 6 4 2

Does diff lut look like this?
12 [2] 8 [2] 4 [2]

How does hardware know that this is increase lut or decrease lut?

> chrome os app set increase lut normally.
> 
> for increase lut example, chrome os user space set lut
> like this(only r chanel for example):
> 2 4 6 8 10 12.
> 1) mt8183 gamma driver should set the gamma lut to hardware
> register like this:
> 2 [2] 6 [8] 10 [2]

2 [2] 6 [2] 10 [2]

> the value with [] is the difference value
> 2) gamma hardware will restore the lut when apply gamma
> function to display

I don't know why do you mention the 'restore', any modification is
related to this?

> 
> Signed-off-by: Yongqiang Niu <
> yongqiang.niu@mediatek.corp-partner.google.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c   |  4 ++-
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  2 +-
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34 +++++++++++++++++++
> ----
>  3 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index f46d4ab73d6a..e6378b074a17 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -23,6 +23,7 @@
>  
>  struct mtk_disp_aal_data {
>  	bool has_gamma;
> +	bool lut_diff;
>  };
>  
>  /**
> @@ -66,7 +67,7 @@ void mtk_aal_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
>  
>  	if (aal->data && aal->data->has_gamma)
> -		mtk_gamma_set_common(aal->regs, state);
> +		mtk_gamma_set_common(aal->regs, state, aal->data-
> >lut_diff);

gamma in aal does not support diff lut, so this would make things
simple.

mtk_gamma_set_common(aal->regs, state, false);

Regards,
CK

>  }
>  
>  void mtk_aal_start(struct device *dev)
> @@ -148,6 +149,7 @@ static int mtk_disp_aal_remove(struct
> platform_device *pdev)
>  
>  static const struct mtk_disp_aal_data mt8173_aal_driver_data = {
>  	.has_gamma = true,
> +	.lut_diff = false,
>  };
>  
>  static const struct of_device_id mtk_disp_aal_driver_dt_match[] = {
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index 86c3068894b1..3380651c6707 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -51,7 +51,7 @@ void mtk_gamma_config(struct device *dev, unsigned
> int w,
>  		      unsigned int h, unsigned int vrefresh,
>  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> *state);
> -void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state
> *state);
> +void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state
> *state, bool lut_diff);
>  void mtk_gamma_start(struct device *dev);
>  void mtk_gamma_stop(struct device *dev);
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 3a5815ab4079..fec2e9a5b60d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -27,6 +27,7 @@
>  
>  struct mtk_disp_gamma_data {
>  	bool has_dither;
> +	bool lut_diff;
>  };
>  
>  /*
> @@ -53,12 +54,13 @@ void mtk_gamma_clk_disable(struct device *dev)
>  	clk_disable_unprepare(gamma->clk);
>  }
>  
> -void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state
> *state)
> +void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state
> *state, bool lut_diff)
>  {
>  	unsigned int i, reg;
>  	struct drm_color_lut *lut;
>  	void __iomem *lut_base;
>  	u32 word;
> +	u32 diff[3] = {0};
>  
>  	if (state->gamma_lut) {
>  		reg = readl(regs + DISP_GAMMA_CFG);
> @@ -67,9 +69,20 @@ void mtk_gamma_set_common(void __iomem *regs,
> struct drm_crtc_state *state)
>  		lut_base = regs + DISP_GAMMA_LUT;
>  		lut = (struct drm_color_lut *)state->gamma_lut->data;
>  		for (i = 0; i < MTK_LUT_SIZE; i++) {
> -			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) <<
> 20) +
> -				(((lut[i].green >> 6) & LUT_10BIT_MASK)
> << 10) +
> -				((lut[i].blue >> 6) & LUT_10BIT_MASK);
> +
> +			if (!lut_diff || (i % 2 == 0)) {
> +				word = (((lut[i].red >> 6) &
> LUT_10BIT_MASK) << 20) +
> +					(((lut[i].green >> 6) &
> LUT_10BIT_MASK) << 10) +
> +					((lut[i].blue >> 6) &
> LUT_10BIT_MASK);
> +			} else {
> +				diff[0] = abs((lut[i].red >> 6) -
> (lut[i - 1].red >> 6));
> +				diff[1] = abs((lut[i].green >> 6) -
> (lut[i - 1].green >> 6));
> +				diff[2] = abs((lut[i].blue >> 6) -
> (lut[i - 1].blue >> 6));
> +
> +				word = ((diff[0] & LUT_10BIT_MASK) <<
> 20) +
> +					((diff[1] & LUT_10BIT_MASK) <<
> 10) +
> +					(diff[2] & LUT_10BIT_MASK);
> +			}
>  			writel(word, (lut_base + i * 4));
>  		}
>  	}
> @@ -78,8 +91,12 @@ void mtk_gamma_set_common(void __iomem *regs,
> struct drm_crtc_state *state)
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +	bool lut_diff = false;
> +
> +	if (gamma->data)
> +		lut_diff = gamma->data->lut_diff;
>  
> -	mtk_gamma_set_common(gamma->regs, state);
> +	mtk_gamma_set_common(gamma->regs, state, lut_diff);
>  }
>  
>  void mtk_gamma_config(struct device *dev, unsigned int w,
> @@ -176,10 +193,15 @@ static const struct mtk_disp_gamma_data
> mt8173_gamma_driver_data = {
>  	.has_dither = true,
>  };
>  
> +static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
> +	.lut_diff = true,
> +};
> +
>  static const struct of_device_id mtk_disp_gamma_driver_dt_match[] =
> {
>  	{ .compatible = "mediatek,mt8173-disp-gamma",
>  	  .data = &mt8173_gamma_driver_data},
> -	{ .compatible = "mediatek,mt8183-disp-gamma"},
> +	{ .compatible = "mediatek,mt8183-disp-gamma",
> +	  .data = &mt8183_gamma_driver_data},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
Yongqiang Niu April 26, 2022, 2:39 a.m. UTC | #4
On Tue, 2022-04-26 at 09:58 +0800, CK Hu wrote:
> Hi, Yongqiang:
> 
> On Wed, 2022-04-20 at 21:06 +0800, Yongqiang Niu wrote:
> > From: Yongqiang Niu <yongqiang.niu@mediatek.corp-partner.google.com
> > >
> > 
> > mt8183 gamma module usage is different with before soc,
> > gamma odd(index start from 0) lut value set to hardware
> > register should be
> > the difference of current lut value with last lut value.
> > 
> > gamma function support both increase and decrease lut.
> 
> How to set decrease lut?
> 
> Original lut:
> 12 10 8 6 4 2
> 
> Does diff lut look like this?
> 12 [2] 8 [2] 4 [2]
> 
yes

> How does hardware know that this is increase lut or decrease lut?

gamma register 0x20 , bit 2, 
if this bit set to 1, hardware will know lut is decrease.
this bit default is 0, that means lut is increase

> 
> > chrome os app set increase lut normally.
> > 
> > for increase lut example, chrome os user space set lut
> > like this(only r chanel for example):
> > 2 4 6 8 10 12.
> > 1) mt8183 gamma driver should set the gamma lut to hardware
> > register like this:
> > 2 [2] 6 [8] 10 [2]
> 
> 2 [2] 6 [2] 10 [2]
> 
> > the value with [] is the difference value
> > 2) gamma hardware will restore the lut when apply gamma
> > function to display
> 
> I don't know why do you mention the 'restore', any modification is
> related to this?
> 
that means gamma
hardware process display data with original lut
> > 
> > Signed-off-by: Yongqiang Niu <
> > yongqiang.niu@mediatek.corp-partner.google.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_aal.c   |  4 ++-
> >  drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  2 +-
> >  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34 +++++++++++++++++++
> > ----
> >  3 files changed, 32 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > index f46d4ab73d6a..e6378b074a17 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > @@ -23,6 +23,7 @@
> >  
> >  struct mtk_disp_aal_data {
> >  	bool has_gamma;
> > +	bool lut_diff;
> >  };
> >  
> >  /**
> > @@ -66,7 +67,7 @@ void mtk_aal_gamma_set(struct device *dev, struct
> > drm_crtc_state *state)
> >  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> >  
> >  	if (aal->data && aal->data->has_gamma)
> > -		mtk_gamma_set_common(aal->regs, state);
> > +		mtk_gamma_set_common(aal->regs, state, aal->data-
> > > lut_diff);
> 
> gamma in aal does not support diff lut, so this would make things
> simple.
> 
> mtk_gamma_set_common(aal->regs, state, false);

fix next version
> 
> Regards,
> CK
> 
> >  }
> >  
> >  void mtk_aal_start(struct device *dev)
> > @@ -148,6 +149,7 @@ static int mtk_disp_aal_remove(struct
> > platform_device *pdev)
> >  
> >  static const struct mtk_disp_aal_data mt8173_aal_driver_data = {
> >  	.has_gamma = true,
> > +	.lut_diff = false,
> >  };
> >  
> >  static const struct of_device_id mtk_disp_aal_driver_dt_match[] =
> > {
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > index 86c3068894b1..3380651c6707 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -51,7 +51,7 @@ void mtk_gamma_config(struct device *dev,
> > unsigned
> > int w,
> >  		      unsigned int h, unsigned int vrefresh,
> >  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> >  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > *state);
> > -void mtk_gamma_set_common(void __iomem *regs, struct
> > drm_crtc_state
> > *state);
> > +void mtk_gamma_set_common(void __iomem *regs, struct
> > drm_crtc_state
> > *state, bool lut_diff);
> >  void mtk_gamma_start(struct device *dev);
> >  void mtk_gamma_stop(struct device *dev);
> >  
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > index 3a5815ab4079..fec2e9a5b60d 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > @@ -27,6 +27,7 @@
> >  
> >  struct mtk_disp_gamma_data {
> >  	bool has_dither;
> > +	bool lut_diff;
> >  };
> >  
> >  /*
> > @@ -53,12 +54,13 @@ void mtk_gamma_clk_disable(struct device *dev)
> >  	clk_disable_unprepare(gamma->clk);
> >  }
> >  
> > -void mtk_gamma_set_common(void __iomem *regs, struct
> > drm_crtc_state
> > *state)
> > +void mtk_gamma_set_common(void __iomem *regs, struct
> > drm_crtc_state
> > *state, bool lut_diff)
> >  {
> >  	unsigned int i, reg;
> >  	struct drm_color_lut *lut;
> >  	void __iomem *lut_base;
> >  	u32 word;
> > +	u32 diff[3] = {0};
> >  
> >  	if (state->gamma_lut) {
> >  		reg = readl(regs + DISP_GAMMA_CFG);
> > @@ -67,9 +69,20 @@ void mtk_gamma_set_common(void __iomem *regs,
> > struct drm_crtc_state *state)
> >  		lut_base = regs + DISP_GAMMA_LUT;
> >  		lut = (struct drm_color_lut *)state->gamma_lut->data;
> >  		for (i = 0; i < MTK_LUT_SIZE; i++) {
> > -			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) <<
> > 20) +
> > -				(((lut[i].green >> 6) & LUT_10BIT_MASK)
> > << 10) +
> > -				((lut[i].blue >> 6) & LUT_10BIT_MASK);
> > +
> > +			if (!lut_diff || (i % 2 == 0)) {
> > +				word = (((lut[i].red >> 6) &
> > LUT_10BIT_MASK) << 20) +
> > +					(((lut[i].green >> 6) &
> > LUT_10BIT_MASK) << 10) +
> > +					((lut[i].blue >> 6) &
> > LUT_10BIT_MASK);
> > +			} else {
> > +				diff[0] = abs((lut[i].red >> 6) -
> > (lut[i - 1].red >> 6));
> > +				diff[1] = abs((lut[i].green >> 6) -
> > (lut[i - 1].green >> 6));
> > +				diff[2] = abs((lut[i].blue >> 6) -
> > (lut[i - 1].blue >> 6));
> > +
> > +				word = ((diff[0] & LUT_10BIT_MASK) <<
> > 20) +
> > +					((diff[1] & LUT_10BIT_MASK) <<
> > 10) +
> > +					(diff[2] & LUT_10BIT_MASK);
> > +			}
> >  			writel(word, (lut_base + i * 4));
> >  		}
> >  	}
> > @@ -78,8 +91,12 @@ void mtk_gamma_set_common(void __iomem *regs,
> > struct drm_crtc_state *state)
> >  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > *state)
> >  {
> >  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> > +	bool lut_diff = false;
> > +
> > +	if (gamma->data)
> > +		lut_diff = gamma->data->lut_diff;
> >  
> > -	mtk_gamma_set_common(gamma->regs, state);
> > +	mtk_gamma_set_common(gamma->regs, state, lut_diff);
> >  }
> >  
> >  void mtk_gamma_config(struct device *dev, unsigned int w,
> > @@ -176,10 +193,15 @@ static const struct mtk_disp_gamma_data
> > mt8173_gamma_driver_data = {
> >  	.has_dither = true,
> >  };
> >  
> > +static const struct mtk_disp_gamma_data mt8183_gamma_driver_data =
> > {
> > +	.lut_diff = true,
> > +};
> > +
> >  static const struct of_device_id mtk_disp_gamma_driver_dt_match[]
> > =
> > {
> >  	{ .compatible = "mediatek,mt8173-disp-gamma",
> >  	  .data = &mt8173_gamma_driver_data},
> > -	{ .compatible = "mediatek,mt8183-disp-gamma"},
> > +	{ .compatible = "mediatek,mt8183-disp-gamma",
> > +	  .data = &mt8183_gamma_driver_data},
> >  	{},
> >  };
> >  MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
> 
>
CK Hu (胡俊光) April 26, 2022, 2:47 a.m. UTC | #5
On Tue, 2022-04-26 at 10:39 +0800, yongqiang.niu wrote:
> On Tue, 2022-04-26 at 09:58 +0800, CK Hu wrote:
> > Hi, Yongqiang:
> > 
> > On Wed, 2022-04-20 at 21:06 +0800, Yongqiang Niu wrote:
> > > From: Yongqiang Niu <
> > > yongqiang.niu@mediatek.corp-partner.google.com
> > > > 
> > > 
> > > mt8183 gamma module usage is different with before soc,
> > > gamma odd(index start from 0) lut value set to hardware
> > > register should be
> > > the difference of current lut value with last lut value.
> > > 
> > > gamma function support both increase and decrease lut.
> > 
> > How to set decrease lut?
> > 
> > Original lut:
> > 12 10 8 6 4 2
> > 
> > Does diff lut look like this?
> > 12 [2] 8 [2] 4 [2]
> > 
> 
> yes
> 
> > How does hardware know that this is increase lut or decrease lut?
> 
> gamma register 0x20 , bit 2, 
> if this bit set to 1, hardware will know lut is decrease.
> this bit default is 0, that means lut is increase
> 
> > 
> > > chrome os app set increase lut normally.
> > > 
> > > for increase lut example, chrome os user space set lut
> > > like this(only r chanel for example):
> > > 2 4 6 8 10 12.
> > > 1) mt8183 gamma driver should set the gamma lut to hardware
> > > register like this:
> > > 2 [2] 6 [8] 10 [2]
> > 
> > 2 [2] 6 [2] 10 [2]
> > 
> > > the value with [] is the difference value
> > > 2) gamma hardware will restore the lut when apply gamma
> > > function to display
> > 
> > I don't know why do you mention the 'restore', any modification is
> > related to this?
> > 
> 
> that means gamma
> hardware process display data with original lut
> > > 
> > > Signed-off-by: Yongqiang Niu <
> > > yongqiang.niu@mediatek.corp-partner.google.com>
> > > ---
> > >  drivers/gpu/drm/mediatek/mtk_disp_aal.c   |  4 ++-
> > >  drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  2 +-
> > >  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34
> > > +++++++++++++++++++
> > > ----
> > >  3 files changed, 32 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > index f46d4ab73d6a..e6378b074a17 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > @@ -23,6 +23,7 @@
> > >  
> > >  struct mtk_disp_aal_data {
> > >  	bool has_gamma;
> > > +	bool lut_diff;
> > >  };
> > >  
> > >  /**
> > > @@ -66,7 +67,7 @@ void mtk_aal_gamma_set(struct device *dev,
> > > struct
> > > drm_crtc_state *state)
> > >  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> > >  
> > >  	if (aal->data && aal->data->has_gamma)
> > > -		mtk_gamma_set_common(aal->regs, state);
> > > +		mtk_gamma_set_common(aal->regs, state, aal->data-
> > > > lut_diff);
> > 
> > gamma in aal does not support diff lut, so this would make things
> > simple.
> > 
> > mtk_gamma_set_common(aal->regs, state, false);
> 
> fix next version
> > 
> > Regards,
> > CK
> > 
> > >  }
> > >  
> > >  void mtk_aal_start(struct device *dev)
> > > @@ -148,6 +149,7 @@ static int mtk_disp_aal_remove(struct
> > > platform_device *pdev)
> > >  
> > >  static const struct mtk_disp_aal_data mt8173_aal_driver_data = {
> > >  	.has_gamma = true,
> > > +	.lut_diff = false,
> > >  };
> > >  
> > >  static const struct of_device_id mtk_disp_aal_driver_dt_match[]
> > > =
> > > {
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > index 86c3068894b1..3380651c6707 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > @@ -51,7 +51,7 @@ void mtk_gamma_config(struct device *dev,
> > > unsigned
> > > int w,
> > >  		      unsigned int h, unsigned int vrefresh,
> > >  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> > >  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > > *state);
> > > -void mtk_gamma_set_common(void __iomem *regs, struct
> > > drm_crtc_state
> > > *state);
> > > +void mtk_gamma_set_common(void __iomem *regs, struct
> > > drm_crtc_state
> > > *state, bool lut_diff);
> > >  void mtk_gamma_start(struct device *dev);
> > >  void mtk_gamma_stop(struct device *dev);
> > >  
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > > b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > > index 3a5815ab4079..fec2e9a5b60d 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > > @@ -27,6 +27,7 @@
> > >  
> > >  struct mtk_disp_gamma_data {
> > >  	bool has_dither;
> > > +	bool lut_diff;
> > >  };
> > >  
> > >  /*
> > > @@ -53,12 +54,13 @@ void mtk_gamma_clk_disable(struct device
> > > *dev)
> > >  	clk_disable_unprepare(gamma->clk);
> > >  }
> > >  
> > > -void mtk_gamma_set_common(void __iomem *regs, struct
> > > drm_crtc_state
> > > *state)
> > > +void mtk_gamma_set_common(void __iomem *regs, struct
> > > drm_crtc_state
> > > *state, bool lut_diff)
> > >  {
> > >  	unsigned int i, reg;
> > >  	struct drm_color_lut *lut;
> > >  	void __iomem *lut_base;
> > >  	u32 word;
> > > +	u32 diff[3] = {0};
> > >  
> > >  	if (state->gamma_lut) {
> > >  		reg = readl(regs + DISP_GAMMA_CFG);
> > > @@ -67,9 +69,20 @@ void mtk_gamma_set_common(void __iomem *regs,
> > > struct drm_crtc_state *state)
> > >  		lut_base = regs + DISP_GAMMA_LUT;
> > >  		lut = (struct drm_color_lut *)state->gamma_lut->data;
> > >  		for (i = 0; i < MTK_LUT_SIZE; i++) {
> > > -			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) <<
> > > 20) +
> > > -				(((lut[i].green >> 6) & LUT_10BIT_MASK)
> > > << 10) +
> > > -				((lut[i].blue >> 6) & LUT_10BIT_MASK);
> > > +
> > > +			if (!lut_diff || (i % 2 == 0)) {
> > > +				word = (((lut[i].red >> 6) &
> > > LUT_10BIT_MASK) << 20) +
> > > +					(((lut[i].green >> 6) &
> > > LUT_10BIT_MASK) << 10) +
> > > +					((lut[i].blue >> 6) &
> > > LUT_10BIT_MASK);
> > > +			} else {
> > > +				diff[0] = abs((lut[i].red >> 6) -
> > > (lut[i - 1].red >> 6));

You does not set register 0x20[2], so this patch only support increase
lut and abs() is not necessary.

Regards,
CK

> > > +				diff[1] = abs((lut[i].green >> 6) -
> > > (lut[i - 1].green >> 6));
> > > +				diff[2] = abs((lut[i].blue >> 6) -
> > > (lut[i - 1].blue >> 6));
> > > +
> > > +				word = ((diff[0] & LUT_10BIT_MASK) <<
> > > 20) +
> > > +					((diff[1] & LUT_10BIT_MASK) <<
> > > 10) +
> > > +					(diff[2] & LUT_10BIT_MASK);
> > > +			}
> > >  			writel(word, (lut_base + i * 4));
> > >  		}
> > >  	}
> > > @@ -78,8 +91,12 @@ void mtk_gamma_set_common(void __iomem *regs,
> > > struct drm_crtc_state *state)
> > >  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > > *state)
> > >  {
> > >  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> > > +	bool lut_diff = false;
> > > +
> > > +	if (gamma->data)
> > > +		lut_diff = gamma->data->lut_diff;
> > >  
> > > -	mtk_gamma_set_common(gamma->regs, state);
> > > +	mtk_gamma_set_common(gamma->regs, state, lut_diff);
> > >  }
> > >  
> > >  void mtk_gamma_config(struct device *dev, unsigned int w,
> > > @@ -176,10 +193,15 @@ static const struct mtk_disp_gamma_data
> > > mt8173_gamma_driver_data = {
> > >  	.has_dither = true,
> > >  };
> > >  
> > > +static const struct mtk_disp_gamma_data mt8183_gamma_driver_data
> > > =
> > > {
> > > +	.lut_diff = true,
> > > +};
> > > +
> > >  static const struct of_device_id
> > > mtk_disp_gamma_driver_dt_match[]
> > > =
> > > {
> > >  	{ .compatible = "mediatek,mt8173-disp-gamma",
> > >  	  .data = &mt8173_gamma_driver_data},
> > > -	{ .compatible = "mediatek,mt8183-disp-gamma"},
> > > +	{ .compatible = "mediatek,mt8183-disp-gamma",
> > > +	  .data = &mt8183_gamma_driver_data},
> > >  	{},
> > >  };
> > >  MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
> > 
> > 
> 
>
Yongqiang Niu April 26, 2022, 2:50 a.m. UTC | #6
On Tue, 2022-04-26 at 10:47 +0800, CK Hu wrote:
> On Tue, 2022-04-26 at 10:39 +0800, yongqiang.niu wrote:
> > On Tue, 2022-04-26 at 09:58 +0800, CK Hu wrote:
> > > Hi, Yongqiang:
> > > 
> > > On Wed, 2022-04-20 at 21:06 +0800, Yongqiang Niu wrote:
> > > > From: Yongqiang Niu <
> > > > yongqiang.niu@mediatek.corp-partner.google.com
> > > > > 
> > > > 
> > > > mt8183 gamma module usage is different with before soc,
> > > > gamma odd(index start from 0) lut value set to hardware
> > > > register should be
> > > > the difference of current lut value with last lut value.
> > > > 
> > > > gamma function support both increase and decrease lut.
> > > 
> > > How to set decrease lut?
> > > 
> > > Original lut:
> > > 12 10 8 6 4 2
> > > 
> > > Does diff lut look like this?
> > > 12 [2] 8 [2] 4 [2]
> > > 
> > 
> > yes
> > 
> > > How does hardware know that this is increase lut or decrease lut?
> > 
> > gamma register 0x20 , bit 2, 
> > if this bit set to 1, hardware will know lut is decrease.
> > this bit default is 0, that means lut is increase
> > 
> > > 
> > > > chrome os app set increase lut normally.
> > > > 
> > > > for increase lut example, chrome os user space set lut
> > > > like this(only r chanel for example):
> > > > 2 4 6 8 10 12.
> > > > 1) mt8183 gamma driver should set the gamma lut to hardware
> > > > register like this:
> > > > 2 [2] 6 [8] 10 [2]
> > > 
> > > 2 [2] 6 [2] 10 [2]
> > > 
> > > > the value with [] is the difference value
> > > > 2) gamma hardware will restore the lut when apply gamma
> > > > function to display
> > > 
> > > I don't know why do you mention the 'restore', any modification
> > > is
> > > related to this?
> > > 
> > 
> > that means gamma
> > hardware process display data with original lut
> > > > 
> > > > Signed-off-by: Yongqiang Niu <
> > > > yongqiang.niu@mediatek.corp-partner.google.com>
> > > > ---
> > > >  drivers/gpu/drm/mediatek/mtk_disp_aal.c   |  4 ++-
> > > >  drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  2 +-
> > > >  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34
> > > > +++++++++++++++++++
> > > > ----
> > > >  3 files changed, 32 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > > b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > > index f46d4ab73d6a..e6378b074a17 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > > @@ -23,6 +23,7 @@
> > > >  
> > > >  struct mtk_disp_aal_data {
> > > >  	bool has_gamma;
> > > > +	bool lut_diff;
> > > >  };
> > > >  
> > > >  /**
> > > > @@ -66,7 +67,7 @@ void mtk_aal_gamma_set(struct device *dev,
> > > > struct
> > > > drm_crtc_state *state)
> > > >  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> > > >  
> > > >  	if (aal->data && aal->data->has_gamma)
> > > > -		mtk_gamma_set_common(aal->regs, state);
> > > > +		mtk_gamma_set_common(aal->regs, state, aal-
> > > > >data-
> > > > > lut_diff);
> > > 
> > > gamma in aal does not support diff lut, so this would make things
> > > simple.
> > > 
> > > mtk_gamma_set_common(aal->regs, state, false);
> > 
> > fix next version
> > > 
> > > Regards,
> > > CK
> > > 
> > > >  }
> > > >  
> > > >  void mtk_aal_start(struct device *dev)
> > > > @@ -148,6 +149,7 @@ static int mtk_disp_aal_remove(struct
> > > > platform_device *pdev)
> > > >  
> > > >  static const struct mtk_disp_aal_data mt8173_aal_driver_data =
> > > > {
> > > >  	.has_gamma = true,
> > > > +	.lut_diff = false,
> > > >  };
> > > >  
> > > >  static const struct of_device_id
> > > > mtk_disp_aal_driver_dt_match[]
> > > > =
> > > > {
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > > index 86c3068894b1..3380651c6707 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > > @@ -51,7 +51,7 @@ void mtk_gamma_config(struct device *dev,
> > > > unsigned
> > > > int w,
> > > >  		      unsigned int h, unsigned int vrefresh,
> > > >  		      unsigned int bpc, struct cmdq_pkt
> > > > *cmdq_pkt);
> > > >  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > > > *state);
> > > > -void mtk_gamma_set_common(void __iomem *regs, struct
> > > > drm_crtc_state
> > > > *state);
> > > > +void mtk_gamma_set_common(void __iomem *regs, struct
> > > > drm_crtc_state
> > > > *state, bool lut_diff);
> > > >  void mtk_gamma_start(struct device *dev);
> > > >  void mtk_gamma_stop(struct device *dev);
> > > >  
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > > > b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > > > index 3a5815ab4079..fec2e9a5b60d 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > > > @@ -27,6 +27,7 @@
> > > >  
> > > >  struct mtk_disp_gamma_data {
> > > >  	bool has_dither;
> > > > +	bool lut_diff;
> > > >  };
> > > >  
> > > >  /*
> > > > @@ -53,12 +54,13 @@ void mtk_gamma_clk_disable(struct device
> > > > *dev)
> > > >  	clk_disable_unprepare(gamma->clk);
> > > >  }
> > > >  
> > > > -void mtk_gamma_set_common(void __iomem *regs, struct
> > > > drm_crtc_state
> > > > *state)
> > > > +void mtk_gamma_set_common(void __iomem *regs, struct
> > > > drm_crtc_state
> > > > *state, bool lut_diff)
> > > >  {
> > > >  	unsigned int i, reg;
> > > >  	struct drm_color_lut *lut;
> > > >  	void __iomem *lut_base;
> > > >  	u32 word;
> > > > +	u32 diff[3] = {0};
> > > >  
> > > >  	if (state->gamma_lut) {
> > > >  		reg = readl(regs + DISP_GAMMA_CFG);
> > > > @@ -67,9 +69,20 @@ void mtk_gamma_set_common(void __iomem
> > > > *regs,
> > > > struct drm_crtc_state *state)
> > > >  		lut_base = regs + DISP_GAMMA_LUT;
> > > >  		lut = (struct drm_color_lut *)state->gamma_lut-
> > > > >data;
> > > >  		for (i = 0; i < MTK_LUT_SIZE; i++) {
> > > > -			word = (((lut[i].red >> 6) &
> > > > LUT_10BIT_MASK) <<
> > > > 20) +
> > > > -				(((lut[i].green >> 6) &
> > > > LUT_10BIT_MASK)
> > > > << 10) +
> > > > -				((lut[i].blue >> 6) &
> > > > LUT_10BIT_MASK);
> > > > +
> > > > +			if (!lut_diff || (i % 2 == 0)) {
> > > > +				word = (((lut[i].red >> 6) &
> > > > LUT_10BIT_MASK) << 20) +
> > > > +					(((lut[i].green >> 6) &
> > > > LUT_10BIT_MASK) << 10) +
> > > > +					((lut[i].blue >> 6) &
> > > > LUT_10BIT_MASK);
> > > > +			} else {
> > > > +				diff[0] = abs((lut[i].red >> 6)
> > > > -
> > > > (lut[i - 1].red >> 6));
> 
> You does not set register 0x20[2], so this patch only support
> increase
> lut and abs() is not necessary.
> 
> Regards,
> CK

abs will be removed in next version
> 
> > > > +				diff[1] = abs((lut[i].green >>
> > > > 6) -
> > > > (lut[i - 1].green >> 6));
> > > > +				diff[2] = abs((lut[i].blue >>
> > > > 6) -
> > > > (lut[i - 1].blue >> 6));
> > > > +
> > > > +				word = ((diff[0] &
> > > > LUT_10BIT_MASK) <<
> > > > 20) +
> > > > +					((diff[1] &
> > > > LUT_10BIT_MASK) <<
> > > > 10) +
> > > > +					(diff[2] &
> > > > LUT_10BIT_MASK);
> > > > +			}
> > > >  			writel(word, (lut_base + i * 4));
> > > >  		}
> > > >  	}
> > > > @@ -78,8 +91,12 @@ void mtk_gamma_set_common(void __iomem
> > > > *regs,
> > > > struct drm_crtc_state *state)
> > > >  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > > > *state)
> > > >  {
> > > >  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> > > > +	bool lut_diff = false;
> > > > +
> > > > +	if (gamma->data)
> > > > +		lut_diff = gamma->data->lut_diff;
> > > >  
> > > > -	mtk_gamma_set_common(gamma->regs, state);
> > > > +	mtk_gamma_set_common(gamma->regs, state, lut_diff);
> > > >  }
> > > >  
> > > >  void mtk_gamma_config(struct device *dev, unsigned int w,
> > > > @@ -176,10 +193,15 @@ static const struct mtk_disp_gamma_data
> > > > mt8173_gamma_driver_data = {
> > > >  	.has_dither = true,
> > > >  };
> > > >  
> > > > +static const struct mtk_disp_gamma_data
> > > > mt8183_gamma_driver_data
> > > > =
> > > > {
> > > > +	.lut_diff = true,
> > > > +};
> > > > +
> > > >  static const struct of_device_id
> > > > mtk_disp_gamma_driver_dt_match[]
> > > > =
> > > > {
> > > >  	{ .compatible = "mediatek,mt8173-disp-gamma",
> > > >  	  .data = &mt8173_gamma_driver_data},
> > > > -	{ .compatible = "mediatek,mt8183-disp-gamma"},
> > > > +	{ .compatible = "mediatek,mt8183-disp-gamma",
> > > > +	  .data = &mt8183_gamma_driver_data},
> > > >  	{},
> > > >  };
> > > >  MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
> > > 
> > > 
> > 
> > 
> 
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index f46d4ab73d6a..e6378b074a17 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -23,6 +23,7 @@ 
 
 struct mtk_disp_aal_data {
 	bool has_gamma;
+	bool lut_diff;
 };
 
 /**
@@ -66,7 +67,7 @@  void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
 	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
 
 	if (aal->data && aal->data->has_gamma)
-		mtk_gamma_set_common(aal->regs, state);
+		mtk_gamma_set_common(aal->regs, state, aal->data->lut_diff);
 }
 
 void mtk_aal_start(struct device *dev)
@@ -148,6 +149,7 @@  static int mtk_disp_aal_remove(struct platform_device *pdev)
 
 static const struct mtk_disp_aal_data mt8173_aal_driver_data = {
 	.has_gamma = true,
+	.lut_diff = false,
 };
 
 static const struct of_device_id mtk_disp_aal_driver_dt_match[] = {
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index 86c3068894b1..3380651c6707 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -51,7 +51,7 @@  void mtk_gamma_config(struct device *dev, unsigned int w,
 		      unsigned int h, unsigned int vrefresh,
 		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
 void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state);
-void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state);
+void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state, bool lut_diff);
 void mtk_gamma_start(struct device *dev);
 void mtk_gamma_stop(struct device *dev);
 
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 3a5815ab4079..fec2e9a5b60d 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -27,6 +27,7 @@ 
 
 struct mtk_disp_gamma_data {
 	bool has_dither;
+	bool lut_diff;
 };
 
 /*
@@ -53,12 +54,13 @@  void mtk_gamma_clk_disable(struct device *dev)
 	clk_disable_unprepare(gamma->clk);
 }
 
-void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state)
+void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state, bool lut_diff)
 {
 	unsigned int i, reg;
 	struct drm_color_lut *lut;
 	void __iomem *lut_base;
 	u32 word;
+	u32 diff[3] = {0};
 
 	if (state->gamma_lut) {
 		reg = readl(regs + DISP_GAMMA_CFG);
@@ -67,9 +69,20 @@  void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state)
 		lut_base = regs + DISP_GAMMA_LUT;
 		lut = (struct drm_color_lut *)state->gamma_lut->data;
 		for (i = 0; i < MTK_LUT_SIZE; i++) {
-			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
-				(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
-				((lut[i].blue >> 6) & LUT_10BIT_MASK);
+
+			if (!lut_diff || (i % 2 == 0)) {
+				word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
+					(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
+					((lut[i].blue >> 6) & LUT_10BIT_MASK);
+			} else {
+				diff[0] = abs((lut[i].red >> 6) - (lut[i - 1].red >> 6));
+				diff[1] = abs((lut[i].green >> 6) - (lut[i - 1].green >> 6));
+				diff[2] = abs((lut[i].blue >> 6) - (lut[i - 1].blue >> 6));
+
+				word = ((diff[0] & LUT_10BIT_MASK) << 20) +
+					((diff[1] & LUT_10BIT_MASK) << 10) +
+					(diff[2] & LUT_10BIT_MASK);
+			}
 			writel(word, (lut_base + i * 4));
 		}
 	}
@@ -78,8 +91,12 @@  void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state)
 void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 {
 	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
+	bool lut_diff = false;
+
+	if (gamma->data)
+		lut_diff = gamma->data->lut_diff;
 
-	mtk_gamma_set_common(gamma->regs, state);
+	mtk_gamma_set_common(gamma->regs, state, lut_diff);
 }
 
 void mtk_gamma_config(struct device *dev, unsigned int w,
@@ -176,10 +193,15 @@  static const struct mtk_disp_gamma_data mt8173_gamma_driver_data = {
 	.has_dither = true,
 };
 
+static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
+	.lut_diff = true,
+};
+
 static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
 	{ .compatible = "mediatek,mt8173-disp-gamma",
 	  .data = &mt8173_gamma_driver_data},
-	{ .compatible = "mediatek,mt8183-disp-gamma"},
+	{ .compatible = "mediatek,mt8183-disp-gamma",
+	  .data = &mt8183_gamma_driver_data},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);