Re: [Mesa-dev] [PATCH] egl/wayland: Don't use dmabuf with no modifiers

[Daniel Stone]

Hey Emil,

On 2 October 2017 at 17:08, Emil Velikov  wrote:
> On 2 October 2017 at 16:55, Daniel Stone  wrote:
>> The dmabuf interface requires a valid modifier to be sent. If we don't
>> explicitly get a modifier from the driver, we can't know what to send;
>> it must be inferred from legacy side-channels (or assumed to linear, if
>> none exists).
>>
>> If we have no modifier, then we can only have a single-plane format
>> anyway, so fall back to the old wl_drm buffer import path.
>
> IIRC one my earlier questions was about this. The reply was that
> dmabuf should work fine with single-plane formats.

Should but doesn't, at least with Weston. :P Not sure about Mutter.

Either way, given that one of the primary testing targets is broken,
and we get no real advantage from using the dmabuf interface here,
might as well just revert it for now.

> Is that the root issue or it's the missing queryImage() error handling?
>
> Can I suggest separating the two?

OK.

>> Fixes: a65db0ad1c ("st/dri: don't expose modifiers in EGL if the driver 
>> doesn't implement them")
> I think this should be
> Fixes: 02cc3593727 ("egl/wayland: Use linux-dmabuf interface for buffers")

I'm pretty sure it only came up after Marek's change? I can try to
bisect and see I guess, but yeah. Might as well take both of them,
that way stable@ should notice and pull in Marek's fix, which is also
entirely valid.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl/wayland: Don't use dmabuf with no modifiers

[Emil Velikov]

On 2 October 2017 at 16:55, Daniel Stone  wrote:
> The dmabuf interface requires a valid modifier to be sent. If we don't
> explicitly get a modifier from the driver, we can't know what to send;
> it must be inferred from legacy side-channels (or assumed to linear, if
> none exists).
>
> If we have no modifier, then we can only have a single-plane format
> anyway, so fall back to the old wl_drm buffer import path.
>
IIRC one my earlier questions was about this. The reply was that
dmabuf should work fine with single-plane formats.
Is that the root issue or it's the missing queryImage() error handling?

Can I suggest separating the two?

> Fixes: a65db0ad1c ("st/dri: don't expose modifiers in EGL if the driver 
> doesn't implement them")
I think this should be
Fixes: 02cc3593727 ("egl/wayland: Use linux-dmabuf interface for buffers")

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] egl/wayland: Don't use dmabuf with no modifiers

[Daniel Stone]

The dmabuf interface requires a valid modifier to be sent. If we don't
explicitly get a modifier from the driver, we can't know what to send;
it must be inferred from legacy side-channels (or assumed to linear, if
none exists).

If we have no modifier, then we can only have a single-plane format
anyway, so fall back to the old wl_drm buffer import path.

Fixes: a65db0ad1c ("st/dri: don't expose modifiers in EGL if the driver doesn't 
implement them")
Reported-by: Andy Furniss 
Cc: Marek Olšák 
---
 src/egl/drivers/dri2/platform_wayland.c | 45 +++--
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index 011dddfabf..b2c41507d1 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -678,23 +678,42 @@ create_wl_buffer(struct dri2_egl_display *dri2_dpy,
  __DRIimage *image)
 {
struct wl_buffer *ret;
+   GLboolean query;
int width, height, fourcc, num_planes;
+   uint64_t modifier;
+
+   query = dri2_dpy->image->queryImage(image, __DRI_IMAGE_ATTRIB_WIDTH, 
);
+   query &= dri2_dpy->image->queryImage(image, __DRI_IMAGE_ATTRIB_HEIGHT,
+);
+   query &= dri2_dpy->image->queryImage(image, __DRI_IMAGE_ATTRIB_FOURCC,
+);
+   if (!query)
+  return NULL;
 
-   dri2_dpy->image->queryImage(image, __DRI_IMAGE_ATTRIB_WIDTH, );
-   dri2_dpy->image->queryImage(image, __DRI_IMAGE_ATTRIB_HEIGHT, );
-   dri2_dpy->image->queryImage(image, __DRI_IMAGE_ATTRIB_FOURCC, );
-   dri2_dpy->image->queryImage(image, __DRI_IMAGE_ATTRIB_NUM_PLANES,
-   _planes);
+   query = dri2_dpy->image->queryImage(image, __DRI_IMAGE_ATTRIB_NUM_PLANES,
+   _planes);
+   if (!query)
+  num_planes = 1;
 
-   if (dri2_dpy->wl_dmabuf && dri2_dpy->image->base.version >= 15) {
-  struct zwp_linux_buffer_params_v1 *params;
+   modifier = DRM_FORMAT_MOD_INVALID;
+   if (dri2_dpy->image->base.version >= 15) {
   int mod_hi, mod_lo;
-  int i;
 
-  dri2_dpy->image->queryImage(image, __DRI_IMAGE_ATTRIB_MODIFIER_UPPER,
-  _hi);
-  dri2_dpy->image->queryImage(image, __DRI_IMAGE_ATTRIB_MODIFIER_LOWER,
-  _lo);
+  query = dri2_dpy->image->queryImage(image,
+  __DRI_IMAGE_ATTRIB_MODIFIER_UPPER,
+  _hi);
+  query &= dri2_dpy->image->queryImage(image,
+   __DRI_IMAGE_ATTRIB_MODIFIER_LOWER,
+   _lo);
+  if (query) {
+ modifier = (uint64_t) mod_hi << 32;
+ modifier |= (uint64_t) (mod_lo & 0x);
+  }
+   }
+
+   if (dri2_dpy->wl_dmabuf && modifier != DRM_FORMAT_MOD_INVALID) {
+  struct zwp_linux_buffer_params_v1 *params;
+  int i;
 
   /* We don't need a wrapper for wl_dmabuf objects, because we have to
* create the intermediate params object; we can set the queue on this,
@@ -725,7 +744,7 @@ create_wl_buffer(struct dri2_egl_display *dri2_dpy,
 dri2_dpy->image->destroyImage(p_image);
 
  zwp_linux_buffer_params_v1_add(params, fd, i, offset, stride,
-mod_hi, mod_lo);
+modifier >> 32, modifier & 0x);
  close(fd);
   }
 
-- 
2.14.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev