On underrun situations and SAGV enabled we can face hard hangs. So let's reuse the FBC workaround and expand that to SAGV on the hope that it is not already too late for that. Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> Cc: Ashar Shaikh <azhar.shaikh@xxxxxxxxx> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> --- drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_fbc.c | 3 ++ drivers/gpu/drm/i915/intel_fifo_underrun.c | 17 +++++------ drivers/gpu/drm/i915/intel_pm.c | 45 ++++++++++++++++++++++++------ 4 files changed, 50 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 3ca27cc8700d..169af853f6ff 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2003,10 +2003,12 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc, struct skl_pipe_wm *out); void g4x_wm_sanitize(struct drm_i915_private *dev_priv); void vlv_wm_sanitize(struct drm_i915_private *dev_priv); +bool intel_has_sagv(struct drm_i915_private *dev_priv); bool intel_can_enable_sagv(struct drm_atomic_state *state); u8 intel_sagv_block_time(const struct drm_i915_private *dev_priv); void intel_enable_sagv(struct drm_i915_private *dev_priv); void intel_disable_sagv(struct drm_i915_private *dev_priv); +void intel_sagv_underrun_detected(struct drm_i915_private *dev_priv); bool skl_wm_level_equals(const struct skl_wm_level *l1, const struct skl_wm_level *l2); bool skl_ddb_allocation_overlaps(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index c63568564b77..a5d980227708 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -1235,6 +1235,9 @@ void intel_fbc_underrun_detected(struct drm_i915_private *dev_priv) { struct intel_fbc *fbc = &dev_priv->fbc; + if (!fbc_supported(dev_priv)) + return; + mutex_lock(&fbc->lock); /* Maybe we were scheduled twice. */ diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c index 6a290621177b..dea189b83827 100644 --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c @@ -356,23 +356,24 @@ static void intel_underrun_work_fn(struct work_struct *work) container_of(work, struct drm_i915_private, underrun.work); intel_fbc_underrun_detected(dev_priv); + intel_sagv_underrun_detected(dev_priv); } /* - * intel_handle_fifo_underrun_irq - disable FBC when we get a FIFO underrun + * intel_handle_fifo_underrun_irq - disable FBC and SAGV on a FIFO underrun * - * Without FBC, most underruns are harmless and don't really cause too many - * problems, except for an annoying message on dmesg. With FBC, underruns can - * become black screens or even worse, especially when paired with bad - * watermarks. So in order for us to be on the safe side, completely disable FBC - * in case we ever detect a FIFO underrun on any pipe. An underrun on any pipe - * already suggests that watermarks may be bad, so try to be as safe as + * Without FBC or SAGV, most underruns are harmless and don't really cause too + * many problems, except for an annoying message on dmesg. With them, underruns + * can become black screens or even worse, especially when paired with bad + * watermarks. So in order for us to be on the safe side, completely disable + * them in case we ever detect a FIFO underrun on any pipe. An underrun on any + * pipe already suggests that watermarks may be bad, so try to be as safe as * possible. * */ static void intel_handle_fifo_underrun_irq(struct drm_i915_private *dev_priv) { - if (!HAS_FBC(dev_priv)) + if (!HAS_FBC(dev_priv) && !intel_has_sagv(dev_priv)) return; spin_lock(&dev_priv->underrun.lock); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e1ec9c2fd08a..84edaf161fe2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3581,8 +3581,7 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state) return false; } -static bool -intel_has_sagv(struct drm_i915_private *dev_priv) +bool intel_has_sagv(struct drm_i915_private *dev_priv) { bool ret = false; @@ -3648,6 +3647,22 @@ void intel_enable_sagv(struct drm_i915_private *dev_priv) mutex_unlock(&dev_priv->sagv.lock); } +static int __intel_disable_sagv(struct drm_i915_private *dev_priv) +{ + int ret; + + mutex_lock(&dev_priv->pcu_lock); + + /* bspec says to keep retrying for at least 1 ms */ + ret = skl_pcode_request(dev_priv, GEN9_PCODE_SAGV_CONTROL, + GEN9_SAGV_DISABLE, + GEN9_SAGV_IS_DISABLED, GEN9_SAGV_IS_DISABLED, + 1); + mutex_unlock(&dev_priv->pcu_lock); + + return ret; +} + void intel_disable_sagv(struct drm_i915_private *dev_priv) { int ret; @@ -3660,14 +3675,8 @@ void intel_disable_sagv(struct drm_i915_private *dev_priv) goto out; DRM_DEBUG_KMS("Disabling the SAGV\n"); - mutex_lock(&dev_priv->pcu_lock); - /* bspec says to keep retrying for at least 1 ms */ - ret = skl_pcode_request(dev_priv, GEN9_PCODE_SAGV_CONTROL, - GEN9_SAGV_DISABLE, - GEN9_SAGV_IS_DISABLED, GEN9_SAGV_IS_DISABLED, - 1); - mutex_unlock(&dev_priv->pcu_lock); + ret = __intel_disable_sagv(dev_priv); /* * Some skl systems, pre-release machines in particular, @@ -3687,6 +3696,24 @@ void intel_disable_sagv(struct drm_i915_private *dev_priv) mutex_unlock(&dev_priv->sagv.lock); } +void intel_sagv_underrun_detected(struct drm_i915_private *dev_priv) +{ + if (!intel_has_sagv(dev_priv)) + return; + + mutex_lock(&dev_priv->sagv.lock); + + if (dev_priv->sagv.status == I915_SAGV_NOT_CONTROLLED) + goto out; + + DRM_DEBUG_KMS("Disabling SAGV due to FIFO underrun.\n"); + __intel_disable_sagv(dev_priv); + dev_priv->sagv.status = I915_SAGV_NOT_CONTROLLED; + +out: + mutex_unlock(&dev_priv->sagv.lock); +} + bool intel_can_enable_sagv(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; -- 2.13.6 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx