[PATCH weston v2 3/3] compositor-drm: Support linux_dmabuf output for sprite planes without gbm

[Daniel Stone]

Hello Esaki-san,

On 1 November 2016 at 19:09, Daniel Stone <daniel at fooishbar.org> wrote:
>> @@ -1101,15 +1096,9 @@ drm_output_prepare_overlay_view(struct drm_output *output,
>>         if (!found)
>>                 return NULL;
>>
>> -       if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource))) {
>> +       if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource)) &&
>> +           b->no_addfb2 && b->gbm) {
>>  #ifdef HAVE_GBM_FD_IMPORT
>> -               /* XXX: TODO:
>> -                *
>> -                * Use AddFB2 directly, do not go via GBM.
>> -                * Add support for multiplanar formats.
>> -                * Both require refactoring in the DRM-backend to
>> -                * support a mix of gbm_bos and drmfbs.
>> -                */
>>                 struct gbm_import_fd_data gbm_dmabuf = {
>>                         .fd     = dmabuf->attributes.fd[0],
>>                         .width  = dmabuf->attributes.width,
>
> Here I think there is no benefit to using GBM import: I think it would
> be more consistent if we always bypassed GBM and just used our own
> non-GBM drm_fb type. What do you think? If we integrate this patch
> into the drm_plane_state/atomic tree, the changes for this patch are
> quite simple: mainly we need to add a BUFFER_DMABUF type to drm_fb,
> and change the prepare_{overlay_scanout}_view paths to put the new
> buffer into the plane state directly. I am happy to make this change
> myself if you think it is OK.

Unfortunately I have had to pull this patch from my tree. The reason
is that the handle returned by drmPrimeFDToHandle and closed with
DRM_IOCTL_GEM_CLOSE is shared but not refcounted.

GEM drivers have a rule that each buffer _must_ map back to a single
GEM handle for each DRM connection. So, multiple drmPrimeFDToHandle
calls from the same client will return the same GEM handle if the
underlying buffer is the same, even if it is through a different FD.
The first DRM_IOCTL_GEM_CLOSE call will invalidate that handle
completely.

This is a problem for EGL drivers which use dmabuf and GEM: for
instance, on Mesa, the driver will import the dmabuf and create one
handle, then we will create another handle from our own dmabuf import
here, and then later close it from underneath the EGL driver. The EGL
driver can no longer render anything, since the handle is invalid when
submitting commands.

GBM now has a GBM_IMPORT_FD_MODIFIER path, which allows specifying
multiple planes and format modifiers when importing. So, I have
rewritten the patch to use this new path instead: as GBM uses the same
DRM BO cache as EGL does, we no longer have the lifetime issue.

Cheers,
Daniel