linux-kernel - Re: [PATCH v2 2/7] drm/mediatek: fix CCORR mtk_ctm_s31_32_to_s1

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <8b589879a5e4b60041bee5303697b2a6a707ee9e.camel@mediatek.com>
Date: Thu, 7 Aug 2025 06:19:05 +0000
From: Jay Liu (刘博) <Jay.Liu@...iatek.com>
To: Yongqiang Niu (牛永强)
	<yongqiang.niu@...iatek.com>, "chunkuang.hu@...nel.org"
	<chunkuang.hu@...nel.org>, "tzimmermann@...e.de" <tzimmermann@...e.de>,
	"simona@...ll.ch" <simona@...ll.ch>, "mripard@...nel.org"
	<mripard@...nel.org>, "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
	CK Hu (胡俊光) <ck.hu@...iatek.com>,
	"maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>, "robh@...nel.org"
	<robh@...nel.org>, "hsinyi@...omium.org" <hsinyi@...omium.org>,
	"airlied@...il.com" <airlied@...il.com>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
CC: "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>
Subject: Re: [PATCH v2 2/7] drm/mediatek: fix CCORR mtk_ctm_s31_32_to_s1_n
 function issue

On Wed, 2025-08-06 at 06:37 +0000, CK Hu (胡俊光) wrote:
> On Sun, 2025-07-27 at 15:15 +0800, Jay Liu wrote:
> > if matrixbit is 11,
> > The range of color matrix is from 0 to (BIT(12) - 1).
> > Values from 0 to (BIT(11) - 1) represent positive numbers,
> > values from BIT(11) to (BIT(12) - 1) represent negative numbers.
> > For example, -1 need converted to 8191.
> > so convert S31.32 to HW Q2.11 format by
> > drm_color_ctm_s31_32_to_qm_n,
> > and set int_bits to 2.
> 
> You change the behavior of MT8183 CCORR and MT8192 CCORR.
> These two SoC has work for a long time.
> Does both SoC really have bug?
> 
> In comment below, it shows that HW S1.n format.
> The patch sender has much information about the hardware information.
> Would they make mistake?
> If only MT8196 CCORR has the format qm_n, use private data to
> distinguish the behavior.
> 
> Regards,
> CK
> 
Yes. We received an email from customer, they found that the previous
patch failed to handle negative values correctly, so the result was not
as expected and they would like this issue fixed in the current update.
We also consulted with the DE; the hardware itself supports negative
numbers and all ICs behave the same in this regard. The current patch
converts negative values into a format that the hardware can properly
recognize.

Best Regards,
Jay Liu

> > 
> > Fixes: 738ed4156fba ("drm/mediatek: Add matrix_bits private data
> > for ccorr")
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@...labora.com>
> > Signed-off-by: Jay Liu <jay.liu@...iatek.com>
> > Signed-off-by: 20220315152503 created <jay.liu@...iatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_ccorr.c | 24 ++-----------------
> > ----
> >  1 file changed, 2 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> > index 85ba109d6383..b097c20877f3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> > @@ -80,27 +80,6 @@ void mtk_ccorr_stop(struct device *dev)
> >  	writel_relaxed(0x0, ccorr->regs + DISP_CCORR_EN);
> >  }
> >  
> > -/* Converts a DRM S31.32 value to the HW S1.n format. */
> > -static u16 mtk_ctm_s31_32_to_s1_n(u64 in, u32 n)
> > -{
> > -	u16 r;
> > -
> > -	/* Sign bit. */
> > -	r = in & BIT_ULL(63) ? BIT(n + 1) : 0;
> > -
> > -	if ((in & GENMASK_ULL(62, 33)) > 0) {
> > -		/* identity value 0x100000000 -> 0x400(mt8183), */
> > -		/* identity value 0x100000000 -> 0x800(mt8192), */
> > -		/* if bigger this, set it to max 0x7ff. */
> > -		r |= GENMASK(n, 0);
> > -	} else {
> > -		/* take the n+1 most important bits. */
> > -		r |= (in >> (32 - n)) & GENMASK(n, 0);
> > -	}
> > -
> > -	return r;
> > -}
> > -
> >  bool mtk_ccorr_ctm_set(struct device *dev, struct drm_crtc_state
> > *state)
> >  {
> >  	struct mtk_disp_ccorr *ccorr = dev_get_drvdata(dev);
> > @@ -109,6 +88,7 @@ bool mtk_ccorr_ctm_set(struct device *dev,
> > struct drm_crtc_state *state)
> >  	const u64 *input;
> >  	uint16_t coeffs[9] = { 0 };
> >  	int i;
> > +	int int_bits = 2;
> >  	struct cmdq_pkt *cmdq_pkt = NULL;
> >  	u32 matrix_bits = ccorr->data->matrix_bits;
> >  
> > @@ -119,7 +99,7 @@ bool mtk_ccorr_ctm_set(struct device *dev,
> > struct drm_crtc_state *state)
> >  	input = ctm->matrix;
> >  
> >  	for (i = 0; i < ARRAY_SIZE(coeffs); i++)
> > -		coeffs[i] = mtk_ctm_s31_32_to_s1_n(input[i],
> > matrix_bits);
> > +		coeffs[i] = drm_color_ctm_s31_32_to_qm_n(input[i],
> > int_bits, matrix_bits);
> >  
> >  	mtk_ddp_write(cmdq_pkt, coeffs[0] << 16 | coeffs[1],
> >  		      &ccorr->cmdq_reg, ccorr->regs,
> > DISP_CCORR_COEF_0);
> 
>