[PATCH v2 1/6] drm/i915: Lock out execlist tasklet while peeking inside for busy-stats

[PATCH v2 1/6] drm/i915: Lock out execlist tasklet while peeking inside for busy-stats

[Chris Wilson]

In order to prevent a race condition where we may end up overaccounting
the active state and leaving the busy-stats believing the GPU is 100%
busy, lock out the tasklet while we reconstruct the busy state. There is
no direct spinlock guard for the execlists->port[], so we need to
utilise tasklet_disable() as a synchronous barrier to prevent the only
writer to execlists->port[] from running at the same time as the enable.

Fixes: 4900727d35bb ("drm/i915/pmu: Reconstruct active state on starting busy-stats")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index d790bdc227ff..b221610f2365 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1943,16 +1943,22 @@ intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance)
  */
 int intel_enable_engine_stats(struct intel_engine_cs *engine)
 {
+	struct intel_engine_execlists *execlists = &engine->execlists;
 	unsigned long flags;
+	int err = 0;
 
 	if (!intel_engine_supports_stats(engine))
 		return -ENODEV;
 
+	tasklet_disable(&execlists->tasklet);
 	spin_lock_irqsave(&engine->stats.lock, flags);
-	if (engine->stats.enabled == ~0)
-		goto busy;
+
+	if (unlikely(engine->stats.enabled == ~0)) {
+		err = -EBUSY;
+		goto unlock;
+	}
+
 	if (engine->stats.enabled++ == 0) {
-		struct intel_engine_execlists *execlists = &engine->execlists;
 		const struct execlist_port *port = execlists->port;
 		unsigned int num_ports = execlists_num_ports(execlists);
 
@@ -1967,14 +1973,12 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
 		if (engine->stats.active)
 			engine->stats.start = engine->stats.enabled_at;
 	}
-	spin_unlock_irqrestore(&engine->stats.lock, flags);
 
-	return 0;
-
-busy:
+unlock:
 	spin_unlock_irqrestore(&engine->stats.lock, flags);
+	tasklet_enable(&execlists->tasklet);
 
-	return -EBUSY;
+	return err;
 }
 
 static ktime_t __intel_engine_get_busy_time(struct intel_engine_cs *engine)
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/6] drm/i915: Disable preemption and sleeping while using the punit sideband

[Chris Wilson]

While we talk to the punit over its sideband, we need to prevent the cpu
from sleeping in order to prevent a potential machine hang.

Note that by itself, it appears that pm_qos_update_request (via
intel_idle) doesn't provide a sufficient barrier to ensure that all core
are indeed awake (out of Cstate) and that the package is awake. To do so,
we need to supplement the pm_qos with a manual ping on_each_cpu.

v2: Restrict the heavy-weight wakeup to just the ISOF_PORT_PUNIT, there
is insufficient evidence to implicate a wider problem atm. Similarly,
restrict the w/a to Valleyview, as Cherryview doesn't have an angry cadre
of users.

The working theory, courtesy of Ville and Hans, is the issue lies within
the power delivery and so is likely to be unit and board specific and
occurs when both the unit/fw require extra power at the same time as the
cpu package is changing its own power state.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109051
References: https://bugs.freedesktop.org/show_bug.cgi?id=102657
References: https://bugzilla.kernel.org/show_bug.cgi?id=195255
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c       |  6 +++
 drivers/gpu/drm/i915/i915_drv.h       |  1 +
 drivers/gpu/drm/i915/intel_sideband.c | 89 +++++++++++++++++++++++++++--------
 3 files changed, 77 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6c8da9d20c33..d4b90cc0130b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -902,6 +902,9 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	spin_lock_init(&dev_priv->uncore.lock);
 
 	mutex_init(&dev_priv->sb_lock);
+	pm_qos_add_request(&dev_priv->sb_qos,
+			   PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
+
 	mutex_init(&dev_priv->modeset_restore_lock);
 	mutex_init(&dev_priv->av_mutex);
 	mutex_init(&dev_priv->wm.wm_mutex);
@@ -953,6 +956,9 @@ static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
 	intel_irq_fini(dev_priv);
 	i915_workqueues_cleanup(dev_priv);
 	i915_engines_cleanup(dev_priv);
+
+	pm_qos_remove_request(&dev_priv->sb_qos);
+	mutex_destroy(&dev_priv->sb_lock);
 }
 
 static int i915_mmio_setup(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c42015b05b47..d95d8c3d04aa 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1886,6 +1886,7 @@ struct drm_i915_private {
 
 	/* Sideband mailbox protection */
 	struct mutex sb_lock;
+	struct pm_qos_request sb_qos;
 
 	/** Cached value of IMR to avoid reads in updating the bitfield */
 	union {
diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index 75c872bb8cc9..d56eda33734e 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -22,6 +22,8 @@
  *
  */
 
+#include <asm/iosf_mbi.h>
+
 #include "i915_drv.h"
 #include "intel_drv.h"
 
@@ -39,18 +41,48 @@
 /* Private register write, double-word addressing, non-posted */
 #define SB_CRWRDA_NP	0x07
 
-static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
-			   u32 port, u32 opcode, u32 addr, u32 *val)
+static void ping(void *info)
 {
-	u32 cmd, be = 0xf, bar = 0;
-	bool is_read = (opcode == SB_MRD_NP || opcode == SB_CRRDDA_NP);
+}
 
-	cmd = (devfn << IOSF_DEVFN_SHIFT) | (opcode << IOSF_OPCODE_SHIFT) |
-		(port << IOSF_PORT_SHIFT) | (be << IOSF_BYTE_ENABLES_SHIFT) |
-		(bar << IOSF_BAR_SHIFT);
+static void __vlv_punit_get(struct drm_i915_private *dev_priv)
+{
+	iosf_mbi_punit_acquire();
 
-	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
+	/*
+	 * Prevent the cpu from sleeping while we use this sideband, otherwise
+	 * the punit may cause a machine hang. The issue appears to be isolated
+	 * with changing the power state of the CPU package while changing
+	 * the power state via the punit, and we have only observed it
+	 * reliably on 4-core Baytail systems suggesting the issue is in the
+	 * power delivery mechanism and likely to be be board/function
+	 * specific. Hence we presume the workaround needs only be applied
+	 * to the Valleyview P-unit and not all sideband communications.
+	 */
+	if (IS_VALLEYVIEW(dev_priv)) {
+		pm_qos_update_request(&dev_priv->sb_qos, 0);
+		on_each_cpu(ping, NULL, 1);
+	}
+}
+
+static void __vlv_punit_put(struct drm_i915_private *dev_priv)
+{
+	if (IS_VALLEYVIEW(dev_priv))
+		pm_qos_update_request(&dev_priv->sb_qos, PM_QOS_DEFAULT_VALUE);
 
+	iosf_mbi_punit_release();
+}
+
+static int vlv_sideband_rw(struct drm_i915_private *dev_priv,
+			   u32 devfn, u32 port, u32 opcode,
+			   u32 addr, u32 *val)
+{
+	const bool is_read = (opcode == SB_MRD_NP || opcode == SB_CRRDDA_NP);
+	int err;
+
+	lockdep_assert_held(&dev_priv->sb_lock);
+
+	/* Flush the previous comms, just in case it failed last time. */
 	if (intel_wait_for_register(dev_priv,
 				    VLV_IOSF_DOORBELL_REQ, IOSF_SB_BUSY, 0,
 				    5)) {
@@ -59,22 +91,33 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
 		return -EAGAIN;
 	}
 
-	I915_WRITE(VLV_IOSF_ADDR, addr);
-	I915_WRITE(VLV_IOSF_DATA, is_read ? 0 : *val);
-	I915_WRITE(VLV_IOSF_DOORBELL_REQ, cmd);
-
-	if (intel_wait_for_register(dev_priv,
-				    VLV_IOSF_DOORBELL_REQ, IOSF_SB_BUSY, 0,
-				    5)) {
+	preempt_disable();
+
+	I915_WRITE_FW(VLV_IOSF_ADDR, addr);
+	I915_WRITE_FW(VLV_IOSF_DATA, is_read ? 0 : *val);
+	I915_WRITE_FW(VLV_IOSF_DOORBELL_REQ,
+		      (devfn << IOSF_DEVFN_SHIFT) |
+		      (opcode << IOSF_OPCODE_SHIFT) |
+		      (port << IOSF_PORT_SHIFT) |
+		      (0xf << IOSF_BYTE_ENABLES_SHIFT) |
+		      (0 << IOSF_BAR_SHIFT) |
+		      IOSF_SB_BUSY);
+
+	if (__intel_wait_for_register_fw(dev_priv,
+					 VLV_IOSF_DOORBELL_REQ, IOSF_SB_BUSY, 0,
+					 10000, 0, NULL) == 0) {
+		if (is_read)
+			*val = I915_READ_FW(VLV_IOSF_DATA);
+		err = 0;
+	} else {
 		DRM_DEBUG_DRIVER("IOSF sideband finish wait (%s) timed out\n",
 				 is_read ? "read" : "write");
-		return -ETIMEDOUT;
+		err = -ETIMEDOUT;
 	}
 
-	if (is_read)
-		*val = I915_READ(VLV_IOSF_DATA);
+	preempt_enable();
 
-	return 0;
+	return err;
 }
 
 u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr)
@@ -84,8 +127,12 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr)
 	WARN_ON(!mutex_is_locked(&dev_priv->pcu_lock));
 
 	mutex_lock(&dev_priv->sb_lock);
+	__vlv_punit_get(dev_priv);
+
 	vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
 			SB_CRRDDA_NP, addr, &val);
+
+	__vlv_punit_put(dev_priv);
 	mutex_unlock(&dev_priv->sb_lock);
 
 	return val;
@@ -98,8 +145,12 @@ int vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val)
 	WARN_ON(!mutex_is_locked(&dev_priv->pcu_lock));
 
 	mutex_lock(&dev_priv->sb_lock);
+	__vlv_punit_get(dev_priv);
+
 	err = vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
 			      SB_CRWRDA_NP, addr, &val);
+
+	__vlv_punit_put(dev_priv);
 	mutex_unlock(&dev_priv->sb_lock);
 
 	return err;
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/6] drm/i915: Lift acquiring the vlv punit magic to a common sb-get

[Chris Wilson]

As we now employ a very heavy pm_qos around the punit access, we want to
minimise the number of synchronous requests by performing one for the
whole punit sequence rather than around individual accesses. The
sideband lock is used for this, so push the pm_qos into the sideband
lock acquisition and release, moving it from the lowlevel punit rw
routine to the callers. In the first step, we move the punit magic into
the common sideband lock so that we can acquire a bunch of ports
simultaneously, and if need be extend the workaround protection later.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h         |  44 ++++++++++--
 drivers/gpu/drm/i915/intel_cdclk.c      |   6 +-
 drivers/gpu/drm/i915/intel_display.c    |  37 +++++-----
 drivers/gpu/drm/i915/intel_dp.c         |   4 +-
 drivers/gpu/drm/i915/intel_dpio_phy.c   |  37 +++++-----
 drivers/gpu/drm/i915/intel_dsi.c        |   8 +--
 drivers/gpu/drm/i915/intel_dsi_pll.c    |  14 ++--
 drivers/gpu/drm/i915/intel_dsi_vbt.c    |   8 +--
 drivers/gpu/drm/i915/intel_hdmi.c       |   4 +-
 drivers/gpu/drm/i915/intel_pm.c         |   4 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c |   8 +--
 drivers/gpu/drm/i915/intel_sideband.c   | 115 +++++++++++++++++++++++++++-----
 12 files changed, 207 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d95d8c3d04aa..80280f27601e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3723,25 +3723,61 @@ int skl_pcode_request(struct drm_i915_private *dev_priv, u32 mbox, u32 request,
 		      u32 reply_mask, u32 reply, int timeout_base_ms);
 
 /* intel_sideband.c */
+
+enum {
+	VLV_IOSF_SB_BUNIT,
+	VLV_IOSF_SB_CCK,
+	VLV_IOSF_SB_CCU,
+	VLV_IOSF_SB_DPIO,
+	VLV_IOSF_SB_FLISDSI,
+	VLV_IOSF_SB_GPIO,
+	VLV_IOSF_SB_NC,
+	VLV_IOSF_SB_PUNIT,
+};
+
+void vlv_iosf_sb_get(struct drm_i915_private *dev_priv, unsigned long ports);
+u32 vlv_iosf_sb_read(struct drm_i915_private *dev_priv, u8 port, u32 reg);
+void vlv_iosf_sb_write(struct drm_i915_private *dev_priv, u8 port, u32 reg, u32 val);
+void vlv_iosf_sb_put(struct drm_i915_private *dev_priv, unsigned long ports);
+
+void vlv_punit_get(struct drm_i915_private *dev_priv);
 u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr);
 int vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val);
+void vlv_punit_put(struct drm_i915_private *dev_priv);
+
+void vlv_nc_get(struct drm_i915_private *dev_priv);
 u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr);
-u32 vlv_iosf_sb_read(struct drm_i915_private *dev_priv, u8 port, u32 reg);
-void vlv_iosf_sb_write(struct drm_i915_private *dev_priv, u8 port, u32 reg, u32 val);
+void vlv_nc_put(struct drm_i915_private *dev_priv);
+
+void vlv_cck_get(struct drm_i915_private *dev_priv);
 u32 vlv_cck_read(struct drm_i915_private *dev_priv, u32 reg);
 void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
+void vlv_cck_put(struct drm_i915_private *dev_priv);
+
+void vlv_ccu_get(struct drm_i915_private *dev_priv);
 u32 vlv_ccu_read(struct drm_i915_private *dev_priv, u32 reg);
 void vlv_ccu_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
+void vlv_ccu_put(struct drm_i915_private *dev_priv);
+
+void vlv_bunit_get(struct drm_i915_private *dev_priv);
 u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg);
 void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
+void vlv_bunit_put(struct drm_i915_private *dev_priv);
+
+void vlv_dpio_get(struct drm_i915_private *dev_priv);
 u32 vlv_dpio_read(struct drm_i915_private *dev_priv, enum pipe pipe, int reg);
 void vlv_dpio_write(struct drm_i915_private *dev_priv, enum pipe pipe, int reg, u32 val);
+void vlv_dpio_put(struct drm_i915_private *dev_priv);
+
+void vlv_flisdsi_get(struct drm_i915_private *dev_priv);
+u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg);
+void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
+void vlv_flisdsi_put(struct drm_i915_private *dev_priv);
+
 u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
 		   enum intel_sbi_destination destination);
 void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
 		     enum intel_sbi_destination destination);
-u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg);
-void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
 
 /* intel_dpio_phy.c */
 void bxt_port_to_phy_channel(struct drm_i915_private *dev_priv, enum port port,
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index ca36321eafac..177bb79beee7 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -552,7 +552,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
 	}
 	mutex_unlock(&dev_priv->pcu_lock);
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_iosf_sb_get(dev_priv,
+		       	BIT(VLV_IOSF_SB_CCK) | BIT(VLV_IOSF_SB_BUNIT));
 
 	if (cdclk == 400000) {
 		u32 divider;
@@ -586,7 +587,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
 		val |= 3000 / 250; /* 3.0 usec */
 	vlv_bunit_write(dev_priv, BUNIT_REG_BISOC, val);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_iosf_sb_put(dev_priv,
+		       	BIT(VLV_IOSF_SB_CCK) | BIT(VLV_IOSF_SB_BUNIT));
 
 	intel_update_cdclk(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 221e3a183d36..5f0b9f66c8d8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -165,10 +165,10 @@ int vlv_get_hpll_vco(struct drm_i915_private *dev_priv)
 	int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
 
 	/* Obtain SKU information */
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_cck_get(dev_priv);
 	hpll_freq = vlv_cck_read(dev_priv, CCK_FUSE_REG) &
 		CCK_FUSE_HPLL_FREQ_MASK;
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_cck_put(dev_priv);
 
 	return vco_freq[hpll_freq] * 1000;
 }
@@ -179,9 +179,9 @@ int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
 	u32 val;
 	int divider;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_cck_get(dev_priv);
 	val = vlv_cck_read(dev_priv, reg);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_cck_put(dev_priv);
 
 	divider = val & CCK_FREQUENCY_VALUES;
 
@@ -1078,9 +1078,9 @@ void assert_dsi_pll(struct drm_i915_private *dev_priv, bool state)
 	u32 val;
 	bool cur_state;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_cck_get(dev_priv);
 	val = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_cck_put(dev_priv);
 
 	cur_state = val & DSI_PLL_VCO_EN;
 	I915_STATE_WARN(cur_state != state,
@@ -1428,14 +1428,14 @@ static void _chv_enable_pll(struct intel_crtc *crtc,
 	enum dpio_channel port = vlv_pipe_to_channel(pipe);
 	u32 tmp;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* Enable back the 10bit clock to display controller */
 	tmp = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
 	tmp |= DPIO_DCLKP_EN;
 	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), tmp);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 
 	/*
 	 * Need to wait > 100ns between dclkp clock enable bit and PLL enable.
@@ -1620,14 +1620,14 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	I915_WRITE(DPLL(pipe), val);
 	POSTING_READ(DPLL(pipe));
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* Disable 10bit clock to display controller */
 	val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port));
 	val &= ~DPIO_DCLKP_EN;
 	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW14(port), val);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 }
 
 void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
@@ -6638,7 +6638,7 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
 		return;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	bestn = pipe_config->dpll.n;
 	bestm1 = pipe_config->dpll.m1;
@@ -6715,7 +6715,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW7(pipe), coreclk);
 
 	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW11(pipe), 0x87871000);
-	mutex_unlock(&dev_priv->sb_lock);
+
+	vlv_dpio_put(dev_priv);
 }
 
 static void chv_prepare_pll(struct intel_crtc *crtc,
@@ -6748,7 +6749,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
 	dpio_val = 0;
 	loopfilter = 0;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* p1 and p2 divider */
 	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW13(port),
@@ -6820,7 +6821,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
 			vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW14(port)) |
 			DPIO_AFC_RECAL);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 }
 
 /**
@@ -7422,9 +7423,9 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc,
 	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
 		return;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 	mdiv = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe));
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 
 	clock.m1 = (mdiv >> DPIO_M1DIV_SHIFT) & 7;
 	clock.m2 = mdiv & DPIO_M2DIV_MASK;
@@ -7524,13 +7525,13 @@ static void chv_crtc_clock_get(struct intel_crtc *crtc,
 	if ((pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE) == 0)
 		return;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 	cmn_dw13 = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW13(port));
 	pll_dw0 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW0(port));
 	pll_dw1 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW1(port));
 	pll_dw2 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW2(port));
 	pll_dw3 = vlv_dpio_read(dev_priv, pipe, CHV_PLL_DW3(port));
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 
 	clock.m1 = (pll_dw1 & 0x7) == DPIO_CHV_M1_DIV_BY_2 ? 2 : 0;
 	clock.m2 = (pll_dw0 & 0xff) << 22;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 68229f53d5b8..168f2c0de1b6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2771,12 +2771,12 @@ static void chv_post_disable_dp(struct intel_encoder *encoder,
 
 	intel_dp_link_down(encoder, old_crtc_state);
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* Assert data lane reset */
 	chv_data_lane_soft_reset(encoder, old_crtc_state, true);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
index 76473e9836c6..662d71cd501f 100644
--- a/drivers/gpu/drm/i915/intel_dpio_phy.c
+++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
@@ -645,7 +645,7 @@ void chv_set_phy_signal_level(struct intel_encoder *encoder,
 	u32 val;
 	int i;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* Clear calc init */
 	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW10(ch));
@@ -726,8 +726,7 @@ void chv_set_phy_signal_level(struct intel_encoder *encoder,
 		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW10(ch), val);
 	}
 
-	mutex_unlock(&dev_priv->sb_lock);
-
+	vlv_dpio_put(dev_priv);
 }
 
 void chv_data_lane_soft_reset(struct intel_encoder *encoder,
@@ -797,7 +796,7 @@ void chv_phy_pre_pll_enable(struct intel_encoder *encoder,
 
 	chv_phy_powergate_lanes(encoder, true, lane_mask);
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* Assert data lane reset */
 	chv_data_lane_soft_reset(encoder, crtc_state, true);
@@ -852,7 +851,7 @@ void chv_phy_pre_pll_enable(struct intel_encoder *encoder,
 		val |= CHV_CMN_USEDCLKCHANNEL;
 	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW19(ch), val);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 }
 
 void chv_phy_pre_encoder_enable(struct intel_encoder *encoder,
@@ -867,7 +866,7 @@ void chv_phy_pre_encoder_enable(struct intel_encoder *encoder,
 	int data, i, stagger;
 	u32 val;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* allow hardware to manage TX FIFO reset source */
 	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW11(ch));
@@ -932,7 +931,7 @@ void chv_phy_pre_encoder_enable(struct intel_encoder *encoder,
 	/* Deassert data lane reset */
 	chv_data_lane_soft_reset(encoder, crtc_state, false);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 }
 
 void chv_phy_release_cl2_override(struct intel_encoder *encoder)
@@ -953,7 +952,7 @@ void chv_phy_post_pll_disable(struct intel_encoder *encoder,
 	enum pipe pipe = to_intel_crtc(old_crtc_state->base.crtc)->pipe;
 	u32 val;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* disable left/right clock distribution */
 	if (pipe != PIPE_B) {
@@ -966,7 +965,7 @@ void chv_phy_post_pll_disable(struct intel_encoder *encoder,
 		vlv_dpio_write(dev_priv, pipe, _CHV_CMN_DW1_CH1, val);
 	}
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 
 	/*
 	 * Leave the power down bit cleared for at least one
@@ -990,7 +989,8 @@ void vlv_set_phy_signal_level(struct intel_encoder *encoder,
 	enum dpio_channel port = vlv_dport_to_channel(dport);
 	enum pipe pipe = intel_crtc->pipe;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
+
 	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW5(port), 0x00000000);
 	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW4(port), demph_reg_value);
 	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW2(port),
@@ -1003,7 +1003,8 @@ void vlv_set_phy_signal_level(struct intel_encoder *encoder,
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW11(port), 0x00030000);
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW9(port), preemph_reg_value);
 	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW5(port), DPIO_TX_OCALINIT_EN);
-	mutex_unlock(&dev_priv->sb_lock);
+
+	vlv_dpio_put(dev_priv);
 }
 
 void vlv_phy_pre_pll_enable(struct intel_encoder *encoder,
@@ -1016,7 +1017,8 @@ void vlv_phy_pre_pll_enable(struct intel_encoder *encoder,
 	enum pipe pipe = crtc->pipe;
 
 	/* Program Tx lane resets to default */
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
+
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW0(port),
 			 DPIO_PCS_TX_LANE2_RESET |
 			 DPIO_PCS_TX_LANE1_RESET);
@@ -1030,7 +1032,8 @@ void vlv_phy_pre_pll_enable(struct intel_encoder *encoder,
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW12(port), 0x00750f00);
 	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW11(port), 0x00001500);
 	vlv_dpio_write(dev_priv, pipe, VLV_TX_DW14(port), 0x40400000);
-	mutex_unlock(&dev_priv->sb_lock);
+
+	vlv_dpio_put(dev_priv);
 }
 
 void vlv_phy_pre_encoder_enable(struct intel_encoder *encoder,
@@ -1044,7 +1047,7 @@ void vlv_phy_pre_encoder_enable(struct intel_encoder *encoder,
 	enum pipe pipe = crtc->pipe;
 	u32 val;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* Enable clock channels for this port */
 	val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW8(port));
@@ -1060,7 +1063,7 @@ void vlv_phy_pre_encoder_enable(struct intel_encoder *encoder,
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW14(port), 0x00760018);
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW23(port), 0x00400888);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 }
 
 void vlv_phy_reset_lanes(struct intel_encoder *encoder,
@@ -1072,8 +1075,8 @@ void vlv_phy_reset_lanes(struct intel_encoder *encoder,
 	enum dpio_channel port = vlv_dport_to_channel(dport);
 	enum pipe pipe = crtc->pipe;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW0(port), 0x00000000);
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW1(port), 0x00e00060);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 }
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index f67d321376e4..0b503b6971ff 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -278,7 +278,7 @@ static int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs,
 
 static void band_gap_reset(struct drm_i915_private *dev_priv)
 {
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_flisdsi_get(dev_priv);
 
 	vlv_flisdsi_write(dev_priv, 0x08, 0x0001);
 	vlv_flisdsi_write(dev_priv, 0x0F, 0x0005);
@@ -287,7 +287,7 @@ static void band_gap_reset(struct drm_i915_private *dev_priv)
 	vlv_flisdsi_write(dev_priv, 0x0F, 0x0000);
 	vlv_flisdsi_write(dev_priv, 0x08, 0x0000);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_flisdsi_put(dev_priv);
 }
 
 static inline bool is_vid_mode(struct intel_dsi *intel_dsi)
@@ -509,11 +509,11 @@ static void vlv_dsi_device_ready(struct intel_encoder *encoder)
 
 	DRM_DEBUG_KMS("\n");
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_flisdsi_get(dev_priv);
 	/* program rcomp for compliance, reduce from 50 ohms to 45 ohms
 	 * needed everytime after power gate */
 	vlv_flisdsi_write(dev_priv, 0x04, 0x0004);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_flisdsi_put(dev_priv);
 
 	/* bandgap reset is needed after everytime we do power gate */
 	band_gap_reset(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 2ff2ee7f3b78..b73336e7dcd2 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -149,7 +149,7 @@ static void vlv_enable_dsi_pll(struct intel_encoder *encoder,
 
 	DRM_DEBUG_KMS("\n");
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_cck_get(dev_priv);
 
 	vlv_cck_write(dev_priv, CCK_REG_DSI_PLL_CONTROL, 0);
 	vlv_cck_write(dev_priv, CCK_REG_DSI_PLL_DIVIDER, config->dsi_pll.div);
@@ -166,11 +166,11 @@ static void vlv_enable_dsi_pll(struct intel_encoder *encoder,
 	if (wait_for(vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL) &
 						DSI_PLL_LOCK, 20)) {
 
-		mutex_unlock(&dev_priv->sb_lock);
+		vlv_cck_put(dev_priv);
 		DRM_ERROR("DSI PLL lock failed\n");
 		return;
 	}
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_cck_put(dev_priv);
 
 	DRM_DEBUG_KMS("DSI PLL locked\n");
 }
@@ -182,14 +182,14 @@ static void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 
 	DRM_DEBUG_KMS("\n");
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_cck_get(dev_priv);
 
 	tmp = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
 	tmp &= ~DSI_PLL_VCO_EN;
 	tmp |= DSI_PLL_LDO_GATE;
 	vlv_cck_write(dev_priv, CCK_REG_DSI_PLL_CONTROL, tmp);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_cck_put(dev_priv);
 }
 
 static bool bxt_dsi_pll_is_enabled(struct drm_i915_private *dev_priv)
@@ -274,10 +274,10 @@ static u32 vlv_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
 
 	DRM_DEBUG_KMS("\n");
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_cck_get(dev_priv);
 	pll_ctl = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
 	pll_div = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_cck_put(dev_priv);
 
 	config->dsi_pll.ctrl = pll_ctl & ~DSI_PLL_LOCK;
 	config->dsi_pll.div = pll_div;
diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
index 91c07b0c8db9..f1168b6e8592 100644
--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
@@ -234,7 +234,7 @@ static void vlv_exec_gpio(struct drm_i915_private *dev_priv,
 	pconf0 = VLV_GPIO_PCONF0(map->base_offset);
 	padval = VLV_GPIO_PAD_VAL(map->base_offset);
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_GPIO));
 	if (!map->init) {
 		/* FIXME: remove constant below */
 		vlv_iosf_sb_write(dev_priv, port, pconf0, 0x2000CC00);
@@ -243,7 +243,7 @@ static void vlv_exec_gpio(struct drm_i915_private *dev_priv,
 
 	tmp = 0x4 | value;
 	vlv_iosf_sb_write(dev_priv, port, padval, tmp);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_GPIO));
 }
 
 static void chv_exec_gpio(struct drm_i915_private *dev_priv,
@@ -289,12 +289,12 @@ static void chv_exec_gpio(struct drm_i915_private *dev_priv,
 	cfg0 = CHV_GPIO_PAD_CFG0(family_num, gpio_index);
 	cfg1 = CHV_GPIO_PAD_CFG1(family_num, gpio_index);
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_GPIO));
 	vlv_iosf_sb_write(dev_priv, port, cfg1, 0);
 	vlv_iosf_sb_write(dev_priv, port, cfg0,
 			  CHV_GPIO_GPIOEN | CHV_GPIO_GPIOCFG_GPO |
 			  CHV_GPIO_GPIOTXSTATE(value));
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_GPIO));
 }
 
 static void bxt_exec_gpio(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 691f15b59124..2b5b7d4f73d7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1995,12 +1995,12 @@ static void chv_hdmi_post_disable(struct intel_encoder *encoder,
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* Assert data lane reset */
 	chv_data_lane_soft_reset(encoder, old_crtc_state, true);
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 }
 
 static void chv_hdmi_pre_enable(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1db79a860b96..60c666424d65 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7173,9 +7173,9 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
 
 	vlv_init_gpll_ref_freq(dev_priv);
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_cck_get(dev_priv);
 	val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_cck_put(dev_priv);
 
 	switch ((val >> 2) & 0x7) {
 	case 3:
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 4996c4ea8a80..34ffa2dd3996 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1182,7 +1182,7 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 				    1))
 		DRM_ERROR("Display PHY %d is not power up\n", phy);
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 
 	/* Enable dynamic power down */
 	tmp = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW28);
@@ -1205,7 +1205,7 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 		vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW30, tmp);
 	}
 
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 
 	dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(phy);
 	I915_WRITE(DISPLAY_PHY_CONTROL, dev_priv->chv_phy_control);
@@ -1268,9 +1268,9 @@ static void assert_chv_phy_powergate(struct drm_i915_private *dev_priv, enum dpi
 	else
 		reg = _CHV_CMN_DW6_CH1;
 
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_dpio_get(dev_priv);
 	val = vlv_dpio_read(dev_priv, pipe, reg);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_dpio_put(dev_priv);
 
 	/*
 	 * This assumes !override is only used when the port is disabled.
diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index d56eda33734e..3d7c5917b97c 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -73,6 +73,22 @@ static void __vlv_punit_put(struct drm_i915_private *dev_priv)
 	iosf_mbi_punit_release();
 }
 
+void vlv_iosf_sb_get(struct drm_i915_private *dev_priv, unsigned long ports)
+{
+	if (ports & BIT(VLV_IOSF_SB_PUNIT))
+		__vlv_punit_get(dev_priv);
+
+	mutex_lock(&dev_priv->sb_lock);
+}
+
+void vlv_iosf_sb_put(struct drm_i915_private *dev_priv, unsigned long ports)
+{
+	mutex_unlock(&dev_priv->sb_lock);
+
+	if (ports & BIT(VLV_IOSF_SB_PUNIT))
+		__vlv_punit_put(dev_priv);
+}
+
 static int vlv_sideband_rw(struct drm_i915_private *dev_priv,
 			   u32 devfn, u32 port, u32 opcode,
 			   u32 addr, u32 *val)
@@ -81,6 +97,8 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv,
 	int err;
 
 	lockdep_assert_held(&dev_priv->sb_lock);
+	if (port == IOSF_PORT_PUNIT)
+		iosf_mbi_assert_punit_acquired();
 
 	/* Flush the previous comms, just in case it failed last time. */
 	if (intel_wait_for_register(dev_priv,
@@ -124,16 +142,14 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr)
 {
 	u32 val = 0;
 
-	WARN_ON(!mutex_is_locked(&dev_priv->pcu_lock));
+	lockdep_assert_held(&dev_priv->pcu_lock);
 
-	mutex_lock(&dev_priv->sb_lock);
-	__vlv_punit_get(dev_priv);
+	vlv_punit_get(dev_priv);
 
 	vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
 			SB_CRRDDA_NP, addr, &val);
 
-	__vlv_punit_put(dev_priv);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_punit_put(dev_priv);
 
 	return val;
 }
@@ -142,20 +158,28 @@ int vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val)
 {
 	int err;
 
-	WARN_ON(!mutex_is_locked(&dev_priv->pcu_lock));
+	lockdep_assert_held(&dev_priv->pcu_lock);
 
-	mutex_lock(&dev_priv->sb_lock);
-	__vlv_punit_get(dev_priv);
+	vlv_punit_get(dev_priv);
 
 	err = vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
 			      SB_CRWRDA_NP, addr, &val);
 
-	__vlv_punit_put(dev_priv);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_punit_put(dev_priv);
 
 	return err;
 }
 
+void vlv_punit_get(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_PUNIT));
+}
+
+void vlv_punit_put(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_PUNIT));
+}
+
 u32 vlv_bunit_read(struct drm_i915_private *dev_priv, u32 reg)
 {
 	u32 val = 0;
@@ -172,20 +196,38 @@ void vlv_bunit_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
 			SB_CRWRDA_NP, reg, &val);
 }
 
+void vlv_bunit_get(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_BUNIT));
+}
+
+void vlv_bunit_put(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_BUNIT));
+}
+
 u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
 {
 	u32 val = 0;
 
-	WARN_ON(!mutex_is_locked(&dev_priv->pcu_lock));
-
-	mutex_lock(&dev_priv->sb_lock);
+	vlv_nc_get(dev_priv);
 	vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_NC,
 			SB_CRRDDA_NP, addr, &val);
-	mutex_unlock(&dev_priv->sb_lock);
+	vlv_nc_put(dev_priv);
 
 	return val;
 }
 
+void vlv_nc_get(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_NC));
+}
+
+void vlv_nc_put(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_NC));
+}
+
 u32 vlv_iosf_sb_read(struct drm_i915_private *dev_priv, u8 port, u32 reg)
 {
 	u32 val = 0;
@@ -215,6 +257,16 @@ void vlv_cck_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
 			SB_CRWRDA_NP, reg, &val);
 }
 
+void vlv_cck_get(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_CCK));
+}
+
+void vlv_cck_put(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_CCK));
+}
+
 u32 vlv_ccu_read(struct drm_i915_private *dev_priv, u32 reg)
 {
 	u32 val = 0;
@@ -229,6 +281,16 @@ void vlv_ccu_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
 			SB_CRWRDA_NP, reg, &val);
 }
 
+void vlv_ccu_get(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_CCU));
+}
+
+void vlv_ccu_put(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_CCU));
+}
+
 u32 vlv_dpio_read(struct drm_i915_private *dev_priv, enum pipe pipe, int reg)
 {
 	u32 val = 0;
@@ -252,12 +314,23 @@ void vlv_dpio_write(struct drm_i915_private *dev_priv, enum pipe pipe, int reg,
 			SB_MWR_NP, reg, &val);
 }
 
+void vlv_dpio_get(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_DPIO));
+}
+
+void vlv_dpio_put(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_DPIO));
+}
+
 /* SBI access */
 u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
 		   enum intel_sbi_destination destination)
 {
 	u32 value = 0;
-	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
+
+	lockdep_assert_held(&dev_priv->sb_lock);
 
 	if (intel_wait_for_register(dev_priv,
 				    SBI_CTL_STAT, SBI_BUSY, 0,
@@ -297,7 +370,7 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
 {
 	u32 tmp;
 
-	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
+	lockdep_assert_held(&dev_priv->sb_lock);
 
 	if (intel_wait_for_register(dev_priv,
 				    SBI_CTL_STAT, SBI_BUSY, 0,
@@ -344,3 +417,13 @@ void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
 	vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_FLISDSI, SB_CRWRDA_NP,
 			reg, &val);
 }
+
+void vlv_flisdsi_get(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_FLISDSI));
+}
+
+void vlv_flisdsi_put(struct drm_i915_private *dev_priv)
+{
+	vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_FLISDSI));
+}
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 4/6] drm/i915: Lift sideband locking for vlv_punit_(read|write)

[Chris Wilson]

Lift the sideband acquisition for vlv_punit_read and vlv_punit_write
into their callers, so that we can lock the sideband once for a sequence
of operations, rather than perform the heavyweight acquisition on each
request.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |  3 +++
 drivers/gpu/drm/i915/i915_sysfs.c       | 14 +++++-----
 drivers/gpu/drm/i915/intel_cdclk.c      | 24 +++++++++++++----
 drivers/gpu/drm/i915/intel_display.c    | 16 +++++++-----
 drivers/gpu/drm/i915/intel_pm.c         | 46 +++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_runtime_pm.c |  8 ++++++
 drivers/gpu/drm/i915/intel_sideband.c   | 18 ++-----------
 7 files changed, 86 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index cc659b4b2a45..28062bd60793 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1033,7 +1033,10 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 			   yesno((rpmodectl & GEN6_RP_MEDIA_MODE_MASK) ==
 				  GEN6_RP_MEDIA_SW_MODE));
 
+		vlv_punit_get(dev_priv);
 		freq_sts = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
+		vlv_punit_put(dev_priv);
+
 		seq_printf(m, "PUNIT_REG_GPU_FREQ_STS: 0x%08x\n", freq_sts);
 		seq_printf(m, "DDR freq: %d MHz\n", dev_priv->mem_freq);
 
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index c74a20b80182..0475a13d751b 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -258,25 +258,25 @@ static ssize_t gt_act_freq_mhz_show(struct device *kdev,
 				    struct device_attribute *attr, char *buf)
 {
 	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
-	int ret;
+	u32 freq;
 
 	intel_runtime_pm_get(dev_priv);
 
 	mutex_lock(&dev_priv->pcu_lock);
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		u32 freq;
+		vlv_punit_get(dev_priv);
 		freq = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
-		ret = intel_gpu_freq(dev_priv, (freq >> 8) & 0xff);
+		vlv_punit_put(dev_priv);
+
+		freq = (freq >> 8) & 0xff;
 	} else {
-		ret = intel_gpu_freq(dev_priv,
-				     intel_get_cagf(dev_priv,
-						    I915_READ(GEN6_RPSTAT1)));
+		freq = intel_get_cagf(dev_priv, I915_READ(GEN6_RPSTAT1));
 	}
 	mutex_unlock(&dev_priv->pcu_lock);
 
 	intel_runtime_pm_put(dev_priv);
 
-	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
+	return snprintf(buf, PAGE_SIZE, "%d\n", intel_gpu_freq(dev_priv, freq));
 }
 
 static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 177bb79beee7..fd6aa8da373f 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -461,13 +461,19 @@ static void vlv_get_cdclk(struct drm_i915_private *dev_priv,
 {
 	u32 val;
 
+	mutex_lock(&dev_priv->pcu_lock);
+	vlv_iosf_sb_get(dev_priv,
+			BIT(VLV_IOSF_SB_CCK) | BIT(VLV_IOSF_SB_PUNIT));
+
 	cdclk_state->vco = vlv_get_hpll_vco(dev_priv);
 	cdclk_state->cdclk = vlv_get_cck_clock(dev_priv, "cdclk",
 					       CCK_DISPLAY_CLOCK_CONTROL,
 					       cdclk_state->vco);
 
-	mutex_lock(&dev_priv->pcu_lock);
 	val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
+
+	vlv_iosf_sb_put(dev_priv,
+			BIT(VLV_IOSF_SB_CCK) | BIT(VLV_IOSF_SB_PUNIT));
 	mutex_unlock(&dev_priv->pcu_lock);
 
 	if (IS_VALLEYVIEW(dev_priv))
@@ -540,6 +546,11 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
 	 */
 	intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
 
+	vlv_iosf_sb_get(dev_priv,
+			BIT(VLV_IOSF_SB_CCK) |
+			BIT(VLV_IOSF_SB_BUNIT) |
+			BIT(VLV_IOSF_SB_PUNIT));
+
 	mutex_lock(&dev_priv->pcu_lock);
 	val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
 	val &= ~DSPFREQGUAR_MASK;
@@ -552,9 +563,6 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
 	}
 	mutex_unlock(&dev_priv->pcu_lock);
 
-	vlv_iosf_sb_get(dev_priv,
-		       	BIT(VLV_IOSF_SB_CCK) | BIT(VLV_IOSF_SB_BUNIT));
-
 	if (cdclk == 400000) {
 		u32 divider;
 
@@ -588,7 +596,9 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
 	vlv_bunit_write(dev_priv, BUNIT_REG_BISOC, val);
 
 	vlv_iosf_sb_put(dev_priv,
-		       	BIT(VLV_IOSF_SB_CCK) | BIT(VLV_IOSF_SB_BUNIT));
+			BIT(VLV_IOSF_SB_CCK) |
+			BIT(VLV_IOSF_SB_BUNIT) |
+			BIT(VLV_IOSF_SB_PUNIT));
 
 	intel_update_cdclk(dev_priv);
 
@@ -623,6 +633,8 @@ static void chv_set_cdclk(struct drm_i915_private *dev_priv,
 	intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
 
 	mutex_lock(&dev_priv->pcu_lock);
+	vlv_punit_get(dev_priv);
+
 	val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
 	val &= ~DSPFREQGUAR_MASK_CHV;
 	val |= (cmd << DSPFREQGUAR_SHIFT_CHV);
@@ -632,6 +644,8 @@ static void chv_set_cdclk(struct drm_i915_private *dev_priv,
 		     50)) {
 		DRM_ERROR("timed out waiting for CDclk change\n");
 	}
+
+	vlv_punit_put(dev_priv);
 	mutex_unlock(&dev_priv->pcu_lock);
 
 	intel_update_cdclk(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5f0b9f66c8d8..9e082c6e9364 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -165,10 +165,8 @@ int vlv_get_hpll_vco(struct drm_i915_private *dev_priv)
 	int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
 
 	/* Obtain SKU information */
-	vlv_cck_get(dev_priv);
 	hpll_freq = vlv_cck_read(dev_priv, CCK_FUSE_REG) &
 		CCK_FUSE_HPLL_FREQ_MASK;
-	vlv_cck_put(dev_priv);
 
 	return vco_freq[hpll_freq] * 1000;
 }
@@ -179,10 +177,7 @@ int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
 	u32 val;
 	int divider;
 
-	vlv_cck_get(dev_priv);
 	val = vlv_cck_read(dev_priv, reg);
-	vlv_cck_put(dev_priv);
-
 	divider = val & CCK_FREQUENCY_VALUES;
 
 	WARN((val & CCK_FREQUENCY_STATUS) !=
@@ -195,11 +190,18 @@ int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
 int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
 			   const char *name, u32 reg)
 {
+	int hpll;
+
+	vlv_cck_get(dev_priv);
+
 	if (dev_priv->hpll_freq == 0)
 		dev_priv->hpll_freq = vlv_get_hpll_vco(dev_priv);
 
-	return vlv_get_cck_clock(dev_priv, name, reg,
-				 dev_priv->hpll_freq);
+	hpll = vlv_get_cck_clock(dev_priv, name, reg, dev_priv->hpll_freq);
+
+	vlv_cck_put(dev_priv);
+
+	return hpll;
 }
 
 static void intel_update_czclk(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 60c666424d65..83eef5d04592 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -311,6 +311,7 @@ static void chv_set_memory_dvfs(struct drm_i915_private *dev_priv, bool enable)
 	u32 val;
 
 	mutex_lock(&dev_priv->pcu_lock);
+	vlv_punit_get(dev_priv);
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
 	if (enable)
@@ -325,6 +326,7 @@ static void chv_set_memory_dvfs(struct drm_i915_private *dev_priv, bool enable)
 		      FORCE_DDR_FREQ_REQ_ACK) == 0, 3))
 		DRM_ERROR("timed out waiting for Punit DDR DVFS request\n");
 
+	vlv_punit_put(dev_priv);
 	mutex_unlock(&dev_priv->pcu_lock);
 }
 
@@ -333,6 +335,7 @@ static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool enable)
 	u32 val;
 
 	mutex_lock(&dev_priv->pcu_lock);
+	vlv_punit_get(dev_priv);
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
 	if (enable)
@@ -341,6 +344,7 @@ static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool enable)
 		val &= ~DSP_MAXFIFO_PM5_ENABLE;
 	vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
 
+	vlv_punit_put(dev_priv);
 	mutex_unlock(&dev_priv->pcu_lock);
 }
 
@@ -5636,6 +5640,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 
 	if (IS_CHERRYVIEW(dev_priv)) {
 		mutex_lock(&dev_priv->pcu_lock);
+		vlv_punit_get(dev_priv);
 
 		val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
 		if (val & DSP_MAXFIFO_PM5_ENABLE)
@@ -5665,6 +5670,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 				wm->level = VLV_WM_LEVEL_DDR_DVFS;
 		}
 
+		vlv_punit_put(dev_priv);
 		mutex_unlock(&dev_priv->pcu_lock);
 	}
 
@@ -6211,7 +6217,9 @@ static int valleyview_set_rps(struct drm_i915_private *dev_priv, u8 val)
 	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
 	if (val != dev_priv->gt_pm.rps.cur_freq) {
+		vlv_punit_get(dev_priv);
 		err = vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
+		vlv_punit_put(dev_priv);
 		if (err)
 			return err;
 
@@ -7125,6 +7133,11 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
 
 	valleyview_setup_pctx(dev_priv);
 
+	vlv_iosf_sb_get(dev_priv,
+			BIT(VLV_IOSF_SB_PUNIT) |
+			BIT(VLV_IOSF_SB_NC) |
+			BIT(VLV_IOSF_SB_CCK));
+
 	vlv_init_gpll_ref_freq(dev_priv);
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
@@ -7162,6 +7175,11 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
 	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
 			 intel_gpu_freq(dev_priv, rps->min_freq),
 			 rps->min_freq);
+
+	vlv_iosf_sb_put(dev_priv,
+			BIT(VLV_IOSF_SB_PUNIT) |
+			BIT(VLV_IOSF_SB_NC) |
+			BIT(VLV_IOSF_SB_CCK));
 }
 
 static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
@@ -7171,11 +7189,14 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
 
 	cherryview_setup_pctx(dev_priv);
 
+	vlv_iosf_sb_get(dev_priv,
+			BIT(VLV_IOSF_SB_PUNIT) |
+			BIT(VLV_IOSF_SB_NC) |
+			BIT(VLV_IOSF_SB_CCK));
+
 	vlv_init_gpll_ref_freq(dev_priv);
 
-	vlv_cck_get(dev_priv);
 	val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
-	vlv_cck_put(dev_priv);
 
 	switch ((val >> 2) & 0x7) {
 	case 3:
@@ -7208,6 +7229,11 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
 			 intel_gpu_freq(dev_priv, rps->min_freq),
 			 rps->min_freq);
 
+	vlv_iosf_sb_put(dev_priv,
+			BIT(VLV_IOSF_SB_PUNIT) |
+			BIT(VLV_IOSF_SB_NC) |
+			BIT(VLV_IOSF_SB_CCK));
+
 	WARN_ONCE((rps->max_freq | rps->efficient_freq | rps->rp1_freq |
 		   rps->min_freq) & 1,
 		  "Odd GPU freq values\n");
@@ -7295,13 +7321,15 @@ static void cherryview_enable_rps(struct drm_i915_private *dev_priv)
 		   GEN6_RP_DOWN_IDLE_AVG);
 
 	/* Setting Fixed Bias */
-	val = VLV_OVERRIDE_EN |
-		  VLV_SOC_TDP_EN |
-		  CHV_BIAS_CPU_50_SOC_50;
+	vlv_punit_get(dev_priv);
+
+	val = VLV_OVERRIDE_EN | VLV_SOC_TDP_EN | CHV_BIAS_CPU_50_SOC_50;
 	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
+	vlv_punit_put(dev_priv);
+
 	/* RPS code assumes GPLL is used */
 	WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
 
@@ -7378,14 +7406,16 @@ static void valleyview_enable_rps(struct drm_i915_private *dev_priv)
 		   GEN6_RP_UP_BUSY_AVG |
 		   GEN6_RP_DOWN_IDLE_CONT);
 
+	vlv_punit_get(dev_priv);
+
 	/* Setting Fixed Bias */
-	val = VLV_OVERRIDE_EN |
-		  VLV_SOC_TDP_EN |
-		  VLV_BIAS_CPU_125_SOC_875;
+	val = VLV_OVERRIDE_EN | VLV_SOC_TDP_EN | VLV_BIAS_CPU_125_SOC_875;
 	vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
+	vlv_punit_put(dev_priv);
+
 	/* RPS code assumes GPLL is used */
 	WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
 
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 34ffa2dd3996..d41972087cb7 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -799,6 +799,7 @@ static void vlv_set_power_well(struct drm_i915_private *dev_priv,
 			 PUNIT_PWRGT_PWR_GATE(power_well_id);
 
 	mutex_lock(&dev_priv->pcu_lock);
+	vlv_punit_get(dev_priv);
 
 #define COND \
 	((vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS) & mask) == state)
@@ -819,6 +820,7 @@ static void vlv_set_power_well(struct drm_i915_private *dev_priv,
 #undef COND
 
 out:
+	vlv_punit_put(dev_priv);
 	mutex_unlock(&dev_priv->pcu_lock);
 }
 
@@ -847,6 +849,7 @@ static bool vlv_power_well_enabled(struct drm_i915_private *dev_priv,
 	ctrl = PUNIT_PWRGT_PWR_ON(power_well_id);
 
 	mutex_lock(&dev_priv->pcu_lock);
+	vlv_punit_get(dev_priv);
 
 	state = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS) & mask;
 	/*
@@ -865,6 +868,7 @@ static bool vlv_power_well_enabled(struct drm_i915_private *dev_priv,
 	ctrl = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_CTRL) & mask;
 	WARN_ON(ctrl != state);
 
+	vlv_punit_put(dev_priv);
 	mutex_unlock(&dev_priv->pcu_lock);
 
 	return enabled;
@@ -1378,6 +1382,7 @@ static bool chv_pipe_power_well_enabled(struct drm_i915_private *dev_priv,
 	u32 state, ctrl;
 
 	mutex_lock(&dev_priv->pcu_lock);
+	vlv_punit_get(dev_priv);
 
 	state = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) & DP_SSS_MASK(pipe);
 	/*
@@ -1394,6 +1399,7 @@ static bool chv_pipe_power_well_enabled(struct drm_i915_private *dev_priv,
 	ctrl = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) & DP_SSC_MASK(pipe);
 	WARN_ON(ctrl << 16 != state);
 
+	vlv_punit_put(dev_priv);
 	mutex_unlock(&dev_priv->pcu_lock);
 
 	return enabled;
@@ -1410,6 +1416,7 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
 	state = enable ? DP_SSS_PWR_ON(pipe) : DP_SSS_PWR_GATE(pipe);
 
 	mutex_lock(&dev_priv->pcu_lock);
+	vlv_punit_get(dev_priv);
 
 #define COND \
 	((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) & DP_SSS_MASK(pipe)) == state)
@@ -1430,6 +1437,7 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
 #undef COND
 
 out:
+	vlv_punit_put(dev_priv);
 	mutex_unlock(&dev_priv->pcu_lock);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index 3d7c5917b97c..dc3b491b4d00 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -144,30 +144,18 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr)
 
 	lockdep_assert_held(&dev_priv->pcu_lock);
 
-	vlv_punit_get(dev_priv);
-
 	vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
 			SB_CRRDDA_NP, addr, &val);
 
-	vlv_punit_put(dev_priv);
-
 	return val;
 }
 
 int vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val)
 {
-	int err;
-
 	lockdep_assert_held(&dev_priv->pcu_lock);
 
-	vlv_punit_get(dev_priv);
-
-	err = vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
-			      SB_CRWRDA_NP, addr, &val);
-
-	vlv_punit_put(dev_priv);
-
-	return err;
+	return vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
+			       SB_CRWRDA_NP, addr, &val);
 }
 
 void vlv_punit_get(struct drm_i915_private *dev_priv)
@@ -210,10 +198,8 @@ u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
 {
 	u32 val = 0;
 
-	vlv_nc_get(dev_priv);
 	vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_NC,
 			SB_CRRDDA_NP, addr, &val);
-	vlv_nc_put(dev_priv);
 
 	return val;
 }
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 5/6] drm/i915: Reduce RPS update frequency on Valleyview/Cherryview

[Chris Wilson]

Valleyview and Cherryview update the GPU frequency via the punit, which
is very expensive as we have to ensure the cores do not sleep during the
comms. If we perform frequent RPS evaluations, the frequent punit
requests cause measurable system overhead for little benefit, so
increase the evaluation intervals to reduce the number of times we try
and change frequency.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 83eef5d04592..b2a7cff044cb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6119,6 +6119,19 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 		break;
 	}
 
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		/*
+		 * Baytrail and Braswell control the gpu frequency via the
+		 * punit, which is very slow and expensive to communicate with,
+		 * as we synchronously force the package to C0. If we try and
+		 * update the gpufreq too often we cause measurable system
+		 * load for little benefit (effectively stealing CPU time for
+		 * the GPU, negatively impacting overall throughput).
+		 */
+		ei_up <<= 2;
+		ei_down <<= 2;
+	}
+
 	/* When byt can survive without system hang with dynamic
 	 * sw freq adjustments, this restriction can be lifted.
 	 */
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 6/6] Revert "drm/i915: Avoid tweaking evaluation thresholds on Baytrail v3"

[Chris Wilson]

With the vlv sideband fixed to avoid sleeping while we talk to the
punit, the system should be much more stable and be able to utilise the
punit without risk.

This reverts commit 6067a27d1f01 ("drm/i915: Avoid tweaking evaluation
thresholds on Baytrail v3")

References: 6067a27d1f01 ("drm/i915: Avoid tweaking evaluation thresholds on Baytrail v3")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: fritsch@xbmc.org
---
 drivers/gpu/drm/i915/intel_pm.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b2a7cff044cb..d88ceea031de 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6132,12 +6132,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 		ei_down <<= 2;
 	}
 
-	/* When byt can survive without system hang with dynamic
-	 * sw freq adjustments, this restriction can be lifted.
-	 */
-	if (IS_VALLEYVIEW(dev_priv))
-		goto skip_hw_write;
-
 	I915_WRITE(GEN6_RP_UP_EI,
 		   GT_INTERVAL_FROM_US(dev_priv, ei_up));
 	I915_WRITE(GEN6_RP_UP_THRESHOLD,
@@ -6158,7 +6152,6 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 		   GEN6_RP_UP_BUSY_AVG |
 		   GEN6_RP_DOWN_IDLE_AVG);
 
-skip_hw_write:
 	rps->power = new_power;
 	rps->up_threshold = threshold_up;
 	rps->down_threshold = threshold_down;
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [v2,1/6] drm/i915: Lock out execlist tasklet while peeking inside for busy-stats

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/6] drm/i915: Lock out execlist tasklet while peeking inside for busy-stats
URL   : https://patchwork.freedesktop.org/series/36475/
State : success

== Summary ==

Series 36475v1 series starting with [v2,1/6] drm/i915: Lock out execlist tasklet while peeking inside for busy-stats
https://patchwork.freedesktop.org/api/1.0/series/36475/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> FAIL       (fi-elk-e7500) fdo#103989 +1
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-kbl-r) fdo#104172
        Subgroup suspend-read-crc-pipe-c:
                pass       -> INCOMPLETE (fi-bdw-5557u) fdo#104162

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172
fdo#104162 https://bugs.freedesktop.org/show_bug.cgi?id=104162

fi-bdw-5557u     total:246  pass:228  dwarn:0   dfail:0   fail:0   skip:17 
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:426s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:480s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:282s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:488s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:425s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:422s
fi-cnl-y2        total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:529s
fi-elk-e7500     total:224  pass:167  dwarn:10  dfail:0   fail:1   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:275s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:510s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:399s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:409s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:466s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:413s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:471s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:500s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:457s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:504s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:576s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:436s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:508s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:524s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:494s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-snb-2520m     total:3    pass:2    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:396s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:566s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:469s

254125b984264731491e1eafbe58bc50e84a032d drm-tip: 2018y-01m-15d-09h-31m-31s UTC integration manifest
53921a1b3d74 Revert "drm/i915: Avoid tweaking evaluation thresholds on Baytrail v3"
9b3ca39e4c2b drm/i915: Reduce RPS update frequency on Valleyview/Cherryview
c4ac11972355 drm/i915: Lift sideband locking for vlv_punit_(read|write)
d68b373db490 drm/i915: Lift acquiring the vlv punit magic to a common sb-get
a49b025be825 drm/i915: Disable preemption and sleeping while using the punit sideband
6309fb7f178c drm/i915: Lock out execlist tasklet while peeking inside for busy-stats

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7668/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/6] drm/i915: Disable preemption and sleeping while using the punit sideband

[Hans de Goede]

Hi,

On 15-01-18 11:28, Chris Wilson wrote:
> While we talk to the punit over its sideband, we need to prevent the cpu
> from sleeping in order to prevent a potential machine hang.
> 
> Note that by itself, it appears that pm_qos_update_request (via
> intel_idle) doesn't provide a sufficient barrier to ensure that all core
> are indeed awake (out of Cstate) and that the package is awake. To do so,
> we need to supplement the pm_qos with a manual ping on_each_cpu.
> 
> v2: Restrict the heavy-weight wakeup to just the ISOF_PORT_PUNIT, there
> is insufficient evidence to implicate a wider problem atm. Similarly,
> restrict the w/a to Valleyview, as Cherryview doesn't have an angry cadre
> of users.
> 
> The working theory, courtesy of Ville and Hans, is the issue lies within
> the power delivery and so is likely to be unit and board specific and
> occurs when both the unit/fw require extra power at the same time as the
> cpu package is changing its own power state.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109051
> References: https://bugs.freedesktop.org/show_bug.cgi?id=102657
> References: https://bugzilla.kernel.org/show_bug.cgi?id=195255
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

First of all, Chris many thanks for working on this and tracking this
down!

This looks good to me:

Acked-by: Hans de Goede <hdegoede@redhat.com>

As you may know from: https://bugzilla.kernel.org/show_bug.cgi?id=109051

I've 2 patches which people in that bug are testing to workaround this
issue, but it seems that this is a much better fix. The people testing
are capable of building their own kernels and actually have VLV systems
in production(ish) use, they have been running with my patches for a
couple of weeks now.

If we want to go with this patch instead I think we should ask them to
switch to running a test-kernel with this patch (any other patches needed?)
rather then that they keep testing 2 patches which we've no intent to
upstream ...

So shall I ask people in bug109051 to test this patch, or do you want to
just keep this to the intel-gfx list for now?

Regards,

Hans




> ---
>   drivers/gpu/drm/i915/i915_drv.c       |  6 +++
>   drivers/gpu/drm/i915/i915_drv.h       |  1 +
>   drivers/gpu/drm/i915/intel_sideband.c | 89 +++++++++++++++++++++++++++--------
>   3 files changed, 77 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 6c8da9d20c33..d4b90cc0130b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -902,6 +902,9 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>   	spin_lock_init(&dev_priv->uncore.lock);
>   
>   	mutex_init(&dev_priv->sb_lock);
> +	pm_qos_add_request(&dev_priv->sb_qos,
> +			   PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
> +
>   	mutex_init(&dev_priv->modeset_restore_lock);
>   	mutex_init(&dev_priv->av_mutex);
>   	mutex_init(&dev_priv->wm.wm_mutex);
> @@ -953,6 +956,9 @@ static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
>   	intel_irq_fini(dev_priv);
>   	i915_workqueues_cleanup(dev_priv);
>   	i915_engines_cleanup(dev_priv);
> +
> +	pm_qos_remove_request(&dev_priv->sb_qos);
> +	mutex_destroy(&dev_priv->sb_lock);
>   }
>   
>   static int i915_mmio_setup(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c42015b05b47..d95d8c3d04aa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1886,6 +1886,7 @@ struct drm_i915_private {
>   
>   	/* Sideband mailbox protection */
>   	struct mutex sb_lock;
> +	struct pm_qos_request sb_qos;
>   
>   	/** Cached value of IMR to avoid reads in updating the bitfield */
>   	union {
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index 75c872bb8cc9..d56eda33734e 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -22,6 +22,8 @@
>    *
>    */
>   
> +#include <asm/iosf_mbi.h>
> +
>   #include "i915_drv.h"
>   #include "intel_drv.h"
>   
> @@ -39,18 +41,48 @@
>   /* Private register write, double-word addressing, non-posted */
>   #define SB_CRWRDA_NP	0x07
>   
> -static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
> -			   u32 port, u32 opcode, u32 addr, u32 *val)
> +static void ping(void *info)
>   {
> -	u32 cmd, be = 0xf, bar = 0;
> -	bool is_read = (opcode == SB_MRD_NP || opcode == SB_CRRDDA_NP);
> +}
>   
> -	cmd = (devfn << IOSF_DEVFN_SHIFT) | (opcode << IOSF_OPCODE_SHIFT) |
> -		(port << IOSF_PORT_SHIFT) | (be << IOSF_BYTE_ENABLES_SHIFT) |
> -		(bar << IOSF_BAR_SHIFT);
> +static void __vlv_punit_get(struct drm_i915_private *dev_priv)
> +{
> +	iosf_mbi_punit_acquire();
>   
> -	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
> +	/*
> +	 * Prevent the cpu from sleeping while we use this sideband, otherwise
> +	 * the punit may cause a machine hang. The issue appears to be isolated
> +	 * with changing the power state of the CPU package while changing
> +	 * the power state via the punit, and we have only observed it
> +	 * reliably on 4-core Baytail systems suggesting the issue is in the
> +	 * power delivery mechanism and likely to be be board/function
> +	 * specific. Hence we presume the workaround needs only be applied
> +	 * to the Valleyview P-unit and not all sideband communications.
> +	 */
> +	if (IS_VALLEYVIEW(dev_priv)) {
> +		pm_qos_update_request(&dev_priv->sb_qos, 0);
> +		on_each_cpu(ping, NULL, 1);
> +	}
> +}
> +
> +static void __vlv_punit_put(struct drm_i915_private *dev_priv)
> +{
> +	if (IS_VALLEYVIEW(dev_priv))
> +		pm_qos_update_request(&dev_priv->sb_qos, PM_QOS_DEFAULT_VALUE);
>   
> +	iosf_mbi_punit_release();
> +}
> +
> +static int vlv_sideband_rw(struct drm_i915_private *dev_priv,
> +			   u32 devfn, u32 port, u32 opcode,
> +			   u32 addr, u32 *val)
> +{
> +	const bool is_read = (opcode == SB_MRD_NP || opcode == SB_CRRDDA_NP);
> +	int err;
> +
> +	lockdep_assert_held(&dev_priv->sb_lock);
> +
> +	/* Flush the previous comms, just in case it failed last time. */
>   	if (intel_wait_for_register(dev_priv,
>   				    VLV_IOSF_DOORBELL_REQ, IOSF_SB_BUSY, 0,
>   				    5)) {
> @@ -59,22 +91,33 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
>   		return -EAGAIN;
>   	}
>   
> -	I915_WRITE(VLV_IOSF_ADDR, addr);
> -	I915_WRITE(VLV_IOSF_DATA, is_read ? 0 : *val);
> -	I915_WRITE(VLV_IOSF_DOORBELL_REQ, cmd);
> -
> -	if (intel_wait_for_register(dev_priv,
> -				    VLV_IOSF_DOORBELL_REQ, IOSF_SB_BUSY, 0,
> -				    5)) {
> +	preempt_disable();
> +
> +	I915_WRITE_FW(VLV_IOSF_ADDR, addr);
> +	I915_WRITE_FW(VLV_IOSF_DATA, is_read ? 0 : *val);
> +	I915_WRITE_FW(VLV_IOSF_DOORBELL_REQ,
> +		      (devfn << IOSF_DEVFN_SHIFT) |
> +		      (opcode << IOSF_OPCODE_SHIFT) |
> +		      (port << IOSF_PORT_SHIFT) |
> +		      (0xf << IOSF_BYTE_ENABLES_SHIFT) |
> +		      (0 << IOSF_BAR_SHIFT) |
> +		      IOSF_SB_BUSY);
> +
> +	if (__intel_wait_for_register_fw(dev_priv,
> +					 VLV_IOSF_DOORBELL_REQ, IOSF_SB_BUSY, 0,
> +					 10000, 0, NULL) == 0) {
> +		if (is_read)
> +			*val = I915_READ_FW(VLV_IOSF_DATA);
> +		err = 0;
> +	} else {
>   		DRM_DEBUG_DRIVER("IOSF sideband finish wait (%s) timed out\n",
>   				 is_read ? "read" : "write");
> -		return -ETIMEDOUT;
> +		err = -ETIMEDOUT;
>   	}
>   
> -	if (is_read)
> -		*val = I915_READ(VLV_IOSF_DATA);
> +	preempt_enable();
>   
> -	return 0;
> +	return err;
>   }
>   
>   u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr)
> @@ -84,8 +127,12 @@ u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr)
>   	WARN_ON(!mutex_is_locked(&dev_priv->pcu_lock));
>   
>   	mutex_lock(&dev_priv->sb_lock);
> +	__vlv_punit_get(dev_priv);
> +
>   	vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
>   			SB_CRRDDA_NP, addr, &val);
> +
> +	__vlv_punit_put(dev_priv);
>   	mutex_unlock(&dev_priv->sb_lock);
>   
>   	return val;
> @@ -98,8 +145,12 @@ int vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val)
>   	WARN_ON(!mutex_is_locked(&dev_priv->pcu_lock));
>   
>   	mutex_lock(&dev_priv->sb_lock);
> +	__vlv_punit_get(dev_priv);
> +
>   	err = vlv_sideband_rw(dev_priv, PCI_DEVFN(0, 0), IOSF_PORT_PUNIT,
>   			      SB_CRWRDA_NP, addr, &val);
> +
> +	__vlv_punit_put(dev_priv);
>   	mutex_unlock(&dev_priv->sb_lock);
>   
>   	return err;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [v2,1/6] drm/i915: Lock out execlist tasklet while peeking inside for busy-stats

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/6] drm/i915: Lock out execlist tasklet while peeking inside for busy-stats
URL   : https://patchwork.freedesktop.org/series/36475/
State : failure

== Summary ==

Test kms_flip:
        Subgroup plain-flip-fb-recreate:
                fail       -> PASS       (shard-hsw) fdo#100368
        Subgroup vblank-vs-modeset-suspend-interruptible:
                pass       -> FAIL       (shard-snb)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (shard-snb) fdo#103375 +1
Test gem_tiled_swapping:
        Subgroup non-threaded:
                pass       -> INCOMPLETE (shard-hsw) fdo#104218 +1

fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#104218 https://bugs.freedesktop.org/show_bug.cgi?id=104218

shard-hsw        total:2644 pass:1502 dwarn:1   dfail:0   fail:9   skip:1131 time:8826s
shard-snb        total:2698 pass:1301 dwarn:1   dfail:0   fail:12  skip:1383 time:7669s
Blacklisted hosts:
shard-kbl        total:2713 pass:1810 dwarn:1   dfail:0   fail:23  skip:879 time:10633s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7668/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/6] drm/i915: Lock out execlist tasklet while peeking inside for busy-stats

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> In order to prevent a race condition where we may end up overaccounting
> the active state and leaving the busy-stats believing the GPU is 100%
> busy, lock out the tasklet while we reconstruct the busy state. There is
> no direct spinlock guard for the execlists->port[], so we need to
> utilise tasklet_disable() as a synchronous barrier to prevent the only
> writer to execlists->port[] from running at the same time as the enable.
>
> Fixes: 4900727d35bb ("drm/i915/pmu: Reconstruct active state on starting busy-stats")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This series applied on drm-tip with j1900. Uptime 2h 35min until system
hang. Load was glxgears + video.

-Mika

> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index d790bdc227ff..b221610f2365 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1943,16 +1943,22 @@ intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance)
>   */
>  int intel_enable_engine_stats(struct intel_engine_cs *engine)
>  {
> +	struct intel_engine_execlists *execlists = &engine->execlists;
>  	unsigned long flags;
> +	int err = 0;
>  
>  	if (!intel_engine_supports_stats(engine))
>  		return -ENODEV;
>  
> +	tasklet_disable(&execlists->tasklet);
>  	spin_lock_irqsave(&engine->stats.lock, flags);
> -	if (engine->stats.enabled == ~0)
> -		goto busy;
> +
> +	if (unlikely(engine->stats.enabled == ~0)) {
> +		err = -EBUSY;
> +		goto unlock;
> +	}
> +
>  	if (engine->stats.enabled++ == 0) {
> -		struct intel_engine_execlists *execlists = &engine->execlists;
>  		const struct execlist_port *port = execlists->port;
>  		unsigned int num_ports = execlists_num_ports(execlists);
>  
> @@ -1967,14 +1973,12 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
>  		if (engine->stats.active)
>  			engine->stats.start = engine->stats.enabled_at;
>  	}
> -	spin_unlock_irqrestore(&engine->stats.lock, flags);
>  
> -	return 0;
> -
> -busy:
> +unlock:
>  	spin_unlock_irqrestore(&engine->stats.lock, flags);
> +	tasklet_enable(&execlists->tasklet);
>  
> -	return -EBUSY;
> +	return err;
>  }
>  
>  static ktime_t __intel_engine_get_busy_time(struct intel_engine_cs *engine)
> -- 
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/6] drm/i915: Lock out execlist tasklet while peeking inside for busy-stats

[Chris Wilson]

Quoting Mika Kuoppala (2018-01-17 13:22:09)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > In order to prevent a race condition where we may end up overaccounting
> > the active state and leaving the busy-stats believing the GPU is 100%
> > busy, lock out the tasklet while we reconstruct the busy state. There is
> > no direct spinlock guard for the execlists->port[], so we need to
> > utilise tasklet_disable() as a synchronous barrier to prevent the only
> > writer to execlists->port[] from running at the same time as the enable.
> >
> > Fixes: 4900727d35bb ("drm/i915/pmu: Reconstruct active state on starting busy-stats")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This series applied on drm-tip with j1900. Uptime 2h 35min until system
> hang. Load was glxgears + video.

Same load (the good old bug.sh), currently 48 hours. As stable as
skip_hw_write here. :|

Can you give the full series with the pcu_lock refactoring a whirl?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx