-----Original Message----- From: Thomas Hellström thomas.hellstrom@linux.intel.com Sent: Friday, June 25, 2021 12:18 PM To: Ruhl, Michael J michael.j.ruhl@intel.com; intel- gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: Auld, Matthew matthew.auld@intel.com Subject: Re: [PATCH 4/4] drm/i915/gem: Migrate to system at dma-buf map time
Hi, Michael,
thanks for looking at this.
On 6/25/21 6:02 PM, Ruhl, Michael J wrote:
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Thomas Hellström Sent: Thursday, June 24, 2021 2:31 PM To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: Thomas Hellström thomas.hellstrom@linux.intel.com; Auld,
Matthew
matthew.auld@intel.com Subject: [PATCH 4/4] drm/i915/gem: Migrate to system at dma-buf map
time
Until we support p2p dma or as a complement to that, migrate data to system memory at dma-buf map time if possible.
Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index 616c3a2f1baf..a52f885bc09a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -25,7 +25,14 @@ static struct sg_table
*i915_gem_map_dma_buf(struct
dma_buf_attachment *attachme struct scatterlist *src, *dst; int ret, i;
- ret = i915_gem_object_pin_pages_unlocked(obj);
- ret = i915_gem_object_lock_interruptible(obj, NULL);
Hmm, I believe in most cases that the caller should be holding the lock (object dma-resv) on this object already.
Yes, I agree, In particular for other instances of our own driver, at least since the dma_resv introduction.
But I also think that's a pre-existing bug, since i915_gem_object_pin_pages_unlocked() will also take the lock.
Ouch yes. Missed that.
I Think we need to initially make the exporter dynamic-capable to resolve this, and drop the locking here completely, as dma-buf docs says that we're then guaranteed to get called with the object lock held.
I figure if we make the exporter dynamic, we need to migrate already at dma_buf_pin time so we don't pin the object in the wrong location.
The exporter as dynamic (ops->pin is available) is optional, but importer dynamic (ops->move_notify) is required.
With that in mind, it would seem that there are three possible combinations for the migrate to be attempted:
1) in the ops->pin function (export_dynamic != import_dynamic, during attach) 2) in the ops->pin function (export_dynamic and !CONFIG_DMABUF_MOVE_NOTIFY) during mapping 3) and possibly in ops->map_dma_buf (exort_dynamic iand CONFIG_DMABUF_MOVE_NOTIFY)
Since one possibility has to be in the mapping function, it seems that if we can figure out the locking, that the migrate should probably be available here.
Mike
/Thomas
I know for the dynamic version of dma-buf, there is a check to make sure that the lock is held when called.
I think you will run into some issues if you try to get it here as well.
Mike
- if (ret)
return ERR_PTR(ret);- ret = i915_gem_object_migrate(obj, NULL, INTEL_REGION_SMEM);
- if (!ret)
ret = i915_gem_object_pin_pages(obj);- i915_gem_object_unlock(obj); if (ret) goto err;
-- 2.31.1