[igt-dev] [PATCH i-g-t 1/2] lib/igt_fb: Add XBGR2101010 support via pixman

[Ville Syrjälä]

On Mon, Sep 30, 2019 at 08:35:11PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-09-18 14:51:55)
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Use pixman to swizzle cairo RGB30 into XBGR2101010.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  lib/igt_fb.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index bad3eeca4132..612c25d5baed 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -157,6 +157,12 @@ static const struct format_desc_struct {
> >           .num_planes = 1, .plane_bpp = { 32, },
> >           .hsub = 1, .vsub = 1,
> >         },
> > +       { .name = "XBGB2101010", .depth = -1, .drm_id = DRM_FORMAT_XBGR2101010,
> > +         .cairo_id = CAIRO_FORMAT_INVALID,
> > +         .pixman_id = PIXMAN_x2b10g10r10,
> > +         .num_planes = 1, .plane_bpp = { 32, },
> > +         .hsub = 1, .vsub = 1,
> > +       },
> 
> That's self-consistent.
> 
> >         { .name = "ARGB8888", .depth = 32, .drm_id = DRM_FORMAT_ARGB8888,
> >           .cairo_id = CAIRO_FORMAT_ARGB32,
> >           .pixman_id = PIXMAN_a8r8g8b8,
> > @@ -3107,6 +3113,9 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> >         } else if (PIXMAN_FORMAT_A(f->pixman_id)) {
> >                 cairo_id = CAIRO_FORMAT_ARGB32;
> >                 drm_format = DRM_FORMAT_ARGB8888;
> > +       } else if (PIXMAN_FORMAT_R(f->pixman_id) > 8) {
> > +               cairo_id = CAIRO_FORMAT_RGB30;
> > +               drm_format = DRM_FORMAT_XRGB2101010;
> 
> A little less sure about this rule, but if it ever needs extending for
> more formats, it should hopefully become quickly apparent.

Yeah, it's a bit too fuzzy for my taste. I was pondering about putting
the correct cairo format into the format_desc itself, but that would
require some changes to the "should we convert and how?" logic elsewhere.
Seems doable but needs a little bit of actual thought.

> 
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

Thanks.

-- 
Ville Syrjälä
Intel