[Mesa-dev] [PATCH] egl/wayland: Set __DRI_IMAGE_USE_SCANOUT for shared buffers

[Daniel Stone]

Hi,

On 27 January 2016 at 14:16, Axel Davy <axel.davy at ens.fr> wrote:
> On 27/01/2016 13:43, Daniel Stone wrote:
>> On 27 January 2016 at 09:34, Michel Dänzer <michel at daenzer.net> wrote:
>>> The compositor may have the hardware scan out directly from the buffers
>>> sent by the client, so we must make sure the buffers we create are
>>> suitable for scanout.
>>
>> If the compositor wants to scan out directly, it will import via GBM,
>> which is in a position to reject the import if the buffer is not
>> suitable for scanout. So there's something missing here, either in the
>> GBM implementation to set magic flags when importing, or failure to
>> communicate tiling mode correctly, or whatever.
>>
>> So for now, I would NAK this and fix the underlying problem, before
>> forcing every client buffer to be scanout-capable, which can cause
>> performance issues of its own. This also introduces a
>> resource-contention issue (e.g. if the display controller can only
>> scan out from physically-contiguous memory); you'll cause all client
>> buffer allocations to fail unless it can allocate a potentially
>> contended resource.
>>
>> Please fix the radeonsi/Gallium implementations of GBM and/or wl_drm
>> to deal with tiling/compression correctly instead.
>
> Scanout buffers are already used for DRI2 and DRI3 by default (for Mesa
> drivers),
> and it looks like a sane default behaviour given there is no current
> way to know if the compositor would like to use the buffer as scanout
> or not.

Sure, but the DRI protocols are rather fixed, whereas wl_drm is
completely extensible to better describe the formats. I feel like this
is mainly being used as a proxy to paper over a few issues, including
negotiating tiling formats between different GPUs, which we can now
fully describe via FB modifiers.

The only places I can see this actually happening by default, are in
the GBM drivers (obviously correct), and DRI3 when using a different
GPU to render (less obviously correct, but does paper over the lack of
common-format negotiation, as above). I can't see it happening by
default for DRI2, unless I'm missing something ... ?

> As long as the app has no way to know what the compositor wants (there
> was talks I remember on irc on ways to do that), I think enforcing scanout
> as default is sane.

Again, negotiating scanout is something we can handle internally
through wl_drm without having to have a flag-day API change. If you
assume that scanout negates tiling, then every browser you launch is
going to be rendered to a non-tiled format. This has a non-trivial
impact on a lot of architectures, and is only really mostly kind of
fine on desktop architectures where you have the power to waste
anyway.

> The other thing you mention, ie failing to import non-scanout buffers when
> scanout is requested is, I believe,
> unrelated to this patch, and would require a bit more work. In particular
> this is handled by a function validateUsage,
> which doesn't seem to be fully implemented on gallium. I suggest to file a
> bug on bugzillla.

Well, I could, but if this fix goes in, I assume nothing will ever
happen for Gallium drivers, so not sure there's much point, aside from
having a URL to point to the next time this comes up.

Cheers,
Daniel