[Intel-gfx] [PATCH 03/23] drm/i915: Remove locking from i915_gem_object_prepare_read/write
Execbuffer submission will perform its own WW locking, and we
cannot rely on the implicit lock there.
This also makes it clear that the GVT code will get a lockdep splat when
multiple batchbuffer shadows need to be performed in the same instance,
fix that up.
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c| 20 ++-
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 13 ++--
drivers/gpu/drm/i915/gem/i915_gem_object.h| 1 -
.../gpu/drm/i915/gem/selftests/huge_pages.c | 5 -
.../i915/gem/selftests/i915_gem_coherency.c | 14 +
.../drm/i915/gem/selftests/i915_gem_context.c | 12 ---
drivers/gpu/drm/i915/gt/intel_renderstate.c | 5 -
drivers/gpu/drm/i915/gvt/cmd_parser.c | 9 -
drivers/gpu/drm/i915/i915_gem.c | 20 +--
9 files changed, 70 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index c0acfc97fae3..8ebceebd11b0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -576,19 +576,17 @@ int i915_gem_object_prepare_read(struct
drm_i915_gem_object *obj,
if (!i915_gem_object_has_struct_page(obj))
return -ENODEV;
- ret = i915_gem_object_lock_interruptible(obj, NULL);
- if (ret)
- return ret;
+ assert_object_held(obj);
ret = i915_gem_object_wait(obj,
I915_WAIT_INTERRUPTIBLE,
MAX_SCHEDULE_TIMEOUT);
if (ret)
- goto err_unlock;
+ return ret;
ret = i915_gem_object_pin_pages(obj);
if (ret)
- goto err_unlock;
+ return ret;
if (obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ ||
!static_cpu_has(X86_FEATURE_CLFLUSH)) {
@@ -616,8 +614,6 @@ int i915_gem_object_prepare_read(struct drm_i915_gem_object
*obj,
err_unpin:
i915_gem_object_unpin_pages(obj);
-err_unlock:
- i915_gem_object_unlock(obj);
return ret;
}
@@ -630,20 +626,18 @@ int i915_gem_object_prepare_write(struct
drm_i915_gem_object *obj,
if (!i915_gem_object_has_struct_page(obj))
return -ENODEV;
- ret = i915_gem_object_lock_interruptible(obj, NULL);
- if (ret)
- return ret;
+ assert_object_held(obj);
ret = i915_gem_object_wait(obj,
I915_WAIT_INTERRUPTIBLE |
I915_WAIT_ALL,
MAX_SCHEDULE_TIMEOUT);
if (ret)
- goto err_unlock;
+ return ret;
ret = i915_gem_object_pin_pages(obj);
if (ret)
- goto err_unlock;
+ return ret;
if (obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_WRITE ||
!static_cpu_has(X86_FEATURE_CLFLUSH)) {
@@ -680,7 +674,5 @@ int i915_gem_object_prepare_write(struct
drm_i915_gem_object *obj,
err_unpin:
i915_gem_object_unpin_pages(obj);
-err_unlock:
- i915_gem_object_unlock(obj);
return ret;
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 8861287df27a..f72f96863f8c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1098,11 +1098,14 @@ static void reloc_cache_reset(struct reloc_cache *cache)
vaddr = unmask_page(cache->vaddr);
if (cache->vaddr & KMAP) {
+ struct drm_i915_gem_object *obj =
+ (struct drm_i915_gem_object *)cache->node.mm;
if (cache->vaddr & CLFLUSH_AFTER)
mb();
kunmap_atomic(vaddr);
- i915_gem_object_finish_access((struct drm_i915_gem_object
*)cache->node.mm);
+ i915_gem_object_finish_access(obj);
+ i915_gem_object_unlock(obj);
} else {
struct i915_ggtt *ggtt = cache_to_ggtt(cache);
@@ -1137,10 +1140,16 @@ static void *reloc_kmap(struct drm_i915_gem_object *obj,
unsigned int flushes;
int err;
- err = i915_gem_object_prepare_write(obj, );
+ err = i915_gem_object_lock_interruptible(obj, NULL);
if (err)
return ERR_PTR(err);
+ err = i915_gem_object_prepare_write(obj, );
+ if (err) {
+ i915_gem_object_unlock(obj);
+ return ERR_PTR(err);
+ }
+
BUILD_BUG_ON(KMAP & CLFLUSH_FLAGS);
BUILD_BUG_ON((KMAP | CLFLUSH_FLAGS) & PAGE_MASK);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 37a9ee227ff1..aea18495c7a9 100644
[Intel-gfx] [PATCH 03/23] drm/i915: Remove locking from i915_gem_object_prepare_read/write
Execbuffer submission will perform its own WW locking, and we
cannot rely on the implicit lock there.
This also makes it clear that the GVT code will get a lockdep splat when
multiple batchbuffer shadows need to be performed in the same instance,
fix that up.
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c| 20 ++-
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 13 ++--
drivers/gpu/drm/i915/gem/i915_gem_object.h| 1 -
.../gpu/drm/i915/gem/selftests/huge_pages.c | 5 -
.../i915/gem/selftests/i915_gem_coherency.c | 14 +
.../drm/i915/gem/selftests/i915_gem_context.c | 12 ---
drivers/gpu/drm/i915/gt/intel_renderstate.c | 5 -
drivers/gpu/drm/i915/gvt/cmd_parser.c | 9 -
drivers/gpu/drm/i915/i915_gem.c | 20 +--
9 files changed, 70 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index c0acfc97fae3..8ebceebd11b0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -576,19 +576,17 @@ int i915_gem_object_prepare_read(struct
drm_i915_gem_object *obj,
if (!i915_gem_object_has_struct_page(obj))
return -ENODEV;
- ret = i915_gem_object_lock_interruptible(obj, NULL);
- if (ret)
- return ret;
+ assert_object_held(obj);
ret = i915_gem_object_wait(obj,
I915_WAIT_INTERRUPTIBLE,
MAX_SCHEDULE_TIMEOUT);
if (ret)
- goto err_unlock;
+ return ret;
ret = i915_gem_object_pin_pages(obj);
if (ret)
- goto err_unlock;
+ return ret;
if (obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ ||
!static_cpu_has(X86_FEATURE_CLFLUSH)) {
@@ -616,8 +614,6 @@ int i915_gem_object_prepare_read(struct drm_i915_gem_object
*obj,
err_unpin:
i915_gem_object_unpin_pages(obj);
-err_unlock:
- i915_gem_object_unlock(obj);
return ret;
}
@@ -630,20 +626,18 @@ int i915_gem_object_prepare_write(struct
drm_i915_gem_object *obj,
if (!i915_gem_object_has_struct_page(obj))
return -ENODEV;
- ret = i915_gem_object_lock_interruptible(obj, NULL);
- if (ret)
- return ret;
+ assert_object_held(obj);
ret = i915_gem_object_wait(obj,
I915_WAIT_INTERRUPTIBLE |
I915_WAIT_ALL,
MAX_SCHEDULE_TIMEOUT);
if (ret)
- goto err_unlock;
+ return ret;
ret = i915_gem_object_pin_pages(obj);
if (ret)
- goto err_unlock;
+ return ret;
if (obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_WRITE ||
!static_cpu_has(X86_FEATURE_CLFLUSH)) {
@@ -680,7 +674,5 @@ int i915_gem_object_prepare_write(struct
drm_i915_gem_object *obj,
err_unpin:
i915_gem_object_unpin_pages(obj);
-err_unlock:
- i915_gem_object_unlock(obj);
return ret;
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index a5972a7d0242..10e1af1cab2f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1098,11 +1098,14 @@ static void reloc_cache_reset(struct reloc_cache *cache)
vaddr = unmask_page(cache->vaddr);
if (cache->vaddr & KMAP) {
+ struct drm_i915_gem_object *obj =
+ (struct drm_i915_gem_object *)cache->node.mm;
if (cache->vaddr & CLFLUSH_AFTER)
mb();
kunmap_atomic(vaddr);
- i915_gem_object_finish_access((struct drm_i915_gem_object
*)cache->node.mm);
+ i915_gem_object_finish_access(obj);
+ i915_gem_object_unlock(obj);
} else {
struct i915_ggtt *ggtt = cache_to_ggtt(cache);
@@ -1137,10 +1140,16 @@ static void *reloc_kmap(struct drm_i915_gem_object *obj,
unsigned int flushes;
int err;
- err = i915_gem_object_prepare_write(obj, );
+ err = i915_gem_object_lock_interruptible(obj, NULL);
if (err)
return ERR_PTR(err);
+ err = i915_gem_object_prepare_write(obj, );
+ if (err) {
+ i915_gem_object_unlock(obj);
+ return ERR_PTR(err);
+ }
+
BUILD_BUG_ON(KMAP & CLFLUSH_FLAGS);
BUILD_BUG_ON((KMAP | CLFLUSH_FLAGS) & PAGE_MASK);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 5103067269b0..11b8e2735071 100644
