Re: [igt-dev] [PATCH v3 03/11] fb: Add format conversion routine

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: igt-dev@lists.freedesktop.org,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	eben@raspberrypi.org
Subject: Re: [igt-dev] [PATCH v3 03/11] fb: Add format conversion routine
Date: Thu, 14 Jun 2018 18:13:06 +0300	[thread overview]
Message-ID: <20180614151306.GI20518@intel.com> (raw)
In-Reply-To: <20180614145339.r5g2w7nl4iwxylfa@flea>

On Thu, Jun 14, 2018 at 04:53:39PM +0200, Maxime Ripard wrote:
> On Mon, Jun 04, 2018 at 01:13:53PM +0300, Ville Syrjälä wrote:
> > On Mon, Jun 04, 2018 at 11:54:03AM +0200, Maxime Ripard wrote:
> > > On Thu, May 24, 2018 at 06:40:36PM +0300, Ville Syrjälä wrote:
> > > > On Thu, May 24, 2018 at 05:19:04PM +0200, Maxime Ripard wrote:
> > > > > On Thu, May 24, 2018 at 05:58:13PM +0300, Ville Syrjälä wrote:
> > > > > > On Thu, May 24, 2018 at 04:24:42PM +0200, Maxime Ripard wrote:
> > > > > > > The chamelium format subtests will need to convert the reference pattern to
> > > > > > > the format to be tested on the DRM device.
> > > > > > > 
> > > > > > > However, Cairo is very limited when it comes to format, and while pixman
> > > > > > > has much more support for formats, it's still falling short compared to
> > > > > > > what DRM exposes, especially on the YUV side.
> > > > > > 
> > > > > > We already have the capability to do format conversions automagically.
> > > > > > I'm extending it to handle more YUV stuff here:
> > > > > > https://patchwork.freedesktop.org/series/43651/
> > > > > > 
> > > > > > Can you hook up the pixman stuff in the same way so that we don't
> > > > > > have to any explicit conversion stuff in the tests themselves?
> > > > > 
> > > > > I really think that the assumption that cairo will handle it is not a
> > > > > proper fit. Cairo support is very limited, just like pixman is to a
> > > > > lesser extent, and we should just break the assumption that we will
> > > > > get a cairo surface all the time.
> > > > 
> > > > Not sure what you're saying here. We don't assume cairo handles all
> > > > the formats, well, because it doesn't. We just magically convert
> > > > to/from XRGB888 so that cairo doesn't have to handle anything else.
> > > 
> > > My point was that from what I can tell from your patches, you allow to
> > > create an igt_fb with whatever format we want, and then convert it to
> > > XRGB8888 when one calls igt_get_cairo_surface.
> > > 
> > > I need something more generic, but even if we simplify to the
> > > operation that I need now, I basically needs the exact opposite: to be
> > > able to take an XRGB8888 buffer (that is our pattern) and get an
> > > igt_fb in an arbitrary format.
> > 
> > The current thing (and my patches) converts both ways. Can't get much
> > more generic than that.
> > 
> > > 
> > > And that buffer cannot be created by cairo,
> > 
> > Why not?
> 
> In order to have control over the lower bits that would be lost in the
> resampling. Let's say you convert the initial XRGB8888 pattern to an
> RGB565 one, feed that to the scanout, and capture the frame on the
> other end using a chamelium.
> 
> The Chamelium will capture a CRC over an XRGB8888 frame internall, and
> you will have the CRC of the XRGB8888 pattern. However, since that
> pattern was downsampled to RGB565 and the resampled back to 8 bits,
> you lost the least significant bits, and without a pattern that was
> carefully chosen, the CRCs don't match anymore.
> 
> As far as I know, Cairo only allows you to specify a color component
> as a float, and tries to map that to a 24bits color components. But
> you have no way to force it to use a particular value for the 3 (or
> more) lower bits.

You can always draw solid rectangles and such with cairo. Should retain
full control over the bits that way.

Alternative is to just throw away the low bits with a gamma unit etc.
Granted, you might actually want to check that the low bits have the
correct data. Depedns on what you're testing I suppose.

That said, I don't have any real objections to adding a function to
do the format conversion explicitly, but I'd definitely want to see
this stuff hooked up to the cairo path as well. And conversely the
things we already have in the cairo path should probably be hooked
up to the explicit format conversion function.

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev