On 6/25/21 7:38 PM, Ruhl, Michael J wrote:
-----Original Message-----
From: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
Sent: Friday, June 25, 2021 12:18 PM
To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>; intel-
gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
Cc: Auld, Matthew <matthew.auld@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
Thomas Hellström
Sent: Thursday, June 24, 2021 2:31 PM
To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx
Cc: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>; Auld,
Matthew
<matthew.auld@xxxxxxxxx>
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@xxxxxxxxxxxxxxx>
---
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
So perhaps just to initially fix the bug, we could just implement NOP
pin() and unpin() callbacks and drop the locking in map_attach() and
replace it with an assert_object_held();
/Thomas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
