diff mbox series

[v3,13/14] drm/mediatek: Support DRM plane alpha in OVL

Message ID 20240620-igt-v3-13-a9d62d2e2c7e@mediatek.com (mailing list archive)
State New
Headers show
Series This series fixes the errors of MediaTek display driver found by IGT. | expand

Commit Message

Hsiao Chien Sung via B4 Relay June 19, 2024, 4:38 p.m. UTC
From: Hsiao Chien Sung <shawn.sung@mediatek.com>

Set the plane alpha according to DRM plane property.

Reviewed-by: CK Hu <ck.hu@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Adam Thiede Sept. 30, 2024, 5:48 p.m. UTC | #1
On 6/19/24 11:38, Hsiao Chien Sung via B4 Relay wrote:
> From: Hsiao Chien Sung <shawn.sung@mediatek.com>
> 
> Set the plane alpha according to DRM plane property.
> 
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 943db4f1bd6b..4b370bc0746d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -458,8 +458,10 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>   	}
>   
>   	con = ovl_fmt_convert(ovl, fmt);
> -	if (state->base.fb && state->base.fb->format->has_alpha)
> -		con |= OVL_CON_AEN | OVL_CON_ALPHA;
> +	if (state->base.fb) {
> +		con |= OVL_CON_AEN;
> +		con |= state->base.alpha & OVL_CON_ALPHA;
> +	}
>   
>   	/* CONST_BLD must be enabled for XRGB formats although the alpha channel
>   	 * can be ignored, or OVL will still read the value from memory.
> 
Hello, I believe that this commit has caused a problem for my Lenovo 
C330 Chromebook running postmarketOS.

With kernel 6.11 this device didn't show any text on the tty or splash 
screen during booting, but graphical environments (wayland, xorg) do 
appear. With a few bisects I found it to be this commit. With it 
reverted I'm able to get text on the tty again.

The kernel config is here: 
https://gitlab.com/adamthiede/pmaports/-/tree/mt8173-611/device/community/linux-postmarketos-mediatek-mt8173/
To be perfectly clear, this device is not running Chrome OS.

I'm still rather new at this so it's also likely I got something wrong 
or have a bad configuration option. If there is any more information I 
can provide please let me know. Thank you.

- Adam Thiede
CK Hu (胡俊光) Oct. 1, 2024, 8:55 a.m. UTC | #2
Hi, Jason:

Would you clarify this problem?

Regards,
CK

On Mon, 2024-09-30 at 12:48 -0500, Adam Thiede wrote:
>  	 
> External email : Please do not click links or open attachments until you have verified the sender or the content.
>  On 6/19/24 11:38, Hsiao Chien Sung via B4 Relay wrote:
> > From: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > 
> > Set the plane alpha according to DRM plane property.
> > 
> > Reviewed-by: CK Hu <ck.hu@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > ---
> >   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index 943db4f1bd6b..4b370bc0746d 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -458,8 +458,10 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
> >   }
> >   
> >   con = ovl_fmt_convert(ovl, fmt);
> > -if (state->base.fb && state->base.fb->format->has_alpha)
> > -con |= OVL_CON_AEN | OVL_CON_ALPHA;
> > +if (state->base.fb) {
> > +con |= OVL_CON_AEN;
> > +con |= state->base.alpha & OVL_CON_ALPHA;
> > +}
> >   
> >   /* CONST_BLD must be enabled for XRGB formats although the alpha channel
> >    * can be ignored, or OVL will still read the value from memory.
> > 
> Hello, I believe that this commit has caused a problem for my Lenovo 
> C330 Chromebook running postmarketOS.
> 
> With kernel 6.11 this device didn't show any text on the tty or splash 
> screen during booting, but graphical environments (wayland, xorg) do 
> appear. With a few bisects I found it to be this commit. With it 
> reverted I'm able to get text on the tty again.
> 
> The kernel config is here: 
> https://gitlab.com/adamthiede/pmaports/-/tree/mt8173-611/device/community/linux-postmarketos-mediatek-mt8173/
> To be perfectly clear, this device is not running Chrome OS.
> 
> I'm still rather new at this so it's also likely I got something wrong 
> or have a bad configuration option. If there is any more information I 
> can provide please let me know. Thank you.
> 
> - Adam Thiede
Jason-JH Lin (林睿祥) Oct. 1, 2024, 6:02 p.m. UTC | #3
On Tue, 2024-10-01 at 08:55 +0000, CK Hu (胡俊光) wrote:
> Hi, Jason:
> 
> Would you clarify this problem?
> 

OK~

> Regards,
> CK
> 
> On Mon, 2024-09-30 at 12:48 -0500, Adam Thiede wrote:
> >  	 
> > External email : Please do not click links or open attachments
> > until you have verified the sender or the content.
> >  On 6/19/24 11:38, Hsiao Chien Sung via B4 Relay wrote:
> > > From: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > > 
> > > Set the plane alpha according to DRM plane property.
> > > 
> > > Reviewed-by: CK Hu <ck.hu@mediatek.com>
> > > Reviewed-by: AngeloGioacchino Del Regno <
> > > angelogioacchino.delregno@collabora.com>
> > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek
> > > SoC MT8173.")
> > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > > ---
> > >   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++++--
> > >   1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > index 943db4f1bd6b..4b370bc0746d 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > > @@ -458,8 +458,10 @@ void mtk_ovl_layer_config(struct device
> > > *dev, unsigned int idx,
> > >   }
> > >   
> > >   con = ovl_fmt_convert(ovl, fmt);
> > > -if (state->base.fb && state->base.fb->format->has_alpha)
> > > -con |= OVL_CON_AEN | OVL_CON_ALPHA;
> > > +if (state->base.fb) {
> > > +con |= OVL_CON_AEN;
> > > +con |= state->base.alpha & OVL_CON_ALPHA;

Hi Adam,

Could you print out the "fmt", "state->base.fb->format-
>has_alpha", "state->base.alpha" and "con" here?

pr_info("fmt:0x%x, has_alpha:0x%x, alpha:0x%x, con:0x%x \n",
        fmt, state->base.fb->format->has_alpha,
        state->base.alpha, con);

I'm not sure if it's the color format setting problem, maybe there is
something wire configuration here, such as XRGB8888 with alpha or
ARGB8888 without alpha.

So I want these information to compare with my MT8188. Thanks!

Regards,
Jason-JH.Lin

> > > +}
> > >   
> > >   /* CONST_BLD must be enabled for XRGB formats although the
> > > alpha channel
> > >    * can be ignored, or OVL will still read the value from
> > > memory.
> > > 
> > 
> > Hello, I believe that this commit has caused a problem for my
> > Lenovo 
> > C330 Chromebook running postmarketOS.
> > 
> > With kernel 6.11 this device didn't show any text on the tty or
> > splash 
> > screen during booting, but graphical environments (wayland, xorg)
> > do 
> > appear. With a few bisects I found it to be this commit. With it 
> > reverted I'm able to get text on the tty again.
> > 
> > The kernel config is here: 
> > 
https://gitlab.com/adamthiede/pmaports/-/tree/mt8173-611/device/community/linux-postmarketos-mediatek-mt8173/
> > To be perfectly clear, this device is not running Chrome OS.
> > 
> > I'm still rather new at this so it's also likely I got something
> > wrong 
> > or have a bad configuration option. If there is any more
> > information I 
> > can provide please let me know. Thank you.
> > 
> > - Adam Thiede
Adam Thiede Oct. 1, 2024, 7:51 p.m. UTC | #4
On 10/1/24 13:02, Jason-JH Lin (林睿祥) wrote:
> On Tue, 2024-10-01 at 08:55 +0000, CK Hu (胡俊光) wrote:
>> Hi, Jason:
>> 
>> Would you clarify this problem?
>> 
> 
> OK~
> 
>> Regards,
>> CK
>> 
>> On Mon, 2024-09-30 at 12:48 -0500, Adam Thiede wrote:
>> >   
>> > External email : Please do not click links or open attachments
>> > until you have verified the sender or the content.
>> >  On 6/19/24 11:38, Hsiao Chien Sung via B4 Relay wrote:
>> > > From: Hsiao Chien Sung <shawn.sung@mediatek.com>
>> > > 
>> > > Set the plane alpha according to DRM plane property.
>> > > 
>> > > Reviewed-by: CK Hu <ck.hu@mediatek.com>
>> > > Reviewed-by: AngeloGioacchino Del Regno <
>> > > angelogioacchino.delregno@collabora.com>
>> > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek
>> > > SoC MT8173.")
>> > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
>> > > ---
>> > >   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++++--
>> > >   1 file changed, 4 insertions(+), 2 deletions(-)
>> > > 
>> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>> > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>> > > index 943db4f1bd6b..4b370bc0746d 100644
>> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>> > > @@ -458,8 +458,10 @@ void mtk_ovl_layer_config(struct device
>> > > *dev, unsigned int idx,
>> > >   }
>> > >   
>> > >   con = ovl_fmt_convert(ovl, fmt);
>> > > -if (state->base.fb && state->base.fb->format->has_alpha)
>> > > -con |= OVL_CON_AEN | OVL_CON_ALPHA;
>> > > +if (state->base.fb) {
>> > > +con |= OVL_CON_AEN;
>> > > +con |= state->base.alpha & OVL_CON_ALPHA;
> 
> Hi Adam,
> 
> Could you print out the "fmt", "state->base.fb->format-
>>has_alpha", "state->base.alpha" and "con" here?
> 
> pr_info("fmt:0x%x, has_alpha:0x%x, alpha:0x%x, con:0x%x \n",
>          fmt, state->base.fb->format->has_alpha,
>          state->base.alpha, con);
> 
> I'm not sure if it's the color format setting problem, maybe there is
> something wire configuration here, such as XRGB8888 with alpha or
> ARGB8888 without alpha.
> 
> So I want these information to compare with my MT8188. Thanks!
> 
> Regards,
> Jason-JH.Lin
> 
Jason, thank you for your timely reply. I added the code you provided to 
my patch, and now get this line on endless repeat in dmesg:

fmt:0x34325258, has_alpha:0x0, alpha:0xffff, con:0x2000

This line also shows up sometimes in there, but I have no idea what 
triggers it.

fmt:0x34325241, has_alpha:0x1, alpha:0xffff, con:0x21ff

Does that help?

-Adam

>> > > +}
>> > >   
>> > >   /* CONST_BLD must be enabled for XRGB formats although the
>> > > alpha channel
>> > >    * can be ignored, or OVL will still read the value from
>> > > memory.
>> > > 
>> > 
>> > Hello, I believe that this commit has caused a problem for my
>> > Lenovo 
>> > C330 Chromebook running postmarketOS.
>> > 
>> > With kernel 6.11 this device didn't show any text on the tty or
>> > splash 
>> > screen during booting, but graphical environments (wayland, xorg)
>> > do 
>> > appear. With a few bisects I found it to be this commit. With it 
>> > reverted I'm able to get text on the tty again.
>> > 
>> > The kernel config is here: 
>> > 
> https://gitlab.com/adamthiede/pmaports/-/tree/mt8173-611/device/community/linux-postmarketos-mediatek-mt8173/
>> > To be perfectly clear, this device is not running Chrome OS.
>> > 
>> > I'm still rather new at this so it's also likely I got something
>> > wrong 
>> > or have a bad configuration option. If there is any more
>> > information I 
>> > can provide please let me know. Thank you.
>> > 
>> > - Adam Thiede
> 
> ************* MEDIATEK Confidentiality Notice ********************
> The information contained in this e-mail message (including any
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be
> conveyed only to the designated recipient(s). Any use, dissemination,
> distribution, printing, retaining or copying of this e-mail (including its
> attachments) by unintended recipient(s) is strictly prohibited and may
> be unlawful. If you are not an intended recipient of this e-mail, or believe
> that you have received this e-mail in error, please notify the sender
> immediately (by replying to this e-mail), delete any and all copies of
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!
Jason-JH Lin (林睿祥) Oct. 2, 2024, 7:50 a.m. UTC | #5
> >> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> >> > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> >> > > index 943db4f1bd6b..4b370bc0746d 100644
> >> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> >> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> >> > > @@ -458,8 +458,10 @@ void mtk_ovl_layer_config(struct device
> >> > > *dev, unsigned int idx,
> >> > >   }
> >> > >   
> >> > >   con = ovl_fmt_convert(ovl, fmt);
> >> > > -if (state->base.fb && state->base.fb->format->has_alpha)
> >> > > -con |= OVL_CON_AEN | OVL_CON_ALPHA;
> >> > > +if (state->base.fb) {
> >> > > +con |= OVL_CON_AEN;
> >> > > +con |= state->base.alpha & OVL_CON_ALPHA;
> > 
> > Hi Adam,
> > 
> > Could you print out the "fmt", "state->base.fb->format-
> >>has_alpha", "state->base.alpha" and "con" here?
> > 
> > pr_info("fmt:0x%x, has_alpha:0x%x, alpha:0x%x, con:0x%x \n",
> >          fmt, state->base.fb->format->has_alpha,
> >          state->base.alpha, con);
> > 
> > I'm not sure if it's the color format setting problem, maybe there
> is
> > something wire configuration here, such as XRGB8888 with alpha or
> > ARGB8888 without alpha.
> > 
> > So I want these information to compare with my MT8188. Thanks!
> > 
> > Regards,
> > Jason-JH.Lin
> > 
> Jason, thank you for your timely reply. I added the code you provided
> to 
> my patch, and now get this line on endless repeat in dmesg:
> 
> fmt:0x34325258, has_alpha:0x0, alpha:0xffff, con:0x2000
> 

This function is used to configure the 4 OVL hardware layer per-frame,
so it may be called 4 times in every VSYNC. If your display device is
60fps, then this line would be called N layers times in every 16.66ms.

> This line also shows up sometimes in there, but I have no idea what 
> triggers it.
> 
> fmt:0x34325241, has_alpha:0x1, alpha:0xffff, con:0x21ff
> 

From the DRM color format definition here:

https://elixir.bootlin.com/linux/v6.11.1/source/include/uapi/drm/drm_fourcc.h#L198

We can see the MACROs:
#define fourcc_code(a, b, c, d) \
        (((uint32_t)(a) << 0) | ((uint32_t)(b) << 8) | \
        ((uint32_t)(c) << 16) | ((uint32_t)(d) << 24))
...
#define DRM_FORMAT_XRGB8888 fourcc_code('X', 'R', '2', '4')
...
#define DRM_FORMAT_ARGB8888 fourcc_code('A', 'R', '2', '4')

Given the fourcc_code macro as previously described,
the DRM_FORMAT_XRGB8888 macro would translate the characters
'X', 'R', '2', '4' into a 32-bit integer value, with each character
occupying 8 bits in the order from least significant byte to most 
significant byte.

Here are the ASCII values for these characters:

'A' has an ASCII value of 65 (0x41)
'X' has an ASCII value of 88 (0x58)
'R' has an ASCII value of 82 (0x52)
'2' has an ASCII value of 50 (0x32)
'4' has an ASCII value of 52 (0x34)

Therefore, the integer value of XR24 with hex format would be:
0x34325258, and AR24 would be: 0x34325241

> Does that help?
> 
> -Adam

Here is the translation from your logs :

fmt:0x34325258, has_alpha:0x0, alpha:0xffff, con:0x2000
- DRM color format=XRGB8888
- user set has_alpha=0
- user set alpha value=0xff
- configure value to OVL hardware=0x2000

fmt:0x34325241, has_alpha:0x1, alpha:0xffff, con:0x21ff
- DRM color format=ARGB8888
- user set has_alpha=1
- user set alpha value=0xff
- configure value to OVL hardware=0x21ff

Could you tell me in which log you can see and not see the text on the
tty?



Here is some of my analysis:

In original condition:
if (state->base.fb && state->base.fb->format->has_alpha)
	con |= OVL_CON_AEN | OVL_CON_ALPHA;
- XRGB8888 will get con = 0x2000
- ARGB8888 will get con = 0x21ff
	
In current condition:
if (state->base.fb) {
	con |= OVL_CON_AEN;
	con |= state->base.alpha & OVL_CON_ALPHA;
}
- XRGB8888 will get con = 0x21ff
- ARGB8888 will get con = 0x21ff

But then XRGB8888 will set the ignore_pixel_alpha by the code below:
/* 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;

Does your code include this patch?

https://patchwork.kernel.org/project/linux-mediatek/patch/20240620-igt-v3-3-a9d62d2e2c7e@mediatek.com/

If you have included this patch, I would then check with the OVL
hardware designers whether the MT8173 supports OVL_CONST_BLEND.

Regards,
Jason-JH.Lin
Adam Thiede Oct. 2, 2024, 3:28 p.m. UTC | #6
On 10/2/24 02:50, Jason-JH Lin (林睿祥) wrote:
>> >> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>> >> > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>> >> > > index 943db4f1bd6b..4b370bc0746d 100644
>> >> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>> >> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>> >> > > @@ -458,8 +458,10 @@ void mtk_ovl_layer_config(struct device
>> >> > > *dev, unsigned int idx,
>> >> > >   }
>> >> > >   
>> >> > >   con = ovl_fmt_convert(ovl, fmt);
>> >> > > -if (state->base.fb && state->base.fb->format->has_alpha)
>> >> > > -con |= OVL_CON_AEN | OVL_CON_ALPHA;
>> >> > > +if (state->base.fb) {
>> >> > > +con |= OVL_CON_AEN;
>> >> > > +con |= state->base.alpha & OVL_CON_ALPHA;
>> > 
>> > Hi Adam,
>> > 
>> > Could you print out the "fmt", "state->base.fb->format-
>> >>has_alpha", "state->base.alpha" and "con" here?
>> > 
>> > pr_info("fmt:0x%x, has_alpha:0x%x, alpha:0x%x, con:0x%x \n",
>> >          fmt, state->base.fb->format->has_alpha,
>> >          state->base.alpha, con);
>> > 
>> > I'm not sure if it's the color format setting problem, maybe there
>> is
>> > something wire configuration here, such as XRGB8888 with alpha or
>> > ARGB8888 without alpha.
>> > 
>> > So I want these information to compare with my MT8188. Thanks!
>> > 
>> > Regards,
>> > Jason-JH.Lin
>> > 
>> Jason, thank you for your timely reply. I added the code you provided
>> to 
>> my patch, and now get this line on endless repeat in dmesg:
>> 
>> fmt:0x34325258, has_alpha:0x0, alpha:0xffff, con:0x2000
>> 
> 
> This function is used to configure the 4 OVL hardware layer per-frame,
> so it may be called 4 times in every VSYNC. If your display device is
> 60fps, then this line would be called N layers times in every 16.66ms.
> 
>> This line also shows up sometimes in there, but I have no idea what 
>> triggers it.
>> 
>> fmt:0x34325241, has_alpha:0x1, alpha:0xffff, con:0x21ff
>> 
> 
>  From the DRM color format definition here:
> 
> https://elixir.bootlin.com/linux/v6.11.1/source/include/uapi/drm/drm_fourcc.h#L198
> 
> We can see the MACROs:
> #define fourcc_code(a, b, c, d) \
>          (((uint32_t)(a) << 0) | ((uint32_t)(b) << 8) | \
>          ((uint32_t)(c) << 16) | ((uint32_t)(d) << 24))
> ...
> #define DRM_FORMAT_XRGB8888 fourcc_code('X', 'R', '2', '4')
> ...
> #define DRM_FORMAT_ARGB8888 fourcc_code('A', 'R', '2', '4')
> 
> Given the fourcc_code macro as previously described,
> the DRM_FORMAT_XRGB8888 macro would translate the characters
> 'X', 'R', '2', '4' into a 32-bit integer value, with each character
> occupying 8 bits in the order from least significant byte to most
> significant byte.
> 
> Here are the ASCII values for these characters:
> 
> 'A' has an ASCII value of 65 (0x41)
> 'X' has an ASCII value of 88 (0x58)
> 'R' has an ASCII value of 82 (0x52)
> '2' has an ASCII value of 50 (0x32)
> '4' has an ASCII value of 52 (0x34)
> 
> Therefore, the integer value of XR24 with hex format would be:
> 0x34325258, and AR24 would be: 0x34325241
> 
>> Does that help?
>> 
>> -Adam
> 
> Here is the translation from your logs :
> 
> fmt:0x34325258, has_alpha:0x0, alpha:0xffff, con:0x2000
> - DRM color format=XRGB8888
> - user set has_alpha=0
> - user set alpha value=0xff
> - configure value to OVL hardware=0x2000
> 
> fmt:0x34325241, has_alpha:0x1, alpha:0xffff, con:0x21ff
> - DRM color format=ARGB8888
> - user set has_alpha=1
> - user set alpha value=0xff
> - configure value to OVL hardware=0x21ff
> 
> Could you tell me in which log you can see and not see the text on the
> tty?
> 
> 
> 
> Here is some of my analysis:
> 
> In original condition:
> if (state->base.fb && state->base.fb->format->has_alpha)
> con |= OVL_CON_AEN | OVL_CON_ALPHA;
> - XRGB8888 will get con = 0x2000
> - ARGB8888 will get con = 0x21ff
> 
> In current condition:
> if (state->base.fb) {
> con |= OVL_CON_AEN;
> con |= state->base.alpha & OVL_CON_ALPHA;
> }
> - XRGB8888 will get con = 0x21ff
> - ARGB8888 will get con = 0x21ff
> 
> But then XRGB8888 will set the ignore_pixel_alpha by the code below:
> /* 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;
> 
> Does your code include this patch?
> 
> https://patchwork.kernel.org/project/linux-mediatek/patch/20240620-igt-v3-3-a9d62d2e2c7e@mediatek.com/
> 
> If you have included this patch, I would then check with the OVL
> hardware designers whether the MT8173 supports OVL_CONST_BLEND.
> 
> Regards,
> Jason-JH.Lin
Jason:
That is a lot of information, and quite above my head! Thank you though.

I should note that the log items I sent you are from the "good" kernel - 
6.11 with the commit reverted. Here is a much longer set of logs: 
https://termbin.com/co6v

I've rebuild 6.11 with the log statement enabled and the "bad" behavior.
Here is a dmesg from that: https://termbin.com/xiev

These logs are both from `dmesg`.

I'm fairly certain I've built with the patch you referenced enabled. The 
kernels I run are just release kernels, not RCs or git branches or 
anything. The mainline v6.11 kernel is the one that has this problem. If 
that patch has been merged into 6.11 (which, looks like it has) then 
it's in the kernel I'm building.

- Adam Thiede
Jason-JH Lin (林睿祥) Oct. 3, 2024, 5:17 a.m. UTC | #7
> Jason:
> That is a lot of information, and quite above my head! Thank you
> though.
> 
> I should note that the log items I sent you are from the "good"
> kernel - 
> 6.11 with the commit reverted. Here is a much longer set of logs: 
> https://termbin.com/co6v
> 
> I've rebuild 6.11 with the log statement enabled and the "bad"
> behavior.
> Here is a dmesg from that: https://termbin.com/xiev
> 
Hi Adam,

I think something wrong with your dmesg links, both logs look the same.
We should see this log in the "bad" one:
fmt:0x34325258, has_alpha:0x0, alpha:0xffff, con:0x2000

But anyway, I think the reason for the downgrade is clear enough to me.
So let's try to figure out the solution.

> These logs are both from `dmesg`.
> 
> I'm fairly certain I've built with the patch you referenced enabled.
> The 
> kernels I run are just release kernels, not RCs or git branches or 
> anything. The mainline v6.11 kernel is the one that has this problem.
> If 
> that patch has been merged into 6.11 (which, looks like it has) then 
> it's in the kernel I'm building.

Got it.
Then OVL_CONST_BLEND might be the unsupported configuration in MT8173,
I think we should remove the XRGB8888 format for MT8173.

Could you please try this modification and see if it'll change to use
others supported format to show the text?

--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -102,12 +102,9 @@ static inline bool is_10bit_rgb(u32 fmt)
 }

 static const u32 mt8173_formats[] = {
-       DRM_FORMAT_XRGB8888,
        DRM_FORMAT_ARGB8888,
-       DRM_FORMAT_BGRX8888,
        DRM_FORMAT_BGRA8888,
        DRM_FORMAT_ABGR8888,
-       DRM_FORMAT_XBGR8888,
        DRM_FORMAT_RGB888,
        DRM_FORMAT_BGR888,
        DRM_FORMAT_RGB565,


Regards,
Jason-JH.Lin

> 
> - Adam Thiede
Adam Thiede Oct. 3, 2024, 3:29 p.m. UTC | #8
On 10/3/24 00:17, Jason-JH Lin (林睿祥) wrote:
>> Jason:
>> That is a lot of information, and quite above my head! Thank you
>> though.
>> 
>> I should note that the log items I sent you are from the "good"
>> kernel - 
>> 6.11 with the commit reverted. Here is a much longer set of logs: 
>> https://termbin.com/co6v
>> 
>> I've rebuild 6.11 with the log statement enabled and the "bad"
>> behavior.
>> Here is a dmesg from that: https://termbin.com/xiev
>> 
> Hi Adam,
> 
> I think something wrong with your dmesg links, both logs look the same.
> We should see this log in the "bad" one:
> fmt:0x34325258, has_alpha:0x0, alpha:0xffff, con:0x2000

Apologies, I did indeed upload the same file twice. Here is the "good" one:
https://termbin.com/tb33

And the "bad" one:
https://termbin.com/yhxx

I think the fact that we're not seeing that line in the "bad" one is 
part of the problem?
> 
> But anyway, I think the reason for the downgrade is clear enough to me.
> So let's try to figure out the solution.
> 
>> These logs are both from `dmesg`.
>> 
>> I'm fairly certain I've built with the patch you referenced enabled.
>> The 
>> kernels I run are just release kernels, not RCs or git branches or 
>> anything. The mainline v6.11 kernel is the one that has this problem.
>> If 
>> that patch has been merged into 6.11 (which, looks like it has) then 
>> it's in the kernel I'm building.
> 
> Got it.
> Then OVL_CONST_BLEND might be the unsupported configuration in MT8173,
> I think we should remove the XRGB8888 format for MT8173.
> 
> Could you please try this modification and see if it'll change to use
> others supported format to show the text?
> 
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -102,12 +102,9 @@ static inline bool is_10bit_rgb(u32 fmt)
>   }
> 
>   static const u32 mt8173_formats[] = {
> -       DRM_FORMAT_XRGB8888,
>          DRM_FORMAT_ARGB8888,
> -       DRM_FORMAT_BGRX8888,
>          DRM_FORMAT_BGRA8888,
>          DRM_FORMAT_ABGR8888,
> -       DRM_FORMAT_XBGR8888,
>          DRM_FORMAT_RGB888,
>          DRM_FORMAT_BGR888,
>          DRM_FORMAT_RGB565,
> 
I've not been able to get the kernel to build with that patch; it keeps 
segfaulting at the end. I will keep attempting though.

> 
> Regards,
> Jason-JH.Lin
> 
>> 
>> - Adam Thiede
Yassine Oudjana Oct. 5, 2024, 5:54 a.m. UTC | #9
On 03/10/2024 8:17 am, Jason-JH Lin (林睿祥) wrote:
>> Jason:
>> That is a lot of information, and quite above my head! Thank you
>> though.
>>
>> I should note that the log items I sent you are from the "good"
>> kernel -
>> 6.11 with the commit reverted. Here is a much longer set of logs:
>> https://termbin.com/co6v
>>
>> I've rebuild 6.11 with the log statement enabled and the "bad"
>> behavior.
>> Here is a dmesg from that: https://termbin.com/xiev
>>
> Hi Adam,
> 
> I think something wrong with your dmesg links, both logs look the same.
> We should see this log in the "bad" one:
> fmt:0x34325258, has_alpha:0x0, alpha:0xffff, con:0x2000
> 
> But anyway, I think the reason for the downgrade is clear enough to me.
> So let's try to figure out the solution.
> 
>> These logs are both from `dmesg`.
>>
>> I'm fairly certain I've built with the patch you referenced enabled.
>> The
>> kernels I run are just release kernels, not RCs or git branches or
>> anything. The mainline v6.11 kernel is the one that has this problem.
>> If
>> that patch has been merged into 6.11 (which, looks like it has) then
>> it's in the kernel I'm building.
> 
> Got it.
> Then OVL_CONST_BLEND might be the unsupported configuration in MT8173,
> I think we should remove the XRGB8888 format for MT8173.

I've carried patches that add MT6735 support in my tree for a while and 
MT6735 broke as well with this patch. Turns out MT6735's OVL doesn't 
have the CONST_BLEND bit. It's highly likely MT8173 also doesn't since 
MT8173's DISPSYS is very similar to MT6735's.

> 
> Could you please try this modification and see if it'll change to use
> others supported format to show the text?
> 
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -102,12 +102,9 @@ static inline bool is_10bit_rgb(u32 fmt)
>   }
> 
>   static const u32 mt8173_formats[] = {
> -       DRM_FORMAT_XRGB8888,
>          DRM_FORMAT_ARGB8888,
> -       DRM_FORMAT_BGRX8888,
>          DRM_FORMAT_BGRA8888,
>          DRM_FORMAT_ABGR8888,
> -       DRM_FORMAT_XBGR8888,
>          DRM_FORMAT_RGB888,
>          DRM_FORMAT_BGR888,
>          DRM_FORMAT_RGB565,
> 
> 
> Regards,
> Jason-JH.Lin
> 
>>
>> - Adam Thiede
Yassine Oudjana Oct. 5, 2024, 6:33 a.m. UTC | #10
On 03/10/2024 8:17 am, Jason-JH Lin (林睿祥) wrote:
>> Jason:
>> That is a lot of information, and quite above my head! Thank you
>> though.
>>
>> I should note that the log items I sent you are from the "good"
>> kernel -
>> 6.11 with the commit reverted. Here is a much longer set of logs:
>> https://termbin.com/co6v
>>
>> I've rebuild 6.11 with the log statement enabled and the "bad"
>> behavior.
>> Here is a dmesg from that: https://termbin.com/xiev
>>
> Hi Adam,
> 
> I think something wrong with your dmesg links, both logs look the same.
> We should see this log in the "bad" one:
> fmt:0x34325258, has_alpha:0x0, alpha:0xffff, con:0x2000
> 
> But anyway, I think the reason for the downgrade is clear enough to me.
> So let's try to figure out the solution.
> 
>> These logs are both from `dmesg`.
>>
>> I'm fairly certain I've built with the patch you referenced enabled.
>> The
>> kernels I run are just release kernels, not RCs or git branches or
>> anything. The mainline v6.11 kernel is the one that has this problem.
>> If
>> that patch has been merged into 6.11 (which, looks like it has) then
>> it's in the kernel I'm building.
> 
> Got it.
> Then OVL_CONST_BLEND might be the unsupported configuration in MT8173,
> I think we should remove the XRGB8888 format for MT8173.
> 
> Could you please try this modification and see if it'll change to use
> others supported format to show the text?
> 
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -102,12 +102,9 @@ static inline bool is_10bit_rgb(u32 fmt)
>   }
> 
>   static const u32 mt8173_formats[] = {
> -       DRM_FORMAT_XRGB8888,
>          DRM_FORMAT_ARGB8888,
> -       DRM_FORMAT_BGRX8888,
>          DRM_FORMAT_BGRA8888,
>          DRM_FORMAT_ABGR8888,
> -       DRM_FORMAT_XBGR8888,
>          DRM_FORMAT_RGB888,
>          DRM_FORMAT_BGR888,
>          DRM_FORMAT_RGB565,

This is what I get on MT6735:

[    1.729467] mediatek-drm mediatek-drm.1.auto: [drm] bpp/depth value 
of 32/24 not supported
[    1.737777] mediatek-drm mediatek-drm.1.auto: [drm] No compatible 
format found
[    1.745943] mediatek-drm mediatek-drm.1.auto: [drm] *ERROR* 
fbdev-dma: Failed to setup generic emulation (ret=-22)

> 
> Regards,
> Jason-JH.Lin
> 
>>
>> - Adam Thiede
Jason-JH Lin (林睿祥) Oct. 5, 2024, 10:02 a.m. UTC | #11
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -102,12 +102,9 @@ static inline bool is_10bit_rgb(u32 fmt)
> >   }
> > 
> >   static const u32 mt8173_formats[] = {
> > -       DRM_FORMAT_XRGB8888,
> >          DRM_FORMAT_ARGB8888,
> > -       DRM_FORMAT_BGRX8888,
> >          DRM_FORMAT_BGRA8888,
> >          DRM_FORMAT_ABGR8888,
> > -       DRM_FORMAT_XBGR8888,
> >          DRM_FORMAT_RGB888,
> >          DRM_FORMAT_BGR888,
> >          DRM_FORMAT_RGB565,
> 
> This is what I get on MT6735:
> 
> [    1.729467] mediatek-drm mediatek-drm.1.auto: [drm] bpp/depth
> value 
> of 32/24 not supported
> [    1.737777] mediatek-drm mediatek-drm.1.auto: [drm] No compatible 
> format found
> [    1.745943] mediatek-drm mediatek-drm.1.auto: [drm] *ERROR* 
> fbdev-dma: Failed to setup generic emulation (ret=-22)
> 

Hi Adam, Yassine,

Please try the patches below and check if they can fix the downgrade
issue:
[1] Fix degradation problem of alpha blending series
- 
https://patchwork.kernel.org/project/linux-mediatek/list/?series=893634
[2] drm/mediatek: Fix XRGB format breakage for blend_modes unsupported
SoCs
- 
https://patchwork.kernel.org/project/linux-mediatek/patch/20241005095234.12925-1-jason-jh.lin@mediatek.com/

Regards,
Jason-JH.Lin

> > 
> > Regards,
> > Jason-JH.Lin
> > 
> >>
> >> - Adam Thiede
>
Adam Thiede Oct. 5, 2024, 5:32 p.m. UTC | #12
On 10/5/24 05:02, Jason-JH Lin (林睿祥) wrote:
>> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>> > @@ -102,12 +102,9 @@ static inline bool is_10bit_rgb(u32 fmt)
>> >   }
>> > 
>> >   static const u32 mt8173_formats[] = {
>> > -       DRM_FORMAT_XRGB8888,
>> >          DRM_FORMAT_ARGB8888,
>> > -       DRM_FORMAT_BGRX8888,
>> >          DRM_FORMAT_BGRA8888,
>> >          DRM_FORMAT_ABGR8888,
>> > -       DRM_FORMAT_XBGR8888,
>> >          DRM_FORMAT_RGB888,
>> >          DRM_FORMAT_BGR888,
>> >          DRM_FORMAT_RGB565,
>> 
>> This is what I get on MT6735:
>> 
>> [    1.729467] mediatek-drm mediatek-drm.1.auto: [drm] bpp/depth
>> value 
>> of 32/24 not supported
>> [    1.737777] mediatek-drm mediatek-drm.1.auto: [drm] No compatible 
>> format found
>> [    1.745943] mediatek-drm mediatek-drm.1.auto: [drm] *ERROR* 
>> fbdev-dma: Failed to setup generic emulation (ret=-22)
>> 
> 
> Hi Adam, Yassine,
> 
> Please try the patches below and check if they can fix the downgrade
> issue:
> [1] Fix degradation problem of alpha blending series
> -
> https://patchwork.kernel.org/project/linux-mediatek/list/?series=893634
> [2] drm/mediatek: Fix XRGB format breakage for blend_modes unsupported
> SoCs
> -
> https://patchwork.kernel.org/project/linux-mediatek/patch/20241005095234.12925-1-jason-jh.lin@mediatek.com/
> 
> Regards,
> Jason-JH.Lin

Jason,
I've built 6.12-rc1 with those patch series applied. (I am also not 
reverting the other commit.) This fixes the issue - I'm able to see the 
console now. Thank you! Hopefully these can go into 6.12?
- Adam Thiede
Jason-JH Lin (林睿祥) Oct. 7, 2024, 7:22 a.m. UTC | #13
> > 
> > Hi Adam, Yassine,
> > 
> > Please try the patches below and check if they can fix the
> downgrade
> > issue:
> > [1] Fix degradation problem of alpha blending series
> > -
> > 
> https://patchwork.kernel.org/project/linux-mediatek/list/?series=893634
> > [2] drm/mediatek: Fix XRGB format breakage for blend_modes
> unsupported
> > SoCs
> > -
> > 
> https://patchwork.kernel.org/project/linux-mediatek/patch/20241005095234.12925-1-jason-jh.lin@mediatek.com/
> > 
> > Regards,
> > Jason-JH.Lin
> 
> Jason,
> I've built 6.12-rc1 with those patch series applied. (I am also not 
> reverting the other commit.) This fixes the issue - I'm able to see
> the 
> console now. Thank you! Hopefully these can go into 6.12?
> - Adam Thiede

Yes, they will go into 6.12.



Hi Adam, Yassine,

Since the maintainer, CK, had some comments at the [2], I made some
changes for it.

Could you please test again only with this single fix patch:
[3] drm/mediatek: ovl: Fix XRGB format breakage for blend_modes
unsupported SoCs
- 
https://patchwork.kernel.org/project/linux-mediatek/patch/20241007070101.23263-2-jason-jh.lin@mediatek.com/
 
and see if it can fix your problem?

Regards,
Jason-JH.Lin
Adam Thiede Oct. 7, 2024, 10:54 a.m. UTC | #14
On 10/7/24 02:22, Jason-JH Lin (林睿祥) wrote:
>> > 
>> > Hi Adam, Yassine,
>> > 
>> > Please try the patches below and check if they can fix the
>> downgrade
>> > issue:
>> > [1] Fix degradation problem of alpha blending series
>> > -
>> > 
>> https://patchwork.kernel.org/project/linux-mediatek/list/?series=893634
>> > [2] drm/mediatek: Fix XRGB format breakage for blend_modes
>> unsupported
>> > SoCs
>> > -
>> > 
>> https://patchwork.kernel.org/project/linux-mediatek/patch/20241005095234.12925-1-jason-jh.lin@mediatek.com/
>> > 
>> > Regards,
>> > Jason-JH.Lin
>> 
>> Jason,
>> I've built 6.12-rc1 with those patch series applied. (I am also not 
>> reverting the other commit.) This fixes the issue - I'm able to see
>> the 
>> console now. Thank you! Hopefully these can go into 6.12?
>> - Adam Thiede
> 
> Yes, they will go into 6.12.
> 
> 
> 
> Hi Adam, Yassine,
> 
> Since the maintainer, CK, had some comments at the [2], I made some
> changes for it.
> 
> Could you please test again only with this single fix patch:
> [3] drm/mediatek: ovl: Fix XRGB format breakage for blend_modes
> unsupported SoCs
> -
> https://patchwork.kernel.org/project/linux-mediatek/patch/20241007070101.23263-2-jason-jh.lin@mediatek.com/
>   
> and see if it can fix your problem?
> 
> Regards,
> Jason-JH.Lin

Jason,
Just this patch on 6.12-rc1 does fix my problem too.
Thank you.

-Adam
Jason-JH Lin (林睿祥) Oct. 7, 2024, 2:38 p.m. UTC | #15
> > Hi Adam, Yassine,
> > 
> > Since the maintainer, CK, had some comments at the [2], I made some
> > changes for it.
> > 
> > Could you please test again only with this single fix patch:
> > [3] drm/mediatek: ovl: Fix XRGB format breakage for blend_modes
> > unsupported SoCs
> > -
> > 
> https://patchwork.kernel.org/project/linux-mediatek/patch/20241007070101.23263-2-jason-jh.lin@mediatek.com/
> >   
> > and see if it can fix your problem?
> > 
> > Regards,
> > Jason-JH.Lin
> 
> Jason,
> Just this patch on 6.12-rc1 does fix my problem too.
> Thank you.

Hi Adam,

Thanks for your verification.
I'll make these fix patches get reviewed soon.

Regards,
Jason-JH.Lin

> 
> -Adam
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 943db4f1bd6b..4b370bc0746d 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -458,8 +458,10 @@  void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
 	}
 
 	con = ovl_fmt_convert(ovl, fmt);
-	if (state->base.fb && state->base.fb->format->has_alpha)
-		con |= OVL_CON_AEN | OVL_CON_ALPHA;
+	if (state->base.fb) {
+		con |= OVL_CON_AEN;
+		con |= state->base.alpha & OVL_CON_ALPHA;
+	}
 
 	/* CONST_BLD must be enabled for XRGB formats although the alpha channel
 	 * can be ignored, or OVL will still read the value from memory.