diff mbox series

[v9,16/21] drm/mediatek: Support "Pre-multiplied" blending in OVL

Message ID 20240614024620.19011-17-shawn.sung@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Support IGT in display driver | expand

Commit Message

Shawn Sung (宋孝謙) June 14, 2024, 2:46 a.m. UTC
From: Hsiao Chien Sung <shawn.sung@mediatek.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@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 42 ++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 8 deletions(-)

Comments

CK Hu (胡俊光) June 14, 2024, 5:42 a.m. UTC | #1
On Fri, 2024-06-14 at 10:46 +0800, Shawn Sung wrote:
> From: Hsiao Chien Sung <shawn.sung@mediatek.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@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 42 ++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 6567806cf4e2..47d0b039a616 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -52,8 +52,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)
> @@ -61,6 +65,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)

#define OVL_CON_CLRFMT_PARGB8888 (OVL_CON_CLRFMT_RGBA8888 | OVL_CON_CLRFMT_MAN)

But I'm confused with the naming.


> +#define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_PARGB8888 | OVL_CON_RGB_SWAP)

#define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_ABGR8888 | OVL_CON_CLRFMT_MAN)

> +#define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_PARGB8888 | OVL_CON_BYTE_SWAP)

#define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_BGRA8888 | OVL_CON_CLRFMT_MAN)

> +#define OVL_CON_CLRFMT_PRGBA8888 (OVL_CON_CLRFMT_PABGR8888 | OVL_CON_BYTE_SWAP)
>  #define OVL_CON_CLRFMT_RGB565(ovl)	((ovl)->data->fmt_rgb565_is_0 ? \
>  					0 : OVL_CON_CLRFMT_RGB)
>  #define OVL_CON_CLRFMT_RGB888(ovl)	((ovl)->data->fmt_rgb565_is_0 ? \
> @@ -74,6 +83,8 @@
>  #define	OVL_CON_VIRT_FLIP	BIT(9)
>  #define	OVL_CON_HORZ_FLIP	BIT(10)
>  
> +#define OVL_COLOR_ALPHA		GENMASK(31, 24)
> +
>  static inline bool is_10bit_rgb(u32 fmt)
>  {
>  	switch (fmt) {
> @@ -298,7 +309,13 @@ void mtk_ovl_config(struct device *dev, unsigned int w,
>  	if (w != 0 && h != 0)
>  		mtk_ddp_write_relaxed(cmdq_pkt, h << 16 | w, &ovl->cmdq_reg, ovl->regs,
>  				      DISP_REG_OVL_ROI_SIZE);
> -	mtk_ddp_write_relaxed(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_ROI_BGCLR);
> +
> +	/*
> +	 * The background color must be opaque black (ARGB),
> +	 * otherwise the alpha blending will have no effect
> +	 */
> +	mtk_ddp_write_relaxed(cmdq_pkt, OVL_COLOR_ALPHA, &ovl->cmdq_reg,
> +			      ovl->regs, DISP_REG_OVL_ROI_BGCLR);

The coverage mode formula is:
dst.RGB = src.RGB * src.A + dst.RGB * (1 - src.A)

The pre-multiplied mode formula is:
dst.RGB = src.RGB + dst.RGB * (1 - src.A)

Both formula has no destination alpha (I think background color is last destination),
why coverage mode work fine but pre-multiplied mode has something wrong?

Regards,
CK

>  
>  	mtk_ddp_write(cmdq_pkt, 0x1, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_RST);
>  	mtk_ddp_write(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_RST);
> @@ -374,7 +391,8 @@ void mtk_ovl_layer_off(struct device *dev, unsigned int idx,
>  		      DISP_REG_OVL_RDMA_CTRL(idx));
>  }
>  
> -static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt)
> +static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt,
> +				    unsigned int blend_mode)
>  {
>  	/* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX"
>  	 * is defined in mediatek HW data sheet.
> @@ -395,22 +413,30 @@ static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt)
>  	case DRM_FORMAT_RGBA8888:
>  	case DRM_FORMAT_RGBX1010102:
>  	case DRM_FORMAT_RGBA1010102:
> -		return OVL_CON_CLRFMT_RGBA8888;
> +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_RGBA8888 :
> +		       OVL_CON_CLRFMT_PRGBA8888;
>  	case DRM_FORMAT_BGRX8888:
>  	case DRM_FORMAT_BGRA8888:
>  	case DRM_FORMAT_BGRX1010102:
>  	case DRM_FORMAT_BGRA1010102:
> -		return OVL_CON_CLRFMT_BGRA8888;
> +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_BGRA8888 :
> +		       OVL_CON_CLRFMT_PBGRA8888;
>  	case DRM_FORMAT_XRGB8888:
>  	case DRM_FORMAT_ARGB8888:
>  	case DRM_FORMAT_XRGB2101010:
>  	case DRM_FORMAT_ARGB2101010:
> -		return OVL_CON_CLRFMT_ARGB8888;
> +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_ARGB8888 :
> +		       OVL_CON_CLRFMT_PARGB8888;
>  	case DRM_FORMAT_XBGR8888:
>  	case DRM_FORMAT_ABGR8888:
>  	case DRM_FORMAT_XBGR2101010:
>  	case DRM_FORMAT_ABGR2101010:
> -		return OVL_CON_CLRFMT_ABGR8888;
> +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_ABGR8888 :
> +		       OVL_CON_CLRFMT_PABGR8888;
>  	case DRM_FORMAT_UYVY:
>  		return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB;
>  	case DRM_FORMAT_YUYV:
> @@ -450,7 +476,7 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>  		return;
>  	}
>  
> -	con = ovl_fmt_convert(ovl, fmt);
> +	con = ovl_fmt_convert(ovl, fmt, blend_mode);
>  	if (state->base.fb) {
>  		con |= OVL_CON_AEN;
>  		con |= state->base.alpha & OVL_CON_ALPHA;
CK Hu (胡俊光) June 14, 2024, 5:49 a.m. UTC | #2
Hi, Shawn:

On Fri, 2024-06-14 at 10:46 +0800, Shawn Sung wrote:
> From: Hsiao Chien Sung <shawn.sung@mediatek.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@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 42 ++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 6567806cf4e2..47d0b039a616 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -52,8 +52,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)
> @@ -61,6 +65,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)
> +#define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_PARGB8888 | OVL_CON_RGB_SWAP)
> +#define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_PARGB8888 | OVL_CON_BYTE_SWAP)
> +#define OVL_CON_CLRFMT_PRGBA8888 (OVL_CON_CLRFMT_PABGR8888 | OVL_CON_BYTE_SWAP)
>  #define OVL_CON_CLRFMT_RGB565(ovl)	((ovl)->data->fmt_rgb565_is_0 ? \
>  					0 : OVL_CON_CLRFMT_RGB)
>  #define OVL_CON_CLRFMT_RGB888(ovl)	((ovl)->data->fmt_rgb565_is_0 ? \
> @@ -74,6 +83,8 @@
>  #define	OVL_CON_VIRT_FLIP	BIT(9)
>  #define	OVL_CON_HORZ_FLIP	BIT(10)
>  
> 

[snip]

>  
> -static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt)
> +static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt,
> +				    unsigned int blend_mode)
>  {
>  	/* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX"
>  	 * is defined in mediatek HW data sheet.
> @@ -395,22 +413,30 @@ static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt)
>  	case DRM_FORMAT_RGBA8888:
>  	case DRM_FORMAT_RGBX1010102:
>  	case DRM_FORMAT_RGBA1010102:
> -		return OVL_CON_CLRFMT_RGBA8888;
> +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_RGBA8888 :
> +		       OVL_CON_CLRFMT_PRGBA8888;
>  	case DRM_FORMAT_BGRX8888:
>  	case DRM_FORMAT_BGRA8888:
>  	case DRM_FORMAT_BGRX1010102:
>  	case DRM_FORMAT_BGRA1010102:
> -		return OVL_CON_CLRFMT_BGRA8888;
> +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_BGRA8888 :
> +		       OVL_CON_CLRFMT_PBGRA8888;
>  	case DRM_FORMAT_XRGB8888:
>  	case DRM_FORMAT_ARGB8888:
>  	case DRM_FORMAT_XRGB2101010:
>  	case DRM_FORMAT_ARGB2101010:
> -		return OVL_CON_CLRFMT_ARGB8888;
> +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_ARGB8888 :
> +		       OVL_CON_CLRFMT_PARGB8888;
>  	case DRM_FORMAT_XBGR8888:
>  	case DRM_FORMAT_ABGR8888:
>  	case DRM_FORMAT_XBGR2101010:
>  	case DRM_FORMAT_ABGR2101010:
> -		return OVL_CON_CLRFMT_ABGR8888;
> +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> +		       OVL_CON_CLRFMT_ABGR8888 :
> +		       OVL_CON_CLRFMT_PABGR8888;
>  	case DRM_FORMAT_UYVY:
>  		return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB;
>  	case DRM_FORMAT_YUYV:
> @@ -450,7 +476,7 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>  		return;
>  	}
>  
> -	con = ovl_fmt_convert(ovl, fmt);
> +	con = ovl_fmt_convert(ovl, fmt, blend_mode);

The simple way is

con |= blend_mode == DRM_MODE_BLEND_PREMULTI ? OVL_CON_CLRFMT_MAN : 0;

Regards,
CK


>  	if (state->base.fb) {
>  		con |= OVL_CON_AEN;
>  		con |= state->base.alpha & OVL_CON_ALPHA;
Jason-JH Lin (林睿祥) June 18, 2024, 5 p.m. UTC | #3
Hi CK,

On Fri, 2024-06-14 at 05:42 +0000, CK Hu (胡俊光) wrote:
> On Fri, 2024-06-14 at 10:46 +0800, Shawn Sung wrote:
> > From: Hsiao Chien Sung <shawn.sung@mediatek.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@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 42 ++++++++++++++++++++-
> > ----
> >  1 file changed, 34 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index 6567806cf4e2..47d0b039a616 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -52,8 +52,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)
> > @@ -61,6 +65,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)
> 
> #define OVL_CON_CLRFMT_PARGB8888 (OVL_CON_CLRFMT_RGBA8888 |
> OVL_CON_CLRFMT_MAN)
> 
> But I'm confused with the naming.
> 

We found that original naming is not correct, so we fix it with the
patch:

https://patchwork.kernel.org/project/linux-mediatek/patch/20240616-mediatek-drm-next-v1-11-7e8f9cf785d8@mediatek.com/

PARGB8888 is not ARGB8888 + MAN.

If DISP_REG_OVL_CON open the MAN bit, some original color format
settings will be changed, for example:

When MAN = 0:
RGB565 = 0000 in bit[15:12]
RGB888 = 0001 in bit[15:12]
ARGB8888 = 0010 in bit[15:12]
RGBA8888 = 0011 in bit[15:12]
UYVY = 0100 in bit[15:12]
YUYV = 0101 in bit[15:12]

When MAN = 1:
RGB888 = 0000 in bit[15:12]
RGB565 = 0001 in bit[15:12]
BGRA
8888 = 0010 in bit[15:12]
PARGB8888 = 0011 in bit[15:12]BGRA4444 = 0100
in bit[15:12]
BGRA5551 = 0101 in bit[15:12]
YUYV = 1000 in bit[15:12]
UYVY
= 1001 in bit[15:12]
YVYU = 1010 in bit[15:12]
VYUY = 1011 in bit[15:12]

> 
> > +#define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_PARGB8888 |
> > OVL_CON_RGB_SWAP)
> 
> #define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_ABGR8888 |
> OVL_CON_CLRFMT_MAN)
> 
> > +#define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_PARGB8888 |
> > OVL_CON_BYTE_SWAP)
> 
> #define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_BGRA8888 |
> OVL_CON_CLRFMT_MAN)
> 
> > +#define OVL_CON_CLRFMT_PRGBA8888 (OVL_CON_CLRFMT_PABGR8888 |
> > OVL_CON_BYTE_SWAP)
> >  #define OVL_CON_CLRFMT_RGB565(ovl)	((ovl)->data->fmt_rgb565_is_0 ?
> > \
> >  					0 : OVL_CON_CLRFMT_RGB)
> >  #define OVL_CON_CLRFMT_RGB888(ovl)	((ovl)->data->fmt_rgb565_is_0 ?
> > \
> > @@ -74,6 +83,8 @@
> >  #define	OVL_CON_VIRT_FLIP	BIT(9)
> >  #define	OVL_CON_HORZ_FLIP	BIT(10)
> >  
> > +#define OVL_COLOR_ALPHA		GENMASK(31, 24)
> > +
> >  static inline bool is_10bit_rgb(u32 fmt)
> >  {
> >  	switch (fmt) {
> > @@ -298,7 +309,13 @@ void mtk_ovl_config(struct device *dev,
> > unsigned int w,
> >  	if (w != 0 && h != 0)
> >  		mtk_ddp_write_relaxed(cmdq_pkt, h << 16 | w, &ovl-
> > >cmdq_reg, ovl->regs,
> >  				      DISP_REG_OVL_ROI_SIZE);
> > -	mtk_ddp_write_relaxed(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, 
> > DISP_REG_OVL_ROI_BGCLR);
> > +
> > +	/*
> > +	 * The background color must be opaque black (ARGB),
> > +	 * otherwise the alpha blending will have no effect
> > +	 */
> > +	mtk_ddp_write_relaxed(cmdq_pkt, OVL_COLOR_ALPHA, &ovl-
> > >cmdq_reg,
> > +			      ovl->regs, DISP_REG_OVL_ROI_BGCLR);
> 
> The coverage mode formula is:
> dst.RGB = src.RGB * src.A + dst.RGB * (1 - src.A)
> 
> The pre-multiplied mode formula is:
> dst.RGB = src.RGB + dst.RGB * (1 - src.A)
> 
> Both formula has no destination alpha (I think background color is
> last destination),
> why coverage mode work fine but pre-multiplied mode has something
> wrong?
> 

alpha of coverage mode formula is:
dst.a = dst.a x (0xff - src.a x SCA/0xff)/0xff + src.a x SCA/0xff;

alpha of pre-multiplied mode is:
dst.a = round( {{dst.a x [0xff - round((src.a x SCA)/0xff)]} / 0xff} +
round((src.a x SCA)/0xff) );

* SCA is constant alpha

If SCA = 0xff, then the formula will be:
dst.a = dst.a x (0xff - src.a) + src.a

If dst.a is 0, then dst.a will be src.a
If dst.a is 0xff, then dst.a will be 0xff


Regards,
Jason-JH Lin

> Regards,
> CK
> 
> >  
> >  	mtk_ddp_write(cmdq_pkt, 0x1, &ovl->cmdq_reg, ovl->regs,
> > DISP_REG_OVL_RST);
> >  	mtk_ddp_write(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs,
> > DISP_REG_OVL_RST);
> > @@ -374,7 +391,8 @@ void mtk_ovl_layer_off(struct device *dev,
> > unsigned int idx,
> >  		      DISP_REG_OVL_RDMA_CTRL(idx));
> >  }
> >  
> > -static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> > unsigned int fmt)
> > +static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> > unsigned int fmt,
> > +				    unsigned int blend_mode)
> >  {
> >  	/* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX"
> >  	 * is defined in mediatek HW data sheet.
> > @@ -395,22 +413,30 @@ static unsigned int ovl_fmt_convert(struct
> > mtk_disp_ovl *ovl, unsigned int fmt)
> >  	case DRM_FORMAT_RGBA8888:
> >  	case DRM_FORMAT_RGBX1010102:
> >  	case DRM_FORMAT_RGBA1010102:
> > -		return OVL_CON_CLRFMT_RGBA8888;
> > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > +		       OVL_CON_CLRFMT_RGBA8888 :
> > +		       OVL_CON_CLRFMT_PRGBA8888;
> >  	case DRM_FORMAT_BGRX8888:
> >  	case DRM_FORMAT_BGRA8888:
> >  	case DRM_FORMAT_BGRX1010102:
> >  	case DRM_FORMAT_BGRA1010102:
> > -		return OVL_CON_CLRFMT_BGRA8888;
> > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > +		       OVL_CON_CLRFMT_BGRA8888 :
> > +		       OVL_CON_CLRFMT_PBGRA8888;
> >  	case DRM_FORMAT_XRGB8888:
> >  	case DRM_FORMAT_ARGB8888:
> >  	case DRM_FORMAT_XRGB2101010:
> >  	case DRM_FORMAT_ARGB2101010:
> > -		return OVL_CON_CLRFMT_ARGB8888;
> > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > +		       OVL_CON_CLRFMT_ARGB8888 :
> > +		       OVL_CON_CLRFMT_PARGB8888;
> >  	case DRM_FORMAT_XBGR8888:
> >  	case DRM_FORMAT_ABGR8888:
> >  	case DRM_FORMAT_XBGR2101010:
> >  	case DRM_FORMAT_ABGR2101010:
> > -		return OVL_CON_CLRFMT_ABGR8888;
> > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > +		       OVL_CON_CLRFMT_ABGR8888 :
> > +		       OVL_CON_CLRFMT_PABGR8888;
> >  	case DRM_FORMAT_UYVY:
> >  		return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB;
> >  	case DRM_FORMAT_YUYV:
> > @@ -450,7 +476,7 @@ void mtk_ovl_layer_config(struct device *dev,
> > unsigned int idx,
> >  		return;
> >  	}
> >  
> > -	con = ovl_fmt_convert(ovl, fmt);
> > +	con = ovl_fmt_convert(ovl, fmt, blend_mode);
> >  	if (state->base.fb) {
> >  		con |= OVL_CON_AEN;
> >  		con |= state->base.alpha & OVL_CON_ALPHA;
CK Hu (胡俊光) June 19, 2024, 3:17 a.m. UTC | #4
Hi, Jason:

On Tue, 2024-06-18 at 17:00 +0000, Jason-JH Lin (林睿祥) wrote:
> Hi CK,
> 
> On Fri, 2024-06-14 at 05:42 +0000, CK Hu (胡俊光) wrote:
> > On Fri, 2024-06-14 at 10:46 +0800, Shawn Sung wrote:
> > > From: Hsiao Chien Sung <shawn.sung@mediatek.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@mediatek.com>
> > > ---
> > >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 42 ++++++++++++++++++++-
> > > ----
> > >  1 file changed, 34 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > index 6567806cf4e2..47d0b039a616 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > @@ -52,8 +52,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)
> > > @@ -61,6 +65,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)
> > 
> > #define OVL_CON_CLRFMT_PARGB8888 (OVL_CON_CLRFMT_RGBA8888 |
> > OVL_CON_CLRFMT_MAN)
> > 
> > But I'm confused with the naming.
> > 
> 
> We found that original naming is not correct, so we fix it with the
> patch:
> 
> https://patchwork.kernel.org/project/linux-mediatek/patch/20240616-mediatek-drm-next-v1-11-7e8f9cf785d8@mediatek.com/
> 
> PARGB8888 is not ARGB8888 + MAN.
> 
> If DISP_REG_OVL_CON open the MAN bit, some original color format
> settings will be changed, for example:
> 
> When MAN = 0:
> RGB565 = 0000 in bit[15:12]
> RGB888 = 0001 in bit[15:12]
> ARGB8888 = 0010 in bit[15:12]
> RGBA8888 = 0011 in bit[15:12]
> UYVY = 0100 in bit[15:12]
> YUYV = 0101 in bit[15:12]
> 
> When MAN = 1:
> RGB888 = 0000 in bit[15:12]
> RGB565 = 0001 in bit[15:12]
> BGRA
> 8888 = 0010 in bit[15:12]
> PARGB8888 = 0011 in bit[15:12]BGRA4444 = 0100
> in bit[15:12]
> BGRA5551 = 0101 in bit[15:12]
> YUYV = 1000 in bit[15:12]
> UYVY
> = 1001 in bit[15:12]
> YVYU = 1010 in bit[15:12]
> VYUY = 1011 in bit[15:12]

I think you could define premultiplied format as:

#define PRGB888		(0 << 12) | OVL_CON_CLRFMT_MAN
#define PRGB565		(1 << 12) | OVL_CON_CLRFMT_MAN
#define PBGRA8888	(2 << 12) | OVL_CON_CLRFMT_MAN
#define PARGB8888	(3 << 12) | OVL_CON_CLRFMT_MAN
#define PBGRA4444	(4 << 12) | OVL_CON_CLRFMT_MAN
#define PBGRA5551	(5 << 12) | OVL_CON_CLRFMT_MAN
#define PYUYV		(8 << 12) | OVL_CON_CLRFMT_MAN
#define PVYUY		(9 << 12) | OVL_CON_CLRFMT_MAN

> 
> > 
> > > +#define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_PARGB8888 |
> > > OVL_CON_RGB_SWAP)
> > 
> > #define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_ABGR8888 |
> > OVL_CON_CLRFMT_MAN)
> > 
> > > +#define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_PARGB8888 |
> > > OVL_CON_BYTE_SWAP)
> > 
> > #define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_BGRA8888 |
> > OVL_CON_CLRFMT_MAN)
> > 
> > > +#define OVL_CON_CLRFMT_PRGBA8888 (OVL_CON_CLRFMT_PABGR8888 |
> > > OVL_CON_BYTE_SWAP)
> > >  #define OVL_CON_CLRFMT_RGB565(ovl)	((ovl)->data->fmt_rgb565_is_0 ?
> > > \
> > >  					0 : OVL_CON_CLRFMT_RGB)
> > >  #define OVL_CON_CLRFMT_RGB888(ovl)	((ovl)->data->fmt_rgb565_is_0 ?
> > > \
> > > @@ -74,6 +83,8 @@
> > >  #define	OVL_CON_VIRT_FLIP	BIT(9)
> > >  #define	OVL_CON_HORZ_FLIP	BIT(10)
> > >  
> > > +#define OVL_COLOR_ALPHA		GENMASK(31, 24)
> > > +
> > >  static inline bool is_10bit_rgb(u32 fmt)
> > >  {
> > >  	switch (fmt) {
> > > @@ -298,7 +309,13 @@ void mtk_ovl_config(struct device *dev,
> > > unsigned int w,
> > >  	if (w != 0 && h != 0)
> > >  		mtk_ddp_write_relaxed(cmdq_pkt, h << 16 | w, &ovl-
> > > > cmdq_reg, ovl->regs,
> > > 
> > >  				      DISP_REG_OVL_ROI_SIZE);
> > > -	mtk_ddp_write_relaxed(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, 
> > > DISP_REG_OVL_ROI_BGCLR);
> > > +
> > > +	/*
> > > +	 * The background color must be opaque black (ARGB),
> > > +	 * otherwise the alpha blending will have no effect
> > > +	 */
> > > +	mtk_ddp_write_relaxed(cmdq_pkt, OVL_COLOR_ALPHA, &ovl-
> > > > cmdq_reg,
> > > 
> > > +			      ovl->regs, DISP_REG_OVL_ROI_BGCLR);
> > 
> > The coverage mode formula is:
> > dst.RGB = src.RGB * src.A + dst.RGB * (1 - src.A)
> > 
> > The pre-multiplied mode formula is:
> > dst.RGB = src.RGB + dst.RGB * (1 - src.A)
> > 
> > Both formula has no destination alpha (I think background color is
> > last destination),
> > why coverage mode work fine but pre-multiplied mode has something
> > wrong?
> > 
> 
> alpha of coverage mode formula is:
> dst.a = dst.a x (0xff - src.a x SCA/0xff)/0xff + src.a x SCA/0xff;
> 
> alpha of pre-multiplied mode is:
> dst.a = round( {{dst.a x [0xff - round((src.a x SCA)/0xff)]} / 0xff} +
> round((src.a x SCA)/0xff) );
> 
> * SCA is constant alpha
> 
> If SCA = 0xff, then the formula will be:
> dst.a = dst.a x (0xff - src.a) + src.a
> 
> If dst.a is 0, then dst.a will be src.a
> If dst.a is 0xff, then dst.a will be 0xff

I'm confused that dst.a exist in both side of '='. Let me name them as dst.a.new and dst.a.old.
So calculate dst.a.new first, then calculate dst.RGB according to dst.a.new not dst.a.old?

If so, dst.a.new is influenced by SCA either in coverage mode and premultiplied mode.
This is to fix the bug already exist in coverage mode.

Regards,
CK

> 
> 
> Regards,
> Jason-JH Lin
> 
> > Regards,
> > CK
> > 
> > >  
> > >  	mtk_ddp_write(cmdq_pkt, 0x1, &ovl->cmdq_reg, ovl->regs,
> > > DISP_REG_OVL_RST);
> > >  	mtk_ddp_write(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs,
> > > DISP_REG_OVL_RST);
> > > @@ -374,7 +391,8 @@ void mtk_ovl_layer_off(struct device *dev,
> > > unsigned int idx,
> > >  		      DISP_REG_OVL_RDMA_CTRL(idx));
> > >  }
> > >  
> > > -static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> > > unsigned int fmt)
> > > +static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> > > unsigned int fmt,
> > > +				    unsigned int blend_mode)
> > >  {
> > >  	/* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX"
> > >  	 * is defined in mediatek HW data sheet.
> > > @@ -395,22 +413,30 @@ static unsigned int ovl_fmt_convert(struct
> > > mtk_disp_ovl *ovl, unsigned int fmt)
> > >  	case DRM_FORMAT_RGBA8888:
> > >  	case DRM_FORMAT_RGBX1010102:
> > >  	case DRM_FORMAT_RGBA1010102:
> > > -		return OVL_CON_CLRFMT_RGBA8888;
> > > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > > +		       OVL_CON_CLRFMT_RGBA8888 :
> > > +		       OVL_CON_CLRFMT_PRGBA8888;
> > >  	case DRM_FORMAT_BGRX8888:
> > >  	case DRM_FORMAT_BGRA8888:
> > >  	case DRM_FORMAT_BGRX1010102:
> > >  	case DRM_FORMAT_BGRA1010102:
> > > -		return OVL_CON_CLRFMT_BGRA8888;
> > > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > > +		       OVL_CON_CLRFMT_BGRA8888 :
> > > +		       OVL_CON_CLRFMT_PBGRA8888;
> > >  	case DRM_FORMAT_XRGB8888:
> > >  	case DRM_FORMAT_ARGB8888:
> > >  	case DRM_FORMAT_XRGB2101010:
> > >  	case DRM_FORMAT_ARGB2101010:
> > > -		return OVL_CON_CLRFMT_ARGB8888;
> > > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > > +		       OVL_CON_CLRFMT_ARGB8888 :
> > > +		       OVL_CON_CLRFMT_PARGB8888;
> > >  	case DRM_FORMAT_XBGR8888:
> > >  	case DRM_FORMAT_ABGR8888:
> > >  	case DRM_FORMAT_XBGR2101010:
> > >  	case DRM_FORMAT_ABGR2101010:
> > > -		return OVL_CON_CLRFMT_ABGR8888;
> > > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > > +		       OVL_CON_CLRFMT_ABGR8888 :
> > > +		       OVL_CON_CLRFMT_PABGR8888;
> > >  	case DRM_FORMAT_UYVY:
> > >  		return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB;
> > >  	case DRM_FORMAT_YUYV:
> > > @@ -450,7 +476,7 @@ void mtk_ovl_layer_config(struct device *dev,
> > > unsigned int idx,
> > >  		return;
> > >  	}
> > >  
> > > -	con = ovl_fmt_convert(ovl, fmt);
> > > +	con = ovl_fmt_convert(ovl, fmt, blend_mode);
> > >  	if (state->base.fb) {
> > >  		con |= OVL_CON_AEN;
> > >  		con |= state->base.alpha & OVL_CON_ALPHA;
Jason-JH Lin (林睿祥) June 19, 2024, 4 p.m. UTC | #5
Hi CK,

On Wed, 2024-06-19 at 03:17 +0000, CK Hu (胡俊光) wrote:
> Hi, Jason:
> 
> On Tue, 2024-06-18 at 17:00 +0000, Jason-JH Lin (林睿祥) wrote:
> > Hi CK,
> > 
> > On Fri, 2024-06-14 at 05:42 +0000, CK Hu (胡俊光) wrote:
> > > On Fri, 2024-06-14 at 10:46 +0800, Shawn Sung wrote:
> > > > From: Hsiao Chien Sung <shawn.sung@mediatek.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@mediatek.com>
> > > > ---
> > > >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 42
> > > > ++++++++++++++++++++-
> > > > ----
> > > >  1 file changed, 34 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > > index 6567806cf4e2..47d0b039a616 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > > @@ -52,8 +52,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)
> > > > @@ -61,6 +65,11 @@
> > > >  #define OVL_CON_CLRFMT_BGRA8888	(OVL_CON_CLRFMT_ARGB888
> > > > 8 |
> > > > 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)
> > > 
> > > #define OVL_CON_CLRFMT_PARGB8888 (OVL_CON_CLRFMT_RGBA8888 |
> > > OVL_CON_CLRFMT_MAN)
> > > 
> > > But I'm confused with the naming.
> > > 
> > 
> > We found that original naming is not correct, so we fix it with the
> > patch:
> > 
> > 
https://patchwork.kernel.org/project/linux-mediatek/patch/20240616-mediatek-drm-next-v1-11-7e8f9cf785d8@mediatek.com/
> > 
> > PARGB8888 is not ARGB8888 + MAN.
> > 
> > If DISP_REG_OVL_CON open the MAN bit, some original color format
> > settings will be changed, for example:
> > 
> > When MAN = 0:
> > RGB565 = 0000 in bit[15:12]
> > RGB888 = 0001 in bit[15:12]
> > ARGB8888 = 0010 in bit[15:12]
> > RGBA8888 = 0011 in bit[15:12]
> > UYVY = 0100 in bit[15:12]
> > YUYV = 0101 in bit[15:12]
> > 
> > When MAN = 1:
> > RGB888 = 0000 in bit[15:12]
> > RGB565 = 0001 in bit[15:12]
> > BGRA
> > 8888 = 0010 in bit[15:12]
> > PARGB8888 = 0011 in bit[15:12]BGRA4444 = 0100
> > in bit[15:12]
> > BGRA5551 = 0101 in bit[15:12]
> > YUYV = 1000 in bit[15:12]
> > UYVY
> > = 1001 in bit[15:12]
> > YVYU = 1010 in bit[15:12]
> > VYUY = 1011 in bit[15:12]
> 
> I think you could define premultiplied format as:
> 
> #define PRGB888		(0 << 12) | OVL_CON_CLRFMT_MAN
> #define PRGB565		(1 << 12) | OVL_CON_CLRFMT_MAN
> #define PBGRA8888	(2 << 12) | OVL_CON_CLRFMT_MAN
> #define PARGB8888	(3 << 12) | OVL_CON_CLRFMT_MAN
> #define PBGRA4444	(4 << 12) | OVL_CON_CLRFMT_MAN
> #define PBGRA5551	(5 << 12) | OVL_CON_CLRFMT_MAN
> #define PYUYV		(8 << 12) | OVL_CON_CLRFMT_MAN
> #define PVYUY		(9 << 12) | OVL_CON_CLRFMT_MAN
> 

OVL_CON_CLRFMT_MAN = 1 is for extending the color format, not for
enabling the Pre-multipled, so I think we only need to define the new
format PARGB8888  (3 << 12) | OVL_CON_CLRFMT_MAN.

Then we can use RGB_SAP and BYTE_SWAP to change PARGB8888 to other Pre-
Multiplied 8888 color formats.

Keep using the color format that is support with OVL_CON_CLRFMT_MAN = 0
for the previous SoC.

> > 
> > > 
> > > > +#define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_PARGB8888 |
> > > > OVL_CON_RGB_SWAP)
> > > 
> > > #define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_ABGR8888 |
> > > OVL_CON_CLRFMT_MAN)
> > > 
> > > > +#define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_PARGB8888 |
> > > > OVL_CON_BYTE_SWAP)
> > > 
> > > #define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_BGRA8888 |
> > > OVL_CON_CLRFMT_MAN)
> > > 
> > > > +#define OVL_CON_CLRFMT_PRGBA8888 (OVL_CON_CLRFMT_PABGR8888 |
> > > > OVL_CON_BYTE_SWAP)
> > > >  #define OVL_CON_CLRFMT_RGB565(ovl)	((ovl)->data-
> > > > >fmt_rgb565_is_0 ?
> > > > \
> > > >  					0 : OVL_CON_CLRFMT_RGB)
> > > >  #define OVL_CON_CLRFMT_RGB888(ovl)	((ovl)->data-
> > > > >fmt_rgb565_is_0 ?
> > > > \
> > > > @@ -74,6 +83,8 @@
> > > >  #define	OVL_CON_VIRT_FLIP	BIT(9)
> > > >  #define	OVL_CON_HORZ_FLIP	BIT(10)
> > > >  
> > > > +#define OVL_COLOR_ALPHA		GENMASK(31, 24)
> > > > +
> > > >  static inline bool is_10bit_rgb(u32 fmt)
> > > >  {
> > > >  	switch (fmt) {
> > > > @@ -298,7 +309,13 @@ void mtk_ovl_config(struct device *dev,
> > > > unsigned int w,
> > > >  	if (w != 0 && h != 0)
> > > >  		mtk_ddp_write_relaxed(cmdq_pkt, h << 16 | w,
> > > > &ovl-
> > > > > cmdq_reg, ovl->regs,
> > > > 
> > > >  				      DISP_REG_OVL_ROI_SIZE);
> > > > -	mtk_ddp_write_relaxed(cmdq_pkt, 0x0, &ovl->cmdq_reg,
> > > > ovl->regs, 
> > > > DISP_REG_OVL_ROI_BGCLR);
> > > > +
> > > > +	/*
> > > > +	 * The background color must be opaque black (ARGB),
> > > > +	 * otherwise the alpha blending will have no effect
> > > > +	 */
> > > > +	mtk_ddp_write_relaxed(cmdq_pkt, OVL_COLOR_ALPHA, &ovl-
> > > > > cmdq_reg,
> > > > 
> > > > +			      ovl->regs,
> > > > DISP_REG_OVL_ROI_BGCLR);
> > > 
> > > The coverage mode formula is:
> > > dst.RGB = src.RGB * src.A + dst.RGB * (1 - src.A)
> > > 
> > > The pre-multiplied mode formula is:
> > > dst.RGB = src.RGB + dst.RGB * (1 - src.A)
> > > 
> > > Both formula has no destination alpha (I think background color
> > > is
> > > last destination),
> > > why coverage mode work fine but pre-multiplied mode has something
> > > wrong?
> > > 
> > 
> > alpha of coverage mode formula is:
> > dst.a = dst.a x (0xff - src.a x SCA/0xff)/0xff + src.a x SCA/0xff;
> > 
> > alpha of pre-multiplied mode is:
> > dst.a = round( {{dst.a x [0xff - round((src.a x SCA)/0xff)]} /
> > 0xff} +
> > round((src.a x SCA)/0xff) );
> > 
> > * SCA is constant alpha
> > 
> > If SCA = 0xff, then the formula will be:
> > dst.a = dst.a x (0xff - src.a) + src.a
> > 
> > If dst.a is 0, then dst.a will be src.a
> > If dst.a is 0xff, then dst.a will be 0xff
> 
> I'm confused that dst.a exist in both side of '='. Let me name them
> as dst.a.new and dst.a.old.
> So calculate dst.a.new first, then calculate dst.RGB according to
> dst.a.new not dst.a.old?
> 
> If so, dst.a.new is influenced by SCA either in coverage mode and
> premultiplied mode.
> This is to fix the bug already exist in coverage mode.
> 
Yes, I'll separate this to another patch.

Regards,
Jason-JH Lin

> Regards,
> CK
> 
> > 
> > 
> > Regards,
> > Jason-JH Lin
> > 
> > > Regards,
> > > CK
> > > 
> > > >  
> > > >  	mtk_ddp_write(cmdq_pkt, 0x1, &ovl->cmdq_reg, ovl->regs,
> > > > DISP_REG_OVL_RST);
> > > >  	mtk_ddp_write(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs,
> > > > DISP_REG_OVL_RST);
> > > > @@ -374,7 +391,8 @@ void mtk_ovl_layer_off(struct device *dev,
> > > > unsigned int idx,
> > > >  		      DISP_REG_OVL_RDMA_CTRL(idx));
> > > >  }
> > > >  
> > > > -static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> > > > unsigned int fmt)
> > > > +static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> > > > unsigned int fmt,
> > > > +				    unsigned int blend_mode)
> > > >  {
> > > >  	/* The return value in switch
> > > > "MEM_MODE_INPUT_FORMAT_XXX"
> > > >  	 * is defined in mediatek HW data sheet.
> > > > @@ -395,22 +413,30 @@ static unsigned int
> > > > ovl_fmt_convert(struct
> > > > mtk_disp_ovl *ovl, unsigned int fmt)
> > > >  	case DRM_FORMAT_RGBA8888:
> > > >  	case DRM_FORMAT_RGBX1010102:
> > > >  	case DRM_FORMAT_RGBA1010102:
> > > > -		return OVL_CON_CLRFMT_RGBA8888;
> > > > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > > > +		       OVL_CON_CLRFMT_RGBA8888 :
> > > > +		       OVL_CON_CLRFMT_PRGBA8888;
> > > >  	case DRM_FORMAT_BGRX8888:
> > > >  	case DRM_FORMAT_BGRA8888:
> > > >  	case DRM_FORMAT_BGRX1010102:
> > > >  	case DRM_FORMAT_BGRA1010102:
> > > > -		return OVL_CON_CLRFMT_BGRA8888;
> > > > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > > > +		       OVL_CON_CLRFMT_BGRA8888 :
> > > > +		       OVL_CON_CLRFMT_PBGRA8888;
> > > >  	case DRM_FORMAT_XRGB8888:
> > > >  	case DRM_FORMAT_ARGB8888:
> > > >  	case DRM_FORMAT_XRGB2101010:
> > > >  	case DRM_FORMAT_ARGB2101010:
> > > > -		return OVL_CON_CLRFMT_ARGB8888;
> > > > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > > > +		       OVL_CON_CLRFMT_ARGB8888 :
> > > > +		       OVL_CON_CLRFMT_PARGB8888;
> > > >  	case DRM_FORMAT_XBGR8888:
> > > >  	case DRM_FORMAT_ABGR8888:
> > > >  	case DRM_FORMAT_XBGR2101010:
> > > >  	case DRM_FORMAT_ABGR2101010:
> > > > -		return OVL_CON_CLRFMT_ABGR8888;
> > > > +		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
> > > > +		       OVL_CON_CLRFMT_ABGR8888 :
> > > > +		       OVL_CON_CLRFMT_PABGR8888;
> > > >  	case DRM_FORMAT_UYVY:
> > > >  		return OVL_CON_CLRFMT_UYVY |
> > > > OVL_CON_MTX_YUV_TO_RGB;
> > > >  	case DRM_FORMAT_YUYV:
> > > > @@ -450,7 +476,7 @@ void mtk_ovl_layer_config(struct device
> > > > *dev,
> > > > unsigned int idx,
> > > >  		return;
> > > >  	}
> > > >  
> > > > -	con = ovl_fmt_convert(ovl, fmt);
> > > > +	con = ovl_fmt_convert(ovl, fmt, blend_mode);
> > > >  	if (state->base.fb) {
> > > >  		con |= OVL_CON_AEN;
> > > >  		con |= state->base.alpha & OVL_CON_ALPHA;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 6567806cf4e2..47d0b039a616 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -52,8 +52,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)
@@ -61,6 +65,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)
+#define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_PARGB8888 | OVL_CON_RGB_SWAP)
+#define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_PARGB8888 | OVL_CON_BYTE_SWAP)
+#define OVL_CON_CLRFMT_PRGBA8888 (OVL_CON_CLRFMT_PABGR8888 | OVL_CON_BYTE_SWAP)
 #define OVL_CON_CLRFMT_RGB565(ovl)	((ovl)->data->fmt_rgb565_is_0 ? \
 					0 : OVL_CON_CLRFMT_RGB)
 #define OVL_CON_CLRFMT_RGB888(ovl)	((ovl)->data->fmt_rgb565_is_0 ? \
@@ -74,6 +83,8 @@ 
 #define	OVL_CON_VIRT_FLIP	BIT(9)
 #define	OVL_CON_HORZ_FLIP	BIT(10)
 
+#define OVL_COLOR_ALPHA		GENMASK(31, 24)
+
 static inline bool is_10bit_rgb(u32 fmt)
 {
 	switch (fmt) {
@@ -298,7 +309,13 @@  void mtk_ovl_config(struct device *dev, unsigned int w,
 	if (w != 0 && h != 0)
 		mtk_ddp_write_relaxed(cmdq_pkt, h << 16 | w, &ovl->cmdq_reg, ovl->regs,
 				      DISP_REG_OVL_ROI_SIZE);
-	mtk_ddp_write_relaxed(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_ROI_BGCLR);
+
+	/*
+	 * The background color must be opaque black (ARGB),
+	 * otherwise the alpha blending will have no effect
+	 */
+	mtk_ddp_write_relaxed(cmdq_pkt, OVL_COLOR_ALPHA, &ovl->cmdq_reg,
+			      ovl->regs, DISP_REG_OVL_ROI_BGCLR);
 
 	mtk_ddp_write(cmdq_pkt, 0x1, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_RST);
 	mtk_ddp_write(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_RST);
@@ -374,7 +391,8 @@  void mtk_ovl_layer_off(struct device *dev, unsigned int idx,
 		      DISP_REG_OVL_RDMA_CTRL(idx));
 }
 
-static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt)
+static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt,
+				    unsigned int blend_mode)
 {
 	/* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX"
 	 * is defined in mediatek HW data sheet.
@@ -395,22 +413,30 @@  static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt)
 	case DRM_FORMAT_RGBA8888:
 	case DRM_FORMAT_RGBX1010102:
 	case DRM_FORMAT_RGBA1010102:
-		return OVL_CON_CLRFMT_RGBA8888;
+		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
+		       OVL_CON_CLRFMT_RGBA8888 :
+		       OVL_CON_CLRFMT_PRGBA8888;
 	case DRM_FORMAT_BGRX8888:
 	case DRM_FORMAT_BGRA8888:
 	case DRM_FORMAT_BGRX1010102:
 	case DRM_FORMAT_BGRA1010102:
-		return OVL_CON_CLRFMT_BGRA8888;
+		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
+		       OVL_CON_CLRFMT_BGRA8888 :
+		       OVL_CON_CLRFMT_PBGRA8888;
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_ARGB8888:
 	case DRM_FORMAT_XRGB2101010:
 	case DRM_FORMAT_ARGB2101010:
-		return OVL_CON_CLRFMT_ARGB8888;
+		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
+		       OVL_CON_CLRFMT_ARGB8888 :
+		       OVL_CON_CLRFMT_PARGB8888;
 	case DRM_FORMAT_XBGR8888:
 	case DRM_FORMAT_ABGR8888:
 	case DRM_FORMAT_XBGR2101010:
 	case DRM_FORMAT_ABGR2101010:
-		return OVL_CON_CLRFMT_ABGR8888;
+		return blend_mode == DRM_MODE_BLEND_COVERAGE ?
+		       OVL_CON_CLRFMT_ABGR8888 :
+		       OVL_CON_CLRFMT_PABGR8888;
 	case DRM_FORMAT_UYVY:
 		return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB;
 	case DRM_FORMAT_YUYV:
@@ -450,7 +476,7 @@  void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
 		return;
 	}
 
-	con = ovl_fmt_convert(ovl, fmt);
+	con = ovl_fmt_convert(ovl, fmt, blend_mode);
 	if (state->base.fb) {
 		con |= OVL_CON_AEN;
 		con |= state->base.alpha & OVL_CON_ALPHA;