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
