Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Ashar Shaikh <azhar.shaikh@intel.com>,
	Paulo Zanoni <paulo.r.zanoni@intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: [PATCH 4/4] drm/i915: Also disable SAGV on fifo underrun.
Date: Mon, 26 Feb 2018 12:53:09 -0800	[thread overview]
Message-ID: <20180226205309.2438-4-rodrigo.vivi@intel.com> (raw)
In-Reply-To: <20180226205309.2438-1-rodrigo.vivi@intel.com>

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@intel.com>
Cc: Ashar Shaikh <azhar.shaikh@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2018-02-26 20:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-26 20:53 [PATCH 1/4] drm/i915: Don't propagate SAGV errnos in vain Rodrigo Vivi
2018-02-26 20:53 ` [PATCH 2/4] drm/i915: Introduce SAGV mutex Rodrigo Vivi
2018-02-26 21:21   ` Chris Wilson
2018-02-26 22:20     ` Rodrigo Vivi
2018-02-26 20:53 ` [PATCH 3/4] drm/i915: Move underrun work from fbc to fifo_underrun Rodrigo Vivi
2018-02-26 21:00   ` Chris Wilson
2018-02-26 22:23     ` Rodrigo Vivi
2018-02-26 20:53 ` Rodrigo Vivi [this message]
2018-02-26 21:25 ` ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Don't propagate SAGV errnos in vain Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180226205309.2438-4-rodrigo.vivi@intel.com \
    --to=rodrigo.vivi@intel.com \
    --cc=azhar.shaikh@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox