Re: [PATCH v2 5/6] virtio-gpu: add basic prime support

[Daniel Vetter]

On Tue, Sep 22, 2015 at 05:24:29PM +0200, Gerd Hoffmann wrote:
> > > +int virtgpu_gem_prime_mmap(struct drm_gem_object *obj,
> > > +struct vm_area_struct *area)
> > > +{
> > > + WARN_ONCE(1, "not implemented");
> > > + return ENOSYS;
> > 
> > This can get called by userspace, so please don't WARN here. Also missing
> > negate sign:
> > 
> > return -ENOSYS;
> 
> Hmm now checkpatch throws a warning at me:
> 
> 
>WARNING: ENOSYS means 'invalid syscall nr' and nothing else
>#12: FILE: drivers/gpu/drm/virtio/virtgpu_prime.c:70:
>+   return -ENOSYS;
> 
> 
> I guess I should use something else then (here and elsewhere in the
> file)?  Maybe -EINVAL?  Other suggestions?

-ENODEV is what we occasionally pick. drm is fairly creative at errno
abuse, e.g. we already use -ENOENT to signal any kind of lookup failure in
ioctls (even if the fd itself is obviously there so not possible that the
fd isn't there).

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 5/6] virtio-gpu: add basic prime support

[Daniel Vetter]

On Mon, Sep 21, 2015 at 11:40:16AM +0200, Gerd Hoffmann wrote:
> From: Dave Airlie 
> 
> This is enough to enable DRI3.
> 
> Signed-off-by: Dave Airlie 
> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/virtio/Makefile|  2 +-
>  drivers/gpu/drm/virtio/virtgpu_drv.c   | 13 +-
>  drivers/gpu/drm/virtio/virtgpu_drv.h   | 12 ++
>  drivers/gpu/drm/virtio/virtgpu_prime.c | 72 
> ++
>  4 files changed, 97 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/drm/virtio/virtgpu_prime.c
> 
> diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile
> index da7bf19..3fb8eac 100644
> --- a/drivers/gpu/drm/virtio/Makefile
> +++ b/drivers/gpu/drm/virtio/Makefile
> @@ -7,6 +7,6 @@ ccflags-y := -Iinclude/drm
>  virtio-gpu-y := virtgpu_drv.o virtgpu_kms.o virtgpu_drm_bus.o virtgpu_gem.o \
>   virtgpu_fb.o virtgpu_display.o virtgpu_vq.o virtgpu_ttm.o \
>   virtgpu_fence.o virtgpu_object.o virtgpu_debugfs.o virtgpu_plane.o \
> - virtgpu_ioctl.o
> + virtgpu_ioctl.o virtgpu_prime.o
>  
>  obj-$(CONFIG_DRM_VIRTIO_GPU) += virtio-gpu.o
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c 
> b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index 957e455..1245d09 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -113,7 +113,7 @@ static const struct file_operations 
> virtio_gpu_driver_fops = {
>  
>  
>  static struct drm_driver driver = {
> - .driver_features = DRIVER_MODESET | DRIVER_GEM,
> + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME,
>   .set_busid = drm_virtio_set_busid,
>   .load = virtio_gpu_driver_load,
>   .unload = virtio_gpu_driver_unload,
> @@ -128,6 +128,17 @@ static struct drm_driver driver = {
>   .debugfs_init = virtio_gpu_debugfs_init,
>   .debugfs_cleanup = virtio_gpu_debugfs_takedown,
>  #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 = virtgpu_gem_prime_pin,
> + .gem_prime_unpin = virtgpu_gem_prime_unpin,
> + .gem_prime_get_sg_table = virtgpu_gem_prime_get_sg_table,
> + .gem_prime_import_sg_table = virtgpu_gem_prime_import_sg_table,
> + .gem_prime_vmap = virtgpu_gem_prime_vmap,
> + .gem_prime_vunmap = virtgpu_gem_prime_vunmap,
> + .gem_prime_mmap = virtgpu_gem_prime_mmap,
>  
>   .gem_free_object = virtio_gpu_gem_free_object,
>   .gem_open_object = virtio_gpu_gem_object_open,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 2719108..79f0abe 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -359,6 +359,18 @@ int virtio_gpu_object_get_sg_table(struct 
> virtio_gpu_device *qdev,
>  void virtio_gpu_object_free_sg_table(struct virtio_gpu_object *bo);
>  int virtio_gpu_object_wait(struct virtio_gpu_object *bo, bool no_wait);
>  
> +/* virtgpu_prime.c */
> +int virtgpu_gem_prime_pin(struct drm_gem_object *obj);
> +void virtgpu_gem_prime_unpin(struct drm_gem_object *obj);
> +struct sg_table *virtgpu_gem_prime_get_sg_table(struct drm_gem_object *obj);
> +struct drm_gem_object *virtgpu_gem_prime_import_sg_table(
> +struct drm_device *dev, struct dma_buf_attachment *attach,
> +struct sg_table *sgt);
> +void *virtgpu_gem_prime_vmap(struct drm_gem_object *obj);
> +void virtgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
> +int virtgpu_gem_prime_mmap(struct drm_gem_object *obj,
> +struct vm_area_struct *vma);
> +
>  static inline struct virtio_gpu_object*
>  virtio_gpu_object_ref(struct virtio_gpu_object *bo)
>  {
> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c 
> b/drivers/gpu/drm/virtio/virtgpu_prime.c
> new file mode 100644
> index 000..724c231
> --- /dev/null
> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
> @@ -0,0 +1,72 @@
> +/*
> + * Copyright 2014 Canonical
> + *
> + * 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 

Re: [PATCH v2 5/6] virtio-gpu: add basic prime support

[Gerd Hoffmann]

> > +int virtgpu_gem_prime_mmap(struct drm_gem_object *obj,
> > +  struct vm_area_struct *area)
> > +{
> > +   WARN_ONCE(1, "not implemented");
> > +   return ENOSYS;
> 
> This can get called by userspace, so please don't WARN here. Also missing
> negate sign:
> 
>   return -ENOSYS;

Hmm now checkpatch throws a warning at me:


   WARNING: ENOSYS means 'invalid syscall nr' and nothing else
   #12: FILE: drivers/gpu/drm/virtio/virtgpu_prime.c:70:
   +   return -ENOSYS;


I guess I should use something else then (here and elsewhere in the
file)?  Maybe -EINVAL?  Other suggestions?

thanks,
  Gerd


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization