Skip to contentSkip to conversation

Patch "drm/i915/perf: fix perf stream opening lock" has been added to the 4.15-stable tree

PATCH [email protected] 1 participant 1 message Mar 12, 2018

Subsystems (3)
Stable branch DRM drivers Intel i915
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

drm/i915/perf: fix perf stream opening lock

to the 4.15-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
drm-i915-perf-fix-perf-stream-opening-lock.patch
and it can be found in the queue-4.15 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From f616f2830c1ed79245cfeca900f7e8a3b3c08c06 Mon Sep 17 00:00:00 2001

From: Lionel Landwerlin <[email protected]>
Date: Thu, 1 Mar 2018 11:06:13 +0000
Subject: drm/i915/perf: fix perf stream opening lock

From: Lionel Landwerlin <[email protected]>

commit f616f2830c1ed79245cfeca900f7e8a3b3c08c06 upstream.

We're seeing on CI that some contexts don't have the programmed OA period timer that directs the OA unit on how often to write reports.

The issue is that we're not holding the drm lock from when we edit the context images down to when we set the exclusive_stream variable. This leaves a window for the deferred context allocation to call i915_oa_init_reg_state() that will not program the expected OA timer value, because we haven't set the exclusive_stream yet.

v2: Drop need_lock from gen8_configure_all_contexts() (Matt)

Signed-off-by Lionel Landwerlin <[email protected]>
Reviewed-by Matthew Auld <[email protected]>
Reviewed-by Chris Wilson <[email protected]>
Fixes 701f8231a2f ("drm/i915/perf: prune OA configs")

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102254
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103715
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103755

Link https://patchwork.freedesktop.org/patch/msgid/[email protected]
Cc Jani Nikula <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Cc [email protected]
Cc <[email protected]> # v4.14+
(cherry picked from commit 41d3fdcd15d5ecf29cc73e8b79c2327ebb54b960)
Signed-off-by Rodrigo Vivi <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/gpu/drm/i915/i915_perf.c |   38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)
drivers/gpu/drm/i915/i915_perf.c +12 -26
@@ -1301,9 +1301,8 @@ static void i915_oa_stream_destroy(struc
 	 */
 	mutex_lock(&dev_priv->drm.struct_mutex);
 	dev_priv->perf.oa.exclusive_stream = NULL;
-	mutex_unlock(&dev_priv->drm.struct_mutex);
-
 	dev_priv->perf.oa.ops.disable_metric_set(dev_priv);
+	mutex_unlock(&dev_priv->drm.struct_mutex);
 
 	free_oa_buffer(dev_priv);
 

@@ -1755,22 +1754,13 @@ static int gen8_switch_to_updated_kernel
  * Note: it's only the RCS/Render context that has any OA state.
  */
 static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
-				       const struct i915_oa_config *oa_config,
-				       bool interruptible)
+				       const struct i915_oa_config *oa_config)
 {
 	struct i915_gem_context *ctx;
 	int ret;
 	unsigned int wait_flags = I915_WAIT_LOCKED;
 
-	if (interruptible) {
-		ret = i915_mutex_lock_interruptible(&dev_priv->drm);
-		if (ret)
-			return ret;
-
-		wait_flags |= I915_WAIT_INTERRUPTIBLE;
-	} else {
-		mutex_lock(&dev_priv->drm.struct_mutex);
-	}
+	lockdep_assert_held(&dev_priv->drm.struct_mutex);
 
 	/* Switch away from any user context. */
 	ret = gen8_switch_to_updated_kernel_context(dev_priv, oa_config);

@@ -1818,8 +1808,6 @@ static int gen8_configure_all_contexts(s
 	}
 
  out:
-	mutex_unlock(&dev_priv->drm.struct_mutex);
-
 	return ret;
 }
 

@@ -1862,7 +1850,7 @@ static int gen8_enable_metric_set(struct
 	 * to make sure all slices/subslices are ON before writing to NOA
 	 * registers.
 	 */
-	ret = gen8_configure_all_contexts(dev_priv, oa_config, true);
+	ret = gen8_configure_all_contexts(dev_priv, oa_config);
 	if (ret)
 		return ret;
 

@@ -1877,7 +1865,7 @@ static int gen8_enable_metric_set(struct
 static void gen8_disable_metric_set(struct drm_i915_private *dev_priv)
 {
 	/* Reset all contexts' slices/subslices configurations. */
-	gen8_configure_all_contexts(dev_priv, NULL, false);
+	gen8_configure_all_contexts(dev_priv, NULL);
 
 	I915_WRITE(GDT_CHICKEN_BITS, (I915_READ(GDT_CHICKEN_BITS) &
 				      ~GT_NOA_ENABLE));

@@ -2127,6 +2115,10 @@ static int i915_oa_stream_init(struct i9
 	if (ret)
 		goto err_oa_buf_alloc;
 
+	ret = i915_mutex_lock_interruptible(&dev_priv->drm);
+	if (ret)
+		goto err_lock;
+
 	ret = dev_priv->perf.oa.ops.enable_metric_set(dev_priv,
 						      stream->oa_config);
 	if (ret)

@@ -2134,23 +2126,17 @@ static int i915_oa_stream_init(struct i9
 
 	stream->ops = &i915_oa_stream_ops;
 
-	/* Lock device for exclusive_stream access late because
-	 * enable_metric_set() might lock as well on gen8+.
-	 */
-	ret = i915_mutex_lock_interruptible(&dev_priv->drm);
-	if (ret)
-		goto err_lock;
-
 	dev_priv->perf.oa.exclusive_stream = stream;
 
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
 	return 0;
 
-err_lock:
+err_enable:
 	dev_priv->perf.oa.ops.disable_metric_set(dev_priv);
+	mutex_unlock(&dev_priv->drm.struct_mutex);
 
-err_enable:
+err_lock:
 	free_oa_buffer(dev_priv);
 
 err_oa_buf_alloc:

Patches currently in stable-queue which might be from [email protected] are

queue-4.15/drm-i915-perf-fix-perf-stream-opening-lock.patch