Re: [PATCH v4 2/8] drm/vkms: Add support for ARGB8888 formats

[Maíra Canal]

Hi Louis,

On 13/06/25 14:28, Louis Chauvet wrote:
> Le 11/06/2025 à 21:55, Maíra Canal a écrit :
> > Hi Louis,
> >
> > On 5/30/25 11:05, Louis Chauvet wrote:
> > > The formats XRGB8888 and ARGB8888 were already supported.
> > > Add the support for:
> > > - XBGR8888
> > > - RGBX8888
> > > - BGRX8888
> > > - ABGR8888
> > > - RGBA8888
> > > - BGRA8888
> > >
> > > Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> > > ---
> >
> > [...]
> >
> > > +READ_LINE_ARGB8888(RGBX8888_read_line, px, 0xFF, px[3], px[2], px[1])
> > > +READ_LINE_ARGB8888(BGRX8888_read_line, px, 0xFF, px[1], px[2], px[3])
> >
> > How did you test those two formats? I noticed that IGT (kms_plane tests)
> > doesn't test them.
>
> Hi Maíra,
>
> Thanks for your review!
>
> I wrote this a long time ago, so I don't remember. I was probably greedy 
> and added all the "trivial" formats I was able to do and missed that 
> this format was not tested.
>
> For this revision, I just started kms_plane to check if it was happy 
> after the rebase, I did not check the formats one by one.
>
> Do you want me to remove those formats? I think it costs nothing to keep 
> them, especially with the new READ_LINE_ARGB8888 macro, but I will 
> comply if you think we should only merge tested formats.

If we don't have a use-case for those formats and we haven't tested
them, I can't see a compelling reason to keep them. Otherwise, we might
keep untested/unused code around that might stale over time.

Best Regards,
- Maíra

> Thanks,
> Louis Chauvet
>
> > Best Regards,
> > - Maíra