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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> On Wed, Apr 3, 2024 at 3:43 AM Pekka Paalanen
> <pekka.paalanen@xxxxxxxxxxxxx> wrote:
> >
> > On Tue,  2 Apr 2024 19:28:13 -0400
> > Zack Rusin <zack.rusin@xxxxxxxxxxxx> 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@xxxxxxxxxxxxx>


Thanks,
pq

Attachment: pgp7p6XfsApIe.pgp
Description: OpenPGP digital signature


[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux