[PATCH] unstable/linux-dmabuf: Add 'direct_display' flag

[Scott Anderson]

>> Has any thought been into how this would need to interact with
>> dmabuf-hints[1]? Without that, it seems like it would be a total
>> crapshoot for clients to try and use this flag, since they have no idea
>> what formats+modifers the display controller supports, and instead only
>> has the list that the GPU supports.
>> dmabuf-hints would also need to explicitly state that a tranche of
>> formats+modifiers are supported for this flag.
> 
> Well I'm not aware of that hint extension, but I think this wasn't taken
> into consideration because it isn't the same thing. There's no assurance
> from the compositor that the buffer will not read/be imported by the
> GPU.
Yeah, dmabuf-hints wouldn't replace this flag, but does interact with 
it. It's supposed to give the client the best possible chance of being 
able to be scanned out if it's possible, and is dynamic with how the 
surface is currently being used. I consider it a much better way to deal 
with any of the performance-related aspects of this, but doesn't say 
anything about the content-protection cases.

Regarding formats+modifiers, the set of them that is valid is 
potentially very different from the ones advertised by wp_linux_dmabuf.
The client has no reliable way to know what they are. dmabuf-hints does 
give a good idea of what the direct-scanout formats are, but doesn't 
really fit the content-protection case _that_ well.

> A hint is merely a hint. The compositor can abide or not by that.
> This flag will explicitly close the client connection if the buffer
> can't be scanned out when this flag is passed.

This flag doesn't sound like a hint to me, but a hard requirement. From 
the discussion I saw on the MR [1], if we don't abide, we risk being killed.

> Regarding the screenshot bit not sure what is the concern here. Are you
> afraid you won't able to take screenshots? The placeholder graphics is
> in place only if view to which the buffer is attached can no longer be
> assigned to a HW plane. >
> It would be nice to know what basic functionality will this break, as I
> not aware of any.

Yes, the functionality is the ability to take correct screenshots.

We can't take a screenshot of a plane, so we're forced to compose. But 
this flag basically forces us to never compose, at risk of being killed. 
This is intended in the content-protection case, but not for anything else.

When an application think it's being clever by using this flag (and they 
have no reason to NOT set it; it sounds like free performance), it 
breaks basic screenshots for us, and users are going to be annoyed by 
placeholders for random surfaces.

For wlroots, which is never going to support content-protection, maybe 
we'd just completely ignore the issue, but the most sensible 
implementation to me would be to just completely disallow this flag.

This flag should be completely re-contextualized about being 
content-protection only, or otherwise buffers we literally cannot access 
from the GPU. There should be no implications about performance, and it 
should be clear that 99.9% of clients don't want to use it.

But without the excuse for performance, that also raises another issue 
for me about content-protection living in a "wp" protocol. The 
governance thing hasn't officially been applied yet, and I wouldn't even 
be the official spokesperson for wlroots, but I would personally NACK 
that. Content-protection is a niche use case not generally useful to 
Wayland implementations. I think a "ext" "wl_buffer factory for 
protected dmabufs" would be a better place for this. It means you could 
advertise a correct list of formats+modifiers too.

Scott

[1]: 
https://gitlab.freedesktop.org/marius.vlad0/wayland-protocols/merge_requests/3#note_291389