[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 <imirkin at alum.mit.edu> wrote:

> On Fri, Jun 22, 2018 at 8:22 PM, Miguel Angel Vico <mvicomoya at nvidia.com> wrote:
> >
> >
> > On Thu, 21 Jun 2018 22:09:14 -0400
> > Ilia Mirkin <imirkin at alum.mit.edu> 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 <mvicomoya at nvidia.com> wrote:  
> >> > Add memory object support for nvc0 and nv50
> >> >
> >> > Signed-off-by: Miguel A Vico Moya <mvicomoya at nvidia.com>
> >> > ---
> >> >  .../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