[Mesa-dev] [PATCH piglit] egl_khr_create_context: Proper invalid attributes for EGL 1.5

[Miguel A. Vico]

When EGL_KHR_create_context was originally written,
EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR was an invalid
attribute for OpenGL ES.

After moving the extension to core EGL 1.5, the aforementioned
attribute was made valid for both OpenGL and OpenGL ES.

Check whether the EGL version is lower than 1.5 before checking
EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR is an invalid
attribute.

Signed-off-by: Miguel A Vico Moya 
---
 .../egl/spec/egl_khr_create_context/common.c  |  5 ++--
 .../egl/spec/egl_khr_create_context/common.h  |  8 +++
 .../invalid-attribute-gles.c  | 24 +++
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/tests/egl/spec/egl_khr_create_context/common.c 
b/tests/egl/spec/egl_khr_create_context/common.c
index a443ced97..ba0311bff 100644
--- a/tests/egl/spec/egl_khr_create_context/common.c
+++ b/tests/egl/spec/egl_khr_create_context/common.c
@@ -27,6 +27,8 @@
 
 static Display *dpy = NULL;
 EGLDisplay egl_dpy;
+EGLint egl_major;
+EGLint egl_minor;
 EGLConfig cfg;
 EGLContext ctx;
 
@@ -87,7 +89,6 @@ EGL_KHR_create_context_setup(EGLint renderable_type_mask)
EGL_NONE
};
EGLint count;
-   EGLint major, minor;
 
dpy = XOpenDisplay(NULL);
if (dpy == NULL) {
@@ -101,7 +102,7 @@ EGL_KHR_create_context_setup(EGLint renderable_type_mask)
piglit_report_result(PIGLIT_FAIL);
}
 
-   if (!eglInitialize(egl_dpy, , )) {
+   if (!eglInitialize(egl_dpy, _major, _minor)) {
fprintf(stderr, "eglInitialize() failed\n");
piglit_report_result(PIGLIT_FAIL);
}
diff --git a/tests/egl/spec/egl_khr_create_context/common.h 
b/tests/egl/spec/egl_khr_create_context/common.h
index f4f42760c..269610f01 100644
--- a/tests/egl/spec/egl_khr_create_context/common.h
+++ b/tests/egl/spec/egl_khr_create_context/common.h
@@ -51,6 +51,8 @@
 #endif
 
 extern EGLDisplay egl_dpy;
+extern EGLint egl_major;
+extern EGLint egl_minor;
 extern EGLConfig cfg;
 extern EGLContext ctx;
 
@@ -75,3 +77,9 @@ version_is_valid_for_context(int ctx_major, int major, int 
minor)
}
return false;
 }
+
+static inline bool
+check_egl_version(int major, int minor)
+{
+   return ((egl_major > major) || ((egl_major == major) && (egl_minor >= 
minor)));
+}
diff --git a/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c 
b/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
index 7d23e5673..d2b98818c 100644
--- a/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
+++ b/tests/egl/spec/egl_khr_create_context/invalid-attribute-gles.c
@@ -79,6 +79,22 @@ int main(int argc, char **argv)
 *attribute is only meaningful for OpenGL contexts, and specifying 
it
 *for other types of contexts, including OpenGL ES contexts, will
 *generate an error."
+*
+* However, after making the extension part of core EGL 1.5,
+* EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR was made a valid
+* attribute for OpenGL ES contexts:
+*
+*"The attribute EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY
+*specifies reset notification behavior for a context supporting
+*robust buffer access.  The attribute value may be either
+*EGL_NO_RESET_NOTIFICATION or EGL_LOSE_CONTEXT_ON_RESET, which
+*respectively result in reset notification behavior of
+*GL_NO_RESET_NOTIFICATION_ARB and GL_LOSE_CONTEXT_ON_RESET_ARB, as
+*described by the OpenGL GL_ARB_robustness extension, or by
+*equivalent functionality.
+*
+*This attribute is supported only for OpenGL and OpenGL ES
+*contexts."
 */
EGLint bad_attributes[] = {
0x,
@@ -97,6 +113,14 @@ int main(int argc, char **argv)
}
 
for (i = 0; i < ARRAY_SIZE(bad_attributes); i++) {
+   /* Skip EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR if
+* it's EGL 1.5+
+*/
+   if ((bad_attributes[i] == 
EGL_CONTEXT_OPENGL_RESET_NOTIFICATION_STRATEGY_KHR) &&
+   check_egl_version(1, 5)) {
+   continue;
+   }
+
pass = try_attribute(bad_attributes[i]) && pass;
}
 
-- 
2.18.0

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

Re: [Mesa-dev] [PATCH 1/2] nouveau: Add basic memory object support

[Miguel Angel Vico]

On Fri, 22 Jun 2018 20:37:19 -0400
Ilia Mirkin  wrote:

> On Fri, Jun 22, 2018 at 8:22 PM, Miguel Angel Vico  
> wrote:
> >
> >
> > On Thu, 21 Jun 2018 22:09:14 -0400
> > Ilia Mirkin  wrote:
> >  
> >> Hi Miguel,
> >>
> >> Preface: I know little about this ext, so feel free to educate me on
> >> the wrongness of my thinking.
> >>
> >> On Thu, Jun 21, 2018 at 10:01 PM, Miguel A. Vico  
> >> wrote:  
> >> > Add memory object support for nvc0 and nv50
> >> >
> >> > Signed-off-by: Miguel A Vico Moya 
> >> > ---
> >> >  .../drivers/nouveau/nv50/nv50_miptree.c   | 49 +
> >> >  .../drivers/nouveau/nv50/nv50_resource.c  | 52 +++
> >> >  .../drivers/nouveau/nv50/nv50_resource.h  | 33 
> >> >  .../drivers/nouveau/nvc0/nvc0_resource.c  | 22 
> >> >  4 files changed, 146 insertions(+), 10 deletions(-)
> >> >
> >> > diff --git a/src/gallium/drivers/nouveau/nv50/nv50_miptree.c 
> >> > b/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
> >> > index f2e304fde6..91007d3dac 100644
> >> > --- a/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
> >> > +++ b/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
> >> > @@ -397,13 +397,13 @@ nv50_miptree_create(struct pipe_screen *pscreen,
> >> > return pt;
> >> >  }
> >> >
> >> > -struct pipe_resource *
> >> > -nv50_miptree_from_handle(struct pipe_screen *pscreen,
> >> > - const struct pipe_resource *templ,
> >> > - struct winsys_handle *whandle)
> >> > +static struct pipe_resource *
> >> > +nv50_miptree_from_bo(struct pipe_screen *pscreen,
> >> > + const struct pipe_resource *templ,
> >> > + struct nouveau_bo *bo,
> >> > + uint32_t stride)
> >> >  {
> >> > struct nv50_miptree *mt;
> >> > -   unsigned stride;
> >> >
> >> > /* only supports 2D, non-mipmapped textures for the moment */  
> >>
> >> Won't this be a drag, since you're supposed to be able to "place" 3d
> >> textures, as well as mip-mapped ones?
> >>
> >> The reason I haven't looked at doing VK for nouveau yet is that the
> >> nouveau kernel API does not allow explicit userspace-side VA
> >> management, which would be required to allow something like this. I
> >> believe it would also be required to implement this GL extension. Feel
> >> free to correct my thinking.  
> >
> > My understanding is that EXT_external_objects itself only presents a
> > generic interface for applications to feed external memory handles to
> > OpenGL. It doesn't specify what properties those handles need to
> > satisfy, or whether the memory comes from user-space or any other
> > driver component. It is up for specific extensions to define new memory
> > objects, their properties, and how they can be imported/exported.
> >
> > As I understand it, the initial motivation for putting together this
> > extension was indeed Vulkan-OpenGL interoperability, but it is not
> > limited to that.
> >
> > This initial implementation of the extension adds the logic to allow
> > applications to feed opaque handles to OpenGL, but there's no API that
> > can create compatible opaque handles for the nouveau driver yet.
> >
> > Just to add a bit more context, here's a prototype of an extension
> > defining one of such handles:
> >
> >   
> > https://gitlab.freedesktop.org/mvicomoya/mesa/tree/wip/NVX_unix_allocator_import
> >
> > It is used by the the kmscube prototype that uses the generic allocator
> > to allocate buffers:
> >
> >   https://gitlab.freedesktop.org/allocator/kmscube/merge_requests/1
> >
> > And EXT_external_objects is just a pre-requisite for that.  
> 
> So by exposing GL_EXT_memory_object, the function
> glTexStorageMem3DEXT() becomes available. I don't think that will work
> with nouveau (without further changes), so the extension can't be
> exposed. Right?

Yes, exposing EXT_memory_object, several functions such as
glTexStorageMem3DEXT() become available, but they would not be usable at
all without at least one accompanying platform-specific extension that
defines how memory objects can be imported (e.g.
https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_external_objects_win32.txt),
am I correct?

In any case, we can keep the extension disabled by not applying patch
2/2 in this series, but maybe patch 1/2 is still good as a stepping
stone?

Thanks.

> 
> [I totally get that this is not your desired use-case, but we can't
> expose half-working extensions...]
> 
>   -ilia


-- 
Miguel


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

Re: [Mesa-dev] [PATCH 1/2] nouveau: Add basic memory object support

[Miguel Angel Vico]

On Thu, 21 Jun 2018 22:09:14 -0400
Ilia Mirkin  wrote:

> Hi Miguel,
> 
> Preface: I know little about this ext, so feel free to educate me on
> the wrongness of my thinking.
> 
> On Thu, Jun 21, 2018 at 10:01 PM, Miguel A. Vico  wrote:
> > Add memory object support for nvc0 and nv50
> >
> > Signed-off-by: Miguel A Vico Moya 
> > ---
> >  .../drivers/nouveau/nv50/nv50_miptree.c   | 49 +
> >  .../drivers/nouveau/nv50/nv50_resource.c  | 52 +++
> >  .../drivers/nouveau/nv50/nv50_resource.h  | 33 
> >  .../drivers/nouveau/nvc0/nvc0_resource.c  | 22 
> >  4 files changed, 146 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/gallium/drivers/nouveau/nv50/nv50_miptree.c 
> > b/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
> > index f2e304fde6..91007d3dac 100644
> > --- a/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
> > +++ b/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
> > @@ -397,13 +397,13 @@ nv50_miptree_create(struct pipe_screen *pscreen,
> > return pt;
> >  }
> >
> > -struct pipe_resource *
> > -nv50_miptree_from_handle(struct pipe_screen *pscreen,
> > - const struct pipe_resource *templ,
> > - struct winsys_handle *whandle)
> > +static struct pipe_resource *
> > +nv50_miptree_from_bo(struct pipe_screen *pscreen,
> > + const struct pipe_resource *templ,
> > + struct nouveau_bo *bo,
> > + uint32_t stride)
> >  {
> > struct nv50_miptree *mt;
> > -   unsigned stride;
> >
> > /* only supports 2D, non-mipmapped textures for the moment */  
> 
> Won't this be a drag, since you're supposed to be able to "place" 3d
> textures, as well as mip-mapped ones?
> 
> The reason I haven't looked at doing VK for nouveau yet is that the
> nouveau kernel API does not allow explicit userspace-side VA
> management, which would be required to allow something like this. I
> believe it would also be required to implement this GL extension. Feel
> free to correct my thinking.

My understanding is that EXT_external_objects itself only presents a
generic interface for applications to feed external memory handles to
OpenGL. It doesn't specify what properties those handles need to
satisfy, or whether the memory comes from user-space or any other
driver component. It is up for specific extensions to define new memory
objects, their properties, and how they can be imported/exported.

As I understand it, the initial motivation for putting together this
extension was indeed Vulkan-OpenGL interoperability, but it is not
limited to that.

This initial implementation of the extension adds the logic to allow
applications to feed opaque handles to OpenGL, but there's no API that
can create compatible opaque handles for the nouveau driver yet.

Just to add a bit more context, here's a prototype of an extension
defining one of such handles:

  
https://gitlab.freedesktop.org/mvicomoya/mesa/tree/wip/NVX_unix_allocator_import

It is used by the the kmscube prototype that uses the generic allocator
to allocate buffers:

  https://gitlab.freedesktop.org/allocator/kmscube/merge_requests/1

And EXT_external_objects is just a pre-requisite for that.

Thanks.

> 
> Cheers,
> 
>   -ilia


-- 
Miguel


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

[Mesa-dev] [PATCH 1/2] nouveau: Add basic memory object support

[Miguel A. Vico]

Add memory object support for nvc0 and nv50

Signed-off-by: Miguel A Vico Moya 
---
 .../drivers/nouveau/nv50/nv50_miptree.c   | 49 +
 .../drivers/nouveau/nv50/nv50_resource.c  | 52 +++
 .../drivers/nouveau/nv50/nv50_resource.h  | 33 
 .../drivers/nouveau/nvc0/nvc0_resource.c  | 22 
 4 files changed, 146 insertions(+), 10 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_miptree.c 
b/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
index f2e304fde6..91007d3dac 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_miptree.c
@@ -397,13 +397,13 @@ nv50_miptree_create(struct pipe_screen *pscreen,
return pt;
 }
 
-struct pipe_resource *
-nv50_miptree_from_handle(struct pipe_screen *pscreen,
- const struct pipe_resource *templ,
- struct winsys_handle *whandle)
+static struct pipe_resource *
+nv50_miptree_from_bo(struct pipe_screen *pscreen,
+ const struct pipe_resource *templ,
+ struct nouveau_bo *bo,
+ uint32_t stride)
 {
struct nv50_miptree *mt;
-   unsigned stride;
 
/* only supports 2D, non-mipmapped textures for the moment */
if ((templ->target != PIPE_TEXTURE_2D &&
@@ -417,11 +417,8 @@ nv50_miptree_from_handle(struct pipe_screen *pscreen,
if (!mt)
   return NULL;
 
-   mt->base.bo = nouveau_screen_bo_from_handle(pscreen, whandle, );
-   if (mt->base.bo == NULL) {
-  FREE(mt);
-  return NULL;
-   }
+   nouveau_bo_ref(bo, >base.bo);
+
mt->base.domain = mt->base.bo->flags & NOUVEAU_BO_APER;
mt->base.address = mt->base.bo->offset;
 
@@ -439,6 +436,38 @@ nv50_miptree_from_handle(struct pipe_screen *pscreen,
return >base.base;
 }
 
+struct pipe_resource *
+nv50_miptree_from_handle(struct pipe_screen *pscreen,
+ const struct pipe_resource *templ,
+ struct winsys_handle *whandle)
+{
+   struct pipe_resource *resource;
+   struct nouveau_bo *bo;
+   uint32_t stride;
+
+   bo = nouveau_screen_bo_from_handle(pscreen, whandle, );
+   if (bo == NULL) {
+  return NULL;
+   }
+
+   resource = nv50_miptree_from_bo(pscreen, templ, bo, stride);
+
+   /* nv50_miptree_from_bo will increment bo's refcount if succeeded */
+   nouveau_bo_ref(NULL, );
+
+   return resource;
+}
+
+struct pipe_resource *
+nv50_miptree_from_memobj(struct pipe_screen *pscreen,
+ const struct pipe_resource *templ,
+ struct pipe_memory_object *memobj)
+{
+   struct nv50_memory_object *mo = nv50_memory_object(memobj);
+
+   return nv50_miptree_from_bo(pscreen, templ, mo->bo, mo->stride);
+}
+
 
 /* Offset of zslice @z from start of level @l. */
 inline unsigned
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_resource.c 
b/src/gallium/drivers/nouveau/nv50/nv50_resource.c
index aed8c6241d..2a93c8820e 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_resource.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_resource.c
@@ -91,6 +91,55 @@ nv50_invalidate_resource(struct pipe_context *pipe, struct 
pipe_resource *res)
   nouveau_buffer_invalidate(pipe, res);
 }
 
+struct pipe_resource *
+nv50_resource_from_memobj(struct pipe_screen *screen,
+  const struct pipe_resource *templ,
+  struct pipe_memory_object *memobj,
+  uint64_t offset)
+{
+   if (offset != 0) {
+  debug_printf("%s: attempt to import unsupported winsys offset %lu\n",
+   __FUNCTION__, offset);
+  return NULL;
+   }
+
+   if (templ->target == PIPE_BUFFER)
+  return NULL;
+   else
+  return nv50_miptree_from_memobj(screen, templ, memobj);
+}
+
+struct pipe_memory_object *
+nv50_memobj_from_handle(struct pipe_screen *screen,
+struct winsys_handle *whandle,
+bool dedicated)
+{
+   struct nv50_memory_object *mo;
+
+   mo = CALLOC_STRUCT(nv50_memory_object);
+   if (!mo)
+  return NULL;
+
+   mo->bo = nouveau_screen_bo_from_handle(screen, whandle, >stride);
+   if (mo->bo == NULL) {
+  FREE(mo);
+  return NULL;
+   }
+   mo->base.dedicated = dedicated;
+
+   return >base;
+}
+
+void
+nv50_memobj_destroy(struct pipe_screen *screen,
+struct pipe_memory_object *memobj)
+{
+   struct nv50_memory_object *mo = nv50_memory_object(memobj);
+
+   nouveau_bo_ref(NULL, >bo);
+   FREE(mo);
+}
+
 void
 nv50_init_resource_functions(struct pipe_context *pcontext)
 {
@@ -111,4 +160,7 @@ nv50_screen_init_resource_functions(struct pipe_screen 
*pscreen)
pscreen->resource_from_handle = nv50_resource_from_handle;
pscreen->resource_get_handle = u_resource_get_handle_vtbl;
pscreen->resource_destroy = u_resource_destroy_vtbl;
+   pscree

[Mesa-dev] [PATCH 2/2] nouveau: Enable support for EXT_external_objects

[Miguel A. Vico]

Enable EXT_external_objects for nvc0 and nv50

Signed-off-by: Miguel A Vico Moya 
---
 src/gallium/drivers/nouveau/nv50/nv50_screen.c | 2 +-
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c 
b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
index 3a3c43b774..e5babd5580 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
@@ -201,6 +201,7 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
case PIPE_CAP_TGSI_CLOCK:
case PIPE_CAP_CAN_BIND_CONST_BUFFER_AS_VERTEX:
case PIPE_CAP_ALLOW_MAPPED_BUFFERS_DURING_EXECUTION:
+   case PIPE_CAP_MEMOBJ:
   return 1;
case PIPE_CAP_SEAMLESS_CUBE_MAP:
   return 1; /* class_3d >= NVA0_3D_CLASS; */
@@ -273,7 +274,6 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
case PIPE_CAP_BINDLESS_TEXTURE:
case PIPE_CAP_NIR_SAMPLERS_AS_DEREF:
case PIPE_CAP_QUERY_SO_OVERFLOW:
-   case PIPE_CAP_MEMOBJ:
case PIPE_CAP_LOAD_CONSTBUF:
case PIPE_CAP_TGSI_ANY_REG_AS_ADDRESS:
case PIPE_CAP_TILE_RASTER_ORDER:
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 02890c7165..ce344e33c5 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -259,6 +259,7 @@ nvc0_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
case PIPE_CAP_CAN_BIND_CONST_BUFFER_AS_VERTEX:
case PIPE_CAP_ALLOW_MAPPED_BUFFERS_DURING_EXECUTION:
case PIPE_CAP_QUERY_SO_OVERFLOW:
+   case PIPE_CAP_MEMOBJ:
   return 1;
case PIPE_CAP_PREFER_BLIT_BASED_TEXTURE_TRANSFER:
   return nouveau_screen(pscreen)->vram_domain & NOUVEAU_BO_VRAM ? 1 : 0;
@@ -309,7 +310,6 @@ nvc0_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
case PIPE_CAP_INT64_DIVMOD:
case PIPE_CAP_SPARSE_BUFFER_PAGE_SIZE:
case PIPE_CAP_NIR_SAMPLERS_AS_DEREF:
-   case PIPE_CAP_MEMOBJ:
case PIPE_CAP_LOAD_CONSTBUF:
case PIPE_CAP_TGSI_ANY_REG_AS_ADDRESS:
case PIPE_CAP_TILE_RASTER_ORDER:
-- 
2.17.1

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

[Mesa-dev] [PATCH 0/2] nouveau: Add support for EXT_external_objects

[Miguel A. Vico]

Hi,

These patches have been sitting in my local tree for some time now. They
are kind of related to the Generic Allocator work, but not specific to it.

James Jones's original kmscube port
(https://gitlab.freedesktop.org/allocator/kmscube) relies on the
EXT_external_objects extension to import allocator allocations to OpenGL as a
texture object. However, the Nouveau implementation of these mechanisms is
missing in Mesa.

These two patches will implement and enable the extension on both nv50 and nvc0.

Hoping to get them merged as I believe they might be useful regardless.

You can also check these changes on Gitlab here:

  
https://gitlab.freedesktop.org/mvicomoya/mesa/tree/wip/EXT_external_objects-nouveau

Thanks,
Miguel.


Miguel A. Vico (2):
  nouveau: Add basic memory object support
  nouveau: Enable support for EXT_external_objects

 src/gallium/drivers/nouveau/nv50/nv50_miptree.c  | 49 
+++--
 src/gallium/drivers/nouveau/nv50/nv50_resource.c | 52 

 src/gallium/drivers/nouveau/nv50/nv50_resource.h | 33 
+
 src/gallium/drivers/nouveau/nv50/nv50_screen.c   |  2 +-
 src/gallium/drivers/nouveau/nvc0/nvc0_resource.c | 22 ++
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c   |  2 +-
 6 files changed, 148 insertions(+), 12 deletions(-)


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

Re: [Mesa-dev] Allocator Nouveau driver, Mesa EXT_external_objects, and DRM metadata import interfaces

[Miguel Angel Vico]

Hi,

Besides the DRM modifiers discussion in the other forks or this thread
(I should've probably started separate threads), has anyone gotten the
chance to look at least at the mesa changes and allocator changes I
shared below?

With respect to Mesa changes, I think it might be worth merging the
EXT_external_objects nouveau implementation upstream. Should I just
send the list of patches as a formal RFR?

With respect to Allocator changes, it'd be nice getting someone else's
(out of NVIDIA) feedback.

Thanks.

On Wed, 20 Dec 2017 08:51:51 -0800
Miguel Angel Vico <mvicom...@nvidia.com> wrote:

> Hi all,
> 
> As many of you already know, I've been working with James Jones on the
> Generic Device Allocator project lately. He started a discussion thread
> some weeks ago seeking feedback on the current prototype of the library
> and advice on how to move all this forward, from a prototype stage to
> production. For further reference, see:
> 
>https://lists.freedesktop.org/archives/mesa-dev/2017-November/177632.html
> 
> From the thread above, we came up with very interesting high level
> design ideas for one of the currently missing parts in the library:
> Usage transitions. That's something I'll personally work on during the
> following weeks.
> 
> 
> In the meantime, I've been working on putting together an open source
> implementation of the allocator mechanisms using the Nouveau driver for
> all to be able to play with.
> 
> Below I'm seeking feedback on a bunch of changes I had to make to
> different components of the graphics stack:
> 
> ** Allocator **
> 
>   An allocator driver implementation on top of Nouveau. The current
>   implementation only handles pitch linear layouts, but that's enough
>   to have the kmscube port working using the allocator and Nouveau
>   drivers.
> 
>   You can pull these changes from
> 
>   https://github.com/mvicomoya/allocator/tree/wip/mvicomoya/nouveau-driver
> 
> ** Mesa **
> 
>   James's kmscube port to use the allocator relies on the
>   EXT_external_objects extension to import allocator allocations to
>   OpenGL as a texture object. However, the Nouveau implementation of
>   these mechanisms is missing in Mesa, so I went ahead and added them.
> 
>   You can pull these changes from
> 
>   
> https://github.com/mvicomoya/mesa/tree/wip/mvicomoya/EXT_external_objects-nouveau
> 
>   Also, James's kmscube port uses the NVX_unix_allocator_import
>   extension to attach allocator metadata to texture objects so the
>   driver knows how to deal with the imported memory.
> 
>   Note that there isn't a formal spec for this extension yet. For now,
>   it just serves as an experimental mechanism to import allocator
>   memory in OpenGL, and attach metadata to texture objects.
> 
>   You can pull these changes (written on top of the above) from:
> 
>   
> https://github.com/mvicomoya/mesa/tree/wip/mvicomoya/NVX_unix_allocator_import
> 
> ** kmscube **
> 
>   Mostly minor fixes and improvements on top of James's port to use the
>   allocator. Main thing is the allocator initialization path will use
>   EGL_MESA_platform_surfaceless if EGLDevice platform isn't supported
>   by the underlying EGL implementation.
> 
>   You can pull these changes from:
> 
>   
> https://github.com/mvicomoya/kmscube/tree/wip/mvicomoya/allocator-nouveau
> 
> 
> With all the above you should be able to get kmscube working using the
> allocator on top of the Nouveau driver.
> 
> 
> Another of the missing pieces before we can move this to production is
> importing allocations to DRM FB objects. This is probably one of the
> most sensitive parts of the project as it requires modification/addition
> of kernel driver interfaces.
> 
> At XDC2017, James had several hallway conversations with several people
> about this, all having different opinions. I'd like to take this
> opportunity to also start a discussion about what's the best option to
> create a path to get allocator allocations added as DRM FB objects.
> 
> These are the few options we've considered to start with:
> 
>   A) Have vendor-private ioctls to set properties on GEM objects that
>  are inherited by the FB objects. This is how our (NVIDIA) desktop
>  DRM driver currently works. This would require every vendor to add
>  their own ioctl to process allocator metadata, but the metadata is
>  actually a vendor-agnostic object more like DRM modifiers. We'd
>  like to come up with a vendor-agnostic solutions that can be
>  integrated to core DRM.
> 
>   B) Add a new drmModeAddFBWithMetadata() command that takes allocator
>  metadata blobs for each plane of the FB. Some people in the
>  community have me

Re: [Mesa-dev] Allocator Nouveau driver, Mesa EXT_external_objects, and DRM metadata import interfaces

[Miguel Angel Vico]

(Adding dri-devel back, and trying to respond to some comments from
the different forks)

James Jones wrote:

> Your worst case analysis above isn't far off from our HW, give or take
> some bits and axes here and there.  We've started an internal discussion
> about how to lay out all the bits we need.  It's hard to even enumerate
> them all without having a complete understanding of what capability sets
> are going to include, a fully-optimized implementation of the mechanism
> on our HW, and lot's of test scenarios though.  

(thanks James for most of the info below)

To elaborate a bit, if we want to share an allocation across GPUs for 3D
rendering, it seems we would need 12 bits to express our
swizzling/tiling memory layouts for fermi+. In addition to that,
maxwell uses 3 more bits for this, and we need an extra bit to identify
pre-fermi representations.

We also need one bit to differentiate between Tegra and desktop, and
another one to indicate whether the layout is otherwise linear.

Then things like whether compression is used (one more bit), and we can
probably get by with 3 bits for the type of compression if we are
creative. However, it'd be way easier to just track arch + page kind,
which would be like 32 bits on its own.

Whether Z-culling and/or zero-bandwidth-clears are used may be another 3
bits.

If device-local properties are included, we might need a couple more
bits for caching.

We may also need to express locality information, which may take at
least another 2 or 3 bits.

If we want to share array textures too, you also need to pass the array
pitch. Is it supposed to be encoded in a modifier too? That's 64 bits on
its own.

So yes, as James mentioned, with some effort, we could technically fit
our current allocation parameters in a modifier, but I'm still not
convinced this is as future proof as it could be as our hardware grows
in capabilities.


Daniel Stone wrote:

> So I reflexively
> get a bit itchy when I see the kernel being used to transit magic
> blobs of data which are supplied by userspace, and only interpreted by
> different userspace. Having tiling formats hidden away means that
> we've had real-world bugs in AMD hardware, where we end up displaying
> garbage because we cannot generically reason about the buffer
> attributes.  

I'm a bit confused. Can't modifiers be specified by vendors and only
interpreted by drivers? My understanding was that modifiers could
actually be treated as opaque 64-bit data, in which case they would
qualify as "magic blobs of data". Otherwise, it seems this wouldn't be
scalable. What am I missing?


Daniel Vetter wrote:

> I think in the interim figuring out how to expose kms capabilities
> better (and necessarily standardizing at least some of them which
> matter at the compositor level, like size limits of framebuffers)
> feels like the place to push the ecosystem forward. In some way
> Miguel's proposal looks a bit backwards, since it adds the pitch
> capabilities to addfb, but at addfb time you've allocated everything
> already, so way too late to fix things up. With modifiers we've added
> a very simple per-plane property to list which modifiers can be
> combined with which pixel formats. Tiny start, but obviously very far
> from all that we'll need.  

Not sure whether I might be misunderstanding your statement, but one of
the allocator main features is negotiation of nearly optimal allocation
parameters given a set of uses on different devices/engines by the
capability merge operation. A client should have queried what every
device/engine is capable of for the given uses, find the optimal set of
capabilities, and use it for allocating a buffer. At the moment these
parameters are given to KMS, they are expected to be good. If they
aren't, the client didn't do things right.


Rob Clark wrote:

> It does seem like, if possible, starting out with modifiers for now at
> the kernel interface would make life easier, vs trying to reinvent
> both kernel and userspace APIs at the same time.  Userspace APIs are
> easier to change or throw away.  Presumably by the time we get to the
> point of changing kernel uabi, we are already using, and pretty happy
> with, serialized liballoc data over the wire in userspace so it is
> only a matter of changing the kernel interface.  

I guess we can indeed start with modifiers for now, if that's what it
takes to get the allocator mechanisms rolling. However, it seems to me
that we won't be able to encode the same type of information included
in capability sets with modifiers in all cases. For instance, if we end
up encoding usage transition information in capability sets, how that
would translate to modifiers?

I assume display doesn't really care about a lot of the data capability
sets may encode, but is it correct to think of modifiers as things only
display needs? If we are to treat modifiers as a first-class citizen, I
would expect to use them beyond that.


Kristian Kristensen wrote:

> I agree and let me 

Re: [Mesa-dev] Allocator Nouveau driver, Mesa EXT_external_objects, and DRM metadata import interfaces

[Miguel Angel Vico]

Inline.

On Wed, 20 Dec 2017 11:54:10 -0800
Kristian Høgsberg <hoegsb...@gmail.com> wrote:

> On Wed, Dec 20, 2017 at 11:51 AM, Daniel Vetter <dan...@ffwll.ch> wrote:
> > Since this also involves the kernel let's add dri-devel ...

Yeah, I forgot. Thanks Daniel!

> >
> > On Wed, Dec 20, 2017 at 5:51 PM, Miguel Angel Vico <mvicom...@nvidia.com> 
> > wrote:  
> >> Hi all,
> >>
> >> As many of you already know, I've been working with James Jones on the
> >> Generic Device Allocator project lately. He started a discussion thread
> >> some weeks ago seeking feedback on the current prototype of the library
> >> and advice on how to move all this forward, from a prototype stage to
> >> production. For further reference, see:
> >>
> >>
> >> https://lists.freedesktop.org/archives/mesa-dev/2017-November/177632.html
> >>
> >> From the thread above, we came up with very interesting high level
> >> design ideas for one of the currently missing parts in the library:
> >> Usage transitions. That's something I'll personally work on during the
> >> following weeks.
> >>
> >>
> >> In the meantime, I've been working on putting together an open source
> >> implementation of the allocator mechanisms using the Nouveau driver for
> >> all to be able to play with.
> >>
> >> Below I'm seeking feedback on a bunch of changes I had to make to
> >> different components of the graphics stack:
> >>
> >> ** Allocator **
> >>
> >>   An allocator driver implementation on top of Nouveau. The current
> >>   implementation only handles pitch linear layouts, but that's enough
> >>   to have the kmscube port working using the allocator and Nouveau
> >>   drivers.
> >>
> >>   You can pull these changes from
> >>
> >>   
> >> https://github.com/mvicomoya/allocator/tree/wip/mvicomoya/nouveau-driver
> >>
> >> ** Mesa **
> >>
> >>   James's kmscube port to use the allocator relies on the
> >>   EXT_external_objects extension to import allocator allocations to
> >>   OpenGL as a texture object. However, the Nouveau implementation of
> >>   these mechanisms is missing in Mesa, so I went ahead and added them.
> >>
> >>   You can pull these changes from
> >>
> >>   
> >> https://github.com/mvicomoya/mesa/tree/wip/mvicomoya/EXT_external_objects-nouveau
> >>
> >>   Also, James's kmscube port uses the NVX_unix_allocator_import
> >>   extension to attach allocator metadata to texture objects so the
> >>   driver knows how to deal with the imported memory.
> >>
> >>   Note that there isn't a formal spec for this extension yet. For now,
> >>   it just serves as an experimental mechanism to import allocator
> >>   memory in OpenGL, and attach metadata to texture objects.
> >>
> >>   You can pull these changes (written on top of the above) from:
> >>
> >>   
> >> https://github.com/mvicomoya/mesa/tree/wip/mvicomoya/NVX_unix_allocator_import
> >>
> >> ** kmscube **
> >>
> >>   Mostly minor fixes and improvements on top of James's port to use the
> >>   allocator. Main thing is the allocator initialization path will use
> >>   EGL_MESA_platform_surfaceless if EGLDevice platform isn't supported
> >>   by the underlying EGL implementation.
> >>
> >>   You can pull these changes from:
> >>
> >>   
> >> https://github.com/mvicomoya/kmscube/tree/wip/mvicomoya/allocator-nouveau
> >>
> >>
> >> With all the above you should be able to get kmscube working using the
> >> allocator on top of the Nouveau driver.
> >>
> >>
> >> Another of the missing pieces before we can move this to production is
> >> importing allocations to DRM FB objects. This is probably one of the
> >> most sensitive parts of the project as it requires modification/addition
> >> of kernel driver interfaces.
> >>
> >> At XDC2017, James had several hallway conversations with several people
> >> about this, all having different opinions. I'd like to take this
> >> opportunity to also start a discussion about what's the best option to
> >> create a path to get allocator allocations added as DRM FB objects.
> >>
> >> These are the few options we've considered to start with:
> >>
> >>   A) Have vendor-private ioctls to set properties on GEM 

[Mesa-dev] Allocator Nouveau driver, Mesa EXT_external_objects, and DRM metadata import interfaces

[Miguel Angel Vico]

Hi all,

As many of you already know, I've been working with James Jones on the
Generic Device Allocator project lately. He started a discussion thread
some weeks ago seeking feedback on the current prototype of the library
and advice on how to move all this forward, from a prototype stage to
production. For further reference, see:

   https://lists.freedesktop.org/archives/mesa-dev/2017-November/177632.html

From the thread above, we came up with very interesting high level
design ideas for one of the currently missing parts in the library:
Usage transitions. That's something I'll personally work on during the
following weeks.


In the meantime, I've been working on putting together an open source
implementation of the allocator mechanisms using the Nouveau driver for
all to be able to play with.

Below I'm seeking feedback on a bunch of changes I had to make to
different components of the graphics stack:

** Allocator **

  An allocator driver implementation on top of Nouveau. The current
  implementation only handles pitch linear layouts, but that's enough
  to have the kmscube port working using the allocator and Nouveau
  drivers.

  You can pull these changes from

  https://github.com/mvicomoya/allocator/tree/wip/mvicomoya/nouveau-driver

** Mesa **

  James's kmscube port to use the allocator relies on the
  EXT_external_objects extension to import allocator allocations to
  OpenGL as a texture object. However, the Nouveau implementation of
  these mechanisms is missing in Mesa, so I went ahead and added them.

  You can pull these changes from

  
https://github.com/mvicomoya/mesa/tree/wip/mvicomoya/EXT_external_objects-nouveau

  Also, James's kmscube port uses the NVX_unix_allocator_import
  extension to attach allocator metadata to texture objects so the
  driver knows how to deal with the imported memory.

  Note that there isn't a formal spec for this extension yet. For now,
  it just serves as an experimental mechanism to import allocator
  memory in OpenGL, and attach metadata to texture objects.

  You can pull these changes (written on top of the above) from:

  
https://github.com/mvicomoya/mesa/tree/wip/mvicomoya/NVX_unix_allocator_import

** kmscube **

  Mostly minor fixes and improvements on top of James's port to use the
  allocator. Main thing is the allocator initialization path will use
  EGL_MESA_platform_surfaceless if EGLDevice platform isn't supported
  by the underlying EGL implementation.

  You can pull these changes from:

  https://github.com/mvicomoya/kmscube/tree/wip/mvicomoya/allocator-nouveau


With all the above you should be able to get kmscube working using the
allocator on top of the Nouveau driver.


Another of the missing pieces before we can move this to production is
importing allocations to DRM FB objects. This is probably one of the
most sensitive parts of the project as it requires modification/addition
of kernel driver interfaces.

At XDC2017, James had several hallway conversations with several people
about this, all having different opinions. I'd like to take this
opportunity to also start a discussion about what's the best option to
create a path to get allocator allocations added as DRM FB objects.

These are the few options we've considered to start with:

  A) Have vendor-private ioctls to set properties on GEM objects that
 are inherited by the FB objects. This is how our (NVIDIA) desktop
 DRM driver currently works. This would require every vendor to add
 their own ioctl to process allocator metadata, but the metadata is
 actually a vendor-agnostic object more like DRM modifiers. We'd
 like to come up with a vendor-agnostic solutions that can be
 integrated to core DRM.

  B) Add a new drmModeAddFBWithMetadata() command that takes allocator
 metadata blobs for each plane of the FB. Some people in the
 community have mentioned this is their preferred design. This,
 however, means we'd have to go through the exercise of adding
 another metadata mechanism to the whole graphics stack.

  C) Shove allocator metadata into DRM by defining it to be a separate
 plane in the image, and using the existing DRM modifiers mechanism
 to indicate there is another plane for each "real" plane added. It
 isn't clear how this scales to surfaces that already need several
 planes, but there are some people that see this as the only way
 forward. Also, we would have to create a separate GEM buffer for
 the metadatada itself, which seems excessive.

We personally like option (B) better, and have already started to
prototype the new path (which is actually very similar to the
drmModeAddFB2() one). You can take a look at the new interfaces here:


https://github.com/mvicomoya/linux/tree/wip/mvicomoya/drm_addfb_with_metadata__4.14-rc8

There may be other options that haven't been explored yet that could be
a better choice than the above, so any suggestion will be greatly
appreciated.

Re: [Mesa-dev] GBM and the Device Memory Allocator Proposals

[Miguel Angel Vico]

On Wed, 6 Dec 2017 16:57:45 -0800
James Jones  wrote:

> On 12/06/2017 03:25 AM, Nicolai Hähnle wrote:
> > On 06.12.2017 08:07, James Jones wrote:
> > [snip]  
> >> So lets say you have a setup where both display and GPU supported
> >> FOO/tiled, but only GPU supported compressed (FOO/CC) and cached
> >> (FOO/cached).  But the GPU supported the following transitions:
> >>
> >>     trans_a: FOO/CC -> null
> >>     trans_b: FOO/cached -> null
> >>
> >> Then the sets for each device (in order of preference):
> >>
> >> GPU:
> >>     1: caps(FOO/tiled, FOO/CC, FOO/cached); 
> >> constraints(alignment=32k)
> >>     2: caps(FOO/tiled, FOO/CC); constraints(alignment=32k)
> >>     3: caps(FOO/tiled); constraints(alignment=32k)
> >>
> >> Display:
> >>     1: caps(FOO/tiled); constraints(alignment=64k)
> >>
> >> Merged Result:
> >>     1: caps(FOO/tiled, FOO/CC, FOO/cached); 
> >> constraints(alignment=64k);
> >>    transition(GPU->display: trans_a, trans_b; display->GPU: none)
> >>     2: caps(FOO/tiled, FOO/CC); constraints(alignment=64k);
> >>    transition(GPU->display: trans_a; display->GPU: none)
> >>     3: caps(FOO/tiled); constraints(alignment=64k);
> >>    transition(GPU->display: none; display->GPU: none)  
> >
> >
> > We definitely don't want to expose a way of getting uncached rendering
> > surfaces for radeonsi. I mean, I think we are supposed to be able 
> > to program
> > our hardware so that the backend bypasses all caches, but (a) nobody
> > validates that and (b) it's basically suicide in terms of 
> > performance. Let's
> > build fewer footguns :)  
> 
>  sure, this was just a hypothetical example.  But to take this case as
>  another example, if you didn't want to expose uncached rendering (or
>  cached w/ cache flushes after each draw), you would exclude the entry
>  from the GPU set which didn't have FOO/cached (I'm adding back a
>  cached but not CC config just to make it interesting), and end up
>  with:
> 
>      trans_a: FOO/CC -> null
>      trans_b: FOO/cached -> null
> 
>  GPU:
>     1: caps(FOO/tiled, FOO/CC, FOO/cached); constraints(alignment=32k)
>     2: caps(FOO/tiled, FOO/cached); constraints(alignment=32k)
> 
>  Display:
>     1: caps(FOO/tiled); constraints(alignment=64k)
> 
>  Merged Result:
>     1: caps(FOO/tiled, FOO/CC, FOO/cached); constraints(alignment=64k);
>    transition(GPU->display: trans_a, trans_b; display->GPU: none)
>     2: caps(FOO/tiled, FOO/cached); constraints(alignment=64k);
>    transition(GPU->display: trans_b; display->GPU: none)
> 
>  So there isn't anything in the result set that doesn't have GPU cache,
>  and the cache-flush transition is always in the set of required
>  transitions going from GPU -> display
> 
>  Hmm, I guess this does require the concept of a required cap..  
> >>>
> >>> Which we already introduced to the allocator API when we realized we
> >>> would need them as we were prototyping.  
> >>
> >> Note I also posed the question of whether things like cached (and 
> >> similarly compression, since I view compression as roughly an 
> >> equivalent mechanism to a cache) in one of the open issues on my XDC 
> >> 2017 slides because of this very problem of over-pruning it causes.  
> >> It's on slide 15, as "No device-local capabilities".  You'll have to 
> >> listen to my coverage of it in the recorded presentation for that 
> >> slide to make any sense, but it's the same thing Nicolai has laid out 
> >> here.
> >>
> >> As I continued working through our prototype driver support, I found I 
> >> didn't actually need to include cached or compressed as capabilities: 
> >> The GPU just applies them as needed and the usage transitions make it 
> >> transparent to the non-GPU engines.  That does mean the GPU driver 
> >> currently needs to be the one to realize the allocation from the 
> >> capability set to get optimal behavior.  We could fix that by 
> >> reworking our driver though.  At this point, not including 
> >> device-local properties like on-device caching in capabilities seems 
> >> like the right solution to me.  I'm curious whether this applies 
> >> universally though, or if other hardware doesn't fit the "compression 
> >> and stuff all behaves like a cache" idiom.  
> > 
> > Compression is a part of the memory layout for us: framebuffer 
> > compression uses an additional "meta surface". At the most basic level, 
> > an allocation with loss-less compression support is by necessity bigger 
> > than an allocation without.
> > 
> > We can allocate this meta surface separately, but then we're forced to 
> > decompress when passing the surface around (e.g. to a compositor.)
> > 
> > Consider also the example I gave elsewhere, where a 

[Mesa-dev] [PATCH] libdrm: Fix QNX build by including 'sys/ioctl.h'

[Miguel A. Vico]

drm.h header includes 'sys/ioccom.h' for ioctl
definitions on non-linux Unix platforms, but QNX
seems to define those in 'sys/ioctl.h' instead.

Signed-off-by: Miguel A Vico Moya <mvicom...@nvidia.com>
---
 include/drm/drm.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/drm/drm.h b/include/drm/drm.h
index f0bd91de..ec0e1fd7 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -44,7 +44,12 @@ typedef unsigned int drm_handle_t;
 
 #else /* One of the BSDs */
 
+#if (defined(__QNX__) || defined(__QNXNTO__))
+#include 
+#else
 #include 
+#endif
+
 #include 
 typedef int8_t   __s8;
 typedef uint8_t  __u8;
-- 
2.15.0

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

Re: [Mesa-dev] GBM and the Device Memory Allocator Proposals

[Miguel Angel Vico]

On Fri, 1 Dec 2017 13:38:41 -0500
Rob Clark  wrote:

> On Fri, Dec 1, 2017 at 12:09 PM, Nicolai Hähnle  wrote:
> > On 01.12.2017 16:06, Rob Clark wrote:  
> >>
> >> On Thu, Nov 30, 2017 at 5:43 PM, Nicolai Hähnle 
> >> wrote:  
> >>>
> >>> Hi,
> >>>
> >>> I've had a chance to look a bit more closely at the allocator prototype
> >>> repository now. There's a whole bunch of low-level API design feedback,
> >>> but
> >>> for now let's focus on the high-level stuff first.
> >>>
> >>> Going by the 4.5 major object types (as also seen on slide 5 of your
> >>> presentation [0]), assertions and usages make sense to me.
> >>>
> >>> Capabilities and capability sets should be cleaned up in my opinion, as
> >>> the
> >>> status quo is overly obfuscating things. What capability sets really
> >>> represent, as far as I understand them, is *memory layouts*, and so
> >>> that's
> >>> what they should be called.
> >>>
> >>> This conceptually simplifies `derive_capabilities` significantly without
> >>> any
> >>> loss of expressiveness as far as I can see. Given two lists of memory
> >>> layouts, we simply look for which memory layouts appear in both lists,
> >>> and
> >>> then merge their constraints and capabilities.
> >>>
> >>> Merging constraints looks good to me.
> >>>
> >>> Capabilities need some more thought. The prototype removes capabilities
> >>> when
> >>> merging layouts, but I'd argue that that is often undesirable. (In fact,
> >>> I
> >>> cannot think of capabilities which we'd always want to remove.)
> >>>
> >>> A typical example for this is compression (i.e. DCC in our case). For
> >>> rendering usage, we'd return something like:
> >>>
> >>> Memory layout: AMD/tiled; constraints(alignment=64k); caps(AMD/DCC)
> >>>
> >>> For display usage, we might return (depending on hardware):
> >>>
> >>> Memory layout: AMD/tiled; constraints(alignment=64k); caps(none)
> >>>
> >>> Merging these in the prototype would remove the DCC capability, even
> >>> though
> >>> it might well make sense to keep it there for rendering. Dealing with the
> >>> fact that display usage does not have this capability is precisely one of
> >>> the two things that transitions are about! The other thing that
> >>> transitions
> >>> are about is caches.
> >>>
> >>> I think this is kind of what Rob was saying in one of his mails.  
> >>
> >>
> >> Perhaps "layout" is a better name than "caps".. either way I think of
> >> both AMD/tiled and AMD/DCC as the same type of "thing".. the
> >> difference between AMD/tiled and AMD/DCC is that a transition can be
> >> provided for AMD/DCC.  Other than that they are both things describing
> >> the layout.  
> >
> >
> > The reason that a transition can be provided is that they aren't quite the
> > same thing, though. In a very real sense, AMD/DCC is a "child" property of
> > AMD/tiled: DCC is implemented as a meta surface whose memory layout depends
> > on the layout of the main surface.  
> 
> I suppose this is six-of-one, half-dozen of the other..
> 
> what you are calling a layout is what I'm calling a cap that just
> happens not to have an associated transition
> 
> > Although, if there are GPUs that can do an in-place "transition" between
> > different tiling layouts, then the distinction is perhaps really not as
> > clear-cut. I guess that would only apply to tiled renderers.  
> 
> I suppose the advantage of just calling both layout and caps the same
> thing, and just saying that a "cap" (or "layout" if you prefer that
> name) can optionally have one or more associated transitions, is that
> you can deal with cases where sometimes a tiled format might actually
> have an in-place transition ;-)
> 
> >  
> >> So lets say you have a setup where both display and GPU supported
> >> FOO/tiled, but only GPU supported compressed (FOO/CC) and cached
> >> (FOO/cached).  But the GPU supported the following transitions:
> >>
> >>trans_a: FOO/CC -> null
> >>trans_b: FOO/cached -> null
> >>
> >> Then the sets for each device (in order of preference):
> >>
> >> GPU:
> >>1: caps(FOO/tiled, FOO/CC, FOO/cached); constraints(alignment=32k)
> >>2: caps(FOO/tiled, FOO/CC); constraints(alignment=32k)
> >>3: caps(FOO/tiled); constraints(alignment=32k)
> >>
> >> Display:
> >>1: caps(FOO/tiled); constraints(alignment=64k)
> >>
> >> Merged Result:
> >>1: caps(FOO/tiled, FOO/CC, FOO/cached); constraints(alignment=64k);
> >>   transition(GPU->display: trans_a, trans_b; display->GPU: none)
> >>2: caps(FOO/tiled, FOO/CC); constraints(alignment=64k);
> >>   transition(GPU->display: trans_a; display->GPU: none)
> >>3: caps(FOO/tiled); constraints(alignment=64k);
> >>   transition(GPU->display: none; display->GPU: none)  
> >
> >
> > We definitely don't want to expose a way of getting uncached rendering
> > surfaces for radeonsi. I mean, I think we are supposed to be able to program
> > our hardware so that the backend 

Re: [Mesa-dev] GBM and the Device Memory Allocator Proposals

[Miguel Angel Vico]

On Wed, 29 Nov 2017 16:28:15 -0500
Rob Clark <robdcl...@gmail.com> wrote:

> On Wed, Nov 29, 2017 at 2:41 PM, Miguel Angel Vico <mvicom...@nvidia.com> 
> wrote:
> > Many of you may already know, but James is going to be out for a few
> > weeks and I'll be taking over this in the meantime.
> >
> > See inline for comments.
> >
> > On Wed, 29 Nov 2017 09:33:29 -0800
> > Jason Ekstrand <ja...@jlekstrand.net> wrote:
> >  
> >> On Sat, Nov 25, 2017 at 1:20 PM, Rob Clark <robdcl...@gmail.com> wrote:
> >>  
> >> > On Sat, Nov 25, 2017 at 12:46 PM, Jason Ekstrand <ja...@jlekstrand.net>
> >> > wrote:  
> >> > > On November 24, 2017 09:29:43 Rob Clark <robdcl...@gmail.com> wrote:  
> >> > >>
> >> > >>
> >> > >> On Mon, Nov 20, 2017 at 8:11 PM, James Jones <jajo...@nvidia.com>  
> >> > wrote:  
> >> > >>>
> >> > >>> As many here know at this point, I've been working on solving issues
> >> > >>> related
> >> > >>> to DMA-capable memory allocation for various devices for some time 
> >> > >>> now.
> >> > >>> I'd
> >> > >>> like to take this opportunity to apologize for the way I handled the 
> >> > >>>  
> >> > EGL  
> >> > >>> stream proposals.  I understand now that the development process  
> >> > followed  
> >> > >>> there was unacceptable to the community and likely offended many 
> >> > >>> great
> >> > >>> engineers.
> >> > >>>
> >> > >>> Moving forward, I attempted to reboot talks in a more constructive  
> >> > manner  
> >> > >>> with the generic allocator library proposals & discussion forum at 
> >> > >>> XDC
> >> > >>> 2016.
> >> > >>> Some great design ideas came out of that, and I've since been  
> >> > prototyping  
> >> > >>> some code to prove them out before bringing them back as official
> >> > >>> proposals.
> >> > >>> Again, I understand some people are growing concerned that I've been
> >> > >>> doing
> >> > >>> this off on the side in a github project that has primarily NVIDIA
> >> > >>> contributors.  My goal was only to avoid wasting everyone's time with
> >> > >>> unproven ideas.  The intent was never to dump the prototype code 
> >> > >>> as-is  
> >> > on  
> >> > >>> the community and presume acceptance. It's just a public research
> >> > >>> project.
> >> > >>>
> >> > >>> Now the prototyping is nearing completion, and I'd like to renew
> >> > >>> discussion
> >> > >>> on whether and how the new mechanisms can be integrated with the 
> >> > >>> Linux
> >> > >>> graphics stack.
> >> > >>>
> >> > >>> I'd be interested to know if more work is needed to demonstrate the
> >> > >>> usefulness of the new mechanisms, or whether people think they have  
> >> > value  
> >> > >>> at
> >> > >>> this point.
> >> > >>>
> >> > >>> After talking with people on the hallway track at XDC this year, I've
> >> > >>> heard
> >> > >>> several proposals for incorporating the new mechanisms:
> >> > >>>
> >> > >>> -Include ideas from the generic allocator design into GBM.  This 
> >> > >>> could
> >> > >>> take
> >> > >>> the form of designing a "GBM 2.0" API, or incrementally adding to the
> >> > >>> existing GBM API.
> >> > >>>
> >> > >>> -Develop a library to replace GBM.  The allocator prototype code 
> >> > >>> could  
> >> > be  
> >> > >>> massaged into something production worthy to jump start this process.
> >> > >>>
> >> > >>> -Develop a library that sits beside or on top of GBM, using GBM for
> >> > >>> low-level graphics buffer allocation, while supporting non-graphics
> >> > >>> kernel
> >> > >

Re: [Mesa-dev] GBM and the Device Memory Allocator Proposals

[Miguel Angel Vico]

 GBM sits on top of.  That is an implementation detail.  It could be
> > >> that GBM grows an API to return an instance of $new_thing for
> > >> use-cases that involve sharing a buffer with the GPU.  Or perhaps that
> > >> is exposed via some sort of EGL extension.  (We probably also need a
> > >> way to get an instance from libdrm (?) for display-only KMS drivers,
> > >> to cover cases like etnaviv sharing a buffer with a separate display
> > >> driver.)
> > >>
> > >> [1] where "devices" could be multiple GPUs or multiple APIs for one or
> > >> more GPUs, but also includes non-GPU devices like camera, video
> > >> decoder, "image processor" (which may or may not be part of camera),
> > >> etc, etc  
> > >
> > >
> > > I'm not quite some sure what I think about this.  I think I would like to
> > > see $new_thing at least replace the guts of GBM. Whether GBM becomes a
> > > wrapper around $new_thing or $new_thing implements the GBM API, I'm not
> > > sure.  What I don't think I want is to see GBM development continuing on
> > > it's own so we have two competing solutions.  
> >
> > I don't really view them as competing.. there is *some* overlap, ie.
> > allocating a buffer.. but even if you are using GBM w/out $new_thing
> > you could allocate a buffer externally and import it.  I don't see
> > $new_thing as that much different from GBM PoV.
> >
> > But things like surfaces (aka swap chains) seem a bit out of place
> > when you are thinking about implementing $new_thing for non-gpu
> > devices.  Plus EGL<->GBM tie-ins that seem out of place when talking
> > about a (for ex.) camera.  I kinda don't want to throw out the baby
> > with the bathwater here.
> >  
> 
> Agreed.  GBM is very EGLish and we don't want the new allocator to be that.
> 
> 
> > *maybe* GBM could be partially implemented on top of $new_thing.  I
> > don't quite see how that would work.  Possibly we could deprecate
> > parts of GBM that are no longer needed?  idk..  Either way, I fully
> > expect that GBM and mesa's implementation of $new_thing could perhaps
> > sit on to of some of the same set of internal APIs.  The public
> > interface can be decoupled from the internal implementation.
> >  
> 
> Maybe I should restate things a bit.  My real point was that modifiers +
> $new_thing + Kernel blob should be a complete and more powerful replacement
> for GBM.  I don't know that we really can implement GBM on top of it
> because GBM has lots of wishy-washy concepts such as "cursor plane" which
> may not map well at least not without querying the kernel about specifc
> display planes.  In particular, I don't want someone to feel like they need
> to use $new_thing and GBM at the same time or together.  Ideally, I'd like
> them to never do that unless we decide gbm_bo is a useful abstraction for
> $new_thing.
> 

I'm not really familiar with GBM guts, so I don't know how easy would
it be to make GBM rely on the allocator for the buffer allocations.
Maybe that's something worth exploring. What I wouldn't like is
$new_thing to fall short because we are trying to shove it under GBM's
hood.

It seems to me that $new_thing should grow as a separate thing whether
it ends up replacing GBM or GBM internals are somewhat rewritten on top
of it. If I'm reading you both correctly, you agree with that, so in
order to move forward, should we go ahead and create a project in fd.o?

Before filing the new project request though, we should find an
appropriate name for $new_thing. Creativity isn't one of my strengths,
but I'll go ahead and start the bikeshedding with "Generic Device
Memory Allocator" or "Generic Device Memory Manager".

Once we agree upon something, I can take care of filing the request,
but I'm unclear what the initial list of approvers should be.
Looking at the main contributors of both the initial draft of
$new_thing and git repository, does the following list of people seem
reasonable?

 * Rob Clark
 * Jason Ekstrand
 * James Jones
 * Chad Versace
 * Miguel A Vico

I never started a project in fd.o, so any useful advice will be
appreciated.

Thanks,
Miguel.

> 
> > > I *think* I like the idea of having $new_thing implement GBM as a  
> > deprecated  
> > > legacy API.  Whether that means we start by pulling GBM out into it's own
> > > project or we start over, I don't know.  My feeling is that the current
> > > dri_interface is *not* what we want which is why starting with GBM makes  
> > me  
> > > nervous.  
> >
> > /me expects if we pull GBM out of mesa, the interface

Re: [Mesa-dev] [PATCH 7/7] wayland-egl: rework and simplify wl_egl_window initialization

[Miguel Angel Vico]

> Miguel I believe the comment correctly describes the design plan, while
> addressing Dan's comment that things look a bit ugly.

Yes. Thank you.

Also, the whole series:

Reviewed-by: Miguel A. Vico <mvicom...@nvidia.com>


Thanks.

On Fri, 29 Sep 2017 14:48:58 +0100
Daniel Stone <dan...@fooishbar.org> wrote:

> On 29 September 2017 at 13:14, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> > Use calloc instead of malloc + explicitly zeroing the different fields.
> > We need special handling for the version field which if of type
> > const intptr_t.
> >
> > As we're here document why keeping the constness is a good idea.
> >
> > The wl_egl_window_resize() call is replaced with an explicit set of the
> > width/height.  
> 
> Thanks for doing this; series is:
> Reviewed-by: Daniel Stone <dani...@collabora.com>
> 
> Cheers,
> Daniel
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


-- 
Miguel


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

Re: [Mesa-dev] XDC 2017 feedback

[Miguel Angel Vico]

Hi,

In general, I think the organization was great. I agree having
everything happen in a single room was a good point. Here's some of my
personal feedback:

 * I didn't like the tables layout at all. I found it to be extremely
   uncomfortable to have to look sideways in order to see the screens
   and presenter.

 * There were a very few power strips, and not well distributed along
   the tables.


Also, this is what I've been able to gather from some of my colleagues
here at NVIDIA::

 * Some people watching the conference remotely complained about the
   stream quality, and the recordings wouldn't include the presenter.

   In one of the hallway conversations, Martin mentioned in XDC2016
   they had a team of camera experts doing the job, and will try to
   improve that part in future XDC's.

 * The microphone/audio situation wasn't great either.  I don't know
   how practical it is with something the size of XDC, but at Khronos
   meetings, they usually set up microphones for the audience too, with
   tap-on/tap-off switches, and a ratio of 1:2 or 1:3 for
   microphones:people.  That makes Q a lot easier.  Alternatively,
   having a question queue rather than running mics around the room can
   speed things up, but makes cross-talk harder.

 * The table/chair layout was really cumbersome. Beyond just the
   orientation, some people had a lot of trouble getting in/out to use
   the restroom, grab snacks, etc.


On a positive note:

 * More time for hallway conversations was in general a good thing.
   Some of us got a ton of useful feedback talking to others.

 * The food was great, and having food on-site gave us even more time
   for hallway-tracking.

 * Surprisingly, parking was not an issue.

 * Signage was good, and the security guards were polite/helpful. It
   was easy to find the room and get our badges.

 * The wifi worked great in general, which is a rarity at conferences.
   It was pretty spotty at XDC 2016.


Thank you.

On Tue, 26 Sep 2017 11:49:10 -0700
Manasi Navare wrote:

> Hi,
> 
> XDC was a really great conference and loved the fact that it was
> in just one room which kept all the hallway conversations in that room 
> resulting
> into more networking.
> But I agree with Andres that for the videos, it would be great to split the
> huge youtube video stream per presentation and have seperate links for each
> talk somewhere on the XDC page.
> 
> Regards
> Manasi
> 
> 
> 
> On Tue, Sep 26, 2017 at 01:25:15PM -0400, Andres Rodriguez wrote:
> > Hi,
> > 
> > A small piece of feedback from those of us watching remotely. It would be
> > nice to have a simple to access index for the long livestream videos.
> > 
> > I think the XDC 2017 wiki page would be a good place for this information. A
> > brief example:
> > 
> > Presentation Title | Presenter Name | Link with timestamp
> > ---||-
> > ...| ...| youtu.be/video-id?t=XhYmZs
> > 
> > Or alternatively, a list of hyperlinks with the presentation title as text
> > that point to the correct timestamp in the video would also be sufficient.
> > 
> > Really enjoyed the talks, and would like them to be slightly easier to
> > access and share with others.
> > 
> > Regards,
> > Andres
> > 
> > 
> > On 2017-09-26 12:57 PM, Daniel Vetter wrote:  
> > >Hi all,
> > >
> > >First again big thanks to Stéphane and Jennifer for organizing a great XDC.
> > >
> > >Like last year we'd like to hear feedback on how this year's XDC went,
> > >both the good (and what you'd like to see more of) and the not so
> > >good. Talk selection, organization, location, scheduling of talks,
> > >anything really.
> > >
> > >Here's a few things we took into account from Helsinki and tried to apply:
> > >- More breaks for more hallway track.
> > >- Try to schedule the hot topics on the first day (did we pick the
> > >right ones) for better hallway track.
> > >- More lightning talk time to give all the late/rejected submissions
> > >some place to give a quick showcase.
> > >
> > >Things that didn't work out perfectly this year and that we'll try to
> > >get better at next year:
> > >- Lots of people missed the submission deadline and their talks were
> > >rejected only because of that. We'll do better PR by sending out a
> > >pile of reminders.
> > >- Comparitively few people asked for travel assistance. No idea
> > >whether this was a year with more budget around, or whether this isn't
> > >all that well know and we need to make more PR in maybe the call for
> > >papers about it.
> > >
> > >But that's just the stuff we've gathered already, we'd like to hear
> > >more feedback. Just reply to this mail or send a mail to
> > >bo...@foundation.x.org if you don't want the entire world to read it.
> > >And if you want to send at pseudonymous feedback, drop a pastebin onto
> > >the #xf-bod channel on the OFTC irc server.
> > >
> > >Thanks, Daniel
> > >  
> > 

Re: [Mesa-dev] [PATCH mesa 5/5] wayland-egl: Update ABI checker

[Miguel Angel Vico]

On Wed, 19 Jul 2017 12:19:30 +0100
Emil Velikov <emil.l.veli...@gmail.com> wrote:

> On 18 July 2017 at 21:49, Miguel A. Vico <mvicom...@nvidia.com> wrote:
> > This change updates wayland-egl-abi-check.c with the latest changes to
> > wl_egl_window.
> >
> > Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
> > Reviewed-by: James Jones <jajo...@nvidia.com>
> > ---
> >  .../wayland/wayland-egl/wayland-egl-abi-check.c| 78 
> > ++
> >  1 file changed, 65 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c 
> > b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
> > index 1962f05850..6bdd71b6e0 100644
> > --- a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
> > +++ b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
> > @@ -31,7 +31,28 @@
> >   * DO NOT EVER CHANGE!
> >   */
> >
> > +/* From: a2ab5c2588 - Miguel A. Vico : wayland-egl: Make wl_egl_window a 
> > versioned struct */  
> Please keep the sha as XXX - we'll update it as the commit lands.
> 

Done.

> > +#define WL_EGL_WINDOW_VERSION_v3 3
> > +struct wl_egl_window_v3 {
> > +const intptr_t version;
> > +
> > +int width;
> > +int height;
> > +int dx;
> > +int dy;
> > +
> > +int attached_width;
> > +int attached_height;
> > +
> > +void *private;
> > +void (*resize_callback)(struct wl_egl_window *, void *);
> > +void (*destroy_window_callback)(void *);
> > +
> > +struct wl_surface *surface;
> > +};
> > +
> >  /* From: 690ead4a13 - Stencel, Joanna : egl/wayland-egl: Fix for segfault 
> > in dri2_wl_destroy_surface. */
> > +#define WL_EGL_WINDOW_VERSION_v2 2
> >  struct wl_egl_window_v2 {
> >  struct wl_surface *surface;
> >
> > @@ -123,6 +144,20 @@ struct wl_egl_window_v0 {
> >  }  
> >  \
> >  } while (0)
> >
> > +#define CHECK_VERSION(A_VER, B_VER, MATCH) 
> >  \
> > +do {   
> >  \
> > +if (((MATCH)  && (WL_EGL_WINDOW_VERSION ## A_VER) !=   
> >  \
> > + (WL_EGL_WINDOW_VERSION ## B_VER)) ||  
> >  \
> > +(!(MATCH) && (WL_EGL_WINDOW_VERSION ## A_VER) >=   
> >  \
> > + (WL_EGL_WINDOW_VERSION ## B_VER))) {  
> >  \
> > +printf("Backards incompatible change detected!\n   "   
> >  \
> > +   "WL_EGL_WINDOW_VERSION" #A_VER " %s "   
> >  \
> > +   "WL_EGL_WINDOW_VERSION" #B_VER "\n",
> >  \
> > +   ((MATCH) ? "!=" : ">="));   
> >  \
> > +return 1;  
> >  \
> > +}  
> >  \
> > +} while (0)
> > +  
> Same crazy idea as CHECK_SIZE - worth having separate macros?

Yup. Done.

> 
> >  int main(int argc, char **argv)
> >  {
> >  /* Check wl_egl_window_v1 ABI against wl_egl_window_v0 */
> > @@ -149,19 +184,36 @@ int main(int argc, char **argv)
> >
> >  CHECK_SIZE(_v1, _v2, FALSE);
> >
> > -/* Check wl_egl_window ABI against wl_egl_window_v2 */
> > -CHECK_MEMBER(_v2,, surface, surface);
> > -CHECK_MEMBER(_v2,, width,   width);
> > -CHECK_MEMBER(_v2,, height,  height);
> > -CHECK_MEMBER(_v2,, dx,  dx);
> > -CHECK_MEMBER(_v2,, dy,  dy);
> > -CHECK_MEMBER(_v2,, attached_width,  attached_width);
> > -CHECK_MEMBER(_v2,, attached_height, attached_height);
> > -CHECK_MEMBER(_v2,, private, private);
> > -CHECK_MEMBER(_v2,, resize_callback, resize_callback);
> > -CHECK_MEMBER(_v2,, destroy_window_callback, destroy_window_callback);
> > -
> > -CHECK_SIZE(_v2,, TRUE);
> > +/* Check wl_egl_window_v3 ABI against wl_egl_window_v2 */
> > +CHECK_MEMBER(_v2, _v3, surface, v

Re: [Mesa-dev] [PATCH mesa 4/5] wayland-egl: Make wl_egl_window a versioned struct

[Miguel Angel Vico]

On Wed, 19 Jul 2017 12:06:06 +0100
Emil Velikov <emil.l.veli...@gmail.com> wrote:

> On 18 July 2017 at 21:49, Miguel A. Vico <mvicom...@nvidia.com> wrote:
> > We need wl_egl_window to be a versioned struct in order to keep track of
> > ABI changes.
> >
> > This change makes the first member of wl_egl_window the version number.
> >
> > An heuristic in the wayland driver is added so that we don't break
> > backwards compatibility:
> >
> >  - If the first field (version) is an actual pointer, it is an old
> >implementation of wl_egl_window, and version points to the wl_surface
> >proxy.
> >
> >  - Else, the first field is the version number, and we have
> >wl_egl_window::surface pointing to the wl_surface proxy.
> >
> > Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
> > Reviewed-by: James Jones <jajo...@nvidia.com>  
> 
> This commit will cause a break in the ABI checker. Yet again, I'm
> short on ideas how to avoid that :-(

Yeah... The only think I can think of is pushing both this and 5/5 as a
single commit.

I usually like to keep things separate. Is it much of a deal given that
they'll go in at the same time?

> 
> > ---
> >  src/egl/drivers/dri2/platform_wayland.c| 13 -
> >  src/egl/wayland/wayland-egl/wayland-egl-priv.h |  6 +-
> >  src/egl/wayland/wayland-egl/wayland-egl.c  |  6 +-
> >  3 files changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/platform_wayland.c 
> > b/src/egl/drivers/dri2/platform_wayland.c
> > index ee68284217..0f0a12fd80 100644
> > --- a/src/egl/drivers/dri2/platform_wayland.c
> > +++ b/src/egl/drivers/dri2/platform_wayland.c
> > @@ -41,6 +41,7 @@
> >  #include "egl_dri2.h"
> >  #include "egl_dri2_fallbacks.h"
> >  #include "loader.h"
> > +#include "eglglobals.h"
> >
> >  #include 
> >  #include "wayland-drm-client-protocol.h"
> > @@ -100,6 +101,16 @@ destroy_window_callback(void *data)
> > dri2_surf->wl_win = NULL;
> >  }
> >
> > +static struct wl_surface *
> > +get_wl_surface_proxy(struct wl_egl_window *window)
> > +{
> > +   if (_eglPointerIsDereferencable((void *)(window->version))) {
> > +  /* window->version points to actual wl_surface data */
> > +  return wl_proxy_create_wrapper((void *)(window->version));
> > +   }  
> Please add a comment in there. I'm thinking about the following
> although use whatever you prefer.
> 
> Version 3 of wl_egl_window introduced a version field, at the same
> location where a pointer to wl_surface was stored.

Done. 

> 
> > +   return wl_proxy_create_wrapper(window->surface);
> > +}
> > +
> >  /**
> >   * Called via eglCreateWindowSurface(), drv->API.CreateWindowSurface().
> >   */
> > @@ -171,7 +182,7 @@ dri2_wl_create_window_surface(_EGLDriver *drv, 
> > _EGLDisplay *disp,
> > wl_proxy_set_queue((struct wl_proxy *)dri2_surf->wl_dpy_wrapper,
> >dri2_surf->wl_queue);
> >
> > -   dri2_surf->wl_surface_wrapper = 
> > wl_proxy_create_wrapper(window->surface);
> > +   dri2_surf->wl_surface_wrapper = get_wl_surface_proxy(window);
> > if (!dri2_surf->wl_surface_wrapper) {
> >_eglError(EGL_BAD_ALLOC, "dri2_create_surface");
> >goto cleanup_drm;
> > diff --git a/src/egl/wayland/wayland-egl/wayland-egl-priv.h 
> > b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> > index 92c31d9454..3b59908cc1 100644
> > --- a/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> > +++ b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> > @@ -41,8 +41,10 @@
> >  extern "C" {
> >  #endif
> >
> > +#define WL_EGL_WINDOW_VERSION 3
> > +
> >  struct wl_egl_window {
> > -   struct wl_surface *surface;
> > +   const intptr_t version;
> >
> > int width;
> > int height;
> > @@ -55,6 +57,8 @@ struct wl_egl_window {
> > void *private;
> > void (*resize_callback)(struct wl_egl_window *, void *);
> > void (*destroy_window_callback)(void *);
> > +
> > +   struct wl_surface *surface;
> >  };
> >
> >  #ifdef  __cplusplus
> > diff --git a/src/egl/wayland/wayland-egl/wayland-egl.c 
> > b/src/egl/wayland/wayland-egl/wayland-egl.c
> > index 4a4701a2de..02645549e0 100644
> > --- a/src/egl/wayland/wayland-egl/wayland-egl.c
> > +++ b/src/egl/wayland/wayland-egl/waylan

Re: [Mesa-dev] [PATCH mesa 3/5] egl: Fix _eglPointerIsDereferencable() to ignore page residency

[Miguel Angel Vico]

On Wed, 19 Jul 2017 11:43:43 +0100
Emil Velikov <emil.l.veli...@gmail.com> wrote:

> On 18 July 2017 at 21:49, Miguel A. Vico <mvicom...@nvidia.com> wrote:
> > mincore() returns 0 on success, and -1 on failure.  The last parameter
> > is a vector of bytes with one entry for each page queried.  mincore
> > returns page residency information in the first bit of each byte in the
> > vector.
> >
> > Residency doesn't actually matter when determining whether a pointer is
> > dereferenceable, so the output vector can be ignored.  What matters is
> > whether mincore succeeds. See:
> >
> >   http://man7.org/linux/man-pages/man2/mincore.2.html
> >  
> Makes sense. Can you confirm that the BSD/Solaris manpages are on the same 
> page?

According to the man pages, they all seem to behave the same way in
that regard.

> 
> Considering they all agree
> Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
> 

Thanks. I sent the new rebased v2 version. Could you push on my behalf?

> -Emil

Thanks.

-- 
Miguel


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

Re: [Mesa-dev] [PATCH mesa 2/5] egl: Move _eglPointerIsDereferencable() to eglglobals.[ch]

[Miguel Angel Vico]

On Wed, 19 Jul 2017 11:36:59 +0100
Emil Velikov <emil.l.veli...@gmail.com> wrote:

> On 18 July 2017 at 21:49, Miguel A. Vico <mvicom...@nvidia.com> wrote:
> > More _eglPointerIsDereferencable() to eglglobals.[ch] and make it a
> > non-static function so it can be used out of egldisplay.c
> >  
> s/More/Move/

Done.

> 
> > Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
> > Reviewed-by: James Jones <jajo...@nvidia.com>  
> Reviewed-by: Emil Velikov <emil.veli...@collabora.com>

Thanks. I sent the new rebased v2 version. Could you push on my behalf?

> 
> -Emil

Thanks.

-- 
Miguel


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

Re: [Mesa-dev] [PATCH mesa 1/5] wayland-egl: Add wl_egl_window ABI checker

[Miguel Angel Vico]

Thanks for all the reviews, Emil.

Inline.

On Wed, 19 Jul 2017 11:35:04 +0100
Emil Velikov <emil.l.veli...@gmail.com> wrote:

> Hi Miguel,
> 
> Thanks for looking into this. There's a couple of small nits below but
> it overall looks good.
> 
> On 18 July 2017 at 21:49, Miguel A. Vico <mvicom...@nvidia.com> wrote:
> > Add a small ABI checker for wl_egl_window so that we can check for
> > backwards incompatible changes at 'make check' time.
> >
> > Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
> > Reviewed-by: James Jones <jajo...@nvidia.com>
> > ---
> >  src/egl/wayland/wayland-egl/Makefile.am|  10 +-
> >  .../wayland/wayland-egl/wayland-egl-abi-check.c| 167 
> > +
> >  2 files changed, 176 insertions(+), 1 deletion(-)
> >  create mode 100644 src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
> >
> > diff --git a/src/egl/wayland/wayland-egl/Makefile.am 
> > b/src/egl/wayland/wayland-egl/Makefile.am
> > index 8c45e8e26d..74a52027c6 100644
> > --- a/src/egl/wayland/wayland-egl/Makefile.am
> > +++ b/src/egl/wayland/wayland-egl/Makefile.am
> > @@ -14,7 +14,15 @@ libwayland_egl_la_LDFLAGS = \
> > $(GC_SECTIONS) \
> > $(LD_NO_UNDEFINED)
> >
> > -TESTS = wayland-egl-symbols-check
> > +TESTS =
> > +check_PROGRAMS =
> > +
> > +TESTS += wayland-egl-symbols-check
> >  EXTRA_DIST = wayland-egl-symbols-check
> >
> > +TESTS += wayland-egl-abi-check
> > +check_PROGRAMS += wayland-egl-abi-check
> > +  
> Please add into the respective variables directly.
> 
> TESTS = \
>foo \
>bar
> 
> check_PROGRAMS = wayland-egl-abi-check
> 
> > +wayland_egl_abi_check_SOURCES = wayland-egl-abi-check.c
> > +  
> Feel free to drop this - default extension is ".c" so this will be
> picked automatically.

Done.

> 
> >  include $(top_srcdir)/install-lib-links.mk
> > diff --git a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c 
> > b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
> > new file mode 100644
> > index 00..1962f05850
> > --- /dev/null
> > +++ b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
> > @@ -0,0 +1,167 @@
> > +/*
> > + * Copyright (c) 2017, NVIDIA CORPORATION. All rights reserved.
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the 
> > "Software"),
> > + * to deal in the Software without restriction, including without 
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included 
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > + * DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +#include  // offsetof
> > +#include   // printf
> > +
> > +#include "wayland-egl-priv.h" // Current struct wl_egl_window 
> > implementation
> > +
> > +/*
> > + * Following are previous implementations of wl_egl_window.
> > + *
> > + * DO NOT EVER CHANGE!
> > + */
> > +
> > +/* From: 690ead4a13 - Stencel, Joanna : egl/wayland-egl: Fix for segfault 
> > in dri2_wl_destroy_surface. */
> > +struct wl_egl_window_v2 {  
> 
> > +/* From: ca3ed3e024 - Ander Conselvan de Oliveira : egl/wayland: Don't 
> > invalidate drawable on swap buffers */
> > +struct wl_egl_window_v1 {  
> 
> > +/* From: 214fc6e850 - Benjamin Franzke : egl: Implement libwayland-egl */
> > +struct wl_egl_window_v0 {  
> Hats off for digging all the commits and adding references!
> 
> Can we declare the structs in the same order to how they are used - 0...
> 

Done.

> 
> > +/* This program checks we keep a backwards-compatible struct wl_egl_wind

[Mesa-dev] [PATCH mesa 5/5 v2] wayland-egl: Update ABI checker

[Miguel A. Vico]

This change updates wayland-egl-abi-check.c with the latest changes to
wl_egl_window.

Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
Reviewed-by: James Jones <jajo...@nvidia.com>
---
 .../wayland/wayland-egl/wayland-egl-abi-check.c| 90 ++
 1 file changed, 75 insertions(+), 15 deletions(-)

diff --git a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c 
b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
index 9701ea1453..40b54c678a 100644
--- a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
+++ b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
@@ -61,6 +61,7 @@ struct wl_egl_window_v1 {
 };
 
 /* From: 690ead4a13 - Stencel, Joanna : egl/wayland-egl: Fix for segfault in 
dri2_wl_destroy_surface. */
+#define WL_EGL_WINDOW_VERSION_v2 2
 struct wl_egl_window_v2 {
 struct wl_surface *surface;
 
@@ -77,6 +78,26 @@ struct wl_egl_window_v2 {
 void (*destroy_window_callback)(void *);
 };
 
+/* From: XXX - Miguel A. Vico : wayland-egl: Make wl_egl_window a versioned 
struct */
+#define WL_EGL_WINDOW_VERSION_v3 3
+struct wl_egl_window_v3 {
+const intptr_t version;
+
+int width;
+int height;
+int dx;
+int dy;
+
+int attached_width;
+int attached_height;
+
+void *private;
+void (*resize_callback)(struct wl_egl_window *, void *);
+void (*destroy_window_callback)(void *);
+
+struct wl_surface *surface;
+};
+
 
 /* This program checks we keep a backwards-compatible struct wl_egl_window
  * definition whenever it is modified in wayland-egl-priv.h.
@@ -87,7 +108,7 @@ struct wl_egl_window_v2 {
 
 #define MEMBER_SIZE(type, member) sizeof(((type *)0)->member)
 
-#define CHECK_MEMBERS(a_ver, b_ver, a_member, b_member)
 \
+#define CHECK_RENAMED_MEMBER(a_ver, b_ver, a_member, b_member) 
 \
 do {   
 \
 if (offsetof(struct wl_egl_window ## a_ver, a_member) !=   
 \
 offsetof(struct wl_egl_window ## b_ver, b_member)) {   
 \
@@ -106,7 +127,7 @@ struct wl_egl_window_v2 {
 }  
 \
 } while (0)
 
-#define CHECK_MEMBER(a_ver, b_ver, member) CHECK_MEMBERS(a_ver, b_ver, member, 
member)
+#define CHECK_MEMBER(a_ver, b_ver, member) CHECK_RENAMED_MEMBER(a_ver, b_ver, 
member, member)
 #define CHECK_MEMBER_CURRENT(a_ver, member) CHECK_MEMBER(a_ver,, member)
 
 #define CHECK_SIZE(a_ver, b_ver)   
 \
@@ -131,6 +152,28 @@ struct wl_egl_window_v2 {
 }  
 \
 } while (0)
 
+#define CHECK_VERSION(a_ver, b_ver)
 \
+do {   
 \
+if ((WL_EGL_WINDOW_VERSION ## a_ver) >=
 \
+(WL_EGL_WINDOW_VERSION ## b_ver)) {
 \
+printf("Backards incompatible change detected!\n   "   
 \
+   "WL_EGL_WINDOW_VERSION" #a_ver " >= "   
 \
+   "WL_EGL_WINDOW_VERSION" #b_ver "\n");   
 \
+return 1;  
 \
+}  
 \
+} while (0)
+
+#define CHECK_VERSION_CURRENT(a_ver)   
 \
+do {   
 \
+if ((WL_EGL_WINDOW_VERSION ## a_ver) !=
 \
+(WL_EGL_WINDOW_VERSION)) { 
 \
+printf("Backards incompatible change detected!\n   "   
 \
+   "WL_EGL_WINDOW_VERSION" #a_ver " != "   
 \
+   "WL_EGL_WINDOW_VERSION\n"); 
 \
+return 1;  
 \
+}  
 \
+} while (0)
+
 int main(int argc, char **argv)
 {
 /* Check wl_egl_window_v1 ABI against wl_egl_window_v0 */
@@ -157,19 +200,36 @@ int main(int argc, char **argv)
 
 CHECK_SIZE(_v1, _v2);
 
-/* Check current wl_egl_window ABI against wl_egl_window_v2 */
-CHECK_MEMBER_CURRENT(_v2, surface);
-CHECK_MEMBER_CURRENT(_v2, width);
-CHECK_MEMBER_CURRENT(_v2, height);
-CHECK_MEMBER_CURRENT(_v2, dx);
-CHECK_MEMBER_CURRENT(_v2, dy);
-CHECK_MEMBER_CURRENT(_v2, attached_width);
-CHE

[Mesa-dev] [PATCH mesa 4/5 v2] wayland-egl: Make wl_egl_window a versioned struct

[Miguel A. Vico]

We need wl_egl_window to be a versioned struct in order to keep track of
ABI changes.

This change makes the first member of wl_egl_window the version number.

An heuristic in the wayland driver is added so that we don't break
backwards compatibility:

 - If the first field (version) is an actual pointer, it is an old
   implementation of wl_egl_window, and version points to the wl_surface
   proxy.

 - Else, the first field is the version number, and we have
   wl_egl_window::surface pointing to the wl_surface proxy.

Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
Reviewed-by: James Jones <jajo...@nvidia.com>
---
 src/egl/drivers/dri2/platform_wayland.c| 16 +++-
 src/egl/wayland/wayland-egl/wayland-egl-priv.h |  6 +-
 src/egl/wayland/wayland-egl/wayland-egl.c  |  4 
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index 211036f45f..f4c09ac0bc 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -43,6 +43,7 @@
 #include "egl_dri2_fallbacks.h"
 #include "loader.h"
 #include "util/u_vector.h"
+#include "eglglobals.h"
 
 #include 
 #include "wayland-drm-client-protocol.h"
@@ -111,6 +112,19 @@ destroy_window_callback(void *data)
dri2_surf->wl_win = NULL;
 }
 
+static struct wl_surface *
+get_wl_surface_proxy(struct wl_egl_window *window)
+{
+/* Version 3 of wl_egl_window introduced a version field at the same
+ * location where a pointer to wl_surface was stored. Thus, if
+ * window->version is dereferencable, we've been given an older version of
+ * wl_egl_window, and window->version points to wl_surface */
+   if (_eglPointerIsDereferencable((void *)(window->version))) {
+  return wl_proxy_create_wrapper((void *)(window->version));
+   }
+   return wl_proxy_create_wrapper(window->surface);
+}
+
 /**
  * Called via eglCreateWindowSurface(), drv->API.CreateWindowSurface().
  */
@@ -182,7 +196,7 @@ dri2_wl_create_window_surface(_EGLDriver *drv, _EGLDisplay 
*disp,
wl_proxy_set_queue((struct wl_proxy *)dri2_surf->wl_dpy_wrapper,
   dri2_surf->wl_queue);
 
-   dri2_surf->wl_surface_wrapper = wl_proxy_create_wrapper(window->surface);
+   dri2_surf->wl_surface_wrapper = get_wl_surface_proxy(window);
if (!dri2_surf->wl_surface_wrapper) {
   _eglError(EGL_BAD_ALLOC, "dri2_create_surface");
   goto cleanup_drm;
diff --git a/src/egl/wayland/wayland-egl/wayland-egl-priv.h 
b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
index 92c31d9454..3b59908cc1 100644
--- a/src/egl/wayland/wayland-egl/wayland-egl-priv.h
+++ b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
@@ -41,8 +41,10 @@
 extern "C" {
 #endif
 
+#define WL_EGL_WINDOW_VERSION 3
+
 struct wl_egl_window {
-   struct wl_surface *surface;
+   const intptr_t version;
 
int width;
int height;
@@ -55,6 +57,8 @@ struct wl_egl_window {
void *private;
void (*resize_callback)(struct wl_egl_window *, void *);
void (*destroy_window_callback)(void *);
+
+   struct wl_surface *surface;
 };
 
 #ifdef  __cplusplus
diff --git a/src/egl/wayland/wayland-egl/wayland-egl.c 
b/src/egl/wayland/wayland-egl/wayland-egl.c
index 4a4701a2de..f16324c9f6 100644
--- a/src/egl/wayland/wayland-egl/wayland-egl.c
+++ b/src/egl/wayland/wayland-egl/wayland-egl.c
@@ -28,6 +28,7 @@
  */
 
 #include 
+#include 
 
 #include 
 #include "wayland-egl.h"
@@ -54,6 +55,7 @@ WL_EGL_EXPORT struct wl_egl_window *
 wl_egl_window_create(struct wl_surface *surface,
 int width, int height)
 {
+   struct wl_egl_window _INIT_ = { .version = WL_EGL_WINDOW_VERSION };
struct wl_egl_window *egl_window;
 
if (width <= 0 || height <= 0)
@@ -63,6 +65,8 @@ wl_egl_window_create(struct wl_surface *surface,
if (!egl_window)
return NULL;
 
+   memcpy(egl_window, &_INIT_, sizeof *egl_window);
+
egl_window->surface = surface;
egl_window->private = NULL;
egl_window->resize_callback = NULL;
-- 
2.12.2

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

[Mesa-dev] [PATCH mesa 3/5 v2] egl: Fix _eglPointerIsDereferencable() to ignore page residency

[Miguel A. Vico]

mincore() returns 0 on success, and -1 on failure.  The last parameter
is a vector of bytes with one entry for each page queried.  mincore
returns page residency information in the first bit of each byte in the
vector.

Residency doesn't actually matter when determining whether a pointer is
dereferenceable, so the output vector can be ignored.  What matters is
whether mincore succeeds. See:

  http://man7.org/linux/man-pages/man2/mincore.2.html

Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
---
 src/egl/main/eglglobals.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/egl/main/eglglobals.c b/src/egl/main/eglglobals.c
index 6fdc6c31ce..9071226618 100644
--- a/src/egl/main/eglglobals.c
+++ b/src/egl/main/eglglobals.c
@@ -168,7 +168,18 @@ _eglPointerIsDereferencable(void *p)
   return EGL_FALSE;
}
 
-   return (valid & 0x01) == 0x01;
+   /* mincore() returns 0 on success, and -1 on failure.  The last parameter
+* is a vector of bytes with one entry for each page queried.  mincore
+* returns page residency information in the first bit of each byte in the
+* vector.
+*
+* Residency doesn't actually matter when determining whether a pointer is
+* dereferenceable, so the output vector can be ignored.  What matters is
+* whether mincore succeeds. See:
+*
+*   http://man7.org/linux/man-pages/man2/mincore.2.html
+*/
+   return EGL_TRUE;
 #else
return p != NULL;
 #endif
-- 
2.12.2

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

[Mesa-dev] [PATCH mesa 2/5 v2] egl: Move _eglPointerIsDereferencable() to eglglobals.[ch]

[Miguel A. Vico]

Move _eglPointerIsDereferencable() to eglglobals.[ch] and make it a
non-static function so it can be used out of egldisplay.c

Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
Reviewed-by: James Jones <jajo...@nvidia.com>
Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
---
 src/egl/main/egldisplay.c | 33 -
 src/egl/main/eglglobals.c | 31 +++
 src/egl/main/eglglobals.h |  6 ++
 3 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index 7aaab3c2c9..690728d2f7 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -49,10 +49,6 @@
 #include "eglsync.h"
 
 /* Includes for _eglNativePlatformDetectNativeDisplay */
-#ifdef HAVE_MINCORE
-#include 
-#include 
-#endif
 #ifdef HAVE_WAYLAND_PLATFORM
 #include 
 #endif
@@ -106,35 +102,6 @@ _eglGetNativePlatformFromEnv(void)
 
 
 /**
- * Perform validity checks on a generic pointer.
- */
-static EGLBoolean
-_eglPointerIsDereferencable(void *p)
-{
-#ifdef HAVE_MINCORE
-   uintptr_t addr = (uintptr_t) p;
-   unsigned char valid = 0;
-   const long page_size = getpagesize();
-
-   if (p == NULL)
-  return EGL_FALSE;
-
-   /* align addr to page_size */
-   addr &= ~(page_size - 1);
-
-   if (mincore((void *) addr, page_size, ) < 0) {
-  _eglLog(_EGL_DEBUG, "mincore failed: %m");
-  return EGL_FALSE;
-   }
-
-   return (valid & 0x01) == 0x01;
-#else
-   return p != NULL;
-#endif
-}
-
-
-/**
  * Try detecting native platform with the help of native display 
characteristcs.
  */
 static _EGLPlatformType
diff --git a/src/egl/main/eglglobals.c b/src/egl/main/eglglobals.c
index baf96bb1ec..6fdc6c31ce 100644
--- a/src/egl/main/eglglobals.c
+++ b/src/egl/main/eglglobals.c
@@ -37,6 +37,12 @@
 #include "eglglobals.h"
 #include "egldisplay.h"
 #include "egldriver.h"
+#include "egllog.h"
+
+#ifdef HAVE_MINCORE
+#include 
+#include 
+#endif
 
 
 static mtx_t _eglGlobalMutex = _MTX_INITIALIZER_NP;
@@ -142,3 +148,28 @@ _eglGetClientExtensionString(void)
mtx_unlock(_eglGlobal.Mutex);
return ret;
 }
+
+EGLBoolean
+_eglPointerIsDereferencable(void *p)
+{
+#ifdef HAVE_MINCORE
+   uintptr_t addr = (uintptr_t) p;
+   unsigned char valid = 0;
+   const long page_size = getpagesize();
+
+   if (p == NULL)
+  return EGL_FALSE;
+
+   /* align addr to page_size */
+   addr &= ~(page_size - 1);
+
+   if (mincore((void *) addr, page_size, ) < 0) {
+  _eglLog(_EGL_DEBUG, "mincore failed: %m");
+  return EGL_FALSE;
+   }
+
+   return (valid & 0x01) == 0x01;
+#else
+   return p != NULL;
+#endif
+}
diff --git a/src/egl/main/eglglobals.h b/src/egl/main/eglglobals.h
index c6ef59d482..6655ccab65 100644
--- a/src/egl/main/eglglobals.h
+++ b/src/egl/main/eglglobals.h
@@ -87,4 +87,10 @@ static inline unsigned int DebugBitFromType(EGLenum type)
 extern const char *
 _eglGetClientExtensionString(void);
 
+/**
+ * Perform validity checks on a generic pointer.
+ */
+extern EGLBoolean
+_eglPointerIsDereferencable(void *p);
+
 #endif /* EGLGLOBALS_INCLUDED */
-- 
2.12.2

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

[Mesa-dev] [PATCH mesa 1/5 v2] wayland-egl: Add wl_egl_window ABI checker

[Miguel A. Vico]

Add a small ABI checker for wl_egl_window so that we can check for
backwards incompatible changes at 'make check' time.

Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
Reviewed-by: James Jones <jajo...@nvidia.com>
---
 src/egl/wayland/wayland-egl/Makefile.am|   6 +-
 .../wayland/wayland-egl/wayland-egl-abi-check.c| 175 +
 2 files changed, 180 insertions(+), 1 deletion(-)
 create mode 100644 src/egl/wayland/wayland-egl/wayland-egl-abi-check.c

diff --git a/src/egl/wayland/wayland-egl/Makefile.am 
b/src/egl/wayland/wayland-egl/Makefile.am
index 8c45e8e26d..846fa6247b 100644
--- a/src/egl/wayland/wayland-egl/Makefile.am
+++ b/src/egl/wayland/wayland-egl/Makefile.am
@@ -14,7 +14,11 @@ libwayland_egl_la_LDFLAGS = \
$(GC_SECTIONS) \
$(LD_NO_UNDEFINED)
 
-TESTS = wayland-egl-symbols-check
+TESTS = wayland-egl-symbols-check \
+wayland-egl-abi-check
+
 EXTRA_DIST = wayland-egl-symbols-check
 
+check_PROGRAMS = wayland-egl-abi-check
+
 include $(top_srcdir)/install-lib-links.mk
diff --git a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c 
b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
new file mode 100644
index 00..9701ea1453
--- /dev/null
+++ b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
@@ -0,0 +1,175 @@
+/*
+ * Copyright (c) 2017, NVIDIA CORPORATION. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include  // offsetof
+#include   // printf
+
+#include "wayland-egl-priv.h" // Current struct wl_egl_window implementation
+
+/*
+ * Following are previous implementations of wl_egl_window.
+ *
+ * DO NOT EVER CHANGE!
+ */
+
+/* From: 214fc6e850 - Benjamin Franzke : egl: Implement libwayland-egl */
+struct wl_egl_window_v0 {
+struct wl_surface *surface;
+
+int width;
+int height;
+int dx;
+int dy;
+
+int attached_width;
+int attached_height;
+};
+
+/* From: ca3ed3e024 - Ander Conselvan de Oliveira : egl/wayland: Don't 
invalidate drawable on swap buffers */
+struct wl_egl_window_v1 {
+struct wl_surface *surface;
+
+int width;
+int height;
+int dx;
+int dy;
+
+int attached_width;
+int attached_height;
+
+void *private;
+void (*resize_callback)(struct wl_egl_window *, void *);
+};
+
+/* From: 690ead4a13 - Stencel, Joanna : egl/wayland-egl: Fix for segfault in 
dri2_wl_destroy_surface. */
+struct wl_egl_window_v2 {
+struct wl_surface *surface;
+
+int width;
+int height;
+int dx;
+int dy;
+
+int attached_width;
+int attached_height;
+
+void *private;
+void (*resize_callback)(struct wl_egl_window *, void *);
+void (*destroy_window_callback)(void *);
+};
+
+
+/* This program checks we keep a backwards-compatible struct wl_egl_window
+ * definition whenever it is modified in wayland-egl-priv.h.
+ *
+ * The previous definition should be added above as a new struct
+ * wl_egl_window_vN, and the appropriate checks should be added below
+ */
+
+#define MEMBER_SIZE(type, member) sizeof(((type *)0)->member)
+
+#define CHECK_MEMBERS(a_ver, b_ver, a_member, b_member)
 \
+do {   
 \
+if (offsetof(struct wl_egl_window ## a_ver, a_member) !=   
 \
+offsetof(struct wl_egl_window ## b_ver, b_member)) {   
 \
+printf("Backards incompatible change detected!\n   "   
 \
+   "offsetof(struct wl_egl_window" #a_ver "::" #a_member ") != 
"\
+   &quo

[Mesa-dev] [PATCH mesa 4/5] wayland-egl: Make wl_egl_window a versioned struct

[Miguel A. Vico]

We need wl_egl_window to be a versioned struct in order to keep track of
ABI changes.

This change makes the first member of wl_egl_window the version number.

An heuristic in the wayland driver is added so that we don't break
backwards compatibility:

 - If the first field (version) is an actual pointer, it is an old
   implementation of wl_egl_window, and version points to the wl_surface
   proxy.

 - Else, the first field is the version number, and we have
   wl_egl_window::surface pointing to the wl_surface proxy.

Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
Reviewed-by: James Jones <jajo...@nvidia.com>
---
 src/egl/drivers/dri2/platform_wayland.c| 13 -
 src/egl/wayland/wayland-egl/wayland-egl-priv.h |  6 +-
 src/egl/wayland/wayland-egl/wayland-egl.c  |  6 +-
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index ee68284217..0f0a12fd80 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -41,6 +41,7 @@
 #include "egl_dri2.h"
 #include "egl_dri2_fallbacks.h"
 #include "loader.h"
+#include "eglglobals.h"
 
 #include 
 #include "wayland-drm-client-protocol.h"
@@ -100,6 +101,16 @@ destroy_window_callback(void *data)
dri2_surf->wl_win = NULL;
 }
 
+static struct wl_surface *
+get_wl_surface_proxy(struct wl_egl_window *window)
+{
+   if (_eglPointerIsDereferencable((void *)(window->version))) {
+  /* window->version points to actual wl_surface data */
+  return wl_proxy_create_wrapper((void *)(window->version));
+   }
+   return wl_proxy_create_wrapper(window->surface);
+}
+
 /**
  * Called via eglCreateWindowSurface(), drv->API.CreateWindowSurface().
  */
@@ -171,7 +182,7 @@ dri2_wl_create_window_surface(_EGLDriver *drv, _EGLDisplay 
*disp,
wl_proxy_set_queue((struct wl_proxy *)dri2_surf->wl_dpy_wrapper,
   dri2_surf->wl_queue);
 
-   dri2_surf->wl_surface_wrapper = wl_proxy_create_wrapper(window->surface);
+   dri2_surf->wl_surface_wrapper = get_wl_surface_proxy(window);
if (!dri2_surf->wl_surface_wrapper) {
   _eglError(EGL_BAD_ALLOC, "dri2_create_surface");
   goto cleanup_drm;
diff --git a/src/egl/wayland/wayland-egl/wayland-egl-priv.h 
b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
index 92c31d9454..3b59908cc1 100644
--- a/src/egl/wayland/wayland-egl/wayland-egl-priv.h
+++ b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
@@ -41,8 +41,10 @@
 extern "C" {
 #endif
 
+#define WL_EGL_WINDOW_VERSION 3
+
 struct wl_egl_window {
-   struct wl_surface *surface;
+   const intptr_t version;
 
int width;
int height;
@@ -55,6 +57,8 @@ struct wl_egl_window {
void *private;
void (*resize_callback)(struct wl_egl_window *, void *);
void (*destroy_window_callback)(void *);
+
+   struct wl_surface *surface;
 };
 
 #ifdef  __cplusplus
diff --git a/src/egl/wayland/wayland-egl/wayland-egl.c 
b/src/egl/wayland/wayland-egl/wayland-egl.c
index 4a4701a2de..02645549e0 100644
--- a/src/egl/wayland/wayland-egl/wayland-egl.c
+++ b/src/egl/wayland/wayland-egl/wayland-egl.c
@@ -28,6 +28,7 @@
  */
 
 #include 
+#include 
 
 #include 
 #include "wayland-egl.h"
@@ -54,6 +55,7 @@ WL_EGL_EXPORT struct wl_egl_window *
 wl_egl_window_create(struct wl_surface *surface,
 int width, int height)
 {
+   struct wl_egl_window _INIT_ = { .version = WL_EGL_WINDOW_VERSION };
struct wl_egl_window *egl_window;
 
if (width <= 0 || height <= 0)
@@ -63,6 +65,8 @@ wl_egl_window_create(struct wl_surface *surface,
if (!egl_window)
return NULL;
 
+   memcpy(egl_window, &_INIT_, sizeof *egl_window);
+
egl_window->surface = surface;
egl_window->private = NULL;
egl_window->resize_callback = NULL;
@@ -70,7 +74,7 @@ wl_egl_window_create(struct wl_surface *surface,
wl_egl_window_resize(egl_window, width, height, 0, 0);
egl_window->attached_width  = 0;
egl_window->attached_height = 0;
-   
+
return egl_window;
 }
 
-- 
2.12.2

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

[Mesa-dev] [PATCH mesa 5/5] wayland-egl: Update ABI checker

[Miguel A. Vico]

This change updates wayland-egl-abi-check.c with the latest changes to
wl_egl_window.

Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
Reviewed-by: James Jones <jajo...@nvidia.com>
---
 .../wayland/wayland-egl/wayland-egl-abi-check.c| 78 ++
 1 file changed, 65 insertions(+), 13 deletions(-)

diff --git a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c 
b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
index 1962f05850..6bdd71b6e0 100644
--- a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
+++ b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
@@ -31,7 +31,28 @@
  * DO NOT EVER CHANGE!
  */
 
+/* From: a2ab5c2588 - Miguel A. Vico : wayland-egl: Make wl_egl_window a 
versioned struct */
+#define WL_EGL_WINDOW_VERSION_v3 3
+struct wl_egl_window_v3 {
+const intptr_t version;
+
+int width;
+int height;
+int dx;
+int dy;
+
+int attached_width;
+int attached_height;
+
+void *private;
+void (*resize_callback)(struct wl_egl_window *, void *);
+void (*destroy_window_callback)(void *);
+
+struct wl_surface *surface;
+};
+
 /* From: 690ead4a13 - Stencel, Joanna : egl/wayland-egl: Fix for segfault in 
dri2_wl_destroy_surface. */
+#define WL_EGL_WINDOW_VERSION_v2 2
 struct wl_egl_window_v2 {
 struct wl_surface *surface;
 
@@ -123,6 +144,20 @@ struct wl_egl_window_v0 {
 }  
 \
 } while (0)
 
+#define CHECK_VERSION(A_VER, B_VER, MATCH) 
 \
+do {   
 \
+if (((MATCH)  && (WL_EGL_WINDOW_VERSION ## A_VER) !=   
 \
+ (WL_EGL_WINDOW_VERSION ## B_VER)) ||  
 \
+(!(MATCH) && (WL_EGL_WINDOW_VERSION ## A_VER) >=   
 \
+ (WL_EGL_WINDOW_VERSION ## B_VER))) {  
 \
+printf("Backards incompatible change detected!\n   "   
 \
+   "WL_EGL_WINDOW_VERSION" #A_VER " %s "   
 \
+   "WL_EGL_WINDOW_VERSION" #B_VER "\n",
 \
+   ((MATCH) ? "!=" : ">="));   
 \
+return 1;  
 \
+}  
 \
+} while (0)
+
 int main(int argc, char **argv)
 {
 /* Check wl_egl_window_v1 ABI against wl_egl_window_v0 */
@@ -149,19 +184,36 @@ int main(int argc, char **argv)
 
 CHECK_SIZE(_v1, _v2, FALSE);
 
-/* Check wl_egl_window ABI against wl_egl_window_v2 */
-CHECK_MEMBER(_v2,, surface, surface);
-CHECK_MEMBER(_v2,, width,   width);
-CHECK_MEMBER(_v2,, height,  height);
-CHECK_MEMBER(_v2,, dx,  dx);
-CHECK_MEMBER(_v2,, dy,  dy);
-CHECK_MEMBER(_v2,, attached_width,  attached_width);
-CHECK_MEMBER(_v2,, attached_height, attached_height);
-CHECK_MEMBER(_v2,, private, private);
-CHECK_MEMBER(_v2,, resize_callback, resize_callback);
-CHECK_MEMBER(_v2,, destroy_window_callback, destroy_window_callback);
-
-CHECK_SIZE(_v2,, TRUE);
+/* Check wl_egl_window_v3 ABI against wl_egl_window_v2 */
+CHECK_MEMBER(_v2, _v3, surface, version);
+CHECK_MEMBER(_v2, _v3, width,   width);
+CHECK_MEMBER(_v2, _v3, height,  height);
+CHECK_MEMBER(_v2, _v3, dx,  dx);
+CHECK_MEMBER(_v2, _v3, dy,  dy);
+CHECK_MEMBER(_v2, _v3, attached_width,  attached_width);
+CHECK_MEMBER(_v2, _v3, attached_height, attached_height);
+CHECK_MEMBER(_v2, _v3, private, private);
+CHECK_MEMBER(_v2, _v3, resize_callback, resize_callback);
+CHECK_MEMBER(_v2, _v3, destroy_window_callback, destroy_window_callback);
+
+CHECK_SIZE   (_v2, _v3, FALSE);
+CHECK_VERSION(_v2, _v3, FALSE);
+
+/* Check wl_egl_window ABI against wl_egl_window_v3 */
+CHECK_MEMBER(_v3,, version, version);
+CHECK_MEMBER(_v3,, width,   width);
+CHECK_MEMBER(_v3,, height,  height);
+CHECK_MEMBER(_v3,, dx,  dx);
+CHECK_MEMBER(_v3,, dy,  dy);
+CHECK_MEMBER(_v3,, attached_width,  attached_width);
+CHECK_MEMBER(_v3,, attached_height, attached_height);
+CHECK_MEMBER(_v3,, private, private);
+CHECK_MEMBER(_v3,, resize_callback, resize_callback);
+CHECK_MEMBER(_v3,, destroy_window_callback

[Mesa-dev] [PATCH mesa 3/5] egl: Fix _eglPointerIsDereferencable() to ignore page residency

[Miguel A. Vico]

mincore() returns 0 on success, and -1 on failure.  The last parameter
is a vector of bytes with one entry for each page queried.  mincore
returns page residency information in the first bit of each byte in the
vector.

Residency doesn't actually matter when determining whether a pointer is
dereferenceable, so the output vector can be ignored.  What matters is
whether mincore succeeds. See:

  http://man7.org/linux/man-pages/man2/mincore.2.html

Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
---
 src/egl/main/eglglobals.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/egl/main/eglglobals.c b/src/egl/main/eglglobals.c
index 6fdc6c31ce..9071226618 100644
--- a/src/egl/main/eglglobals.c
+++ b/src/egl/main/eglglobals.c
@@ -168,7 +168,18 @@ _eglPointerIsDereferencable(void *p)
   return EGL_FALSE;
}
 
-   return (valid & 0x01) == 0x01;
+   /* mincore() returns 0 on success, and -1 on failure.  The last parameter
+* is a vector of bytes with one entry for each page queried.  mincore
+* returns page residency information in the first bit of each byte in the
+* vector.
+*
+* Residency doesn't actually matter when determining whether a pointer is
+* dereferenceable, so the output vector can be ignored.  What matters is
+* whether mincore succeeds. See:
+*
+*   http://man7.org/linux/man-pages/man2/mincore.2.html
+*/
+   return EGL_TRUE;
 #else
return p != NULL;
 #endif
-- 
2.12.2

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

[Mesa-dev] [PATCH mesa 1/5] wayland-egl: Add wl_egl_window ABI checker

[Miguel A. Vico]

Add a small ABI checker for wl_egl_window so that we can check for
backwards incompatible changes at 'make check' time.

Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
Reviewed-by: James Jones <jajo...@nvidia.com>
---
 src/egl/wayland/wayland-egl/Makefile.am|  10 +-
 .../wayland/wayland-egl/wayland-egl-abi-check.c| 167 +
 2 files changed, 176 insertions(+), 1 deletion(-)
 create mode 100644 src/egl/wayland/wayland-egl/wayland-egl-abi-check.c

diff --git a/src/egl/wayland/wayland-egl/Makefile.am 
b/src/egl/wayland/wayland-egl/Makefile.am
index 8c45e8e26d..74a52027c6 100644
--- a/src/egl/wayland/wayland-egl/Makefile.am
+++ b/src/egl/wayland/wayland-egl/Makefile.am
@@ -14,7 +14,15 @@ libwayland_egl_la_LDFLAGS = \
$(GC_SECTIONS) \
$(LD_NO_UNDEFINED)
 
-TESTS = wayland-egl-symbols-check
+TESTS =
+check_PROGRAMS =
+
+TESTS += wayland-egl-symbols-check
 EXTRA_DIST = wayland-egl-symbols-check
 
+TESTS += wayland-egl-abi-check
+check_PROGRAMS += wayland-egl-abi-check
+
+wayland_egl_abi_check_SOURCES = wayland-egl-abi-check.c
+
 include $(top_srcdir)/install-lib-links.mk
diff --git a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c 
b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
new file mode 100644
index 00..1962f05850
--- /dev/null
+++ b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
@@ -0,0 +1,167 @@
+/*
+ * Copyright (c) 2017, NVIDIA CORPORATION. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include  // offsetof
+#include   // printf
+
+#include "wayland-egl-priv.h" // Current struct wl_egl_window implementation
+
+/*
+ * Following are previous implementations of wl_egl_window.
+ *
+ * DO NOT EVER CHANGE!
+ */
+
+/* From: 690ead4a13 - Stencel, Joanna : egl/wayland-egl: Fix for segfault in 
dri2_wl_destroy_surface. */
+struct wl_egl_window_v2 {
+struct wl_surface *surface;
+
+int width;
+int height;
+int dx;
+int dy;
+
+int attached_width;
+int attached_height;
+
+void *private;
+void (*resize_callback)(struct wl_egl_window *, void *);
+void (*destroy_window_callback)(void *);
+};
+
+/* From: ca3ed3e024 - Ander Conselvan de Oliveira : egl/wayland: Don't 
invalidate drawable on swap buffers */
+struct wl_egl_window_v1 {
+struct wl_surface *surface;
+
+int width;
+int height;
+int dx;
+int dy;
+
+int attached_width;
+int attached_height;
+
+void *private;
+void (*resize_callback)(struct wl_egl_window *, void *);
+};
+
+/* From: 214fc6e850 - Benjamin Franzke : egl: Implement libwayland-egl */
+struct wl_egl_window_v0 {
+struct wl_surface *surface;
+
+int width;
+int height;
+int dx;
+int dy;
+
+int attached_width;
+int attached_height;
+};
+
+
+/* This program checks we keep a backwards-compatible struct wl_egl_window
+ * definition whenever it is modified in wayland-egl-priv.h.
+ *
+ * The previous definition should be added above as a new struct
+ * wl_egl_window_vN, and the appropriate checks should be added below
+ */
+
+#define TRUE  1
+#define FALSE 0
+
+#define MEMBER_SIZE(TYPE, MEMBER) sizeof(((TYPE *)0)->MEMBER)
+
+#define CHECK_MEMBER(A_VER, B_VER, A_MEMBER, B_MEMBER) 
 \
+do {   
 \
+if (offsetof(struct wl_egl_window ## A_VER, A_MEMBER) !=   
 \
+offsetof(struct wl_egl_window ## B_VER, B_MEMBER)) {   
 \
+printf("Backards incompatible change detected!\n   "   
 \
+   "offsetof(struct wl_egl_window" #A_VER "::" #A_MEMBER ") != 
"\
+   &quo

[Mesa-dev] [PATCH mesa 2/5] egl: Move _eglPointerIsDereferencable() to eglglobals.[ch]

[Miguel A. Vico]

More _eglPointerIsDereferencable() to eglglobals.[ch] and make it a
non-static function so it can be used out of egldisplay.c

Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
Reviewed-by: James Jones <jajo...@nvidia.com>
---
 src/egl/main/egldisplay.c | 33 -
 src/egl/main/eglglobals.c | 31 +++
 src/egl/main/eglglobals.h |  6 ++
 3 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index 7aaab3c2c9..690728d2f7 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -49,10 +49,6 @@
 #include "eglsync.h"
 
 /* Includes for _eglNativePlatformDetectNativeDisplay */
-#ifdef HAVE_MINCORE
-#include 
-#include 
-#endif
 #ifdef HAVE_WAYLAND_PLATFORM
 #include 
 #endif
@@ -106,35 +102,6 @@ _eglGetNativePlatformFromEnv(void)
 
 
 /**
- * Perform validity checks on a generic pointer.
- */
-static EGLBoolean
-_eglPointerIsDereferencable(void *p)
-{
-#ifdef HAVE_MINCORE
-   uintptr_t addr = (uintptr_t) p;
-   unsigned char valid = 0;
-   const long page_size = getpagesize();
-
-   if (p == NULL)
-  return EGL_FALSE;
-
-   /* align addr to page_size */
-   addr &= ~(page_size - 1);
-
-   if (mincore((void *) addr, page_size, ) < 0) {
-  _eglLog(_EGL_DEBUG, "mincore failed: %m");
-  return EGL_FALSE;
-   }
-
-   return (valid & 0x01) == 0x01;
-#else
-   return p != NULL;
-#endif
-}
-
-
-/**
  * Try detecting native platform with the help of native display 
characteristcs.
  */
 static _EGLPlatformType
diff --git a/src/egl/main/eglglobals.c b/src/egl/main/eglglobals.c
index baf96bb1ec..6fdc6c31ce 100644
--- a/src/egl/main/eglglobals.c
+++ b/src/egl/main/eglglobals.c
@@ -37,6 +37,12 @@
 #include "eglglobals.h"
 #include "egldisplay.h"
 #include "egldriver.h"
+#include "egllog.h"
+
+#ifdef HAVE_MINCORE
+#include 
+#include 
+#endif
 
 
 static mtx_t _eglGlobalMutex = _MTX_INITIALIZER_NP;
@@ -142,3 +148,28 @@ _eglGetClientExtensionString(void)
mtx_unlock(_eglGlobal.Mutex);
return ret;
 }
+
+EGLBoolean
+_eglPointerIsDereferencable(void *p)
+{
+#ifdef HAVE_MINCORE
+   uintptr_t addr = (uintptr_t) p;
+   unsigned char valid = 0;
+   const long page_size = getpagesize();
+
+   if (p == NULL)
+  return EGL_FALSE;
+
+   /* align addr to page_size */
+   addr &= ~(page_size - 1);
+
+   if (mincore((void *) addr, page_size, ) < 0) {
+  _eglLog(_EGL_DEBUG, "mincore failed: %m");
+  return EGL_FALSE;
+   }
+
+   return (valid & 0x01) == 0x01;
+#else
+   return p != NULL;
+#endif
+}
diff --git a/src/egl/main/eglglobals.h b/src/egl/main/eglglobals.h
index c6ef59d482..6655ccab65 100644
--- a/src/egl/main/eglglobals.h
+++ b/src/egl/main/eglglobals.h
@@ -87,4 +87,10 @@ static inline unsigned int DebugBitFromType(EGLenum type)
 extern const char *
 _eglGetClientExtensionString(void);
 
+/**
+ * Perform validity checks on a generic pointer.
+ */
+extern EGLBoolean
+_eglPointerIsDereferencable(void *p);
+
 #endif /* EGLGLOBALS_INCLUDED */
-- 
2.12.2

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

[Mesa-dev] [PATCH mesa 0/5] egl/wayland: Make wl_egl_window a versioned struct

[Miguel A. Vico]

Although we've always made backwards compatible changes to wl_egl_window, we
are lacking of a versioning mechanism for EGL drivers that take a native
wl_egl_window to check what set of features are exposed/available by the given
struct.

This series of patches aim to make wl_egl_window a versioned struct in a
backwards compatible way.

Along the way, a wl_egl_window ABI checker program has been added that runs
at 'make check' time.

Also, _eglPointerIsDereferencable() is fixed to return whether is
dereferencable, regardless of memory residency.

Thanks,
Miguel.

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