From: CK Hu <ck.hu@mediatek.com>
To: Yongqiang Niu <yongqiang.niu@mediatek.com>
Cc: Chun-Kuang Hu <chunkuang.hu@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Philipp Zabel <p.zabel@pengutronix.de>,
David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
Mark Rutland <mark.rutland@arm.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
<dri-devel@lists.freedesktop.org>, <devicetree@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-mediatek@lists.infradead.org>,
<linux-kernel@vger.kernel.org>,
<Project_Global_Chrome_Upstream_Group@mediatek.com>
Subject: Re: [PATCH v1, 3/3] drm/mediatek: gamma set with cmdq
Date: Mon, 12 Apr 2021 16:28:47 +0800 [thread overview]
Message-ID: <1618216127.12699.4.camel@mtksdaap41> (raw)
In-Reply-To: <1618209347-10816-4-git-send-email-yongqiang.niu@mediatek.com>
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.niu@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)
next prev parent reply other threads:[~2021-04-12 8:28 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-12 6:35 [PATCH v1, 0/3] gamma set with cmdq Yongqiang Niu
2021-04-12 6:35 ` [PATCH v1, 1/3] drm/mediatek: Separate aal module Yongqiang Niu
2021-04-12 8:26 ` CK Hu
2021-04-12 6:35 ` [PATCH v1, 2/3] arm64: dts: mt8183: refine aal compatible name Yongqiang Niu
2021-04-12 6:35 ` [PATCH v1, 3/3] drm/mediatek: gamma set with cmdq Yongqiang Niu
2021-04-12 8:28 ` CK Hu [this message]
2021-04-12 8:45 ` Yongqiang Niu
2021-04-12 9:05 ` CK Hu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1618216127.12699.4.camel@mtksdaap41 \
--to=ck.hu@mediatek.com \
--cc=Project_Global_Chrome_Upstream_Group@mediatek.com \
--cc=airlied@linux.ie \
--cc=chunkuang.hu@kernel.org \
--cc=daniel@ffwll.ch \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=matthias.bgg@gmail.com \
--cc=p.zabel@pengutronix.de \
--cc=robh+dt@kernel.org \
--cc=yongqiang.niu@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).