| Message ID | 20190614203615.12639-49-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | prime doc polish and ... a few cleanups | expand |
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
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
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;
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(-)