aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChris Wilson <chris@chris-wilson.co.uk>2017-08-28 06:46:31 -0400
committerRodrigo Vivi <rodrigo.vivi@intel.com>2017-08-30 15:08:11 -0400
commit3b24e7e8109bcf2089b3052d2ddd301e33042034 (patch)
tree38a0fb9c9d21441fcdcd41c635b91dd662aecc28
parent9c8eb2d5cbdb8a97a0b2a4da2825a2c4c0a202dc (diff)
drm/i915: Recreate vmapping even when the object is pinned
Sometimes we know we are the only user of the bo, but since we take a protective pin_pages early on, an attempt to change the vmap on the object is denied because it is busy. i915_gem_object_pin_map() cannot tell from our single pin_count if the operation is safe. Instead we must pass that information down from the caller in the manner of I915_MAP_OVERRIDE. This issue has existed from the introduction of the mapping, but was never noticed as the only place where this conflict might happen is for cached kernel buffers (such as allocated by i915_gem_batch_pool_get()). Until recently there was only a single user (the cmdparser) so no conflicts ever occurred. However, we now use it to allocate batches for different operations (using MAP_WC on !llc for writes) in addition to the existing shadow batch (using MAP_WB for reads). We could either keep both mappings cached, or use a different write mechanism if we detect a MAP_WB already exists (i.e. clflush afterwards), but as we haven't seen this issue in the wild (it requires hitting the GPU reloc path in addition to the cmdparser) for simplicity just allow the mappings to be recreated. v2: Include the i915_MAP_OVERRIDE bit in the enum so the compiler knows about all the valid values. Fixes: 7dd4f6729f92 ("drm/i915: Async GPU relocation processing") Testcase: igt/gem_lut_handle # byt, completely by accident Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170828104631.8606-1-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (cherry picked from commit a575c6761757232ea2c7dc9f370640754b90cc69) Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
-rw-r--r--drivers/gpu/drm/i915/i915_cmd_parser.c2
-rw-r--r--drivers/gpu/drm/i915/i915_drv.h3
-rw-r--r--drivers/gpu/drm/i915/i915_gem.c7
-rw-r--r--drivers/gpu/drm/i915/i915_gem_execbuffer.c4
4 files changed, 13 insertions, 3 deletions
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index f0cb22cc0dd6..8ba932b22f7c 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1073,7 +1073,7 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
1073 goto unpin_src; 1073 goto unpin_src;
1074 } 1074 }
1075 1075
1076 dst = i915_gem_object_pin_map(dst_obj, I915_MAP_WB); 1076 dst = i915_gem_object_pin_map(dst_obj, I915_MAP_FORCE_WB);
1077 if (IS_ERR(dst)) 1077 if (IS_ERR(dst))
1078 goto unpin_dst; 1078 goto unpin_dst;
1079 1079
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 60267e375e88..571c4e27a574 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3479,6 +3479,9 @@ void __i915_gem_object_invalidate(struct drm_i915_gem_object *obj);
3479enum i915_map_type { 3479enum i915_map_type {
3480 I915_MAP_WB = 0, 3480 I915_MAP_WB = 0,
3481 I915_MAP_WC, 3481 I915_MAP_WC,
3482#define I915_MAP_OVERRIDE BIT(31)
3483 I915_MAP_FORCE_WB = I915_MAP_WB | I915_MAP_OVERRIDE,
3484 I915_MAP_FORCE_WC = I915_MAP_WC | I915_MAP_OVERRIDE,
3482}; 3485};
3483 3486
3484/** 3487/**
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b9e8e0d6e97b..9c62b18ca03a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2553,6 +2553,9 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj,
2553 GEM_BUG_ON(i != n_pages); 2553 GEM_BUG_ON(i != n_pages);
2554 2554
2555 switch (type) { 2555 switch (type) {
2556 default:
2557 MISSING_CASE(type);
2558 /* fallthrough to use PAGE_KERNEL anyway */
2556 case I915_MAP_WB: 2559 case I915_MAP_WB:
2557 pgprot = PAGE_KERNEL; 2560 pgprot = PAGE_KERNEL;
2558 break; 2561 break;
@@ -2583,7 +2586,9 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
2583 if (ret) 2586 if (ret)
2584 return ERR_PTR(ret); 2587 return ERR_PTR(ret);
2585 2588
2586 pinned = true; 2589 pinned = !(type & I915_MAP_OVERRIDE);
2590 type &= ~I915_MAP_OVERRIDE;
2591
2587 if (!atomic_inc_not_zero(&obj->mm.pages_pin_count)) { 2592 if (!atomic_inc_not_zero(&obj->mm.pages_pin_count)) {
2588 if (unlikely(IS_ERR_OR_NULL(obj->mm.pages))) { 2593 if (unlikely(IS_ERR_OR_NULL(obj->mm.pages))) {
2589 ret = ____i915_gem_object_get_pages(obj); 2594 ret = ____i915_gem_object_get_pages(obj);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 4c2016237d61..25b14d492d91 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1070,7 +1070,9 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
1070 return PTR_ERR(obj); 1070 return PTR_ERR(obj);
1071 1071
1072 cmd = i915_gem_object_pin_map(obj, 1072 cmd = i915_gem_object_pin_map(obj,
1073 cache->has_llc ? I915_MAP_WB : I915_MAP_WC); 1073 cache->has_llc ?
1074 I915_MAP_FORCE_WB :
1075 I915_MAP_FORCE_WC);
1074 i915_gem_object_unpin_pages(obj); 1076 i915_gem_object_unpin_pages(obj);
1075 if (IS_ERR(cmd)) 1077 if (IS_ERR(cmd))
1076 return PTR_ERR(cmd); 1078 return PTR_ERR(cmd);