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>

drivers/gpu/drm/i915/i915_reg.h

@@ -342,27 +342,54 @@
 #define  DEBUG_RESET_DISPLAY		(1<<9)
 
 /*
- * DPIO - a special bus for various display related registers to hide behind:
- *  0x800c: m1, m2, n, p1, p2, k dividers
- *  0x8014: REF and SFR select
- *  0x8014: N divider, VCO select
- *  0x801c/3c: core clock bits
- *  0x8048/68: low pass filter coefficients
- *  0x8100: fast clock controls
+ * IOSF sideband
+ */
+#define VLV_IOSF_DOORBELL_REQ			(VLV_DISPLAY_BASE + 0x2100)
+#define   IOSF_DEVFN_SHIFT			24
+#define   IOSF_OPCODE_SHIFT			16
+#define   IOSF_PORT_SHIFT			8
+#define   IOSF_BYTE_ENABLES_SHIFT		4
+#define   IOSF_BAR_SHIFT			1
+#define   IOSF_SB_BUSY				(1<<0)
+#define   IOSF_PORT_PUNIT			0x4
+#define   IOSF_PORT_NC				0x11
+#define   IOSF_PORT_DPIO			0x12
+#define VLV_IOSF_DATA				(VLV_DISPLAY_BASE + 0x2104)
+#define VLV_IOSF_ADDR				(VLV_DISPLAY_BASE + 0x2108)
+
+#define PUNIT_OPCODE_REG_READ			6
+#define PUNIT_OPCODE_REG_WRITE			7
+
+#define PUNIT_REG_GPU_LFM			0xd3
+#define PUNIT_REG_GPU_FREQ_REQ			0xd4
+#define PUNIT_REG_GPU_FREQ_STS			0xd8
+#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ		0xdc
+
+#define PUNIT_FUSE_BUS2				0xf6 /* bits 47:40 */
+#define PUNIT_FUSE_BUS1				0xf5 /* bits 55:48 */
+
+#define IOSF_NC_FB_GFX_FREQ_FUSE		0x1c
+#define   FB_GFX_MAX_FREQ_FUSE_SHIFT		3
+#define   FB_GFX_MAX_FREQ_FUSE_MASK		0x000007f8
+#define   FB_GFX_FGUARANTEED_FREQ_FUSE_SHIFT	11
+#define   FB_GFX_FGUARANTEED_FREQ_FUSE_MASK	0x0007f800
+#define IOSF_NC_FB_GFX_FMAX_FUSE_HI		0x34
+#define   FB_FMAX_VMIN_FREQ_HI_MASK		0x00000007
+#define IOSF_NC_FB_GFX_FMAX_FUSE_LO		0x30
+#define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
+#define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
+
+/*
+ * DPIO - a special bus for various display related registers to hide behind
  *
  * DPIO is VLV only.
  *
  * Note: digital port B is DDI0, digital pot C is DDI1
  */
-#define DPIO_PKT			(VLV_DISPLAY_BASE + 0x2100)
-#define  DPIO_RID			(0<<24)
-#define  DPIO_OP_WRITE			(1<<16)
-#define  DPIO_OP_READ			(0<<16)
-#define  DPIO_PORTID			(0x12<<8)
-#define  DPIO_BYTE			(0xf<<4)
-#define  DPIO_BUSY			(1<<0) /* status only */
-#define DPIO_DATA			(VLV_DISPLAY_BASE + 0x2104)
-#define DPIO_REG			(VLV_DISPLAY_BASE + 0x2108)
+#define DPIO_DEVFN			0
+#define DPIO_OPCODE_REG_WRITE		1
+#define DPIO_OPCODE_REG_READ		0
+
 #define DPIO_CTL			(VLV_DISPLAY_BASE + 0x2110)
 #define  DPIO_MODSEL1			(1<<3) /* if ref clk b == 27 */
 #define  DPIO_MODSEL0			(1<<2) /* if ref clk a == 27 */
@@ -4541,40 +4568,6 @@
 #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
 #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
 
-#define VLV_IOSF_DOORBELL_REQ			0x182100
-#define   IOSF_DEVFN_SHIFT			24
-#define   IOSF_OPCODE_SHIFT			16
-#define   IOSF_PORT_SHIFT			8
-#define   IOSF_BYTE_ENABLES_SHIFT		4
-#define   IOSF_BAR_SHIFT			1
-#define   IOSF_SB_BUSY				(1<<0)
-#define   IOSF_PORT_PUNIT			0x4
-#define   IOSF_PORT_NC				0x11
-#define VLV_IOSF_DATA				0x182104
-#define VLV_IOSF_ADDR				0x182108
-
-#define PUNIT_OPCODE_REG_READ			6
-#define PUNIT_OPCODE_REG_WRITE			7
-
-#define PUNIT_REG_GPU_LFM			0xd3
-#define PUNIT_REG_GPU_FREQ_REQ			0xd4
-#define PUNIT_REG_GPU_FREQ_STS			0xd8
-#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ		0xdc
-
-#define PUNIT_FUSE_BUS2				0xf6 /* bits 47:40 */
-#define PUNIT_FUSE_BUS1				0xf5 /* bits 55:48 */
-
-#define IOSF_NC_FB_GFX_FREQ_FUSE		0x1c
-#define   FB_GFX_MAX_FREQ_FUSE_SHIFT		3
-#define   FB_GFX_MAX_FREQ_FUSE_MASK		0x000007f8
-#define   FB_GFX_FGUARANTEED_FREQ_FUSE_SHIFT	11
-#define   FB_GFX_FGUARANTEED_FREQ_FUSE_MASK	0x0007f800
-#define IOSF_NC_FB_GFX_FMAX_FUSE_HI		0x34
-#define   FB_FMAX_VMIN_FREQ_HI_MASK		0x00000007
-#define IOSF_NC_FB_GFX_FMAX_FUSE_LO		0x30
-#define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
-#define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
-
 #define GEN6_GT_CORE_STATUS		0x138060
 #define   GEN6_CORE_CPD_STATE_MASK	(7<<4)
 #define   GEN6_RCn_MASK			7

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 */