linux-kernel - Re: [PATCH v3 3/5] drm/mediatek: Support "Pre-multiplied" blending in OVL

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: <3395f4b113a8cb5b689384bdb9599394fadae01e.camel@mediatek.com>
Date: Thu, 11 Jul 2024 01:35:41 +0000
From: Shawn Sung (宋孝謙) <Shawn.Sung@...iatek.com>
To: "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>, "airlied@...il.com"
	<airlied@...il.com>, "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
	"daniel@...ll.ch" <daniel@...ll.ch>,
	"angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>, "chunkuang.hu@...nel.org"
	<chunkuang.hu@...nel.org>
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>,
	"shawn.sung@...iatek.corp-partner.google.com"
	<shawn.sung@...iatek.corp-partner.google.com>,
	CK Hu (胡俊光) <ck.hu@...iatek.com>
Subject: Re: [PATCH v3 3/5] drm/mediatek: Support "Pre-multiplied" blending in
 OVL

Hi Angelo,

On Wed, 2024-07-10 at 14:02 +0200, AngeloGioacchino Del Regno wrote:
> Il 10/07/24 10:52, Hsiao Chien Sung via B4 Relay ha scritto:
> > From: Hsiao Chien Sung <shawn.sung@...iatek.com>
> > 
> > Support "Pre-multiplied" alpha blending mode on in OVL.
> > Before this patch, only the "coverage" mode is supported.
> > 
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@...iatek.com>
> > Reviewed-by: CK Hu <ck.hu@...iatek.com>
> > ---
> >   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 32
> > +++++++++++++++++++++++++-------
> >   1 file changed, 25 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index add671c38613..89b439dcf3a6 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -56,8 +56,12 @@
> >   #define GMC_THRESHOLD_HIGH	((1 << GMC_THRESHOLD_BITS) / 4)
> >   #define GMC_THRESHOLD_LOW	((1 << GMC_THRESHOLD_BITS) / 8)
> >   
> > +#define OVL_CON_CLRFMT_MAN	BIT(23)
> >   #define OVL_CON_BYTE_SWAP	BIT(24)
> > -#define OVL_CON_MTX_YUV_TO_RGB	(6 << 16)
> > +
> > +/* OVL_CON_RGB_SWAP works only if OVL_CON_CLRFMT_MAN is enabled */
> > +#define OVL_CON_RGB_SWAP	BIT(25)
> > +
> >   #define OVL_CON_CLRFMT_RGB	(1 << 12)
> >   #define OVL_CON_CLRFMT_ARGB8888	(2 << 12)
> >   #define OVL_CON_CLRFMT_RGBA8888	(3 << 12)
> > @@ -65,6 +69,11 @@
> >   #define OVL_CON_CLRFMT_BGRA8888	(OVL_CON_CLRFMT_ARGB8888 |
> > OVL_CON_BYTE_SWAP)
> >   #define OVL_CON_CLRFMT_UYVY	(4 << 12)
> >   #define OVL_CON_CLRFMT_YUYV	(5 << 12)
> > +#define OVL_CON_MTX_YUV_TO_RGB	(6 << 16)
> > +#define OVL_CON_CLRFMT_PARGB8888 ((3 << 12) | OVL_CON_CLRFMT_MAN)
> 
> 
> Shouldn't this be (OVL_CON_CLRFMT_RGBA8888 | OVL_CON_CLRFMT_MAN) ??
> 
> That's getting written to the same register, so I'd guess this is
> like that; but
> then, if it is like that, why is it PARGB888 and not PRGBA888?!
> 

Thank you for highlighting this important point. As whether
OVL_CON_CLRFMT_MAN bit is enabled, (3 << 12) means different formats in
the datasheet, and this function also confused us very much at first.

To prevent any similar misunderstandings going forward, instead of
reusing the MACRO you mentioned, we intetionally redefined
OVL_CON_CLRFMT_PARGB8888 using bit operation again, although this
approach seems to cause another confusion.

Regards,
Shawn