[PATCH 5/5] drm/vmwgfx: Sort primary plane formats by order of preference

[Pekka Paalanen]

On Wed, 3 Apr 2024 07:44:54 -0400
Zack Rusin <zack.rusin at broadcom.com> wrote:

> On Wed, Apr 3, 2024 at 3:43 AM Pekka Paalanen
> <pekka.paalanen at collabora.com> wrote:
> >
> > On Tue,  2 Apr 2024 19:28:13 -0400
> > Zack Rusin <zack.rusin at broadcom.com> wrote:
> >  
> > > The table of primary plane formats wasn't sorted at all, leading to
> > > applications picking our least desirable formats by defaults.
> > >
> > > Sort the primary plane formats according to our order of preference.  
> >
> > This is good.
> >  
> > > Fixes IGT's kms_atomic plane-invalid-params which assumes that the
> > > preferred format is a 32bpp format.  
> >
> > That sounds strange, why would IGT depend on preferred format being
> > 32bpp?
> >
> > That must be an oversight. IGT cannot dictate the format that hardware
> > must prefer. XRGB8888 is strongly suggested to be supported in general,
> > but why also preferred?  
> 
> I think it's just a side-effect of the pixman's assert that's failing:
> https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/lib/igt_fb.c#n4190
> i.e. pixman assumes everything is 4 byte aligned.
> I should have rephrased the message as "IGT assumes that the preferred
> fb format is 4 byte aligned because our 16bpp formats are packed and
> pixman can't convert them".

Ah, yes. IIRC Pixman indeed assumes 4-byte alignment for stride and row
start. It should work for 16bpp formats if the FB had even width +
padding in pixels.

I think this is just an indication that Pixman is ill-suited for IGT.
IGT should be able to generate and analyse images in any format any
kernel driver might support.

I've noticed IGT also using Cairo, which limits the possible pixel
formats so severely I'm actually puzzled how it can even be used there.

Anyway, this is not at all about your patch, which looks good and well
to me. Just the comment about adapting to IGT seemed odd. Maybe phrase
that more like a happy accident rather than another justification for
this patch?

This patch:

Acked-by: Pekka Paalanen <pekka.paalanen at collabora.com>


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20240404/b5b0145d/attachment.sig>