diff mbox series

[v3,3/5] drm/mediatek: Support "Pre-multiplied" blending in OVL

Message ID 20240710-alpha-blending-v3-3-289c187f9c6f@mediatek.com (mailing list archive)
State New
Headers show
Series Support alpha blending in MTK display driver | expand

Commit Message

Hsiao Chien Sung via B4 Relay July 10, 2024, 8:52 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>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

Comments

AngeloGioacchino Del Regno July 10, 2024, 12:02 p.m. UTC | #1
Il 10/07/24 10:52, Hsiao Chien Sung via B4 Relay ha scritto:
> 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>
> Reviewed-by: CK Hu <ck.hu@mediatek.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?!

Regards,
Angelo
Shawn Sung (宋孝謙) July 11, 2024, 1:35 a.m. UTC | #2
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@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>
> > Reviewed-by: CK Hu <ck.hu@mediatek.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
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 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)
+#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 ? \
@@ -377,7 +386,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.
@@ -398,22 +408,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:
@@ -453,7 +471,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;