diff mbox series

drm/mediatek: Fix XRGB format breakage for blend_modes unsupported SoCs

Message ID 20241005095234.12925-1-jason-jh.lin@mediatek.com (mailing list archive)
State New, archived
Headers show
Series drm/mediatek: Fix XRGB format breakage for blend_modes unsupported SoCs | expand

Commit Message

Jason-JH Lin (林睿祥) Oct. 5, 2024, 9:52 a.m. UTC 
If the constant alpha always set, the SoCs that is not supported the
ignore 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.

Make the constatnt alpha only set when setting has_alpha or supported
blend_modes SoC 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 | 28 +++++++++++++++----------
 1 file changed, 17 insertions(+), 11 deletions(-)

Comments

Yassine Oudjana Oct. 6, 2024, 8:14 a.m. UTC | #1 
From: Yassine Oudjana <y.oudjana@protonmail.com>

On 05/10/2024 12:52 pm, Jason-JH.Lin wrote:
> If the constant alpha always set, the SoCs that is not supported the
> ignore 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.
> 
> Make the constatnt alpha only set when setting has_alpha or supported
> blend_modes SoC 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>

Tested-by: Yassine Oudjana <y.oudjana@protonmail.com>

> ---
>   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 28 +++++++++++++++----------
>   1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 703cd3f6c6d3..de3addd89442 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -494,19 +494,25 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>   
>   	con = mtk_ovl_fmt_convert(ovl, state);
>   	if (state->base.fb) {
> -		con |= OVL_CON_AEN;
> -		con |= state->base.alpha & OVL_CON_ALPHA;
> +		/*
> +		 * For blend_modes supported SoCs, always set constant alpha.
> +		 * For blend_modes unsupported SoCs, set constant alpha when has_alpha is set.
> +		 */
> +		if (ovl->data->blend_modes || state->base.fb->format->has_alpha) {
> +			con |= OVL_CON_AEN;
> +			con |= state->base.alpha & OVL_CON_ALPHA;
> +		}
> +
> +		/*
> +		 * Although the alpha channel can be ignored, CONST_BLD must be enabled
> +		 * for XRGB format, otherwise OVL will still read the value from memory.
> +		 * For RGB888 related formats, whether CONST_BLD is enabled or not won't
> +		 * affect the result. Therefore we use !has_alpha as the condition.
> +		 */
> +		if (blend_mode == DRM_MODE_BLEND_PIXEL_NONE || !state->base.fb->format->has_alpha)
> +			ignore_pixel_alpha = OVL_CONST_BLEND;
>   	}
>   
> -	/* CONST_BLD must be enabled for XRGB formats although the alpha channel
> -	 * can be ignored, or OVL will still read the value from memory.
> -	 * For RGB888 related formats, whether CONST_BLD is enabled or not won't
> -	 * affect the result. Therefore we use !has_alpha as the condition.
> -	 */
> -	if ((state->base.fb && !state->base.fb->format->has_alpha) ||
> -	    blend_mode == DRM_MODE_BLEND_PIXEL_NONE)
> -		ignore_pixel_alpha = OVL_CONST_BLEND;
> -
>   	if (pending->rotation & DRM_MODE_REFLECT_Y) {
>   		con |= OVL_CON_VIRT_FLIP;
>   		addr += (pending->height - 1) * pending->pitch;
CK Hu (胡俊光) Oct. 7, 2024, 5:57 a.m. UTC | #2 
Hi, Jason:

On Sat, 2024-10-05 at 17:52 +0800, Jason-JH.Lin wrote:
> If the constant alpha always set, the SoCs that is not supported the
> ignore 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.
> 
> Make the constatnt alpha only set when setting has_alpha or supported
> blend_modes SoC to fix the downgrade issue.

I would like this patch to be cherry-picked to kernel 6.11 easily, so let this patch base on 6.12-rc1.

> 
> 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 | 28 +++++++++++++++----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 703cd3f6c6d3..de3addd89442 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -494,19 +494,25 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>  
>  	con = mtk_ovl_fmt_convert(ovl, state);
>  	if (state->base.fb) {
> -		con |= OVL_CON_AEN;
> -		con |= state->base.alpha & OVL_CON_ALPHA;
> +		/*
> +		 * For blend_modes supported SoCs, always set constant alpha.
> +		 * For blend_modes unsupported SoCs, set constant alpha when has_alpha is set.
> +		 */
> +		if (ovl->data->blend_modes || state->base.fb->format->has_alpha) {
> +			con |= OVL_CON_AEN;
> +			con |= state->base.alpha & OVL_CON_ALPHA;

MT8173 does not support ignore pixel alpha, so OVL_CON_AEN would be disabled for XRGB.
And this is not related to plane alpha, so plane alpha setting should be moved out of here.

> +		}
> +
> +		/*
> +		 * Although the alpha channel can be ignored, CONST_BLD must be enabled
> +		 * for XRGB format, otherwise OVL will still read the value from memory.
> +		 * For RGB888 related formats, whether CONST_BLD is enabled or not won't
> +		 * affect the result. Therefore we use !has_alpha as the condition.
> +		 */
> +		if (blend_mode == DRM_MODE_BLEND_PIXEL_NONE || !state->base.fb->format->has_alpha)
> +			ignore_pixel_alpha = OVL_CONST_BLEND;

This modification is not related to the bug fix, so separate to another patch.

Regards,
CK

>  	}
>  
> -	/* CONST_BLD must be enabled for XRGB formats although the alpha channel
> -	 * can be ignored, or OVL will still read the value from memory.
> -	 * For RGB888 related formats, whether CONST_BLD is enabled or not won't
> -	 * affect the result. Therefore we use !has_alpha as the condition.
> -	 */
> -	if ((state->base.fb && !state->base.fb->format->has_alpha) ||
> -	    blend_mode == DRM_MODE_BLEND_PIXEL_NONE)
> -		ignore_pixel_alpha = OVL_CONST_BLEND;
> -
>  	if (pending->rotation & DRM_MODE_REFLECT_Y) {
>  		con |= OVL_CON_VIRT_FLIP;
>  		addr += (pending->height - 1) * pending->pitch;
Jason-JH Lin (林睿祥) Oct. 7, 2024, 6:09 a.m. UTC | #3 
Hi CK,

On Mon, 2024-10-07 at 05:57 +0000, CK Hu (胡俊光) wrote:
> Hi, Jason:
> 
> On Sat, 2024-10-05 at 17:52 +0800, Jason-JH.Lin wrote:
> > If the constant alpha always set, the SoCs that is not supported
> > the
> > ignore 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.
> > 
> > Make the constatnt alpha only set when setting has_alpha or
> > supported
> > blend_modes SoC to fix the downgrade issue.
> 
> I would like this patch to be cherry-picked to kernel 6.11 easily, so
> let this patch base on 6.12-rc1.

OK, I'll move this patch in front of this series:
[1] Fix degradation problem of alpha blending series
- 
https://patchwork.kernel.org/project/linux-mediatek/list/?series=893634
and then send v9 to let you cherry-pick it to kernel-6.11 easily.

> 
> > 
> > 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 | 28 +++++++++++++++----
> > ------
> >  1 file changed, 17 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index 703cd3f6c6d3..de3addd89442 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -494,19 +494,25 @@ void mtk_ovl_layer_config(struct device *dev,
> > unsigned int idx,
> >  
> >  	con = mtk_ovl_fmt_convert(ovl, state);
> >  	if (state->base.fb) {
> > -		con |= OVL_CON_AEN;
> > -		con |= state->base.alpha & OVL_CON_ALPHA;
> > +		/*
> > +		 * For blend_modes supported SoCs, always set constant
> > alpha.
> > +		 * For blend_modes unsupported SoCs, set constant alpha
> > when has_alpha is set.
> > +		 */
> > +		if (ovl->data->blend_modes || state->base.fb->format-
> > >has_alpha) {
> > +			con |= OVL_CON_AEN;
> > +			con |= state->base.alpha & OVL_CON_ALPHA;
> 
> MT8173 does not support ignore pixel alpha, so OVL_CON_AEN would be
> disabled for XRGB.
> And this is not related to plane alpha, so plane alpha setting should
> be moved out of here.

OK, that make sense.
But I would like to ask Yassine and Adam to test this change again.

> 
> > +		}
> > +
> > +		/*
> > +		 * Although the alpha channel can be ignored, CONST_BLD
> > must be enabled
> > +		 * for XRGB format, otherwise OVL will still read the
> > value from memory.
> > +		 * For RGB888 related formats, whether CONST_BLD is
> > enabled or not won't
> > +		 * affect the result. Therefore we use !has_alpha as
> > the condition.
> > +		 */
> > +		if (blend_mode == DRM_MODE_BLEND_PIXEL_NONE || !state-
> > >base.fb->format->has_alpha)
> > +			ignore_pixel_alpha = OVL_CONST_BLEND;
> 
> This modification is not related to the bug fix, so separate to
> another patch.

OK, I'll separate it.

Regards,
Jason-JH.Lin

> 
> Regards,
> CK
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 703cd3f6c6d3..de3addd89442 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -494,19 +494,25 @@  void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
 
 	con = mtk_ovl_fmt_convert(ovl, state);
 	if (state->base.fb) {
-		con |= OVL_CON_AEN;
-		con |= state->base.alpha & OVL_CON_ALPHA;
+		/*
+		 * For blend_modes supported SoCs, always set constant alpha.
+		 * For blend_modes unsupported SoCs, set constant alpha when has_alpha is set.
+		 */
+		if (ovl->data->blend_modes || state->base.fb->format->has_alpha) {
+			con |= OVL_CON_AEN;
+			con |= state->base.alpha & OVL_CON_ALPHA;
+		}
+
+		/*
+		 * Although the alpha channel can be ignored, CONST_BLD must be enabled
+		 * for XRGB format, otherwise OVL will still read the value from memory.
+		 * For RGB888 related formats, whether CONST_BLD is enabled or not won't
+		 * affect the result. Therefore we use !has_alpha as the condition.
+		 */
+		if (blend_mode == DRM_MODE_BLEND_PIXEL_NONE || !state->base.fb->format->has_alpha)
+			ignore_pixel_alpha = OVL_CONST_BLEND;
 	}
 
-	/* CONST_BLD must be enabled for XRGB formats although the alpha channel
-	 * can be ignored, or OVL will still read the value from memory.
-	 * For RGB888 related formats, whether CONST_BLD is enabled or not won't
-	 * affect the result. Therefore we use !has_alpha as the condition.
-	 */
-	if ((state->base.fb && !state->base.fb->format->has_alpha) ||
-	    blend_mode == DRM_MODE_BLEND_PIXEL_NONE)
-		ignore_pixel_alpha = OVL_CONST_BLEND;
-
 	if (pending->rotation & DRM_MODE_REFLECT_Y) {
 		con |= OVL_CON_VIRT_FLIP;
 		addr += (pending->height - 1) * pending->pitch;