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

From: Adam Thiede
Date: Wed Oct 02 2024 - 11:30:30 EST


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@xxxxxxxxxxxx/

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