[Intel-gfx] [PATCH 2/2] drm/i915: Prevent concurrent tiling/framebuffer modifications

Chris Wilson chris at chris-wilson.co.uk
Tue Feb 28 16:22:33 UTC 2017


Reintroduce a lock around tiling vs framebuffer creation to prevent
modification of the obj->tiling_and_stride whilst the framebuffer is
being created. Rather than use struct_mutex once again, use the
per-object lock - this will also be required in future to prevent
changing the tiling whilst submitting rendering.

Fixes: 24dbf51a5517 ("drm/i915: struct_mutex is not required for allocating the framebuffer")
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_object.h   | 18 +++++++++++++++++-
 drivers/gpu/drm/i915/i915_gem_shrinker.c |  2 +-
 drivers/gpu/drm/i915/i915_gem_tiling.c   |  9 ++++++++-
 drivers/gpu/drm/i915/intel_display.c     | 25 ++++++++++++++++---------
 4 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index ad1bc0b1a0c2..8c02c8ec2a3b 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -169,7 +169,7 @@ struct drm_i915_gem_object {
 	struct reservation_object *resv;
 
 	/** References from framebuffers, locks out tiling changes. */
-	atomic_t framebuffer_references;
+	unsigned int framebuffer_references;
 
 	/** Record of address bit 17 of each page at last unbind. */
 	unsigned long *bit_17;
@@ -263,6 +263,16 @@ extern void drm_gem_object_unreference(struct drm_gem_object *);
 __deprecated
 extern void drm_gem_object_unreference_unlocked(struct drm_gem_object *);
 
+static inline void i915_gem_object_lock(struct drm_i915_gem_object *obj)
+{
+	reservation_object_lock(obj->resv, NULL);
+}
+
+static inline void i915_gem_object_unlock(struct drm_i915_gem_object *obj)
+{
+	reservation_object_unlock(obj->resv);
+}
+
 static inline bool
 i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
 {
@@ -303,6 +313,12 @@ i915_gem_object_clear_active_reference(struct drm_i915_gem_object *obj)
 
 void __i915_gem_object_release_unless_active(struct drm_i915_gem_object *obj);
 
+static inline bool
+i915_gem_object_is_framebuffer(const struct drm_i915_gem_object *obj)
+{
+	return READ_ONCE(obj->framebuffer_references);
+}
+
 static inline unsigned int
 i915_gem_object_get_tiling(struct drm_i915_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 7e3bb48e043e..630697001b38 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -210,7 +210,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
 
 			if (!(flags & I915_SHRINK_ACTIVE) &&
 			    (i915_gem_object_is_active(obj) ||
-			     atomic_read(&obj->framebuffer_references)))
+			     i915_gem_object_is_framebuffer(obj)))
 				continue;
 
 			if (!can_release_pages(obj))
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index c1d669e32f41..ad5e05f6b836 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -238,7 +238,7 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
 	if ((tiling | stride) == obj->tiling_and_stride)
 		return 0;
 
-	if (atomic_read(&obj->framebuffer_references))
+	if (i915_gem_object_is_framebuffer(obj))
 		return -EBUSY;
 
 	/* We need to rebind the object if its current allocation
@@ -258,6 +258,12 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
 	if (err)
 		return err;
 
+	i915_gem_object_lock(obj);
+	if (i915_gem_object_is_framebuffer(obj)) {
+		i915_gem_object_unlock(obj);
+		return -EBUSY;
+	}
+
 	/* If the memory has unknown (i.e. varying) swizzling, we pin the
 	 * pages to prevent them being swapped out and causing corruption
 	 * due to the change in swizzling.
@@ -294,6 +300,7 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
 	}
 
 	obj->tiling_and_stride = tiling | stride;
+	i915_gem_object_unlock(obj);
 
 	/* Force the fence to be reacquired for GTT access */
 	i915_gem_release_mmap(obj);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 77936ddd860a..62a1e628e399 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14255,7 +14255,10 @@ static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
 
 	drm_framebuffer_cleanup(fb);
 
-	WARN_ON(atomic_dec_return(&intel_fb->obj->framebuffer_references) < 0);
+	i915_gem_object_lock(intel_fb->obj);
+	WARN_ON(!intel_fb->obj->framebuffer_references--);
+	i915_gem_object_unlock(intel_fb->obj);
+
 	i915_gem_object_put(intel_fb->obj);
 
 	kfree(intel_fb);
@@ -14332,12 +14335,16 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 				  struct drm_mode_fb_cmd2 *mode_cmd)
 {
 	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
-	unsigned int tiling = i915_gem_object_get_tiling(obj);
-	u32 pitch_limit, stride_alignment;
 	struct drm_format_name_buf format_name;
+	u32 pitch_limit, stride_alignment;
+	unsigned int tiling, stride;
 	int ret = -EINVAL;
 
-	atomic_inc(&obj->framebuffer_references);
+	i915_gem_object_lock(obj);
+	obj->framebuffer_references++;
+	tiling = i915_gem_object_get_tiling(obj);
+	stride = i915_gem_object_get_stride(obj);
+	i915_gem_object_unlock(obj);
 
 	if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
 		/*
@@ -14409,11 +14416,9 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 	 * If there's a fence, enforce that
 	 * the fb pitch and fence stride match.
 	 */
-	if (tiling != I915_TILING_NONE &&
-	    mode_cmd->pitches[0] != i915_gem_object_get_stride(obj)) {
+	if (tiling != I915_TILING_NONE && mode_cmd->pitches[0] !=  stride) {
 		DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n",
-			  mode_cmd->pitches[0],
-			  i915_gem_object_get_stride(obj));
+			  mode_cmd->pitches[0], stride);
 		goto err;
 	}
 
@@ -14494,7 +14499,9 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 	return 0;
 
 err:
-	atomic_dec(&obj->framebuffer_references);
+	i915_gem_object_lock(obj);
+	obj->framebuffer_references--;
+	i915_gem_object_unlock(obj);
 	return ret;
 }
 
-- 
2.11.0



More information about the Intel-gfx mailing list