This is a backport of commit f616f2830c1ed79245cfeca900f7e8a3b3c08c06 sent for fixes on 4.14+ (but didn't apply cleanly on linux-4.14.y, linux-4.15.y). This upstream commit for this change is 41d3fdcd15d5ecf29cc73e8b79c2327ebb54b960.
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 lionel.g.landwerlin@intel.com Reviewed-by: Matthew Auld matthew.auld@intel.com Reviewed-by: Chris Wilson chris@chris-wilson.co.uk 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/20180301110613.1737-1-lionel.g... Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: intel-gfx@lists.freedesktop.org Cc: stable@vger.kernel.org # v4.14, v4.15 (cherry picked from commit f616f2830c1ed79245cfeca900f7e8a3b3c08c06) Signed-off-by: Rodrigo Vivi rodrigo.vivi@intel.com --- drivers/gpu/drm/i915/i915_perf.c | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 370b9d248fed..3e49317f3ec3 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1300,9 +1300,8 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream) */ 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);
@@ -1754,22 +1753,13 @@ static int gen8_switch_to_updated_kernel_context(struct drm_i915_private *dev_pr * 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); @@ -1817,8 +1807,6 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv, }
out: - mutex_unlock(&dev_priv->drm.struct_mutex); - return ret; }
@@ -1862,7 +1850,7 @@ static int gen8_enable_metric_set(struct drm_i915_private *dev_priv, * 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 drm_i915_private *dev_priv, 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 i915_perf_stream *stream, 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 i915_perf_stream *stream,
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: