1. Patchew
  2. linux
  3. [PATCH v9 0/5] Fix degradation problem of alpha blending series
  4. View patch

[PATCH v9 1/5] drm/mediatek: ovl: Fix XRGB format breakage for blend_modes unsupported SoCs

Jason-JH.Lin posted 5 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v9 1/5] drm/mediatek: ovl: Fix XRGB format breakage for blend_modes unsupported SoCs
Posted by Jason-JH.Lin 1 month, 3 weeks ago 
If the constant alpha always enable, the SoCs that is not supported the
ignore pixel alpha bit will still use constant alpha. That will break
the original constant alpha setting of XRGB foramt for blend_modes
unsupported SoCs, such as MT8173.

Note that ignore pixel alpha bit is suppored if the SoC support the
blend_modes.
Make the constatnt alpha only enable when having a vliad blend_mode or
setting the has_alpha to fix the downgrade issue.

Fixes: bc46eb5d5d77 ("drm/mediatek: Support DRM plane alpha in OVL")
Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 89b439dcf3a6..8453a72f9e59 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -473,8 +473,14 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
 
 	con = ovl_fmt_convert(ovl, fmt, blend_mode);
 	if (state->base.fb) {
-		con |= OVL_CON_AEN;
 		con |= state->base.alpha & OVL_CON_ALPHA;
+
+		/*
+		 * For blend_modes supported SoCs, always enable constant alpha.
+		 * For blend_modes unsupported SoCs, enable constant alpha when has_alpha is set.
+		 */
+		if (blend_mode || state->base.fb->format->has_alpha)
+			con |= OVL_CON_AEN;
 	}
 
 	/* CONST_BLD must be enabled for XRGB formats although the alpha channel
-- 
2.43.0
Re: [PATCH v9 1/5] drm/mediatek: ovl: Fix XRGB format breakage for blend_modes unsupported SoCs
Posted by CK Hu (胡俊光) 1 month, 2 weeks ago 
Hi, Jason:

On Mon, 2024-10-07 at 15:00 +0800, Jason-JH.Lin wrote:
> If the constant alpha always enable, the SoCs that is not supported the
> ignore pixel alpha bit will still use constant alpha. That will break
> the original constant alpha setting of XRGB foramt for blend_modes
> unsupported SoCs, such as MT8173.
> 
> Note that ignore pixel alpha bit is suppored if the SoC support the
> blend_modes.
> Make the constatnt alpha only enable when having a vliad blend_mode or
> setting the has_alpha to fix the downgrade issue.

It's alpha blending enable, not constant alpha enable.

> 
> Fixes: bc46eb5d5d77 ("drm/mediatek: Support DRM plane alpha in OVL")
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 89b439dcf3a6..8453a72f9e59 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -473,8 +473,14 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>  
>  	con = ovl_fmt_convert(ovl, fmt, blend_mode);
>  	if (state->base.fb) {
> -		con |= OVL_CON_AEN;
>  		con |= state->base.alpha & OVL_CON_ALPHA;
> +
> +		/*
> +		 * For blend_modes supported SoCs, always enable constant alpha.
> +		 * For blend_modes unsupported SoCs, enable constant alpha when has_alpha is set.

It's alpha blending enable, not constant alpha enable.

Regards,
CK

> +		 */
> +		if (blend_mode || state->base.fb->format->has_alpha)
> +			con |= OVL_CON_AEN;
>  	}
>  
>  	/* CONST_BLD must be enabled for XRGB formats although the alpha channel
Re: [PATCH v9 1/5] drm/mediatek: ovl: Fix XRGB format breakage for blend_modes unsupported SoCs
Posted by Jason-JH Lin (林睿祥) 1 month, 2 weeks ago 
On Tue, 2024-10-08 at 03:51 +0000, CK Hu (胡俊光) wrote:
> Hi, Jason:
> 
> On Mon, 2024-10-07 at 15:00 +0800, Jason-JH.Lin wrote:
> > If the constant alpha always enable, the SoCs that is not supported
> > the
> > ignore pixel alpha bit will still use constant alpha. That will
> > break
> > the original constant alpha setting of XRGB foramt for blend_modes
> > unsupported SoCs, such as MT8173.
> > 
> > Note that ignore pixel alpha bit is suppored if the SoC support the
> > blend_modes.
> > Make the constatnt alpha only enable when having a vliad blend_mode
> > or
> > setting the has_alpha to fix the downgrade issue.
> 
> It's alpha blending enable, not constant alpha enable.

OK, I'll correct the commit message.

> 
> > 
> > Fixes: bc46eb5d5d77 ("drm/mediatek: Support DRM plane alpha in
> > OVL")
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index 89b439dcf3a6..8453a72f9e59 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -473,8 +473,14 @@ void mtk_ovl_layer_config(struct device *dev,
> > unsigned int idx,
> >  
> >  	con = ovl_fmt_convert(ovl, fmt, blend_mode);
> >  	if (state->base.fb) {
> > -		con |= OVL_CON_AEN;
> >  		con |= state->base.alpha & OVL_CON_ALPHA;
> > +
> > +		/*
> > +		 * For blend_modes supported SoCs, always enable
> > constant alpha.
> > +		 * For blend_modes unsupported SoCs, enable constant
> > alpha when has_alpha is set.
> 
> It's alpha blending enable, not constant alpha enable.
> 
OK, I'll correct the comment.

Regards,
Jason-JH.Lin

> Regards,
> CK
Re: [PATCH v9 1/5] drm/mediatek: ovl: Fix XRGB format breakage for blend_modes unsupported SoCs
Posted by Markus Elfring 1 month, 3 weeks ago 
…
> Make the constatnt alpha only enable when having a vliad blend_mode or
…

Another wording suggestion:
Enable the constant alpha channel only when having a valid blending mode?


…
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>

Does a dot really belong to this personal name?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.12-rc2#n396

Regards,
Markus
Re: [PATCH v9 1/5] drm/mediatek: ovl: Fix XRGB format breakage for blend_modes unsupported SoCs
Posted by AngeloGioacchino Del Regno 1 month, 3 weeks ago 
Il 07/10/24 09:00, Jason-JH.Lin ha scritto:
> If the constant alpha always enable, the SoCs that is not supported the
> ignore pixel alpha bit will still use constant alpha. That will break
> the original constant alpha setting of XRGB foramt for blend_modes
> unsupported SoCs, such as MT8173.
> 
> Note that ignore pixel alpha bit is suppored if the SoC support the
> blend_modes.
> Make the constatnt alpha only enable when having a vliad blend_mode or
> setting the has_alpha to fix the downgrade issue.
> 
> Fixes: bc46eb5d5d77 ("drm/mediatek: Support DRM plane alpha in OVL")
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
>   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 89b439dcf3a6..8453a72f9e59 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -473,8 +473,14 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>   
>   	con = ovl_fmt_convert(ovl, fmt, blend_mode);
>   	if (state->base.fb) {
> -		con |= OVL_CON_AEN;
>   		con |= state->base.alpha & OVL_CON_ALPHA;
> +
> +		/*
> +		 * For blend_modes supported SoCs, always enable constant alpha.
> +		 * For blend_modes unsupported SoCs, enable constant alpha when has_alpha is set.
> +		 */
> +		if (blend_mode || state->base.fb->format->has_alpha)
> +			con |= OVL_CON_AEN;

I'd say that you should make sure that OVL_CON_AEN is not set when
!blend_mode && !has_alpha, and you can do that like this:

		if (blend_mode || state->base.fb->format->has_alpha)
			con |= OVL_CON_AEN;
		else
			con &= ~OVL_CON_AEN;

After applying the proposed change,
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Re: [PATCH v9 1/5] drm/mediatek: ovl: Fix XRGB format breakage for blend_modes unsupported SoCs
Posted by Jason-JH Lin (林睿祥) 1 month, 3 weeks ago 
Hi Angelo,

Thanks for the reviews.

On Mon, 2024-10-07 at 11:36 +0200, AngeloGioacchino Del Regno wrote:
> Il 07/10/24 09:00, Jason-JH.Lin ha scritto:
> > If the constant alpha always enable, the SoCs that is not supported
> > the
> > ignore pixel alpha bit will still use constant alpha. That will
> > break
> > the original constant alpha setting of XRGB foramt for blend_modes
> > unsupported SoCs, such as MT8173.
> > 
> > Note that ignore pixel alpha bit is suppored if the SoC support the
> > blend_modes.
> > Make the constatnt alpha only enable when having a vliad blend_mode
> > or
> > setting the has_alpha to fix the downgrade issue.
> > 
> > Fixes: bc46eb5d5d77 ("drm/mediatek: Support DRM plane alpha in
> > OVL")
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> >   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index 89b439dcf3a6..8453a72f9e59 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -473,8 +473,14 @@ void mtk_ovl_layer_config(struct device *dev,
> > unsigned int idx,
> >   
> >   	con = ovl_fmt_convert(ovl, fmt, blend_mode);

Because con is a local variable declared in the beginning of
mtk_ovl_layer_config() and then being assigned to the return value of
ovl_fmt_convert() here.

> >   	if (state->base.fb) {
> > -		con |= OVL_CON_AEN;
> >   		con |= state->base.alpha & OVL_CON_ALPHA;
> > +
> > +		/*
> > +		 * For blend_modes supported SoCs, always enable
> > constant alpha.
> > +		 * For blend_modes unsupported SoCs, enable constant
> > alpha when has_alpha is set.
> > +		 */
> > +		if (blend_mode || state->base.fb->format->has_alpha)
> > +			con |= OVL_CON_AEN;
> 
> I'd say that you should make sure that OVL_CON_AEN is not set when
> !blend_mode && !has_alpha, and you can do that like this:
> 
> 		if (blend_mode || state->base.fb->format->has_alpha)
> 			con |= OVL_CON_AEN;

I think the OVL_CON_AEN won't be set when !blend_mode && !has_alpha.
Can I drop the else case here?

> 		else
> 			con &= ~OVL_CON_AEN;

Regards,
Jason-JH.Lin

> 
> After applying the proposed change,
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.