[igt-dev] [PATCH i-g-t 1/2] lib/igt_fb: Use cairo conversion in igt_fb_convert_with_stride, v3.

[Maarten Lankhorst]

Op 10-05-2019 om 15:43 schreef Paul Kocialkowski:
> Hi,
>
> It looks like I fell behind on reviewing earlier version here, sorry
> about that.
>
> On Fri, 2019-05-10 at 14:33 +0200, Maarten Lankhorst wrote:
>> Ever since commit 3fa65f4b532bd9a5b ("fb: Add support for conversions
>> through pixman") we can generate a valid cairo surface for any plane,
>> use this to avoid having to implement our own conversion routine.
>>
>> Instead of duplicating this functionality in igt_fb_convert_with_stride,
>> we can simply convert this to a few cairo calls, because we now support
>> cairo calls to any of the supported framebuffer formats.
> I don't see how that is the case. Cairo definitely does not support our
> tiled formats, so we can't just pipe them into it.
>
> And we already use pixman for converting through the fb_convert call to
> convert_pixman when possible. Also, my understanding is that cairo is
> very limited format-wise, so we're much better off using pixman
> directly (which is is what a non-accelerated cairo surface will do
> anyway).
>
>> This is required to make this function more generic, and convert from any
>> format/modifier to any other format/modifier.
> We've already designed it to be generic, we just need conversion
> helpers from/to (currently only to) hardware-specific tiling modifiers.
>
> We can't expect cairo or pixman to do that job and this change pretty
> much kills support for the vc4 tiling modes we've added.
>
> Unless I'm missing something, that's a NACK on my side.

You have a function that can convert a untiled fb to tiled, but not the other way around in igt_vc4.c
If you could provide that, we could convert from any fb/modifier format to any fb/modifier format, even with VC4 tiling.

Our internal cairo hooks already provide helpers for conversion, see igt_get_cairo_surface() which calls
create_cairo_surface__convert()

Some conversions can only be done through cairo, converting between P01x and XRGB8888 cannot be done directly,
our pixman representation for that are done in the floating point formats.

The only way to convert between some framebuffer formats and modifiers in i915 is by using those conversion functions,
the fact that igt_fb_convert_with_stride doesn't do those, makes a truly random hdmi-cmp-planes-random useless.

I was getting CRC mismatches because the i915 modifiers weren't respected. When I made sure they were being respected
I ended up with a duplicate of the cairo context stuff. So why not lose a little speed and use that?

If you write and test a detiler function, I can hook it up for you. :)
If necessary I can do it myself, to move this patch forward.

Cairo shouldn't be much slower than pixman, because internally it already uses pixman calls for the backend.

~Maarten