Hi, Yongqiang:

On Mon, 2021-04-12 at 14:35 +0800, Yongqiang Niu wrote:
> gamma lut set in vsync active will caused display flash issue
> set gamma lut with cmdq 

In MT8173, it's ok to set gammma out of vblank period. Why do you
setting gamma in vblank in this patch?

Regards,
CK

> 
> Signed-off-by: Yongqiang Niu <yongqiang....@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c     |  4 ++--
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  7 ++++---
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 11 ++++++-----
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     | 18 +++++++++++-------
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  8 +++++---
>  5 files changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c 
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 64b4528..c8e178e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -59,12 +59,12 @@ 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_SIZE);
>  }
>  
> -void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
> +void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state, 
> struct cmdq_pkt *cmdq_pkt)
>  {
>       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, &aal->cmdq_reg, state, 
> cmdq_pkt);
>  }
>  
>  void mtk_aal_start(struct device *dev)
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h 
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index 86c3068..c2e7dcb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -14,7 +14,7 @@
>  void mtk_aal_config(struct device *dev, unsigned int w,
>                   unsigned int h, unsigned int vrefresh,
>                   unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> -void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state);
> +void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state, 
> struct cmdq_pkt *cmdq_pkt);
>  void mtk_aal_start(struct device *dev);
>  void mtk_aal_stop(struct device *dev);
>  
> @@ -50,8 +50,9 @@ void mtk_dither_set_common(void __iomem *regs, struct 
> cmdq_client_reg *cmdq_reg,
>  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(struct device *dev, struct drm_crtc_state *state, struct 
> cmdq_pkt *cmdq_pkt);
> +void mtk_gamma_set_common(void __iomem *regs, struct cmdq_client_reg 
> *cmdq_reg,
> +                       struct drm_crtc_state *state, struct cmdq_pkt 
> *cmdq_pkt);
>  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 3ebf91e..99a4ff3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -55,7 +55,8 @@ 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 cmdq_client_reg 
> *cmdq_reg,
> +                       struct drm_crtc_state *state, struct cmdq_pkt 
> *cmdq_pkt)
>  {
>       unsigned int i, reg;
>       struct drm_color_lut *lut;
> @@ -65,23 +66,23 @@ void mtk_gamma_set_common(void __iomem *regs, struct 
> drm_crtc_state *state)
>       if (state->gamma_lut) {
>               reg = readl(regs + DISP_GAMMA_CFG);
>               reg = reg | GAMMA_LUT_EN;
> -             writel(reg, regs + DISP_GAMMA_CFG);
> +             mtk_ddp_write(cmdq_pkt, reg, cmdq_reg, regs, DISP_GAMMA_CFG);
>               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);
> -                     writel(word, (lut_base + i * 4));
> +                     mtk_ddp_write(cmdq_pkt, word, cmdq_reg, regs, (lut_base 
> + i * 4));
>               }
>       }
>  }
>  
> -void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
> +void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state, struct 
> cmdq_pkt *cmdq_pkt)
>  {
>       struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
>  
> -     mtk_gamma_set_common(gamma->regs, state);
> +     mtk_gamma_set_common(gamma->regs, &gamma->cmdq_reg, state, cmdq_pkt);
>  }
>  
>  void mtk_gamma_config(struct device *dev, unsigned int w,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 8b0de90..73428f0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -423,6 +423,15 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc,
>               }
>               mtk_crtc->pending_async_planes = false;
>       }
> +
> +     if (crtc->state->color_mgmt_changed) {
> +             int i;
> +
> +             for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> +                     mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state, 
> cmdq_handle);
> +                     mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
> +             }
> +     }
>  }
>  
>  static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> @@ -464,7 +473,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc 
> *mtk_crtc)
>  #if IS_REACHABLE(CONFIG_MTK_CMDQ)
>       if (mtk_crtc->cmdq_client) {
>               mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
> -             cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
> +             cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, 2 * 
> PAGE_SIZE);
>               cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
>               cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
>               mtk_crtc_ddp_config(crtc, cmdq_handle);
> @@ -616,15 +625,10 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc 
> *crtc,
>                                     struct drm_atomic_state *state)
>  {
>       struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> -     int i;
>  
>       if (mtk_crtc->event)
>               mtk_crtc->pending_needs_vblank = true;
> -     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);
> -             }
> +
>       mtk_drm_crtc_hw_config(mtk_crtc);
>  }
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h 
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index bb914d9..bffa58d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -60,7 +60,8 @@ struct mtk_ddp_comp_funcs {
>                            struct mtk_plane_state *state,
>                            struct cmdq_pkt *cmdq_pkt);
>       void (*gamma_set)(struct device *dev,
> -                       struct drm_crtc_state *state);
> +                       struct drm_crtc_state *state,
> +                       struct cmdq_pkt *cmdq_pkt);
>       void (*bgclr_in_on)(struct device *dev);
>       void (*bgclr_in_off)(struct device *dev);
>       void (*ctm_set)(struct device *dev,
> @@ -160,10 +161,11 @@ static inline void mtk_ddp_comp_layer_config(struct 
> mtk_ddp_comp *comp,
>  }
>  
>  static inline void mtk_ddp_gamma_set(struct mtk_ddp_comp *comp,
> -                                  struct drm_crtc_state *state)
> +                                  struct drm_crtc_state *state,
> +                                  struct cmdq_pkt *cmdq_pkt)
>  {
>       if (comp->funcs && comp->funcs->gamma_set)
> -             comp->funcs->gamma_set(comp->dev, state);
> +             comp->funcs->gamma_set(comp->dev, state, cmdq_pkt);
>  }
>  
>  static inline void mtk_ddp_comp_bgclr_in_on(struct mtk_ddp_comp *comp)

Reply via email to