On Mon, Jul 11, 2016 at 12:10:40PM -0300, Gustavo Padovan wrote:
> 2016-07-11 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>:
>
> > vGEM buffers are useful for passing data between software clients and
> > hardware renders. By allowing the user to create and attach fences to
> > the exported vGEM buffers (on the dma-buf), the user can implement a
> > deferred renderer and queue hardware operations like flipping and then
> > signal the buffer readiness (i.e. this allows the user to schedule
> > operations out-of-order, but have them complete in-order).
> >
> > This also makes it much easier to write tightly controlled testcases for
> > dma-buf fencing and signaling between hardware drivers.
> >
> > Testcase: igt/vgem_basic/dmabuf-fence
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
> > Cc: Zach Reizner <zachr@xxxxxxxxxx>
> > Cc: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> > Acked-by: Zach Reizner <zachr@xxxxxxxxxx>
> > ---
> > drivers/gpu/drm/vgem/Makefile | 2 +-
> > drivers/gpu/drm/vgem/vgem_drv.c | 34 ++++++
> > drivers/gpu/drm/vgem/vgem_drv.h | 18 ++++
> > drivers/gpu/drm/vgem/vgem_fence.c | 220 ++++++++++++++++++++++++++++++++++++++
> > include/uapi/drm/vgem_drm.h | 62 +++++++++++
> > 5 files changed, 335 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/gpu/drm/vgem/vgem_fence.c
> > create mode 100644 include/uapi/drm/vgem_drm.h
> >
> > diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile
> > index 3f4c7b842028..bfcdea1330e6 100644
> > --- a/drivers/gpu/drm/vgem/Makefile
> > +++ b/drivers/gpu/drm/vgem/Makefile
> > @@ -1,4 +1,4 @@
> > ccflags-y := -Iinclude/drm
> > -vgem-y := vgem_drv.o
> > +vgem-y := vgem_drv.o vgem_fence.o
> >
> > obj-$(CONFIG_DRM_VGEM) += vgem.o
> > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> > index b5fb968d2d5c..2659e5cda857 100644
> > --- a/drivers/gpu/drm/vgem/vgem_drv.c
> > +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> > @@ -83,6 +83,34 @@ static const struct vm_operations_struct vgem_gem_vm_ops = {
> > .close = drm_gem_vm_close,
> > };
> >
> > +static int vgem_open(struct drm_device *dev, struct drm_file *file)
> > +{
> > + struct vgem_file *vfile;
> > + int ret;
> > +
> > + vfile = kzalloc(sizeof(*vfile), GFP_KERNEL);
> > + if (!vfile)
> > + return -ENOMEM;
> > +
> > + file->driver_priv = vfile;
> > +
> > + ret = vgem_fence_open(vfile);
> > + if (ret) {
> > + kfree(vfile);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void vgem_preclose(struct drm_device *dev, struct drm_file *file)
> > +{
> > + struct vgem_file *vfile = file->driver_priv;
> > +
> > + vgem_fence_close(vfile);
> > + kfree(vfile);
> > +}
> > +
> > /* ioctls */
> >
> > static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> > @@ -164,6 +192,8 @@ unref:
> > }
> >
> > static struct drm_ioctl_desc vgem_ioctls[] = {
> > + DRM_IOCTL_DEF_DRV(VGEM_FENCE_ATTACH, vgem_fence_attach_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> > + DRM_IOCTL_DEF_DRV(VGEM_FENCE_SIGNAL, vgem_fence_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> > };
> >
> > static int vgem_mmap(struct file *filp, struct vm_area_struct *vma)
> > @@ -271,9 +301,12 @@ static int vgem_prime_mmap(struct drm_gem_object *obj,
> >
> > static struct drm_driver vgem_driver = {
> > .driver_features = DRIVER_GEM | DRIVER_PRIME,
> > + .open = vgem_open,
> > + .preclose = vgem_preclose,
> > .gem_free_object_unlocked = vgem_gem_free_object,
> > .gem_vm_ops = &vgem_gem_vm_ops,
> > .ioctls = vgem_ioctls,
> > + .num_ioctls = ARRAY_SIZE(vgem_ioctls),
> > .fops = &vgem_driver_fops,
> >
> > .dumb_create = vgem_gem_dumb_create,
> > @@ -328,5 +361,6 @@ module_init(vgem_init);
> > module_exit(vgem_exit);
> >
> > MODULE_AUTHOR("Red Hat, Inc.");
> > +MODULE_AUTHOR("Intel Corporation");
> > MODULE_DESCRIPTION(DRIVER_DESC);
> > MODULE_LICENSE("GPL and additional rights");
> > diff --git a/drivers/gpu/drm/vgem/vgem_drv.h b/drivers/gpu/drm/vgem/vgem_drv.h
> > index 988cbaae7588..88ce21010e28 100644
> > --- a/drivers/gpu/drm/vgem/vgem_drv.h
> > +++ b/drivers/gpu/drm/vgem/vgem_drv.h
> > @@ -32,9 +32,27 @@
> > #include <drm/drmP.h>
> > #include <drm/drm_gem.h>
> >
> > +#include <uapi/drm/vgem_drm.h>
> > +
> > +struct vgem_file {
> > + struct idr fence_idr;
> > + struct mutex fence_mutex;
> > + u64 fence_context;
> > + atomic_t fence_seqno;
> > +};
> > +
> > #define to_vgem_bo(x) container_of(x, struct drm_vgem_gem_object, base)
> > struct drm_vgem_gem_object {
> > struct drm_gem_object base;
> > };
> >
> > +int vgem_fence_open(struct vgem_file *file);
> > +int vgem_fence_attach_ioctl(struct drm_device *dev,
> > + void *data,
> > + struct drm_file *file);
> > +int vgem_fence_signal_ioctl(struct drm_device *dev,
> > + void *data,
> > + struct drm_file *file);
> > +void vgem_fence_close(struct vgem_file *file);
> > +
> > #endif
> > diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c
> > new file mode 100644
> > index 000000000000..649e9e1cee35
> > --- /dev/null
> > +++ b/drivers/gpu/drm/vgem/vgem_fence.c
> > @@ -0,0 +1,220 @@
> > +/*
> > + * Copyright 2016 Intel Corporation
> > + *
> > + * 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
> > + * on the rights to use, copy, modify, merge, publish, distribute, sub
> > + * license, and/or sell copies of the Software, and to permit persons to whom
> > + * them Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) 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 MERCHANTIBILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS 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 <linux/dma-buf.h>
> > +#include <linux/reservation.h>
> > +
> > +#include "vgem_drv.h"
> > +
> > +struct vgem_fence {
> > + struct fence base;
> > + struct spinlock lock;
> > +};
> > +
> > +static const char *vgem_fence_get_driver_name(struct fence *fence)
> > +{
> > + return "vgem";
> > +}
> > +
> > +static const char *vgem_fence_get_timeline_name(struct fence *fence)
> > +{
> > + return "file";
> > +}
> > +
> > +static bool vgem_fence_signaled(struct fence *fence)
> > +{
> > + return false;
> > +}
> > +
> > +static bool vgem_fence_enable_signaling(struct fence *fence)
> > +{
> > + return true;
> > +}
> > +
> > +static void vgem_fence_value_str(struct fence *fence, char *str, int size)
> > +{
> > + snprintf(str, size, "%u", fence->seqno);
> > +}
> > +
> > +static void vgem_fence_timeline_value_str(struct fence *fence, char *str,
> > + int size)
> > +{
> > + snprintf(str, size, "%u", 0);
> > +}
>
> I think this would be mainly used for debug purposes but it would be
> nice to actually return the current timeline value here, i.e., the
> seqno of the last signalled fence.
The fences can and will be signaled in any order, there is no timeline
here under the control of userspace, we only give them fences.
> This would also allow to return if the fence was signaled or not
> in vgem_fence_signaled() instead of just return false.
That doesn't fit the out-of-order unbound nature of the interface. The
interface is just a collection of fences that userspace associates with
the buffer that it may signal at any time. (Having no strict timeline is
an advantage!)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
