Re: [Mesa-dev] [PATCH 11/11] egl/wayland: Use linux-dmabuf interface for buffers

[Daniel Stone]

Hey Emil,

On 12 July 2017 at 16:10, Emil Velikov  wrote:
> There's one small bug that I've missed previously. Other than that the
> patch looks great

Thanks for taking a look!

> On 16 June 2017 at 18:14, Daniel Stone  wrote:
>> +  if (num_modifiers && dri2_dpy->image->base.version >= 15 &&
>> +  dri2_dpy->image->createImageWithModifiers) {
>> + dri2_surf->back->dri_image =
>> +   dri2_dpy->image->createImageWithModifiers(dri2_dpy->dri_screen,
>> + dri2_surf->base.Width,
>> + dri2_surf->base.Height,
>> + dri_image_format,
>> + modifiers,
>> + num_modifiers,
>> + NULL);
>> +  } else {
>> + dri2_surf->back->dri_image =
>> +dri2_dpy->image->createImage(dri2_dpy->dri_screen,
> I believe we should not fallback to the createImage API if num_modifiers != 0.

That was completely intentional, but you're right - it probably
warrants a comment.

Modifiers are just a suggestion from the upstream window system as to
how to allocate. If the driver doesn't support allocating with
modifiers, we don't _need_ to do it. We just fall back to the previous
system, which is for the driver to make an allocation it 'knows' will
succeed, which is either linear, or if on the same GPU, using tiling
with an Intel/VC4-style magic kernel backchannel for the winsys to
determine the tiling on import.

Practically, this would happen when we have i965 used for composition
on the winsys side (offering a list of acceptable modifiers), and
radeonsi used on the client/rendering side (no support for modifiers).
If we refused to use the legacy createImage() hook when modifiers were
provided, we wouldn't be able to use radeonsi at all.

I'll put this into a less wordy comment for v2. :)

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

Re: [Mesa-dev] [PATCH 11/11] egl/wayland: Use linux-dmabuf interface for buffers

[Emil Velikov]

Hi Dan,

There's one small bug that I've missed previously. Other than that the
patch looks great

On 16 June 2017 at 18:14, Daniel Stone  wrote:
> When available, use the zwp_linux_dambuf_v1 interface to create buffers,
> which allows multiple planes and buffer modifiers to be used.
>
> Signed-off-by: Daniel Stone 
> ---

> if (dri2_surf->back->dri_image == NULL) {
> -  dri2_surf->back->dri_image =
> - dri2_dpy->image->createImage(dri2_dpy->dri_screen,
> -  dri2_surf->base.Width,
> -  dri2_surf->base.Height,
> -  dri_image_format,
> -  dri2_dpy->is_different_gpu ?
> - 0 : use_flags,
> -  NULL);
> +  if (num_modifiers && dri2_dpy->image->base.version >= 15 &&
> +  dri2_dpy->image->createImageWithModifiers) {
> + dri2_surf->back->dri_image =
> +   dri2_dpy->image->createImageWithModifiers(dri2_dpy->dri_screen,
> + dri2_surf->base.Width,
> + dri2_surf->base.Height,
> + dri_image_format,
> + modifiers,
> + num_modifiers,
> + NULL);
> +  } else {
> + dri2_surf->back->dri_image =
> +dri2_dpy->image->createImage(dri2_dpy->dri_screen,
I believe we should not fallback to the createImage API if num_modifiers != 0.

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

Re: [Mesa-dev] [PATCH 11/11] egl/wayland: Use linux-dmabuf interface for buffers

[Emil Velikov]

On 3 July 2017 at 13:49, Daniel Stone  wrote:
> Hey Emil,
>
> On 3 July 2017 at 13:36, Emil Velikov  wrote:
>> Not familiar with the linux-dmabuf protocol - Dan, any ideas if we can
>> "get away" w/o using u_vector?
>
> What would you suggest instead of u_vector?
>
> When the client connects, for each format, it will receive a series of
> 'modifier' events, advertising one modifier supported for that format.
> The number of events which will be received for each format is not
> known beforehand. I need to later use these as an array of uint64_t,
> one per format, to hand to the driver in
> DRIImage->createImageWithModifiers(), so the driver can decide which
> modifier to use.
>
> Given that, what would you use instead? My understanding of u_vector
> vs. u_dynarray is that u_dynarray should be used for dynamic
> grow/shrink, and u_vector pretty much just for append-only. I couldn't
> find anything else except open-coding my own malloc() / realloc() /
> num_elements / array_length implementation, which seemed worse.
>
Initial train of thought was - there's some guarantee that there won't
be more than X unprocessed events.
In which case one could simply make it a fixed size array and avoid
the (implicit if using u_vector) malloc/realloc.

It sounds like that's not the case, so it's either u_vector or
open-coding it. Whichever you and others are happy with.

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

Re: [Mesa-dev] [PATCH 11/11] egl/wayland: Use linux-dmabuf interface for buffers

[Daniel Stone]

Hey Emil,

On 3 July 2017 at 13:36, Emil Velikov  wrote:
> Not familiar with the linux-dmabuf protocol - Dan, any ideas if we can
> "get away" w/o using u_vector?

What would you suggest instead of u_vector?

When the client connects, for each format, it will receive a series of
'modifier' events, advertising one modifier supported for that format.
The number of events which will be received for each format is not
known beforehand. I need to later use these as an array of uint64_t,
one per format, to hand to the driver in
DRIImage->createImageWithModifiers(), so the driver can decide which
modifier to use.

Given that, what would you use instead? My understanding of u_vector
vs. u_dynarray is that u_dynarray should be used for dynamic
grow/shrink, and u_vector pretty much just for append-only. I couldn't
find anything else except open-coding my own malloc() / realloc() /
num_elements / array_length implementation, which seemed worse.

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

Re: [Mesa-dev] [PATCH 11/11] egl/wayland: Use linux-dmabuf interface for buffers

[Emil Velikov]

On 22 June 2017 at 17:52, Lucas Stach  wrote:
> Am Freitag, den 16.06.2017, 18:14 +0100 schrieb Daniel Stone:
>> When available, use the zwp_linux_dambuf_v1 interface to create buffers,
>> which allows multiple planes and buffer modifiers to be used.
>>
>> Signed-off-by: Daniel Stone 
>> ---
>>  configure.ac|   5 +-
>>  src/egl/Makefile.am |  22 +++-
>>  src/egl/drivers/dri2/.gitignore |   2 +
>>  src/egl/drivers/dri2/egl_dri2.c |   7 ++
>>  src/egl/drivers/dri2/egl_dri2.h |  10 ++
>>  src/egl/drivers/dri2/platform_wayland.c | 189 
>> +---
>>  6 files changed, 214 insertions(+), 21 deletions(-)
>>  create mode 100644 src/egl/drivers/dri2/.gitignore
>
> This patch is missing the following hunk to fix out of tree builds:
>
> --- a/src/egl/Makefile.am
> +++ b/src/egl/Makefile.am
> @@ -104,6 +104,7 @@ endif
>  AM_CFLAGS += \
> -I$(top_srcdir)/src/loader \

> -I$(top_srcdir)/src/egl/drivers/dri2 \
> +   -I$(top_builddir)/src/egl/drivers/dri2 \

Thanks Lucas, one small nit though:
The builddir line should be first, otherwise we will pick a stale
in-tree file, when doing an OOT build.

Not familiar with the linux-dmabuf protocol - Dan, any ideas if we can
"get away" w/o using u_vector?

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

Re: [Mesa-dev] [PATCH 11/11] egl/wayland: Use linux-dmabuf interface for buffers

[Lucas Stach]

Am Freitag, den 16.06.2017, 18:14 +0100 schrieb Daniel Stone:
> When available, use the zwp_linux_dambuf_v1 interface to create buffers,
> which allows multiple planes and buffer modifiers to be used.
> 
> Signed-off-by: Daniel Stone 
> ---
>  configure.ac|   5 +-
>  src/egl/Makefile.am |  22 +++-
>  src/egl/drivers/dri2/.gitignore |   2 +
>  src/egl/drivers/dri2/egl_dri2.c |   7 ++
>  src/egl/drivers/dri2/egl_dri2.h |  10 ++
>  src/egl/drivers/dri2/platform_wayland.c | 189 
> +---
>  6 files changed, 214 insertions(+), 21 deletions(-)
>  create mode 100644 src/egl/drivers/dri2/.gitignore

This patch is missing the following hunk to fix out of tree builds:

--- a/src/egl/Makefile.am
+++ b/src/egl/Makefile.am
@@ -104,6 +104,7 @@ endif
 AM_CFLAGS += \
-I$(top_srcdir)/src/loader \
-I$(top_srcdir)/src/egl/drivers/dri2 \
+   -I$(top_builddir)/src/egl/drivers/dri2 \
-I$(top_srcdir)/src/gbm/backends/dri \
-I$(top_srcdir)/src/egl/wayland/wayland-egl \
-I$(top_builddir)/src/egl/wayland/wayland-drm \

Regards,
Lucas

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

[Mesa-dev] [PATCH 11/11] egl/wayland: Use linux-dmabuf interface for buffers

[Daniel Stone]

When available, use the zwp_linux_dambuf_v1 interface to create buffers,
which allows multiple planes and buffer modifiers to be used.

Signed-off-by: Daniel Stone 
---
 configure.ac|   5 +-
 src/egl/Makefile.am |  22 +++-
 src/egl/drivers/dri2/.gitignore |   2 +
 src/egl/drivers/dri2/egl_dri2.c |   7 ++
 src/egl/drivers/dri2/egl_dri2.h |  10 ++
 src/egl/drivers/dri2/platform_wayland.c | 189 +---
 6 files changed, 214 insertions(+), 21 deletions(-)
 create mode 100644 src/egl/drivers/dri2/.gitignore

diff --git a/configure.ac b/configure.ac
index 9de1b42933..8adbd07880 100644
--- a/configure.ac
+++ b/configure.ac
@@ -89,6 +89,7 @@ LIBOMXIL_BELLAGIO_REQUIRED=0.0
 LIBVA_REQUIRED=0.38.0
 VDPAU_REQUIRED=1.1
 WAYLAND_REQUIRED=1.11
+WAYLAND_PROTOCOLS_REQUIRED=1.8
 XCB_REQUIRED=1.9.3
 XCBDRI2_REQUIRED=1.8
 XCBGLX_REQUIRED=1.8.1
@@ -1673,7 +1674,9 @@ for plat in $platforms; do
case "$plat" in
wayland)
 
-   PKG_CHECK_MODULES([WAYLAND], [wayland-client >= 
$WAYLAND_REQUIRED wayland-server >= $WAYLAND_REQUIRED])
+   PKG_CHECK_MODULES([WAYLAND], [wayland-client >= 
$WAYLAND_REQUIRED wayland-server >= $WAYLAND_REQUIRED wayland-protocols >= 
$WAYLAND_PROTOCOLS_REQUIRED])
+ac_wayland_protocols_pkgdatadir=`$PKG_CONFIG 
--variable=pkgdatadir wayland-protocols`
+AC_SUBST(WAYLAND_PROTOCOLS_DATADIR, 
$ac_wayland_protocols_pkgdatadir)
 
if test "x$WAYLAND_SCANNER" = "x:"; then
AC_MSG_ERROR([wayland-scanner is needed to compile the 
wayland platform])
diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
index 81090387b5..a95e5c3230 100644
--- a/src/egl/Makefile.am
+++ b/src/egl/Makefile.am
@@ -21,6 +21,8 @@
 
 include Makefile.sources
 
+BUILT_SOURCES =
+
 AM_CFLAGS = \
-I$(top_srcdir)/include \
-I$(top_srcdir)/src/egl/main \
@@ -61,11 +63,27 @@ endif
 endif
 
 if HAVE_PLATFORM_WAYLAND
+WL_DMABUF_XML = 
$(WAYLAND_PROTOCOLS_DATADIR)/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
+
+drivers/dri2/linux-dmabuf-unstable-v1-protocol.c: $(WL_DMABUF_XML)
+   $(MKDIR_GEN)
+   $(AM_V_GEN)$(WAYLAND_SCANNER) code < $< > $@
+
+drivers/dri2/linux-dmabuf-unstable-v1-client-protocol.h: $(WL_DMABUF_XML)
+   $(MKDIR_GEN)
+   $(AM_V_GEN)$(WAYLAND_SCANNER) client-header < $< > $@
+
+BUILT_SOURCES += \
+   drivers/dri2/linux-dmabuf-unstable-v1-protocol.c \
+   drivers/dri2/linux-dmabuf-unstable-v1-client-protocol.h
+
 AM_CFLAGS += $(WAYLAND_CFLAGS)
 libEGL_common_la_LIBADD += $(WAYLAND_LIBS)
 libEGL_common_la_LIBADD += $(LIBDRM_LIBS)
 libEGL_common_la_LIBADD += 
$(top_builddir)/src/egl/wayland/wayland-drm/libwayland-drm.la
-dri2_backend_FILES += drivers/dri2/platform_wayland.c
+libEGL_common_la_LIBADD += $(top_builddir)/src/util/libmesautil.la
+dri2_backend_FILES += drivers/dri2/platform_wayland.c  \
+   drivers/dri2/linux-dmabuf-unstable-v1-protocol.c
 endif
 
 if HAVE_PLATFORM_DRM
@@ -118,7 +136,7 @@ g_egldispatchstubs.h: $(GLVND_GEN_DEPS)
$(top_srcdir)/src/egl/generate/egl.xml \
$(top_srcdir)/src/egl/generate/egl_other.xml > $@
 
-BUILT_SOURCES = g_egldispatchstubs.c g_egldispatchstubs.h
+BUILT_SOURCES += g_egldispatchstubs.c g_egldispatchstubs.h
 CLEANFILES = $(BUILT_SOURCES)
 
 if USE_LIBGLVND
diff --git a/src/egl/drivers/dri2/.gitignore b/src/egl/drivers/dri2/.gitignore
new file mode 100644
index 00..e96becbb54
--- /dev/null
+++ b/src/egl/drivers/dri2/.gitignore
@@ -0,0 +1,2 @@
+linux-dmabuf-unstable-v1-client-protocol.h
+linux-dmabuf-unstable-v1-protocol.c
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 020a0bc639..4bb6f09ed8 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -53,6 +53,7 @@
 #ifdef HAVE_WAYLAND_PLATFORM
 #include "wayland-drm.h"
 #include "wayland-drm-client-protocol.h"
+#include "linux-dmabuf-unstable-v1-client-protocol.h"
 #endif
 
 #ifdef HAVE_X11_PLATFORM
@@ -62,6 +63,7 @@
 #include "egl_dri2.h"
 #include "loader/loader.h"
 #include "util/u_atomic.h"
+#include "util/u_vector.h"
 
 /* The kernel header drm_fourcc.h defines the DRM formats below.  We duplicate
  * some of the definitions here so that building Mesa won't bleeding-edge
@@ -949,11 +951,16 @@ dri2_display_destroy(_EGLDisplay *disp)
case _EGL_PLATFORM_WAYLAND:
   if (dri2_dpy->wl_drm)
   wl_drm_destroy(dri2_dpy->wl_drm);
+  if (dri2_dpy->wl_dmabuf)
+  zwp_linux_dmabuf_v1_destroy(dri2_dpy->wl_dmabuf);
   if (dri2_dpy->wl_shm)
   wl_shm_destroy(dri2_dpy->wl_shm);
   wl_registry_destroy(dri2_dpy->wl_registry);
   wl_event_queue_destroy(dri2_dpy->wl_queue);
   wl_proxy_wrapper_destroy(dri2_dpy->wl_dpy_wrapper);
+  u_vector_finish(_dpy->wl_modifiers.argb);
+  u_vector_finish(_dpy->wl_modifiers.xrgb);
+