[48/59] drm/vgem: Ditch attach trickery in the fence ioctl
diff mbox series

Message ID 20190614203615.12639-49-daniel.vetter@ffwll.ch
State New
Headers show
Series
  • prime doc polish and ... a few cleanups
Related show

Commit Message

Daniel Vetter June 14, 2019, 8:36 p.m. UTC
It looks like this was done purely to get a consistent place to look
up the reservation object pointer. With the drm_prime.c helper code
now also setting gem_object->resv for imported objects we can just use
that pointer directly, instead of first ensuring a dma-buf exists.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/vgem/vgem_fence.c | 22 +---------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

Comments

Chris Wilson June 18, 2019, 12:31 p.m. UTC | #1
Quoting Daniel Vetter (2019-06-14 21:36:04)
> It looks like this was done purely to get a consistent place to look
> up the reservation object pointer. With the drm_prime.c helper code
> now also setting gem_object->resv for imported objects we can just use
> that pointer directly, instead of first ensuring a dma-buf exists.

Oh. commit 1ba627148ef5 ("drm: Add reservation_object to drm_gem_object")
embedded a reservation_object into the struct. I was wondering what on
earth I was doing if the code should have been so simple.

References: 1ba627148ef5 ("drm: Add reservation_object to drm_gem_object")
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Quick leave before I start ranting about the horrors of
reservation_object.
-Chris
Daniel Vetter June 18, 2019, 1:18 p.m. UTC | #2
On Tue, Jun 18, 2019 at 2:31 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Daniel Vetter (2019-06-14 21:36:04)
> > It looks like this was done purely to get a consistent place to look
> > up the reservation object pointer. With the drm_prime.c helper code
> > now also setting gem_object->resv for imported objects we can just use
> > that pointer directly, instead of first ensuring a dma-buf exists.
>
> Oh. commit 1ba627148ef5 ("drm: Add reservation_object to drm_gem_object")
> embedded a reservation_object into the struct. I was wondering what on
> earth I was doing if the code should have been so simple.

Yeah, this is the thing that started all this, plus a lot more (all
the gem locking helper functions that panfrost and v3d are using).

I think next steps might be to ditch ttm_bo.resv|ttm_resv and
i915_bo.resv|__builtin_resv in favour of the one in drm_gem_bo. But my
series here was already getting way to big. The ttm one is only really
a problem for vmwgfx, and that's easy to solve by giving them a
separate pointer. We might need to keep ttm_bo.resv pointer to make
transitioning easier.

> References: 1ba627148ef5 ("drm: Add reservation_object to drm_gem_object")
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Quick leave before I start ranting about the horrors of
> reservation_object.

:-)

I think it's a case of "the devil we have" and all that ...

Cheers, Daniel

Patch
diff mbox series

diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c
index 72d43d5ec5ab..08997fdd3ccb 100644
--- a/drivers/gpu/drm/vgem/vgem_fence.c
+++ b/drivers/gpu/drm/vgem/vgem_fence.c
@@ -100,22 +100,6 @@  static struct dma_fence *vgem_fence_create(struct vgem_file *vfile,
 	return &fence->base;
 }
 
-static int attach_dmabuf(struct drm_device *dev,
-			 struct drm_gem_object *obj)
-{
-	struct dma_buf *dmabuf;
-
-	if (obj->dma_buf)
-		return 0;
-
-	dmabuf = dev->driver->gem_prime_export(obj, 0);
-	if (IS_ERR(dmabuf))
-		return PTR_ERR(dmabuf);
-
-	obj->dma_buf = dmabuf;
-	return 0;
-}
-
 /*
  * vgem_fence_attach_ioctl (DRM_IOCTL_VGEM_FENCE_ATTACH):
  *
@@ -157,10 +141,6 @@  int vgem_fence_attach_ioctl(struct drm_device *dev,
 	if (!obj)
 		return -ENOENT;
 
-	ret = attach_dmabuf(dev, obj);
-	if (ret)
-		goto err;
-
 	fence = vgem_fence_create(vfile, arg->flags);
 	if (!fence) {
 		ret = -ENOMEM;
@@ -168,7 +148,7 @@  int vgem_fence_attach_ioctl(struct drm_device *dev,
 	}
 
 	/* Check for a conflicting fence */
-	resv = obj->dma_buf->resv;
+	resv = obj->resv;
 	if (!reservation_object_test_signaled_rcu(resv,
 						  arg->flags & VGEM_FENCE_WRITE)) {
 		ret = -EBUSY;