drm/i915: refactor VLV IOSF sideband accessors to use one helper

Both the intel_dpio_{read,write} and valleyview_{punit,nc}_{read,write}
use the IOSF sideband interface. They access the same registers and do
mostly the same stuff, but no shared code. There are even duplicate
register defines for the same registers. Both have locking, but the
former use dpio_lock and the latter rps.hw_lock. It's racy.

This patch refactors the sideband access to a single function that
expects dpio_lock to be held. The dpio_lock is only used for sideband
stuff, so it's a better match than rps.hw_lock for the purpose. The rps
stuff still needs rps.hw_lock, since it's used to protect more than just
the register access, so rps code will need to hold both locks.

Based on the work by Shobhit Kumar <shobhit.kumar@intel.com> and Yogesh
Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

1 files changed, 50 insertions, 52 deletions

diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index 81af8857857..a7c4b61e9c3 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -26,42 +26,37 @@
 #include "intel_drv.h"
 
 /* IOSF sideband */
-static int vlv_punit_rw(struct drm_i915_private *dev_priv, u32 port, u8 opcode,
-			u8 addr, u32 *val)
+static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
+			   u32 port, u32 opcode, u32 addr, u32 *val)
 {
-	u32 cmd, devfn, be, bar;
-
-	bar = 0;
-	be = 0xf;
-	devfn = PCI_DEVFN(2, 0);
+	u32 cmd, be = 0xf, bar = 0;
+	bool is_read = (opcode == PUNIT_OPCODE_REG_READ ||
+			opcode == DPIO_OPCODE_REG_READ);
 
 	cmd = (devfn << IOSF_DEVFN_SHIFT) | (opcode << IOSF_OPCODE_SHIFT) |
 		(port << IOSF_PORT_SHIFT) | (be << IOSF_BYTE_ENABLES_SHIFT) |
 		(bar << IOSF_BAR_SHIFT);
 
-	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
+	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
 
-	if (I915_READ(VLV_IOSF_DOORBELL_REQ) & IOSF_SB_BUSY) {
-		DRM_DEBUG_DRIVER("warning: pcode (%s) mailbox access failed\n",
-				 opcode == PUNIT_OPCODE_REG_READ ?
-				 "read" : "write");
+	if (wait_for((I915_READ(VLV_IOSF_DOORBELL_REQ) & IOSF_SB_BUSY) == 0, 5)) {
+		DRM_DEBUG_DRIVER("IOSF sideband idle wait (%s) timed out\n",
+				 is_read ? "read" : "write");
 		return -EAGAIN;
 	}
 
 	I915_WRITE(VLV_IOSF_ADDR, addr);
-	if (opcode == PUNIT_OPCODE_REG_WRITE)
+	if (!is_read)
 		I915_WRITE(VLV_IOSF_DATA, *val);
 	I915_WRITE(VLV_IOSF_DOORBELL_REQ, cmd);
 
-	if (wait_for((I915_READ(VLV_IOSF_DOORBELL_REQ) & IOSF_SB_BUSY) == 0,
-		     5)) {
-		DRM_ERROR("timeout waiting for pcode %s (%d) to finish\n",
-			  opcode == PUNIT_OPCODE_REG_READ ? "read" : "write",
-			  addr);
+	if (wait_for((I915_READ(VLV_IOSF_DOORBELL_REQ) & IOSF_SB_BUSY) == 0, 5)) {
+		DRM_DEBUG_DRIVER("IOSF sideband finish wait (%s) timed out\n",
+				 is_read ? "read" : "write");
 		return -ETIMEDOUT;
 	}
 
-	if (opcode == PUNIT_OPCODE_REG_READ)
+	if (is_read)
 		*val = I915_READ(VLV_IOSF_DATA);
 	I915_WRITE(VLV_IOSF_DATA, 0);
 
@@ -70,57 +65,60 @@ static int vlv_punit_rw(struct drm_i915_private *dev_priv, u32 port, u8 opcode,
 
 int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
 {
-	return vlv_punit_rw(dev_priv, IOSF_PORT_PUNIT, PUNIT_OPCODE_REG_READ,
-			    addr, val);
+	int ret;
+
+	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
+
+	mutex_lock(&dev_priv->dpio_lock);
+	ret = vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
+			      PUNIT_OPCODE_REG_READ, addr, val);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	return ret;
 }
 
 int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
 {
-	return vlv_punit_rw(dev_priv, IOSF_PORT_PUNIT, PUNIT_OPCODE_REG_WRITE,
-			    addr, &val);
+	int ret;
+
+	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
+
+	mutex_lock(&dev_priv->dpio_lock);
+	ret = vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_PUNIT,
+			      PUNIT_OPCODE_REG_WRITE, addr, &val);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	return ret;
 }
 
 int valleyview_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
 {
-	return vlv_punit_rw(dev_priv, IOSF_PORT_NC, PUNIT_OPCODE_REG_READ,
-			    addr, val);
+	int ret;
+
+	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
+
+	mutex_lock(&dev_priv->dpio_lock);
+	ret = vlv_sideband_rw(dev_priv, PCI_DEVFN(2, 0), IOSF_PORT_NC,
+			      PUNIT_OPCODE_REG_READ, addr, val);
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	return ret;
 }
 
 u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg)
 {
-	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
+	u32 val = 0;
 
-	if (wait_for_atomic_us((I915_READ(DPIO_PKT) & DPIO_BUSY) == 0, 100)) {
-		DRM_ERROR("DPIO idle wait timed out\n");
-		return 0;
-	}
+	vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_DPIO,
+			DPIO_OPCODE_REG_READ, reg, &val);
 
-	I915_WRITE(DPIO_REG, reg);
-	I915_WRITE(DPIO_PKT, DPIO_RID | DPIO_OP_READ | DPIO_PORTID |
-		   DPIO_BYTE);
-	if (wait_for_atomic_us((I915_READ(DPIO_PKT) & DPIO_BUSY) == 0, 100)) {
-		DRM_ERROR("DPIO read wait timed out\n");
-		return 0;
-	}
-
-	return I915_READ(DPIO_DATA);
+	return val;
 }
 
 void intel_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val)
 {
-	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
-
-	if (wait_for_atomic_us((I915_READ(DPIO_PKT) & DPIO_BUSY) == 0, 100)) {
-		DRM_ERROR("DPIO idle wait timed out\n");
-		return;
-	}
-
-	I915_WRITE(DPIO_DATA, val);
-	I915_WRITE(DPIO_REG, reg);
-	I915_WRITE(DPIO_PKT, DPIO_RID | DPIO_OP_WRITE | DPIO_PORTID |
-		   DPIO_BYTE);
-	if (wait_for_atomic_us((I915_READ(DPIO_PKT) & DPIO_BUSY) == 0, 100))
-		DRM_ERROR("DPIO write wait timed out\n");
+	vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_DPIO,
+			DPIO_OPCODE_REG_WRITE, reg, &val);
 }
 
 /* SBI access */