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

[Adam Thiede]

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