linux-kernel - Re: [PATCH v3 14/17] drm/mediatek: add OUTPROC support for MT8196

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a05242578fab11f76873e39129066b58c2a0a126.camel@mediatek.com>
Date: Fri, 4 Jul 2025 07:24:40 +0000
From: Paul-pl Chen (陳柏霖) <Paul-pl.Chen@...iatek.com>
To: CK Hu (胡俊光) <ck.hu@...iatek.com>,
	"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>, "AngeloGioacchino Del
 Regno" <angelogioacchino.delregno@...labora.com>
CC: Sunny Shen (沈姍姍) <Sunny.Shen@...iatek.com>,
	Sirius Wang (王皓昱) <Sirius.Wang@...iatek.com>,
	Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
	Xiandong Wang (王先冬)
	<Xiandong.Wang@...iatek.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Project_Global_Chrome_Upstream_Group
	<Project_Global_Chrome_Upstream_Group@...iatek.com>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	Jason-JH Lin (林睿祥) <Jason-JH.Lin@...iatek.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"fshao@...omium.org" <fshao@...omium.org>, "p.zabel@...gutronix.de"
	<p.zabel@...gutronix.de>, Singo Chang (張興國)
	<Singo.Chang@...iatek.com>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>, "treapking@...omium.org" <treapking@...omium.org>
Subject: Re: [PATCH v3 14/17] drm/mediatek: add OUTPROC support for MT8196

On Wed, 2025-06-18 at 08:23 +0000, CK Hu (胡俊光) wrote:
> On Thu, 2025-05-15 at 17:34 +0800, paul-pl.chen wrote:
> > From: Nancy Lin <nancy.lin@...iatek.com>
> > 
> > OUTPROC handles the post-stage of pixel processing in
> > the overlapping procedure.OUTPROC manages pixels for
> > gamma correction and ensures that pixel values are
> > within the correct range.
> > 
> > Signed-off-by: Nancy Lin <nancy.lin@...iatek.com>
> > Signed-off-by: Paul-pl Chen <paul-pl.chen@...iatek.com>
> > ---
> 
> [snip]
> 
> > +void mtk_disp_outproc_config(struct device *dev, unsigned int w,
> > +			     unsigned int h, unsigned int
> > vrefresh,
> > +			     unsigned int bpc, struct cmdq_pkt
> > *cmdq_pkt)
> > +{
> > +	struct mtk_disp_outproc *priv = dev_get_drvdata(dev);
> > +	u32 tmp;
> > +
> > +	dev_dbg(dev, "%s-w:%d, h:%d\n", __func__, w, h);
> > +
> > +	writel((h << 16) | w, priv->regs +
> > DISP_REG_OVL_OUTPROC_ROI_SIZE);
> > +
> > +	tmp = readl(priv->regs +
> > DISP_REG_OVL_OUTPROC_DATAPATH_CON);
> > +	tmp = (tmp & ~OVL_OUTPROC_DATAPATH_CON_OUTPUT_CLAMP) |
> > +		  (OVL_OUTPROC_DATAPATH_CON_OUTPUT_CLAMP &
> > OVL_OUTPROC_DATAPATH_CON_OUTPUT_CLAMP);
> 
> This would simplified as
> 
> tmp |= OVL_OUTPROC_DATAPATH_CON_OUTPUT_CLAMP;
> 

OK, I will refine the code to simplify this part.

> > +	writel(tmp, priv->regs +
> > DISP_REG_OVL_OUTPROC_DATAPATH_CON);
> > +}
> > +
> > +void mtk_disp_outproc_start(struct device *dev)
> > +{
> > +	struct mtk_disp_outproc *priv = dev_get_drvdata(dev);
> > +	unsigned int tmp;
> > +
> > +	tmp = readl(priv->regs +
> > DISP_REG_OVL_OUTPROC_SHADOW_CTRL);
> > +	tmp = tmp | OVL_OUTPROC_BYPASS_SHADOW;
> 
> tmp |= OVL_OUTPROC_BYPASS_SHADOW;

Understood, I will refine the code to simplify this part.
> 
> > +	writel(tmp, priv->regs +
> > DISP_REG_OVL_OUTPROC_SHADOW_CTRL);
> > +
> > +	writel(0, priv->regs + DISP_REG_OVL_OUTPROC_INTSTA);
> 
> To align OVL driver, clear interrupt status in enable_vblank()
> function.
> 
> 

Understood, I will refine the code and move 
"the clear interrupt status" to function of enable_vblank().
> > +
> > +	tmp = readl(priv->regs + DISP_REG_OVL_OUTPROC_EN);
> > +	tmp = (tmp & ~OVL_OUTPROC_OVL_EN) | (OVL_OUTPROC_OVL_EN &
> > OVL_OUTPROC_OVL_EN);
> 
> This would be simplified as
> 
> tmp |= OVL_OUTPROC_OVL_EN;
> 

Understood, I will simplify the code. 

> One question. In mtk_disp_outproc_stop(), you overwrite the whole
> register of  DISP_REG_OVL_OUTPROC_EN.
> Why here not overwrite the whole register?
> 
> Regards,
> CK
> 
Hi Ck, I will perform an experiment to verify this part.

Best, Paul 
> > +	writel(tmp, priv->regs + DISP_REG_OVL_OUTPROC_EN);
> > +}
> > +
> > +void mtk_disp_outproc_stop(struct device *dev)
> > +{
> > +	struct mtk_disp_outproc *priv = dev_get_drvdata(dev);
> > +
> > +	writel(0, priv->regs + DISP_REG_OVL_OUTPROC_EN);
> > +	writel(OVL_OUTPROC_RST, priv->regs +
> > DISP_REG_OVL_OUTPROC_RST);
> > +	writel(0, priv->regs + DISP_REG_OVL_OUTPROC_RST);
> > +}
> > +
>