All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH] drm/i915: kill lpt pch transcoder->crtc mapping code for fifo underruns
Date: Wed, 12 Jun 2013 16:46:04 +0200	[thread overview]
Message-ID: <1371048364-6281-1-git-send-email-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <CA+gsUGRabR5i0rs8Wa_VEpTKM9=tcnYU0xJ_8B8bTFjL8dhNSg@mail.gmail.com>

It's racy: There's no guarantee that we won't walk this code (due to a
pch fifo underrun interrupt) while someone is changing the pointers
around.

The only reason we do this is to use the righ crtc for the pch fifo
underrun accounting. But we never expose this to userspace, so
essentially no one really cares if we use the "wrong" crtc.

So let's just rip it out.

With this patch fifo underrun code will always use crtc A for tracking
underruns on the (only) pch transcoder on LPT.

v2: Add a big comment explaining what's going on. Requested by Paulo.

Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bb26555..e80c610 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -193,13 +193,13 @@ static void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
 	POSTING_READ(SDEIMR);
 }
 
-static void ibx_set_fifo_underrun_reporting(struct intel_crtc *crtc,
+static void ibx_set_fifo_underrun_reporting(struct drm_device *dev,
+					    enum transcoder pch_transcoder,
 					    bool enable)
 {
-	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint32_t bit = (crtc->pipe == PIPE_A) ? SDE_TRANSA_FIFO_UNDER :
-						SDE_TRANSB_FIFO_UNDER;
+	uint32_t bit = (pch_transcoder == TRANSCODER_A) ?
+		       SDE_TRANSA_FIFO_UNDER : SDE_TRANSB_FIFO_UNDER;
 
 	ibx_display_interrupt_update(dev_priv, bit,
 				     enable ? bit : 0);
@@ -292,29 +292,19 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
 					   bool enable)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum pipe p;
-	struct drm_crtc *crtc;
-	struct intel_crtc *intel_crtc;
+	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pch_transcoder];
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	unsigned long flags;
 	bool ret;
 
-	if (HAS_PCH_LPT(dev)) {
-		crtc = NULL;
-		for_each_pipe(p) {
-			struct drm_crtc *c = dev_priv->pipe_to_crtc_mapping[p];
-			if (intel_pipe_has_type(c, INTEL_OUTPUT_ANALOG)) {
-				crtc = c;
-				break;
-			}
-		}
-		if (!crtc) {
-			DRM_ERROR("PCH FIFO underrun, but no CRTC using the PCH found\n");
-			return false;
-		}
-	} else {
-		crtc = dev_priv->pipe_to_crtc_mapping[pch_transcoder];
-	}
-	intel_crtc = to_intel_crtc(crtc);
+	/*
+	 * NOTE: Pre-LPT has a fixed cpu pipe -> pch transcoder mapping, but LPT
+	 * has has only one pch transcoder A that all pipes can use. To avoid
+	 * racy pch transcoder -> pipe lookups from interrupt code simply store
+	 * the underrun statistics in crtc A. Since we never expose this
+	 * anywhere nor use it outside of the fifo underrun code here using the
+	 * "wrong" crtc on LPT won't cause issues.
+	 */
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
 
@@ -326,7 +316,7 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
 	intel_crtc->pch_fifo_underrun_disabled = !enable;
 
 	if (HAS_PCH_IBX(dev))
-		ibx_set_fifo_underrun_reporting(intel_crtc, enable);
+		ibx_set_fifo_underrun_reporting(dev, pch_transcoder, enable);
 	else
 		cpt_set_fifo_underrun_reporting(dev, pch_transcoder, enable);
 
-- 
1.8.1.4

  reply	other threads:[~2013-06-12 14:46 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-12 11:37 [PATCH 00/24] irq locking review Daniel Vetter
2013-06-12 11:37 ` [PATCH 01/24] drm/i915: fix locking around ironlake_enable|disable_display_irq Daniel Vetter
2013-06-25 12:26   ` [PATCH] " Daniel Vetter
2013-06-26 21:15     ` Paulo Zanoni
2013-06-27 10:37       ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 02/24] drm/i915: close tiny race in the ilk pcu even interrupt setup Daniel Vetter
2013-06-26 21:20   ` Paulo Zanoni
2013-06-12 11:37 ` [PATCH 03/24] drm/i915: assert_spin_locked for pipestat interrupt enable/disable Daniel Vetter
2013-06-26 21:44   ` Paulo Zanoni
2013-06-12 11:37 ` [PATCH 04/24] drm/i915: s/hotplug_irq_storm_detect/intel_hpd_irq_handler/ Daniel Vetter
2013-06-12 14:22   ` Egbert Eich
2013-06-12 11:37 ` [PATCH 05/24] drm/i915: fold the hpd_irq_setup call into intel_hpd_irq_handler Daniel Vetter
     [not found]   ` <20920.34096.704203.67316@linux-qknr.site>
2013-06-12 15:00     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 06/24] drm/i915: fold the queue_work " Daniel Vetter
2013-06-12 14:29   ` Egbert Eich
2013-06-12 11:37 ` [PATCH 07/24] drm/i915: fold the no-irq check " Daniel Vetter
2013-06-12 14:33   ` Egbert Eich
2013-06-26 22:35     ` Paulo Zanoni
2013-06-27 10:39       ` Daniel Vetter
2013-06-27 11:44       ` [PATCH 1/8] drm/i915: fix locking around ironlake_enable|disable_display_irq Daniel Vetter
2013-06-27 11:44         ` [PATCH 2/8] drm/i915: close tiny race in the ilk pcu even interrupt setup Daniel Vetter
2013-06-27 11:45         ` [PATCH 3/8] drm/i915: assert_spin_locked for pipestat interrupt enable/disable Daniel Vetter
2013-06-27 11:45         ` [PATCH 4/8] drm/i915: s/hotplug_irq_storm_detect/intel_hpd_irq_handler/ Daniel Vetter
2013-06-27 11:45         ` [PATCH 5/8] drm/i915: fold the hpd_irq_setup call into intel_hpd_irq_handler Daniel Vetter
2013-06-27 11:45         ` [PATCH 6/8] drm/i915: fold the queue_work " Daniel Vetter
2013-06-27 12:13           ` Chris Wilson
2013-06-27 11:45         ` [PATCH 7/8] drm/i915: fold the no-irq check " Daniel Vetter
2013-06-27 12:14           ` Chris Wilson
2013-06-27 11:45         ` [PATCH 8/8] drm/i915: fix hpd interrupt register locking Daniel Vetter
2013-06-27 14:41           ` Paulo Zanoni
2013-06-27 15:52             ` [PATCH 1/6] drm/i915: assert_spin_locked for pipestat interrupt enable/disable Daniel Vetter
2013-06-27 15:52               ` [PATCH 2/6] drm/i915: s/hotplug_irq_storm_detect/intel_hpd_irq_handler/ Daniel Vetter
2013-06-27 15:52               ` [PATCH 3/6] drm/i915: fold the hpd_irq_setup call into intel_hpd_irq_handler Daniel Vetter
2013-06-27 15:52               ` [PATCH 4/6] drm/i915: fold the queue_work " Daniel Vetter
2013-06-27 15:52               ` [PATCH 5/6] drm/i915: fold the no-irq check " Daniel Vetter
2013-06-27 15:52               ` [PATCH 6/6] drm/i915: fix hpd interrupt register locking Daniel Vetter
2013-07-04 19:22               ` [PATCH 1/6] drm/i915: assert_spin_locked for pipestat interrupt enable/disable Daniel Vetter
2013-06-27 17:44             ` [PATCH 8/8] drm/i915: fix hpd interrupt register locking Daniel Vetter
2013-06-12 11:37 ` [PATCH 08/24] " Daniel Vetter
2013-06-12 14:59   ` Egbert Eich
2013-06-12 15:10     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 09/24] drm/i915: extract ibx_display_interrupt_update Daniel Vetter
2013-06-25 12:27   ` [PATCH] " Daniel Vetter
2013-06-12 11:37 ` [PATCH 10/24] drm/i915: remove SERR_INT clearing in the postinstall hook Daniel Vetter
2013-06-27 19:34   ` Paulo Zanoni
2013-07-04 19:49     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 11/24] drm/i915: improve SERR_INT clearing for fifo underrun reporting Daniel Vetter
2013-06-27 20:19   ` Paulo Zanoni
2013-07-04 19:55     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 12/24] drm/i915: improve GEN7_ERR_INT " Daniel Vetter
2013-06-12 11:37 ` [PATCH 13/24] drm/i915: kill lpt pch transcoder->crtc mapping code for fifo underruns Daniel Vetter
2013-06-12 13:04   ` Paulo Zanoni
2013-06-12 14:46     ` Daniel Vetter [this message]
2013-06-27 20:45       ` [PATCH] " Paulo Zanoni
2013-07-04 20:41         ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 14/24] drm/i915: irq handlers don't need interrupt-safe spinlocks Daniel Vetter
2013-06-25 12:27   ` [PATCH] " Daniel Vetter
2013-06-27 21:14     ` Paulo Zanoni
2013-06-27 22:40       ` Daniel Vetter
2013-06-28 16:57         ` Paulo Zanoni
2013-06-12 11:37 ` [PATCH 15/24] drm/i915: streamline hsw_pm_irq_handler Daniel Vetter
2013-06-25 12:28   ` [PATCH] " Daniel Vetter
2013-06-12 11:37 ` [PATCH 16/24] drm/i915: queue work outside spinlock in hsw_pm_irq_handler Daniel Vetter
2013-06-12 11:37 ` [PATCH 17/24] drm/i915: kill dev_priv->rps.lock Daniel Vetter
2013-06-28  3:35   ` Ben Widawsky
2013-06-28  3:35   ` Ben Widawsky
2013-06-12 11:37 ` [PATCH 18/24] drm/i915: unify ring irq refcounts (again) Daniel Vetter
2013-06-28 17:24   ` Ben Widawsky
2013-07-04 20:52     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 19/24] drm/i915: don't enable PM_VEBOX_CS_ERROR_INTERRUPT Daniel Vetter
2013-06-12 17:13   ` Ben Widawsky
2013-06-12 17:18     ` Daniel Vetter
2013-06-12 18:19       ` Ben Widawsky
2013-06-12 18:32         ` Daniel Vetter
2013-06-12 18:51           ` Ben Widawsky
2013-06-28 17:25           ` Ben Widawsky
2013-06-12 11:37 ` [PATCH 20/24] drm/i915: kill bogus GTIIR clearing in vlv_preinstall hook Daniel Vetter
2013-06-28 17:01   ` Ben Widawsky
2013-07-04 20:56     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 21/24] drm/i915: unify PM interrupt preinstall sequence Daniel Vetter
2013-06-28 17:26   ` Ben Widawsky
2013-07-04 21:03     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 22/24] drm/i915: unify GT/PM irq postinstall code Daniel Vetter
2013-06-12 11:37 ` [PATCH 23/24] drm/i915: extract rps interrupt enable/disable helpers Daniel Vetter
2013-06-12 11:37 ` [PATCH 24/24] drm/i915: simplify rps interrupt enabling/disabling sequence Daniel Vetter
2013-06-12 22:32   ` Ben Widawsky

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=1371048364-6281-1-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.