Hello Daniel-san On 2017/09/27 2:09, Daniel Stone wrote: > Hello Esaki-san, > > On 1 November 2016 at 19:09, Daniel Stone <[email protected]> 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. >
I agree to that. Thank you for the update. Cheers, Esaki _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
