| From f616f2830c1ed79245cfeca900f7e8a3b3c08c06 Mon Sep 17 00:00:00 2001 |
| From: Lionel Landwerlin <lionel.g.landwerlin@intel.com> |
| Date: Thu, 1 Mar 2018 11:06:13 +0000 |
| Subject: drm/i915/perf: fix perf stream opening lock |
| |
| From: Lionel Landwerlin <lionel.g.landwerlin@intel.com> |
| |
| 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 <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.landwerlin@intel.com |
| 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+ |
| (cherry picked from commit 41d3fdcd15d5ecf29cc73e8b79c2327ebb54b960) |
| Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/gpu/drm/i915/i915_perf.c | 38 ++++++++++++-------------------------- |
| 1 file changed, 12 insertions(+), 26 deletions(-) |
| |
| --- 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(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); |
| |
| @@ -1754,22 +1753,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); |
| @@ -1817,8 +1807,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: |