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] [day] [month] [year] [list]
Message-ID: <5d87836c4c78f7f98233d5c304ef2770d43b0e2d.camel@mediatek.com>
Date: Thu, 7 Aug 2025 11:30:59 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: "matthias.bgg@...il.com" <matthias.bgg@...il.com>, "tzimmermann@...e.de"
	<tzimmermann@...e.de>, Jay Liu (刘博)
	<Jay.Liu@...iatek.com>, "chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	Yongqiang Niu (牛永强)
	<yongqiang.niu@...iatek.com>, "robh@...nel.org" <robh@...nel.org>,
	"airlied@...il.com" <airlied@...il.com>, "simona@...ll.ch" <simona@...ll.ch>,
	"p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
	"maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
	"mripard@...nel.org" <mripard@...nel.org>, "hsinyi@...omium.org"
	<hsinyi@...omium.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
CC: "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.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 Thu, 2025-08-07 at 06:19 +0000, Jay Liu (刘博) wrote:
> 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.

OK, move this patch out of this series.
Send this patch independently.
And this is a fixup patch, so Add 'Fixes' tag.

Regards,
CK

> 
> 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);
> > 
> >