| Message ID | 20211021103605.735002-15-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/28] drm/i915: Fix i915_request fence wait semantics | expand |
On Thu, 21 Oct 2021 at 11:37, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote: > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Needs a proper commit message. > --- > drivers/gpu/drm/i915/i915_gem.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 981e383d1a5d..6aa9e465b48e 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -931,7 +931,14 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj, > goto new_vma; > } > > - ret = i915_vma_unbind(vma); > + ret = 0; > + if (!ww) > + ret = i915_gem_object_lock_interruptible(obj, NULL); > + if (!ret) { > + ret = i915_vma_unbind(vma); > + if (!ww) > + i915_gem_object_unlock(obj); > + } There is also a wait_for_bind below. Do we need the lock for that also? > if (ret) > return ERR_PTR(ret); > } > -- > 2.33.0 >
On 21-10-2021 19:48, Matthew Auld wrote: > On Thu, 21 Oct 2021 at 11:37, Maarten Lankhorst > <maarten.lankhorst@linux.intel.com> wrote: >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Needs a proper commit message. What about this? >> --- >> drivers/gpu/drm/i915/i915_gem.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index 981e383d1a5d..6aa9e465b48e 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -931,7 +931,14 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj, >> goto new_vma; >> } >> >> - ret = i915_vma_unbind(vma); >> + ret = 0; >> + if (!ww) >> + ret = i915_gem_object_lock_interruptible(obj, NULL); >> + if (!ret) { >> + ret = i915_vma_unbind(vma); >> + if (!ww) >> + i915_gem_object_unlock(obj); >> + } > There is also a wait_for_bind below. Do we need the lock for that also? Hmm good find. Not sure if required in this patch series. I have some patches on top that require the lock because of async binding / unbinding, that would need it for sure. I will fix this function to use WARN_ON(!ww), and add ww handling to i915_gem_object_ggtt_pin(). That should fix all issues without special casing !ww. ~Maarten
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 981e383d1a5d..6aa9e465b48e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -931,7 +931,14 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj, goto new_vma; } - ret = i915_vma_unbind(vma); + ret = 0; + if (!ww) + ret = i915_gem_object_lock_interruptible(obj, NULL); + if (!ret) { + ret = i915_vma_unbind(vma); + if (!ww) + i915_gem_object_unlock(obj); + } if (ret) return ERR_PTR(ret); }
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)