[PATCH] drm/qxl: drop prime import/export callbacks

[PATCH] drm/qxl: drop prime import/export callbacks

[Gerd Hoffmann]

Also set prime_handle_to_fd and prime_fd_to_handle to NULL,
so drm will not advertive DRM_PRIME_CAP_{IMPORT,EXPORT} to
userspace.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_drv.c   |  4 ----
 drivers/gpu/drm/qxl/qxl_prime.c | 14 --------------
 2 files changed, 18 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 13c8a662f9..ccb090f3ab 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -250,14 +250,10 @@ static struct drm_driver qxl_driver = {
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = qxl_debugfs_init,
 #endif
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_export = drm_gem_prime_export,
 	.gem_prime_import = drm_gem_prime_import,
 	.gem_prime_pin = qxl_gem_prime_pin,
 	.gem_prime_unpin = qxl_gem_prime_unpin,
-	.gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
-	.gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
 	.gem_prime_vmap = qxl_gem_prime_vmap,
 	.gem_prime_vunmap = qxl_gem_prime_vunmap,
 	.gem_prime_mmap = qxl_gem_prime_mmap,
diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
index a55dece118..df65d3c1a7 100644
--- a/drivers/gpu/drm/qxl/qxl_prime.c
+++ b/drivers/gpu/drm/qxl/qxl_prime.c
@@ -38,20 +38,6 @@ void qxl_gem_prime_unpin(struct drm_gem_object *obj)
 	WARN_ONCE(1, "not implemented");
 }
 
-struct sg_table *qxl_gem_prime_get_sg_table(struct drm_gem_object *obj)
-{
-	WARN_ONCE(1, "not implemented");
-	return ERR_PTR(-ENOSYS);
-}
-
-struct drm_gem_object *qxl_gem_prime_import_sg_table(
-	struct drm_device *dev, struct dma_buf_attachment *attach,
-	struct sg_table *table)
-{
-	WARN_ONCE(1, "not implemented");
-	return ERR_PTR(-ENOSYS);
-}
-
 void *qxl_gem_prime_vmap(struct drm_gem_object *obj)
 {
 	WARN_ONCE(1, "not implemented");
-- 
2.9.3

Re: [PATCH] drm/qxl: drop prime import/export callbacks

[Dave Airlie]

On Thu, 10 Jan 2019 at 18:17, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Also set prime_handle_to_fd and prime_fd_to_handle to NULL,
> so drm will not advertive DRM_PRIME_CAP_{IMPORT,EXPORT} to
> userspace.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/qxl/qxl_drv.c   |  4 ----
>  drivers/gpu/drm/qxl/qxl_prime.c | 14 --------------
>  2 files changed, 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 13c8a662f9..ccb090f3ab 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -250,14 +250,10 @@ static struct drm_driver qxl_driver = {
>  #if defined(CONFIG_DEBUG_FS)
>         .debugfs_init = qxl_debugfs_init,
>  #endif
> -       .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> -       .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>         .gem_prime_export = drm_gem_prime_export,
>         .gem_prime_import = drm_gem_prime_import,
>         .gem_prime_pin = qxl_gem_prime_pin,
>         .gem_prime_unpin = qxl_gem_prime_unpin,
> -       .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
> -       .gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
>         .gem_prime_vmap = qxl_gem_prime_vmap,
>         .gem_prime_vunmap = qxl_gem_prime_vunmap,
>         .gem_prime_mmap = qxl_gem_prime_mmap,
> diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
> index a55dece118..df65d3c1a7 100644
> --- a/drivers/gpu/drm/qxl/qxl_prime.c
> +++ b/drivers/gpu/drm/qxl/qxl_prime.c
> @@ -38,20 +38,6 @@ void qxl_gem_prime_unpin(struct drm_gem_object *obj)
>         WARN_ONCE(1, "not implemented");
>  }
>
> -struct sg_table *qxl_gem_prime_get_sg_table(struct drm_gem_object *obj)
> -{
> -       WARN_ONCE(1, "not implemented");
> -       return ERR_PTR(-ENOSYS);
> -}
> -
> -struct drm_gem_object *qxl_gem_prime_import_sg_table(
> -       struct drm_device *dev, struct dma_buf_attachment *attach,
> -       struct sg_table *table)
> -{
> -       WARN_ONCE(1, "not implemented");
> -       return ERR_PTR(-ENOSYS);
> -}
> -
>  void *qxl_gem_prime_vmap(struct drm_gem_object *obj)
>  {
>         WARN_ONCE(1, "not implemented");
> --
> 2.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/qxl: drop prime import/export callbacks

[Dave Airlie]

On Sat, 12 Jan 2019 at 07:13, Dave Airlie <airlied@gmail.com> wrote:
>
> On Thu, 10 Jan 2019 at 18:17, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > Also set prime_handle_to_fd and prime_fd_to_handle to NULL,
> > so drm will not advertive DRM_PRIME_CAP_{IMPORT,EXPORT} to
> > userspace.

It's been pointed out to me that disables DRI3 for these devices, I'm
not sure that is the solution we actually wanted.

any ideas?
Dave.

> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>
> Reviewed-by: Dave Airlie <airlied@redhat.com>
> > ---
> >  drivers/gpu/drm/qxl/qxl_drv.c   |  4 ----
> >  drivers/gpu/drm/qxl/qxl_prime.c | 14 --------------
> >  2 files changed, 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > index 13c8a662f9..ccb090f3ab 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > @@ -250,14 +250,10 @@ static struct drm_driver qxl_driver = {
> >  #if defined(CONFIG_DEBUG_FS)
> >         .debugfs_init = qxl_debugfs_init,
> >  #endif
> > -       .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > -       .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> >         .gem_prime_export = drm_gem_prime_export,
> >         .gem_prime_import = drm_gem_prime_import,
> >         .gem_prime_pin = qxl_gem_prime_pin,
> >         .gem_prime_unpin = qxl_gem_prime_unpin,
> > -       .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
> > -       .gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
> >         .gem_prime_vmap = qxl_gem_prime_vmap,
> >         .gem_prime_vunmap = qxl_gem_prime_vunmap,
> >         .gem_prime_mmap = qxl_gem_prime_mmap,
> > diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
> > index a55dece118..df65d3c1a7 100644
> > --- a/drivers/gpu/drm/qxl/qxl_prime.c
> > +++ b/drivers/gpu/drm/qxl/qxl_prime.c
> > @@ -38,20 +38,6 @@ void qxl_gem_prime_unpin(struct drm_gem_object *obj)
> >         WARN_ONCE(1, "not implemented");
> >  }
> >
> > -struct sg_table *qxl_gem_prime_get_sg_table(struct drm_gem_object *obj)
> > -{
> > -       WARN_ONCE(1, "not implemented");
> > -       return ERR_PTR(-ENOSYS);
> > -}
> > -
> > -struct drm_gem_object *qxl_gem_prime_import_sg_table(
> > -       struct drm_device *dev, struct dma_buf_attachment *attach,
> > -       struct sg_table *table)
> > -{
> > -       WARN_ONCE(1, "not implemented");
> > -       return ERR_PTR(-ENOSYS);
> > -}
> > -
> >  void *qxl_gem_prime_vmap(struct drm_gem_object *obj)
> >  {
> >         WARN_ONCE(1, "not implemented");
> > --
> > 2.9.3
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/qxl: drop prime import/export callbacks

[Gerd Hoffmann]

On Tue, Apr 09, 2019 at 02:01:33PM +1000, Dave Airlie wrote:
> On Sat, 12 Jan 2019 at 07:13, Dave Airlie <airlied@gmail.com> wrote:
> >
> > On Thu, 10 Jan 2019 at 18:17, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > Also set prime_handle_to_fd and prime_fd_to_handle to NULL,
> > > so drm will not advertive DRM_PRIME_CAP_{IMPORT,EXPORT} to
> > > userspace.
> 
> It's been pointed out to me that disables DRI3 for these devices, I'm
> not sure that is the solution we actually wanted.
> 
> any ideas?

Well.  Lets have a look at where we stand:

 * drm_gem_prime_export() works with qxl, you'll get a dma-buf handle.

 * Other drivers trying to map that dma-buf (drm_gem_map_dma_buf()
   callback) will not work, due to the ->gem_prime_get_sg_table()
   callback not being there.

 * drm_gem_prime_import() will work with buffers from the same qxl
   device, there is a shortcut for this special case.  Otherwise it
   will not work, due to the ->gem_prime_import_sg_table() callback
   not being there.

Bottom line: you can use prime to pass qxl object handles from one
application to another.  But you can't actually export/import qxl
buffer objects from/to other devices.

Problem is that we have no way to signal to userspace that prime can
be used that way.

Setting DRM_PRIME_CAP_{IMPORT,EXPORT} even though the driver can't
do that leads to other problems.  Userspace thinks it can have other
devices (intel vgpu for example) handle the rendering, then import
the rendered buffer into qxl for scanout.

Should we add something like DRM_PRIME_CAP_SAME_DEVICE?

cheers,
  Gerd

Re: [PATCH] drm/qxl: drop prime import/export callbacks

[David Airlie]

On Tue, Apr 9, 2019 at 4:03 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Tue, Apr 09, 2019 at 02:01:33PM +1000, Dave Airlie wrote:
> > On Sat, 12 Jan 2019 at 07:13, Dave Airlie <airlied@gmail.com> wrote:
> > >
> > > On Thu, 10 Jan 2019 at 18:17, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > > >
> > > > Also set prime_handle_to_fd and prime_fd_to_handle to NULL,
> > > > so drm will not advertive DRM_PRIME_CAP_{IMPORT,EXPORT} to
> > > > userspace.
> >
> > It's been pointed out to me that disables DRI3 for these devices, I'm
> > not sure that is the solution we actually wanted.
> >
> > any ideas?
>
> Well.  Lets have a look at where we stand:
>
>  * drm_gem_prime_export() works with qxl, you'll get a dma-buf handle.
>
>  * Other drivers trying to map that dma-buf (drm_gem_map_dma_buf()
>    callback) will not work, due to the ->gem_prime_get_sg_table()
>    callback not being there.
>
>  * drm_gem_prime_import() will work with buffers from the same qxl
>    device, there is a shortcut for this special case.  Otherwise it
>    will not work, due to the ->gem_prime_import_sg_table() callback
>    not being there.
>
> Bottom line: you can use prime to pass qxl object handles from one
> application to another.  But you can't actually export/import qxl
> buffer objects from/to other devices.
>
> Problem is that we have no way to signal to userspace that prime can
> be used that way.
>
> Setting DRM_PRIME_CAP_{IMPORT,EXPORT} even though the driver can't
> do that leads to other problems.  Userspace thinks it can have other
> devices (intel vgpu for example) handle the rendering, then import
> the rendered buffer into qxl for scanout.
>
> Should we add something like DRM_PRIME_CAP_SAME_DEVICE?

Yeah I expect we need some sort of same device only capability, so
that dri3 userspace can work.

If we just fail importing in these cases what happens? userspace just
gets confused, I know we used to print a backtrace if we hit the mmap
path, but if we didn't do that what happens?

Dave.

Re: [PATCH] drm/qxl: drop prime import/export callbacks

[Gerd Hoffmann]

  Hi,

> > Should we add something like DRM_PRIME_CAP_SAME_DEVICE?
> 
> Yeah I expect we need some sort of same device only capability, so
> that dri3 userspace can work.
> 
> If we just fail importing in these cases what happens? userspace just
> gets confused, I know we used to print a backtrace if we hit the mmap
> path, but if we didn't do that what happens?

Well, we printed a backtrace and returned -EINVAL.  So it looked a bit
scary due to the backtrace which is usually printed for more serious
problems.  But we also returned a proper error code.

Userspace was not happy.  It was wayland (gnome-shell) which ran into it
it, and it didn't came up with a working display.

cheers,
  Gerd