From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com> To: "Ruhl, Michael J" <michael.j.ruhl@intel.com>, "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>, "dri-devel@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 Date: Fri, 25 Jun 2021 18:17:45 +0200 [thread overview] Message-ID: <cd90ba70-15f8-af0a-8810-2ee7939622a2@linux.intel.com> (raw) In-Reply-To: <5304ae661abb43a0816a485db9d3bbb5@intel.com> 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. 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. /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
WARNING: multiple messages have this Message-ID (diff)
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com> To: "Ruhl, Michael J" <michael.j.ruhl@intel.com>, "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org> Cc: "Auld, Matthew" <matthew.auld@intel.com> Subject: Re: [Intel-gfx] [PATCH 4/4] drm/i915/gem: Migrate to system at dma-buf map time Date: Fri, 25 Jun 2021 18:17:45 +0200 [thread overview] Message-ID: <cd90ba70-15f8-af0a-8810-2ee7939622a2@linux.intel.com> (raw) In-Reply-To: <5304ae661abb43a0816a485db9d3bbb5@intel.com> 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. 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. /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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-06-25 16:17 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-24 18:31 [PATCH 0/4] drm/i915/gem: Introduce a migrate interface Thomas Hellström 2021-06-24 18:31 ` [Intel-gfx] " Thomas Hellström 2021-06-24 18:31 ` [PATCH 1/4] drm/i915/gem: Implement object migration Thomas Hellström 2021-06-24 18:31 ` [Intel-gfx] " Thomas Hellström 2021-06-25 10:51 ` Matthew Auld 2021-06-25 10:51 ` [Intel-gfx] " Matthew Auld 2021-06-24 18:31 ` [PATCH 2/4] drm/i915/gem: Introduce a selftest for the gem object migrate functionality Thomas Hellström 2021-06-24 18:31 ` [Intel-gfx] " Thomas Hellström 2021-06-24 18:31 ` [PATCH 3/4] drm/i915/display: Migrate objects to LMEM if possible for display Thomas Hellström 2021-06-24 18:31 ` [Intel-gfx] " Thomas Hellström 2021-06-24 18:31 ` [PATCH 4/4] drm/i915/gem: Migrate to system at dma-buf map time Thomas Hellström 2021-06-24 18:31 ` [Intel-gfx] " Thomas Hellström 2021-06-25 16:02 ` Ruhl, Michael J 2021-06-25 16:02 ` [Intel-gfx] " Ruhl, Michael J 2021-06-25 16:17 ` Thomas Hellström [this message] 2021-06-25 16:17 ` Thomas Hellström 2021-06-25 17:38 ` Ruhl, Michael J 2021-06-25 17:38 ` [Intel-gfx] " Ruhl, Michael J 2021-06-25 17:52 ` Thomas Hellström 2021-06-25 17:52 ` [Intel-gfx] " Thomas Hellström 2021-06-25 17:57 ` Ruhl, Michael J 2021-06-25 17:57 ` [Intel-gfx] " Ruhl, Michael J 2021-06-25 18:49 ` Thomas Hellström 2021-06-25 18:49 ` [Intel-gfx] " Thomas Hellström 2021-06-25 19:07 ` Ruhl, Michael J 2021-06-25 19:07 ` [Intel-gfx] " Ruhl, Michael J 2021-06-25 19:10 ` Thomas Hellström 2021-06-25 19:10 ` [Intel-gfx] " Thomas Hellström 2021-06-25 19:21 ` Ruhl, Michael J 2021-06-25 19:21 ` [Intel-gfx] " Ruhl, Michael J 2021-06-24 22:02 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gem: Introduce a migrate interface Patchwork 2021-06-24 22:04 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork 2021-06-24 22:33 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2021-06-25 6:53 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=cd90ba70-15f8-af0a-8810-2ee7939622a2@linux.intel.com \ --to=thomas.hellstrom@linux.intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=matthew.auld@intel.com \ --cc=michael.j.ruhl@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.