[weston 1/2] linux-dmabuf: implement immediate dmabuf import

[Varad Gautam]

Hi Pekka,

On Fri, Jan 27, 2017 at 6:32 PM, Pekka Paalanen
<pekka.paalanen at collabora.co.uk> wrote:
> On Mon, 21 Nov 2016 21:27:03 +0000
> Daniel Stone <daniel at fooishbar.org> wrote:
>
>> Hi Varad,
>>
>> On 11 November 2016 at 11:40, Varad Gautam <varadgautam at gmail.com> wrote:
>> > handle create_immed() dmabuf import requests and support
>> > zwp_linux_dmabuf_v1_interface version 2.
>>
>> Same caveat about holding off on merging applies, but these two patches are:
>> Reviewed-by: Daniel Stone <daniels at collabora.com>
>
> Hi,
>
> yeah, the Weston patch v1 is probably good, considering my review on
> the protocol patch v2 essentially asking to go back to the v1 with just
> some wording added.
>
> However, it would be good to check and ensure what happens in Weston
> when a client uses a wl_buffer that has failed the dmabuf import. I
> suspect you would need to add some checks somewhere depending on what
> Weston will do.
>
> I think we pretty much agreed that Weston will just send a fatal error
> to the client if it commits a failed wl_buffer.
>
> How to recognize a failed wl_buffer then... maybe NULL user data on the
> wl_resource? Then there will be places that need to check for NULL: the
> commit machinery and destructor come to mind.
>
> Whatever Weston does, it cannot avoid creating the wl_resource. It must
> be created, if the protocol XML says a new object is created. And for
> 'create_immed' request, a new object is created. This is probably
> overlooked in patch v1.

Instead of always always creating a wl_resource and identifying an invalid
wl_buffer on commit in weston, how about treating an invalid import just as
any other cause for failure in linux-dmabuf, and letting the
compositor error-out
at import itself?

So, upon failed import the implementation can choose to either:
1. raise an error and never create an invalid resource; or,
2. create an invalid resource and have its own checks in the commit path
(or elsewhere) to decide how to handle the situation.

Option 1 allows us to have a 'semi-defined' behavior for existing compositors
{weston} that expect a commit guarantee on wl_buffers. The protocol v2 [1]
added an optional invalid_wl_buffer error, to be preventively used by
compositors
that mandate the committed wl_buffers to be valid to handle the failed import
case.

[1] https://patchwork.freedesktop.org/patch/133966/

Thanks,
Varad

>
>
> Thanks,
> pq