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

[Michel Dänzer]

On 27.01.2016 23:54, Daniel Stone wrote:
> 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 you can NAK this because... ? :)


>>> 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 ... ?

DRI2 buffers are allocated by the Xorg driver, which can obviously make
sure they can be scanned out.


>> 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.

You're saying there are architectures where tiling is essential for
energy consumption, but the rendering and display hardware don't have
any common tiling formats? The mind boggles.

FWIW, on our hardware there's no such issue, scanout just requires
special tiling modes.


>> 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.

intel_validate_usage doesn't handle scanout either, but AFAIR at least
older generations of Intel GPUs don't support all tiling modes for
scanout either. So I don't see how that could work in the scheme you
describe other than by luck.

Assuming there was any validateUsage hook which would properly catch
buffers which can't be scanned out, can you point us to other code which
would allow the Wayland compositor to make sure clients allocate buffers
which can be scanned out?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer