All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] drm/i915: sideband refactoring
@ 2013-05-22 12:36 Jani Nikula
  2013-05-22 12:36 ` [PATCH v2 1/5] drm/i915: group sideband register accessors to a new file Jani Nikula
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Jani Nikula @ 2013-05-22 12:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Hi all, v2 of the vlv (and now partially also hsw) sideband refactoring
patches [1], addressing Daniel's comments and with two new patches
cleaning things up a bit.

BR,
Jani.

[1] http://mid.gmane.org/cover.1368642827.git.jani.nikula@intel.com


Jani Nikula (5):
  drm/i915: group sideband register accessors to a new file
  drm/i915: refactor VLV IOSF sideband accessors to use one helper
  drm/i915: drop redundant warnings on not holding dpio_lock
  drm/i915: rename VLV IOSF sideband functions logically
  drm/i915: change VLV IOSF sideband accessors to not return error code

 drivers/gpu/drm/i915/Makefile         |    1 +
 drivers/gpu/drm/i915/i915_debugfs.c   |   25 +++--
 drivers/gpu/drm/i915/i915_drv.h       |   14 ++-
 drivers/gpu/drm/i915/i915_reg.h       |   93 ++++++++---------
 drivers/gpu/drm/i915/i915_sysfs.c     |    2 +-
 drivers/gpu/drm/i915/intel_display.c  |  144 +++++----------------------
 drivers/gpu/drm/i915/intel_dp.c       |   38 +++----
 drivers/gpu/drm/i915/intel_drv.h      |    4 -
 drivers/gpu/drm/i915/intel_hdmi.c     |   46 ++++-----
 drivers/gpu/drm/i915/intel_pm.c       |   80 ++-------------
 drivers/gpu/drm/i915/intel_sideband.c |  177 +++++++++++++++++++++++++++++++++
 11 files changed, 313 insertions(+), 311 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_sideband.c

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/5] drm/i915: group sideband register accessors to a new file
  2013-05-22 12:36 [PATCH v2 0/5] drm/i915: sideband refactoring Jani Nikula
@ 2013-05-22 12:36 ` Jani Nikula
  2013-05-23 18:07   ` Jesse Barnes
  2013-05-22 12:36 ` [PATCH v2 2/5] drm/i915: refactor VLV IOSF sideband accessors to use one helper Jani Nikula
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2013-05-22 12:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Group both the HSW/LPT SBI interface and VLV IOSF sideband register
accessor functions into a new file. No functional changes.

v2: also move intel_sbi_{read,write} (Daniel)

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/Makefile         |    1 +
 drivers/gpu/drm/i915/i915_drv.h       |    8 ++
 drivers/gpu/drm/i915/intel_display.c  |   98 ------------------
 drivers/gpu/drm/i915/intel_drv.h      |    4 -
 drivers/gpu/drm/i915/intel_pm.c       |   60 -----------
 drivers/gpu/drm/i915/intel_sideband.c |  183 +++++++++++++++++++++++++++++++++
 6 files changed, 192 insertions(+), 162 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_sideband.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 91f3ac6..40034ec 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -36,6 +36,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
 	  intel_overlay.o \
 	  intel_sprite.o \
 	  intel_opregion.o \
+	  intel_sideband.o \
 	  dvo_ch7xxx.o \
 	  dvo_ch7017.o \
 	  dvo_ivch.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 639ec0b..e8ee05d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1917,9 +1917,17 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv);
 
 int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val);
 int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
+
+/* intel_sideband.c */
 int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
 int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
 int valleyview_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
+u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg);
+void intel_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val);
+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);
 
 int vlv_gpu_freq(int ddr_freq, int val);
 int vlv_freq_opcode(int ddr_freq, int val);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 684ab64..87767dd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -381,43 +381,6 @@ static const intel_limit_t intel_limits_vlv_dp = {
 	.find_pll = intel_vlv_find_best_pll,
 };
 
-u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg)
-{
-	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 0;
-	}
-
-	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);
-}
-
-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");
-}
-
 static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
 						int refclk)
 {
@@ -1404,67 +1367,6 @@ static void intel_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	POSTING_READ(reg);
 }
 
-/* SBI access */
-static void
-intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
-		enum intel_sbi_destination destination)
-{
-	u32 tmp;
-
-	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
-
-	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
-				100)) {
-		DRM_ERROR("timeout waiting for SBI to become ready\n");
-		return;
-	}
-
-	I915_WRITE(SBI_ADDR, (reg << 16));
-	I915_WRITE(SBI_DATA, value);
-
-	if (destination == SBI_ICLK)
-		tmp = SBI_CTL_DEST_ICLK | SBI_CTL_OP_CRWR;
-	else
-		tmp = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IOWR;
-	I915_WRITE(SBI_CTL_STAT, SBI_BUSY | tmp);
-
-	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
-				100)) {
-		DRM_ERROR("timeout waiting for SBI to complete write transaction\n");
-		return;
-	}
-}
-
-static 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->dpio_lock));
-
-	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
-				100)) {
-		DRM_ERROR("timeout waiting for SBI to become ready\n");
-		return 0;
-	}
-
-	I915_WRITE(SBI_ADDR, (reg << 16));
-
-	if (destination == SBI_ICLK)
-		value = SBI_CTL_DEST_ICLK | SBI_CTL_OP_CRRD;
-	else
-		value = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IORD;
-	I915_WRITE(SBI_CTL_STAT, value | SBI_BUSY);
-
-	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
-				100)) {
-		DRM_ERROR("timeout waiting for SBI to complete read transaction\n");
-		return 0;
-	}
-
-	return I915_READ(SBI_DATA);
-}
-
 void vlv_wait_port_ready(struct drm_i915_private *dev_priv, int port)
 {
 	u32 port_mask;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 75a7f22..83b4fe4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -743,10 +743,6 @@ extern int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 extern int intel_sprite_get_colorkey(struct drm_device *dev, void *data,
 				     struct drm_file *file_priv);
 
-extern u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg);
-extern void intel_dpio_write(struct drm_i915_private *dev_priv, int reg,
-			     u32 val);
-
 /* Power-related functions, located in intel_pm.c */
 extern void intel_init_pm(struct drm_device *dev);
 /* FBC */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8a90cf3..19e7b3e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4955,66 +4955,6 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val)
 	return 0;
 }
 
-static int vlv_punit_rw(struct drm_i915_private *dev_priv, u32 port, u8 opcode,
-			u8 addr, u32 *val)
-{
-	u32 cmd, devfn, be, bar;
-
-	bar = 0;
-	be = 0xf;
-	devfn = PCI_DEVFN(2, 0);
-
-	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));
-
-	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");
-		return -EAGAIN;
-	}
-
-	I915_WRITE(VLV_IOSF_ADDR, addr);
-	if (opcode == PUNIT_OPCODE_REG_WRITE)
-		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);
-		return -ETIMEDOUT;
-	}
-
-	if (opcode == PUNIT_OPCODE_REG_READ)
-		*val = I915_READ(VLV_IOSF_DATA);
-	I915_WRITE(VLV_IOSF_DATA, 0);
-
-	return 0;
-}
-
-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 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 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 vlv_gpu_freq(int ddr_freq, int val)
 {
 	int mult, base;
diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
new file mode 100644
index 0000000..81af885
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -0,0 +1,183 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "i915_drv.h"
+#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)
+{
+	u32 cmd, devfn, be, bar;
+
+	bar = 0;
+	be = 0xf;
+	devfn = PCI_DEVFN(2, 0);
+
+	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));
+
+	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");
+		return -EAGAIN;
+	}
+
+	I915_WRITE(VLV_IOSF_ADDR, addr);
+	if (opcode == PUNIT_OPCODE_REG_WRITE)
+		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);
+		return -ETIMEDOUT;
+	}
+
+	if (opcode == PUNIT_OPCODE_REG_READ)
+		*val = I915_READ(VLV_IOSF_DATA);
+	I915_WRITE(VLV_IOSF_DATA, 0);
+
+	return 0;
+}
+
+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 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 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);
+}
+
+u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg)
+{
+	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 0;
+	}
+
+	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);
+}
+
+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");
+}
+
+/* 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->dpio_lock));
+
+	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
+				100)) {
+		DRM_ERROR("timeout waiting for SBI to become ready\n");
+		return 0;
+	}
+
+	I915_WRITE(SBI_ADDR, (reg << 16));
+
+	if (destination == SBI_ICLK)
+		value = SBI_CTL_DEST_ICLK | SBI_CTL_OP_CRRD;
+	else
+		value = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IORD;
+	I915_WRITE(SBI_CTL_STAT, value | SBI_BUSY);
+
+	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
+				100)) {
+		DRM_ERROR("timeout waiting for SBI to complete read transaction\n");
+		return 0;
+	}
+
+	return I915_READ(SBI_DATA);
+}
+
+void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
+		     enum intel_sbi_destination destination)
+{
+	u32 tmp;
+
+	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
+
+	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
+				100)) {
+		DRM_ERROR("timeout waiting for SBI to become ready\n");
+		return;
+	}
+
+	I915_WRITE(SBI_ADDR, (reg << 16));
+	I915_WRITE(SBI_DATA, value);
+
+	if (destination == SBI_ICLK)
+		tmp = SBI_CTL_DEST_ICLK | SBI_CTL_OP_CRWR;
+	else
+		tmp = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IOWR;
+	I915_WRITE(SBI_CTL_STAT, SBI_BUSY | tmp);
+
+	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
+				100)) {
+		DRM_ERROR("timeout waiting for SBI to complete write transaction\n");
+		return;
+	}
+}
-- 
1.7.10.4

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/5] drm/i915: refactor VLV IOSF sideband accessors to use one helper
  2013-05-22 12:36 [PATCH v2 0/5] drm/i915: sideband refactoring Jani Nikula
  2013-05-22 12:36 ` [PATCH v2 1/5] drm/i915: group sideband register accessors to a new file Jani Nikula
@ 2013-05-22 12:36 ` Jani Nikula
  2013-05-22 15:34   ` Jesse Barnes
  2013-05-22 12:36 ` [PATCH v2 3/5] drm/i915: drop redundant warnings on not holding dpio_lock Jani Nikula
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2013-05-22 12:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

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>
---
 drivers/gpu/drm/i915/i915_reg.h       |   93 ++++++++++++++----------------
 drivers/gpu/drm/i915/intel_sideband.c |  102 ++++++++++++++++-----------------
 2 files changed, 93 insertions(+), 102 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 55caedb..dbd9de5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/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
diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index 81af885..a7c4b61 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 */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/5] drm/i915: drop redundant warnings on not holding dpio_lock
  2013-05-22 12:36 [PATCH v2 0/5] drm/i915: sideband refactoring Jani Nikula
  2013-05-22 12:36 ` [PATCH v2 1/5] drm/i915: group sideband register accessors to a new file Jani Nikula
  2013-05-22 12:36 ` [PATCH v2 2/5] drm/i915: refactor VLV IOSF sideband accessors to use one helper Jani Nikula
@ 2013-05-22 12:36 ` Jani Nikula
  2013-05-23 18:07   ` Jesse Barnes
  2013-05-22 12:36 ` [PATCH v2 4/5] drm/i915: rename VLV IOSF sideband functions logically Jani Nikula
  2013-05-22 12:36 ` [PATCH v2 5/5] drm/i915: change VLV IOSF sideband accessors to not return error code Jani Nikula
  4 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2013-05-22 12:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

The lower level sideband read/write functions already do this.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c   |    6 ------
 drivers/gpu/drm/i915/intel_hdmi.c |    4 ----
 2 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3426a61..76d950e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1442,8 +1442,6 @@ static void intel_pre_enable_dp(struct intel_encoder *encoder)
 		int pipe = intel_crtc->pipe;
 		u32 val;
 
-		WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
-
 		val = intel_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
 		val = 0;
 		if (pipe)
@@ -1470,8 +1468,6 @@ static void intel_dp_pre_pll_enable(struct intel_encoder *encoder)
 	if (!IS_VALLEYVIEW(dev))
 		return;
 
-	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
-
 	/* Program Tx lane resets to default */
 	intel_dpio_write(dev_priv, DPIO_PCS_TX(port),
 			 DPIO_PCS_TX_LANE2_RESET |
@@ -1622,8 +1618,6 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp *intel_dp)
 	uint8_t train_set = intel_dp->train_set[0];
 	int port = vlv_dport_to_channel(dport);
 
-	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
-
 	switch (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) {
 	case DP_TRAIN_PRE_EMPHASIS_0:
 		preemph_reg_value = 0x0004000;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 18f8ce0..83b63d7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1018,8 +1018,6 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
 	if (!IS_VALLEYVIEW(dev))
 		return;
 
-	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
-
 	/* Enable clock channels for this port */
 	val = intel_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
 	val = 0;
@@ -1063,8 +1061,6 @@ static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder)
 	if (!IS_VALLEYVIEW(dev))
 		return;
 
-	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
-
 	/* Program Tx lane resets to default */
 	intel_dpio_write(dev_priv, DPIO_PCS_TX(port),
 			 DPIO_PCS_TX_LANE2_RESET |
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/5] drm/i915: rename VLV IOSF sideband functions logically
  2013-05-22 12:36 [PATCH v2 0/5] drm/i915: sideband refactoring Jani Nikula
                   ` (2 preceding siblings ...)
  2013-05-22 12:36 ` [PATCH v2 3/5] drm/i915: drop redundant warnings on not holding dpio_lock Jani Nikula
@ 2013-05-22 12:36 ` Jani Nikula
  2013-05-23 18:08   ` Jesse Barnes
  2013-05-22 12:36 ` [PATCH v2 5/5] drm/i915: change VLV IOSF sideband accessors to not return error code Jani Nikula
  4 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2013-05-22 12:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Rename all VLV IOSF sideband register accessor functions to
vlv_<port>_{read,write}. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |   24 ++++++++---------
 drivers/gpu/drm/i915/i915_drv.h       |   10 +++----
 drivers/gpu/drm/i915/i915_sysfs.c     |    2 +-
 drivers/gpu/drm/i915/intel_display.c  |   46 ++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_dp.c       |   32 +++++++++++------------
 drivers/gpu/drm/i915/intel_hdmi.c     |   42 +++++++++++++++---------------
 drivers/gpu/drm/i915/intel_pm.c       |   16 ++++++------
 drivers/gpu/drm/i915/intel_sideband.c |   10 +++----
 8 files changed, 91 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a55630a..2151958 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1013,16 +1013,16 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
 		u32 freq_sts, val;
 
 		mutex_lock(&dev_priv->rps.hw_lock);
-		valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS,
+		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS,
 				      &freq_sts);
 		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);
 
-		valleyview_punit_read(dev_priv, PUNIT_FUSE_BUS1, &val);
+		vlv_punit_read(dev_priv, PUNIT_FUSE_BUS1, &val);
 		seq_printf(m, "max GPU freq: %d MHz\n",
 			   vlv_gpu_freq(dev_priv->mem_freq, val));
 
-		valleyview_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
+		vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
 		seq_printf(m, "min GPU freq: %d MHz\n",
 			   vlv_gpu_freq(dev_priv->mem_freq, val));
 
@@ -1663,27 +1663,27 @@ static int i915_dpio_info(struct seq_file *m, void *data)
 	seq_printf(m, "DPIO_CTL: 0x%08x\n", I915_READ(DPIO_CTL));
 
 	seq_printf(m, "DPIO_DIV_A: 0x%08x\n",
-		   intel_dpio_read(dev_priv, _DPIO_DIV_A));
+		   vlv_dpio_read(dev_priv, _DPIO_DIV_A));
 	seq_printf(m, "DPIO_DIV_B: 0x%08x\n",
-		   intel_dpio_read(dev_priv, _DPIO_DIV_B));
+		   vlv_dpio_read(dev_priv, _DPIO_DIV_B));
 
 	seq_printf(m, "DPIO_REFSFR_A: 0x%08x\n",
-		   intel_dpio_read(dev_priv, _DPIO_REFSFR_A));
+		   vlv_dpio_read(dev_priv, _DPIO_REFSFR_A));
 	seq_printf(m, "DPIO_REFSFR_B: 0x%08x\n",
-		   intel_dpio_read(dev_priv, _DPIO_REFSFR_B));
+		   vlv_dpio_read(dev_priv, _DPIO_REFSFR_B));
 
 	seq_printf(m, "DPIO_CORE_CLK_A: 0x%08x\n",
-		   intel_dpio_read(dev_priv, _DPIO_CORE_CLK_A));
+		   vlv_dpio_read(dev_priv, _DPIO_CORE_CLK_A));
 	seq_printf(m, "DPIO_CORE_CLK_B: 0x%08x\n",
-		   intel_dpio_read(dev_priv, _DPIO_CORE_CLK_B));
+		   vlv_dpio_read(dev_priv, _DPIO_CORE_CLK_B));
 
 	seq_printf(m, "DPIO_LFP_COEFF_A: 0x%08x\n",
-		   intel_dpio_read(dev_priv, _DPIO_LFP_COEFF_A));
+		   vlv_dpio_read(dev_priv, _DPIO_LFP_COEFF_A));
 	seq_printf(m, "DPIO_LFP_COEFF_B: 0x%08x\n",
-		   intel_dpio_read(dev_priv, _DPIO_LFP_COEFF_B));
+		   vlv_dpio_read(dev_priv, _DPIO_LFP_COEFF_B));
 
 	seq_printf(m, "DPIO_FASTCLK_DISABLE: 0x%08x\n",
-		   intel_dpio_read(dev_priv, DPIO_FASTCLK_DISABLE));
+		   vlv_dpio_read(dev_priv, DPIO_FASTCLK_DISABLE));
 
 	mutex_unlock(&dev_priv->dpio_lock);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e8ee05d..56ec23e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1919,11 +1919,11 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
 int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
 
 /* intel_sideband.c */
-int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
-int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
-int valleyview_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
-u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg);
-void intel_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val);
+int vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
+int vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
+int vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
+u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg);
+void vlv_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val);
 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,
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index c0d7875..588fa00 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -214,7 +214,7 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
 	mutex_lock(&dev_priv->rps.hw_lock);
 	if (IS_VALLEYVIEW(dev_priv->dev)) {
 		u32 freq;
-		valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &freq);
+		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &freq);
 		ret = vlv_gpu_freq(dev_priv->mem_freq, (freq >> 8) & 0xff);
 	} else {
 		ret = dev_priv->rps.cur_delay * GT_FREQUENCY_MULTIPLIER;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 87767dd..a3b507d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4245,24 +4245,24 @@ static void vlv_pllb_recal_opamp(struct drm_i915_private *dev_priv)
 	 * PLLB opamp always calibrates to max value of 0x3f, force enable it
 	 * and set it to a reasonable value instead.
 	 */
-	reg_val = intel_dpio_read(dev_priv, DPIO_IREF(1));
+	reg_val = vlv_dpio_read(dev_priv, DPIO_IREF(1));
 	reg_val &= 0xffffff00;
 	reg_val |= 0x00000030;
-	intel_dpio_write(dev_priv, DPIO_IREF(1), reg_val);
+	vlv_dpio_write(dev_priv, DPIO_IREF(1), reg_val);
 
-	reg_val = intel_dpio_read(dev_priv, DPIO_CALIBRATION);
+	reg_val = vlv_dpio_read(dev_priv, DPIO_CALIBRATION);
 	reg_val &= 0x8cffffff;
 	reg_val = 0x8c000000;
-	intel_dpio_write(dev_priv, DPIO_CALIBRATION, reg_val);
+	vlv_dpio_write(dev_priv, DPIO_CALIBRATION, reg_val);
 
-	reg_val = intel_dpio_read(dev_priv, DPIO_IREF(1));
+	reg_val = vlv_dpio_read(dev_priv, DPIO_IREF(1));
 	reg_val &= 0xffffff00;
-	intel_dpio_write(dev_priv, DPIO_IREF(1), reg_val);
+	vlv_dpio_write(dev_priv, DPIO_IREF(1), reg_val);
 
-	reg_val = intel_dpio_read(dev_priv, DPIO_CALIBRATION);
+	reg_val = vlv_dpio_read(dev_priv, DPIO_CALIBRATION);
 	reg_val &= 0x00ffffff;
 	reg_val |= 0xb0000000;
-	intel_dpio_write(dev_priv, DPIO_CALIBRATION, reg_val);
+	vlv_dpio_write(dev_priv, DPIO_CALIBRATION, reg_val);
 }
 
 static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
@@ -4337,15 +4337,15 @@ static void vlv_update_pll(struct intel_crtc *crtc)
 		vlv_pllb_recal_opamp(dev_priv);
 
 	/* Set up Tx target for periodic Rcomp update */
-	intel_dpio_write(dev_priv, DPIO_IREF_BCAST, 0x0100000f);
+	vlv_dpio_write(dev_priv, DPIO_IREF_BCAST, 0x0100000f);
 
 	/* Disable target IRef on PLL */
-	reg_val = intel_dpio_read(dev_priv, DPIO_IREF_CTL(pipe));
+	reg_val = vlv_dpio_read(dev_priv, DPIO_IREF_CTL(pipe));
 	reg_val &= 0x00ffffff;
-	intel_dpio_write(dev_priv, DPIO_IREF_CTL(pipe), reg_val);
+	vlv_dpio_write(dev_priv, DPIO_IREF_CTL(pipe), reg_val);
 
 	/* Disable fast lock */
-	intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x610);
+	vlv_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x610);
 
 	/* Set idtafcrecal before PLL is enabled */
 	mdiv = ((bestm1 << DPIO_M1DIV_SHIFT) | (bestm2 & DPIO_M2DIV_MASK));
@@ -4359,47 +4359,47 @@ static void vlv_update_pll(struct intel_crtc *crtc)
 	 * Note: don't use the DAC post divider as it seems unstable.
 	 */
 	mdiv |= (DPIO_POST_DIV_HDMIDP << DPIO_POST_DIV_SHIFT);
-	intel_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv);
+	vlv_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv);
 
 	mdiv |= DPIO_ENABLE_CALIBRATION;
-	intel_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv);
+	vlv_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv);
 
 	/* Set HBR and RBR LPF coefficients */
 	if (adjusted_mode->clock == 162000 ||
 	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI))
-		intel_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
+		vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
 				 0x005f0021);
 	else
-		intel_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
+		vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
 				 0x00d0000f);
 
 	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP) ||
 	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)) {
 		/* Use SSC source */
 		if (!pipe)
-			intel_dpio_write(dev_priv, DPIO_REFSFR(pipe),
+			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
 					 0x0df40000);
 		else
-			intel_dpio_write(dev_priv, DPIO_REFSFR(pipe),
+			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
 					 0x0df70000);
 	} else { /* HDMI or VGA */
 		/* Use bend source */
 		if (!pipe)
-			intel_dpio_write(dev_priv, DPIO_REFSFR(pipe),
+			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
 					 0x0df70000);
 		else
-			intel_dpio_write(dev_priv, DPIO_REFSFR(pipe),
+			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
 					 0x0df40000);
 	}
 
-	coreclk = intel_dpio_read(dev_priv, DPIO_CORE_CLK(pipe));
+	coreclk = vlv_dpio_read(dev_priv, DPIO_CORE_CLK(pipe));
 	coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
 	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT) ||
 	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP))
 		coreclk |= 0x01000000;
-	intel_dpio_write(dev_priv, DPIO_CORE_CLK(pipe), coreclk);
+	vlv_dpio_write(dev_priv, DPIO_CORE_CLK(pipe), coreclk);
 
-	intel_dpio_write(dev_priv, DPIO_PLL_CML(pipe), 0x87871000);
+	vlv_dpio_write(dev_priv, DPIO_PLL_CML(pipe), 0x87871000);
 
 	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
 		if (encoder->pre_pll_enable)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 76d950e..a146704 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1442,18 +1442,18 @@ static void intel_pre_enable_dp(struct intel_encoder *encoder)
 		int pipe = intel_crtc->pipe;
 		u32 val;
 
-		val = intel_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
+		val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
 		val = 0;
 		if (pipe)
 			val |= (1<<21);
 		else
 			val &= ~(1<<21);
 		val |= 0x001000c4;
-		intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
+		vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
 
-		intel_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
+		vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
 				 0x00760018);
-		intel_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
+		vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
 				 0x00400888);
 	}
 }
@@ -1469,19 +1469,19 @@ static void intel_dp_pre_pll_enable(struct intel_encoder *encoder)
 		return;
 
 	/* Program Tx lane resets to default */
-	intel_dpio_write(dev_priv, DPIO_PCS_TX(port),
+	vlv_dpio_write(dev_priv, DPIO_PCS_TX(port),
 			 DPIO_PCS_TX_LANE2_RESET |
 			 DPIO_PCS_TX_LANE1_RESET);
-	intel_dpio_write(dev_priv, DPIO_PCS_CLK(port),
+	vlv_dpio_write(dev_priv, DPIO_PCS_CLK(port),
 			 DPIO_PCS_CLK_CRI_RXEB_EIOS_EN |
 			 DPIO_PCS_CLK_CRI_RXDIGFILTSG_EN |
 			 (1<<DPIO_PCS_CLK_DATAWIDTH_SHIFT) |
 				 DPIO_PCS_CLK_SOFT_RESET);
 
 	/* Fix up inter-pair skew failure */
-	intel_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00);
-	intel_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x00001500);
-	intel_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x40400000);
+	vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00);
+	vlv_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x00001500);
+	vlv_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x40400000);
 }
 
 /*
@@ -1691,14 +1691,14 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp *intel_dp)
 		return 0;
 	}
 
-	intel_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x00000000);
-	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port), demph_reg_value);
-	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port),
+	vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x00000000);
+	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port), demph_reg_value);
+	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port),
 			 uniqtranscale_reg_value);
-	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL3(port), 0x0C782040);
-	intel_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x00030000);
-	intel_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port), preemph_reg_value);
-	intel_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x80000000);
+	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL3(port), 0x0C782040);
+	vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x00030000);
+	vlv_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port), preemph_reg_value);
+	vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x80000000);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 83b63d7..8062a92 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1019,35 +1019,35 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
 		return;
 
 	/* Enable clock channels for this port */
-	val = intel_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
+	val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
 	val = 0;
 	if (pipe)
 		val |= (1<<21);
 	else
 		val &= ~(1<<21);
 	val |= 0x001000c4;
-	intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
+	vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
 
 	/* HDMI 1.0V-2dB */
-	intel_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0);
-	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port),
+	vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0);
+	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port),
 			 0x2b245f5f);
-	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port),
+	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port),
 			 0x5578b83a);
-	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL3(port),
+	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL3(port),
 			 0x0c782040);
-	intel_dpio_write(dev_priv, DPIO_TX3_SWING_CTL4(port),
+	vlv_dpio_write(dev_priv, DPIO_TX3_SWING_CTL4(port),
 			 0x2b247878);
-	intel_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x00030000);
-	intel_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port),
+	vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x00030000);
+	vlv_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port),
 			 0x00002000);
-	intel_dpio_write(dev_priv, DPIO_TX_OCALINIT(port),
+	vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port),
 			 DPIO_TX_OCALINIT_EN);
 
 	/* Program lane clock */
-	intel_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
+	vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
 			 0x00760018);
-	intel_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
+	vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
 			 0x00400888);
 }
 
@@ -1062,23 +1062,23 @@ static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder)
 		return;
 
 	/* Program Tx lane resets to default */
-	intel_dpio_write(dev_priv, DPIO_PCS_TX(port),
+	vlv_dpio_write(dev_priv, DPIO_PCS_TX(port),
 			 DPIO_PCS_TX_LANE2_RESET |
 			 DPIO_PCS_TX_LANE1_RESET);
-	intel_dpio_write(dev_priv, DPIO_PCS_CLK(port),
+	vlv_dpio_write(dev_priv, DPIO_PCS_CLK(port),
 			 DPIO_PCS_CLK_CRI_RXEB_EIOS_EN |
 			 DPIO_PCS_CLK_CRI_RXDIGFILTSG_EN |
 			 (1<<DPIO_PCS_CLK_DATAWIDTH_SHIFT) |
 			 DPIO_PCS_CLK_SOFT_RESET);
 
 	/* Fix up inter-pair skew failure */
-	intel_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00);
-	intel_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x00001500);
-	intel_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x40400000);
+	vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00);
+	vlv_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x00001500);
+	vlv_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x40400000);
 
-	intel_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port),
+	vlv_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port),
 			 0x00002000);
-	intel_dpio_write(dev_priv, DPIO_TX_OCALINIT(port),
+	vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port),
 			 DPIO_TX_OCALINIT_EN);
 }
 
@@ -1090,8 +1090,8 @@ static void intel_hdmi_post_disable(struct intel_encoder *encoder)
 
 	/* Reset lanes to avoid HDMI flicker (VLV w/a) */
 	mutex_lock(&dev_priv->dpio_lock);
-	intel_dpio_write(dev_priv, DPIO_PCS_TX(port), 0x00000000);
-	intel_dpio_write(dev_priv, DPIO_PCS_CLK(port), 0x00e00060);
+	vlv_dpio_write(dev_priv, DPIO_PCS_TX(port), 0x00000000);
+	vlv_dpio_write(dev_priv, DPIO_PCS_CLK(port), 0x00e00060);
 	mutex_unlock(&dev_priv->dpio_lock);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 19e7b3e..17285a3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2566,10 +2566,10 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 	if (val == dev_priv->rps.cur_delay)
 		return;
 
-	valleyview_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
+	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
 
 	do {
-		valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
+		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
 		if (time_after(jiffies, timeout)) {
 			DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
 			break;
@@ -2577,7 +2577,7 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 		udelay(10);
 	} while (pval & 1);
 
-	valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
+	vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
 	if ((pval >> 8) != val)
 		DRM_DEBUG_DRIVER("punit overrode freq: %d requested, but got %d\n",
 			  val, pval >> 8);
@@ -2882,7 +2882,7 @@ int valleyview_rps_max_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val, rp0;
 
-	valleyview_nc_read(dev_priv, IOSF_NC_FB_GFX_FREQ_FUSE, &val);
+	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FREQ_FUSE, &val);
 
 	rp0 = (val & FB_GFX_MAX_FREQ_FUSE_MASK) >> FB_GFX_MAX_FREQ_FUSE_SHIFT;
 	/* Clamp to max */
@@ -2895,9 +2895,9 @@ static int valleyview_rps_rpe_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val, rpe;
 
-	valleyview_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_LO, &val);
+	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_LO, &val);
 	rpe = (val & FB_FMAX_VMIN_FREQ_LO_MASK) >> FB_FMAX_VMIN_FREQ_LO_SHIFT;
-	valleyview_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_HI, &val);
+	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_HI, &val);
 	rpe |= (val & FB_FMAX_VMIN_FREQ_HI_MASK) << 5;
 
 	return rpe;
@@ -2907,7 +2907,7 @@ int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val;
 
-	valleyview_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
+	vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
 
 	return val & 0xff;
 }
@@ -3018,7 +3018,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_RC_CONTROL,
 		   GEN7_RC_CTL_TO_MODE);
 
-	valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val);
+	vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val);
 	switch ((val >> 6) & 3) {
 	case 0:
 	case 1:
diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index a7c4b61..d150972 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -63,7 +63,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
 	return 0;
 }
 
-int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
+int vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
 {
 	int ret;
 
@@ -77,7 +77,7 @@ int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
 	return ret;
 }
 
-int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
+int vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
 {
 	int ret;
 
@@ -91,7 +91,7 @@ int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
 	return ret;
 }
 
-int valleyview_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
+int vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
 {
 	int ret;
 
@@ -105,7 +105,7 @@ int valleyview_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
 	return ret;
 }
 
-u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg)
+u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg)
 {
 	u32 val = 0;
 
@@ -115,7 +115,7 @@ u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg)
 	return val;
 }
 
-void intel_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val)
+void vlv_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val)
 {
 	vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_DPIO,
 			DPIO_OPCODE_REG_WRITE, reg, &val);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/5] drm/i915: change VLV IOSF sideband accessors to not return error code
  2013-05-22 12:36 [PATCH v2 0/5] drm/i915: sideband refactoring Jani Nikula
                   ` (3 preceding siblings ...)
  2013-05-22 12:36 ` [PATCH v2 4/5] drm/i915: rename VLV IOSF sideband functions logically Jani Nikula
@ 2013-05-22 12:36 ` Jani Nikula
  2013-05-23 18:06   ` Jesse Barnes
  4 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2013-05-22 12:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

We never check the return values, and there's not much we could do on
errors anyway. Just simplify the signatures. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |    7 +++----
 drivers/gpu/drm/i915/i915_drv.h       |    6 +++---
 drivers/gpu/drm/i915/i915_sysfs.c     |    2 +-
 drivers/gpu/drm/i915/intel_pm.c       |   18 +++++++-----------
 drivers/gpu/drm/i915/intel_sideband.c |   30 +++++++++++++-----------------
 5 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2151958..28bf3ab 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1013,16 +1013,15 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
 		u32 freq_sts, val;
 
 		mutex_lock(&dev_priv->rps.hw_lock);
-		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS,
-				      &freq_sts);
+		freq_sts = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 		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);
 
-		vlv_punit_read(dev_priv, PUNIT_FUSE_BUS1, &val);
+		val = vlv_punit_read(dev_priv, PUNIT_FUSE_BUS1);
 		seq_printf(m, "max GPU freq: %d MHz\n",
 			   vlv_gpu_freq(dev_priv->mem_freq, val));
 
-		vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
+		val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM);
 		seq_printf(m, "min GPU freq: %d MHz\n",
 			   vlv_gpu_freq(dev_priv->mem_freq, val));
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 56ec23e..cb84f4e9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1919,9 +1919,9 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
 int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
 
 /* intel_sideband.c */
-int vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
-int vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
-int vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
+u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr);
+void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
+u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr);
 u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg);
 void vlv_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val);
 u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 588fa00..6875b56 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -214,7 +214,7 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
 	mutex_lock(&dev_priv->rps.hw_lock);
 	if (IS_VALLEYVIEW(dev_priv->dev)) {
 		u32 freq;
-		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &freq);
+		freq = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 		ret = vlv_gpu_freq(dev_priv->mem_freq, (freq >> 8) & 0xff);
 	} else {
 		ret = dev_priv->rps.cur_delay * GT_FREQUENCY_MULTIPLIER;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 17285a3..c0026d2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2569,7 +2569,7 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
 
 	do {
-		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
+		pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 		if (time_after(jiffies, timeout)) {
 			DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
 			break;
@@ -2577,7 +2577,7 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 		udelay(10);
 	} while (pval & 1);
 
-	vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
+	pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 	if ((pval >> 8) != val)
 		DRM_DEBUG_DRIVER("punit overrode freq: %d requested, but got %d\n",
 			  val, pval >> 8);
@@ -2882,7 +2882,7 @@ int valleyview_rps_max_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val, rp0;
 
-	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FREQ_FUSE, &val);
+	val = vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FREQ_FUSE);
 
 	rp0 = (val & FB_GFX_MAX_FREQ_FUSE_MASK) >> FB_GFX_MAX_FREQ_FUSE_SHIFT;
 	/* Clamp to max */
@@ -2895,9 +2895,9 @@ static int valleyview_rps_rpe_freq(struct drm_i915_private *dev_priv)
 {
 	u32 val, rpe;
 
-	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_LO, &val);
+	val = vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_LO);
 	rpe = (val & FB_FMAX_VMIN_FREQ_LO_MASK) >> FB_FMAX_VMIN_FREQ_LO_SHIFT;
-	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_HI, &val);
+	val = vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_HI);
 	rpe |= (val & FB_FMAX_VMIN_FREQ_HI_MASK) << 5;
 
 	return rpe;
@@ -2905,11 +2905,7 @@ static int valleyview_rps_rpe_freq(struct drm_i915_private *dev_priv)
 
 int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
 {
-	u32 val;
-
-	vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
-
-	return val & 0xff;
+	return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
 }
 
 static void vlv_rps_timer_work(struct work_struct *work)
@@ -3018,7 +3014,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_RC_CONTROL,
 		   GEN7_RC_CTL_TO_MODE);
 
-	vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val);
+	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 	switch ((val >> 6) & 3) {
 	case 0:
 	case 1:
diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
index d150972..9a0e6c5 100644
--- a/drivers/gpu/drm/i915/intel_sideband.c
+++ b/drivers/gpu/drm/i915/intel_sideband.c
@@ -63,46 +63,42 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
 	return 0;
 }
 
-int vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
+u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr)
 {
-	int ret;
+	u32 val = 0;
 
 	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);
+	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;
+	return val;
 }
 
-int vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
+void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 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);
+	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 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
+u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
 {
-	int ret;
+	u32 val = 0;
 
 	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);
+	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;
+	return val;
 }
 
 u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/5] drm/i915: refactor VLV IOSF sideband accessors to use one helper
  2013-05-22 12:36 ` [PATCH v2 2/5] drm/i915: refactor VLV IOSF sideband accessors to use one helper Jani Nikula
@ 2013-05-22 15:34   ` Jesse Barnes
  0 siblings, 0 replies; 12+ messages in thread
From: Jesse Barnes @ 2013-05-22 15:34 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, 22 May 2013 15:36:17 +0300
Jani Nikula <jani.nikula@intel.com> wrote:

> 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.

Nice catch... The docs for each came from different places with no
indication that the req register was re-used, but it's obvious now of
course.

Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 5/5] drm/i915: change VLV IOSF sideband accessors to not return error code
  2013-05-22 12:36 ` [PATCH v2 5/5] drm/i915: change VLV IOSF sideband accessors to not return error code Jani Nikula
@ 2013-05-23 18:06   ` Jesse Barnes
  2013-05-23 21:26     ` Daniel Vetter
  0 siblings, 1 reply; 12+ messages in thread
From: Jesse Barnes @ 2013-05-23 18:06 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, 22 May 2013 15:36:20 +0300
Jani Nikula <jani.nikula@intel.com> wrote:

> We never check the return values, and there's not much we could do on
> errors anyway. Just simplify the signatures. No functional changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |    7 +++----
>  drivers/gpu/drm/i915/i915_drv.h       |    6 +++---
>  drivers/gpu/drm/i915/i915_sysfs.c     |    2 +-
>  drivers/gpu/drm/i915/intel_pm.c       |   18 +++++++-----------
>  drivers/gpu/drm/i915/intel_sideband.c |   30 +++++++++++++-----------------
>  5 files changed, 27 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2151958..28bf3ab 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1013,16 +1013,15 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
>  		u32 freq_sts, val;
>  
>  		mutex_lock(&dev_priv->rps.hw_lock);
> -		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS,
> -				      &freq_sts);
> +		freq_sts = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  		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);
>  
> -		vlv_punit_read(dev_priv, PUNIT_FUSE_BUS1, &val);
> +		val = vlv_punit_read(dev_priv, PUNIT_FUSE_BUS1);
>  		seq_printf(m, "max GPU freq: %d MHz\n",
>  			   vlv_gpu_freq(dev_priv->mem_freq, val));
>  
> -		vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
> +		val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM);
>  		seq_printf(m, "min GPU freq: %d MHz\n",
>  			   vlv_gpu_freq(dev_priv->mem_freq, val));
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 56ec23e..cb84f4e9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1919,9 +1919,9 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
>  int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
>  
>  /* intel_sideband.c */
> -int vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
> -int vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
> -int vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
> +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr);
> +void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
> +u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr);
>  u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg);
>  void vlv_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val);
>  u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index 588fa00..6875b56 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -214,7 +214,7 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  	if (IS_VALLEYVIEW(dev_priv->dev)) {
>  		u32 freq;
> -		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &freq);
> +		freq = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  		ret = vlv_gpu_freq(dev_priv->mem_freq, (freq >> 8) & 0xff);
>  	} else {
>  		ret = dev_priv->rps.cur_delay * GT_FREQUENCY_MULTIPLIER;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 17285a3..c0026d2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2569,7 +2569,7 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
>  	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
>  
>  	do {
> -		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
> +		pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  		if (time_after(jiffies, timeout)) {
>  			DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
>  			break;
> @@ -2577,7 +2577,7 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
>  		udelay(10);
>  	} while (pval & 1);
>  
> -	vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
> +	pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  	if ((pval >> 8) != val)
>  		DRM_DEBUG_DRIVER("punit overrode freq: %d requested, but got %d\n",
>  			  val, pval >> 8);
> @@ -2882,7 +2882,7 @@ int valleyview_rps_max_freq(struct drm_i915_private *dev_priv)
>  {
>  	u32 val, rp0;
>  
> -	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FREQ_FUSE, &val);
> +	val = vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FREQ_FUSE);
>  
>  	rp0 = (val & FB_GFX_MAX_FREQ_FUSE_MASK) >> FB_GFX_MAX_FREQ_FUSE_SHIFT;
>  	/* Clamp to max */
> @@ -2895,9 +2895,9 @@ static int valleyview_rps_rpe_freq(struct drm_i915_private *dev_priv)
>  {
>  	u32 val, rpe;
>  
> -	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_LO, &val);
> +	val = vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_LO);
>  	rpe = (val & FB_FMAX_VMIN_FREQ_LO_MASK) >> FB_FMAX_VMIN_FREQ_LO_SHIFT;
> -	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_HI, &val);
> +	val = vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_HI);
>  	rpe |= (val & FB_FMAX_VMIN_FREQ_HI_MASK) << 5;
>  
>  	return rpe;
> @@ -2905,11 +2905,7 @@ static int valleyview_rps_rpe_freq(struct drm_i915_private *dev_priv)
>  
>  int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
>  {
> -	u32 val;
> -
> -	vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
> -
> -	return val & 0xff;
> +	return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
>  }
>  
>  static void vlv_rps_timer_work(struct work_struct *work)
> @@ -3018,7 +3014,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
>  	I915_WRITE(GEN6_RC_CONTROL,
>  		   GEN7_RC_CTL_TO_MODE);
>  
> -	vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val);
> +	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  	switch ((val >> 6) & 3) {
>  	case 0:
>  	case 1:
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index d150972..9a0e6c5 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -63,46 +63,42 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
>  	return 0;
>  }
>  
> -int vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
> +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr)
>  {
> -	int ret;
> +	u32 val = 0;
>  
>  	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);
> +	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;
> +	return val;
>  }
>  
> -int vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
> +void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 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);
> +	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 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
> +u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
>  {
> -	int ret;
> +	u32 val = 0;
>  
>  	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);
> +	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;
> +	return val;
>  }
>  
>  u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg)

Looks fine.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/5] drm/i915: group sideband register accessors to a new file
  2013-05-22 12:36 ` [PATCH v2 1/5] drm/i915: group sideband register accessors to a new file Jani Nikula
@ 2013-05-23 18:07   ` Jesse Barnes
  0 siblings, 0 replies; 12+ messages in thread
From: Jesse Barnes @ 2013-05-23 18:07 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, 22 May 2013 15:36:16 +0300
Jani Nikula <jani.nikula@intel.com> wrote:

> Group both the HSW/LPT SBI interface and VLV IOSF sideband register
> accessor functions into a new file. No functional changes.
> 
> v2: also move intel_sbi_{read,write} (Daniel)
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile         |    1 +
>  drivers/gpu/drm/i915/i915_drv.h       |    8 ++
>  drivers/gpu/drm/i915/intel_display.c  |   98 ------------------
>  drivers/gpu/drm/i915/intel_drv.h      |    4 -
>  drivers/gpu/drm/i915/intel_pm.c       |   60 -----------
>  drivers/gpu/drm/i915/intel_sideband.c |  183 +++++++++++++++++++++++++++++++++
>  6 files changed, 192 insertions(+), 162 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_sideband.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 91f3ac6..40034ec 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -36,6 +36,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
>  	  intel_overlay.o \
>  	  intel_sprite.o \
>  	  intel_opregion.o \
> +	  intel_sideband.o \
>  	  dvo_ch7xxx.o \
>  	  dvo_ch7017.o \
>  	  dvo_ivch.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 639ec0b..e8ee05d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1917,9 +1917,17 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv);
>  
>  int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val);
>  int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
> +
> +/* intel_sideband.c */
>  int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
>  int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
>  int valleyview_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
> +u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg);
> +void intel_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val);
> +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);
>  
>  int vlv_gpu_freq(int ddr_freq, int val);
>  int vlv_freq_opcode(int ddr_freq, int val);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 684ab64..87767dd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -381,43 +381,6 @@ static const intel_limit_t intel_limits_vlv_dp = {
>  	.find_pll = intel_vlv_find_best_pll,
>  };
>  
> -u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg)
> -{
> -	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 0;
> -	}
> -
> -	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);
> -}
> -
> -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");
> -}
> -
>  static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
>  						int refclk)
>  {
> @@ -1404,67 +1367,6 @@ static void intel_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	POSTING_READ(reg);
>  }
>  
> -/* SBI access */
> -static void
> -intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
> -		enum intel_sbi_destination destination)
> -{
> -	u32 tmp;
> -
> -	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
> -
> -	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
> -				100)) {
> -		DRM_ERROR("timeout waiting for SBI to become ready\n");
> -		return;
> -	}
> -
> -	I915_WRITE(SBI_ADDR, (reg << 16));
> -	I915_WRITE(SBI_DATA, value);
> -
> -	if (destination == SBI_ICLK)
> -		tmp = SBI_CTL_DEST_ICLK | SBI_CTL_OP_CRWR;
> -	else
> -		tmp = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IOWR;
> -	I915_WRITE(SBI_CTL_STAT, SBI_BUSY | tmp);
> -
> -	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
> -				100)) {
> -		DRM_ERROR("timeout waiting for SBI to complete write transaction\n");
> -		return;
> -	}
> -}
> -
> -static 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->dpio_lock));
> -
> -	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
> -				100)) {
> -		DRM_ERROR("timeout waiting for SBI to become ready\n");
> -		return 0;
> -	}
> -
> -	I915_WRITE(SBI_ADDR, (reg << 16));
> -
> -	if (destination == SBI_ICLK)
> -		value = SBI_CTL_DEST_ICLK | SBI_CTL_OP_CRRD;
> -	else
> -		value = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IORD;
> -	I915_WRITE(SBI_CTL_STAT, value | SBI_BUSY);
> -
> -	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
> -				100)) {
> -		DRM_ERROR("timeout waiting for SBI to complete read transaction\n");
> -		return 0;
> -	}
> -
> -	return I915_READ(SBI_DATA);
> -}
> -
>  void vlv_wait_port_ready(struct drm_i915_private *dev_priv, int port)
>  {
>  	u32 port_mask;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 75a7f22..83b4fe4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -743,10 +743,6 @@ extern int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>  extern int intel_sprite_get_colorkey(struct drm_device *dev, void *data,
>  				     struct drm_file *file_priv);
>  
> -extern u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg);
> -extern void intel_dpio_write(struct drm_i915_private *dev_priv, int reg,
> -			     u32 val);
> -
>  /* Power-related functions, located in intel_pm.c */
>  extern void intel_init_pm(struct drm_device *dev);
>  /* FBC */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8a90cf3..19e7b3e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4955,66 +4955,6 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val)
>  	return 0;
>  }
>  
> -static int vlv_punit_rw(struct drm_i915_private *dev_priv, u32 port, u8 opcode,
> -			u8 addr, u32 *val)
> -{
> -	u32 cmd, devfn, be, bar;
> -
> -	bar = 0;
> -	be = 0xf;
> -	devfn = PCI_DEVFN(2, 0);
> -
> -	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));
> -
> -	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");
> -		return -EAGAIN;
> -	}
> -
> -	I915_WRITE(VLV_IOSF_ADDR, addr);
> -	if (opcode == PUNIT_OPCODE_REG_WRITE)
> -		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);
> -		return -ETIMEDOUT;
> -	}
> -
> -	if (opcode == PUNIT_OPCODE_REG_READ)
> -		*val = I915_READ(VLV_IOSF_DATA);
> -	I915_WRITE(VLV_IOSF_DATA, 0);
> -
> -	return 0;
> -}
> -
> -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 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 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 vlv_gpu_freq(int ddr_freq, int val)
>  {
>  	int mult, base;
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> new file mode 100644
> index 0000000..81af885
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -0,0 +1,183 @@
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "i915_drv.h"
> +#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)
> +{
> +	u32 cmd, devfn, be, bar;
> +
> +	bar = 0;
> +	be = 0xf;
> +	devfn = PCI_DEVFN(2, 0);
> +
> +	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));
> +
> +	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");
> +		return -EAGAIN;
> +	}
> +
> +	I915_WRITE(VLV_IOSF_ADDR, addr);
> +	if (opcode == PUNIT_OPCODE_REG_WRITE)
> +		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);
> +		return -ETIMEDOUT;
> +	}
> +
> +	if (opcode == PUNIT_OPCODE_REG_READ)
> +		*val = I915_READ(VLV_IOSF_DATA);
> +	I915_WRITE(VLV_IOSF_DATA, 0);
> +
> +	return 0;
> +}
> +
> +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 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 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);
> +}
> +
> +u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg)
> +{
> +	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 0;
> +	}
> +
> +	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);
> +}
> +
> +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");
> +}
> +
> +/* 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->dpio_lock));
> +
> +	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
> +				100)) {
> +		DRM_ERROR("timeout waiting for SBI to become ready\n");
> +		return 0;
> +	}
> +
> +	I915_WRITE(SBI_ADDR, (reg << 16));
> +
> +	if (destination == SBI_ICLK)
> +		value = SBI_CTL_DEST_ICLK | SBI_CTL_OP_CRRD;
> +	else
> +		value = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IORD;
> +	I915_WRITE(SBI_CTL_STAT, value | SBI_BUSY);
> +
> +	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
> +				100)) {
> +		DRM_ERROR("timeout waiting for SBI to complete read transaction\n");
> +		return 0;
> +	}
> +
> +	return I915_READ(SBI_DATA);
> +}
> +
> +void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value,
> +		     enum intel_sbi_destination destination)
> +{
> +	u32 tmp;
> +
> +	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
> +
> +	if (wait_for((I915_READ(SBI_CTL_STAT) & SBI_BUSY) == 0,
> +				100)) {
> +		DRM_ERROR("timeout waiting for SBI to become ready\n");
> +		return;
> +	}
> +
> +	I915_WRITE(SBI_ADDR, (reg << 16));
> +	I915_WRITE(SBI_DATA, value);
> +
> +	if (destination == SBI_ICLK)
> +		tmp = SBI_CTL_DEST_ICLK | SBI_CTL_OP_CRWR;
> +	else
> +		tmp = SBI_CTL_DEST_MPHY | SBI_CTL_OP_IOWR;
> +	I915_WRITE(SBI_CTL_STAT, SBI_BUSY | tmp);
> +
> +	if (wait_for((I915_READ(SBI_CTL_STAT) & (SBI_BUSY | SBI_RESPONSE_FAIL)) == 0,
> +				100)) {
> +		DRM_ERROR("timeout waiting for SBI to complete write transaction\n");
> +		return;
> +	}
> +}

Yay for shrinking intel_display.c.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/5] drm/i915: drop redundant warnings on not holding dpio_lock
  2013-05-22 12:36 ` [PATCH v2 3/5] drm/i915: drop redundant warnings on not holding dpio_lock Jani Nikula
@ 2013-05-23 18:07   ` Jesse Barnes
  0 siblings, 0 replies; 12+ messages in thread
From: Jesse Barnes @ 2013-05-23 18:07 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, 22 May 2013 15:36:18 +0300
Jani Nikula <jani.nikula@intel.com> wrote:

> The lower level sideband read/write functions already do this.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c   |    6 ------
>  drivers/gpu/drm/i915/intel_hdmi.c |    4 ----
>  2 files changed, 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3426a61..76d950e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1442,8 +1442,6 @@ static void intel_pre_enable_dp(struct intel_encoder *encoder)
>  		int pipe = intel_crtc->pipe;
>  		u32 val;
>  
> -		WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
> -
>  		val = intel_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
>  		val = 0;
>  		if (pipe)
> @@ -1470,8 +1468,6 @@ static void intel_dp_pre_pll_enable(struct intel_encoder *encoder)
>  	if (!IS_VALLEYVIEW(dev))
>  		return;
>  
> -	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
> -
>  	/* Program Tx lane resets to default */
>  	intel_dpio_write(dev_priv, DPIO_PCS_TX(port),
>  			 DPIO_PCS_TX_LANE2_RESET |
> @@ -1622,8 +1618,6 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp *intel_dp)
>  	uint8_t train_set = intel_dp->train_set[0];
>  	int port = vlv_dport_to_channel(dport);
>  
> -	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
> -
>  	switch (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) {
>  	case DP_TRAIN_PRE_EMPHASIS_0:
>  		preemph_reg_value = 0x0004000;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 18f8ce0..83b63d7 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1018,8 +1018,6 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
>  	if (!IS_VALLEYVIEW(dev))
>  		return;
>  
> -	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
> -
>  	/* Enable clock channels for this port */
>  	val = intel_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
>  	val = 0;
> @@ -1063,8 +1061,6 @@ static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder)
>  	if (!IS_VALLEYVIEW(dev))
>  		return;
>  
> -	WARN_ON(!mutex_is_locked(&dev_priv->dpio_lock));
> -
>  	/* Program Tx lane resets to default */
>  	intel_dpio_write(dev_priv, DPIO_PCS_TX(port),
>  			 DPIO_PCS_TX_LANE2_RESET |

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/5] drm/i915: rename VLV IOSF sideband functions logically
  2013-05-22 12:36 ` [PATCH v2 4/5] drm/i915: rename VLV IOSF sideband functions logically Jani Nikula
@ 2013-05-23 18:08   ` Jesse Barnes
  0 siblings, 0 replies; 12+ messages in thread
From: Jesse Barnes @ 2013-05-23 18:08 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, 22 May 2013 15:36:19 +0300
Jani Nikula <jani.nikula@intel.com> wrote:

> Rename all VLV IOSF sideband register accessor functions to
> vlv_<port>_{read,write}. No functional changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |   24 ++++++++---------
>  drivers/gpu/drm/i915/i915_drv.h       |   10 +++----
>  drivers/gpu/drm/i915/i915_sysfs.c     |    2 +-
>  drivers/gpu/drm/i915/intel_display.c  |   46 ++++++++++++++++-----------------
>  drivers/gpu/drm/i915/intel_dp.c       |   32 +++++++++++------------
>  drivers/gpu/drm/i915/intel_hdmi.c     |   42 +++++++++++++++---------------
>  drivers/gpu/drm/i915/intel_pm.c       |   16 ++++++------
>  drivers/gpu/drm/i915/intel_sideband.c |   10 +++----
>  8 files changed, 91 insertions(+), 91 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index a55630a..2151958 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1013,16 +1013,16 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
>  		u32 freq_sts, val;
>  
>  		mutex_lock(&dev_priv->rps.hw_lock);
> -		valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS,
> +		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS,
>  				      &freq_sts);
>  		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);
>  
> -		valleyview_punit_read(dev_priv, PUNIT_FUSE_BUS1, &val);
> +		vlv_punit_read(dev_priv, PUNIT_FUSE_BUS1, &val);
>  		seq_printf(m, "max GPU freq: %d MHz\n",
>  			   vlv_gpu_freq(dev_priv->mem_freq, val));
>  
> -		valleyview_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
> +		vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
>  		seq_printf(m, "min GPU freq: %d MHz\n",
>  			   vlv_gpu_freq(dev_priv->mem_freq, val));
>  
> @@ -1663,27 +1663,27 @@ static int i915_dpio_info(struct seq_file *m, void *data)
>  	seq_printf(m, "DPIO_CTL: 0x%08x\n", I915_READ(DPIO_CTL));
>  
>  	seq_printf(m, "DPIO_DIV_A: 0x%08x\n",
> -		   intel_dpio_read(dev_priv, _DPIO_DIV_A));
> +		   vlv_dpio_read(dev_priv, _DPIO_DIV_A));
>  	seq_printf(m, "DPIO_DIV_B: 0x%08x\n",
> -		   intel_dpio_read(dev_priv, _DPIO_DIV_B));
> +		   vlv_dpio_read(dev_priv, _DPIO_DIV_B));
>  
>  	seq_printf(m, "DPIO_REFSFR_A: 0x%08x\n",
> -		   intel_dpio_read(dev_priv, _DPIO_REFSFR_A));
> +		   vlv_dpio_read(dev_priv, _DPIO_REFSFR_A));
>  	seq_printf(m, "DPIO_REFSFR_B: 0x%08x\n",
> -		   intel_dpio_read(dev_priv, _DPIO_REFSFR_B));
> +		   vlv_dpio_read(dev_priv, _DPIO_REFSFR_B));
>  
>  	seq_printf(m, "DPIO_CORE_CLK_A: 0x%08x\n",
> -		   intel_dpio_read(dev_priv, _DPIO_CORE_CLK_A));
> +		   vlv_dpio_read(dev_priv, _DPIO_CORE_CLK_A));
>  	seq_printf(m, "DPIO_CORE_CLK_B: 0x%08x\n",
> -		   intel_dpio_read(dev_priv, _DPIO_CORE_CLK_B));
> +		   vlv_dpio_read(dev_priv, _DPIO_CORE_CLK_B));
>  
>  	seq_printf(m, "DPIO_LFP_COEFF_A: 0x%08x\n",
> -		   intel_dpio_read(dev_priv, _DPIO_LFP_COEFF_A));
> +		   vlv_dpio_read(dev_priv, _DPIO_LFP_COEFF_A));
>  	seq_printf(m, "DPIO_LFP_COEFF_B: 0x%08x\n",
> -		   intel_dpio_read(dev_priv, _DPIO_LFP_COEFF_B));
> +		   vlv_dpio_read(dev_priv, _DPIO_LFP_COEFF_B));
>  
>  	seq_printf(m, "DPIO_FASTCLK_DISABLE: 0x%08x\n",
> -		   intel_dpio_read(dev_priv, DPIO_FASTCLK_DISABLE));
> +		   vlv_dpio_read(dev_priv, DPIO_FASTCLK_DISABLE));
>  
>  	mutex_unlock(&dev_priv->dpio_lock);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e8ee05d..56ec23e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1919,11 +1919,11 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
>  int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
>  
>  /* intel_sideband.c */
> -int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
> -int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
> -int valleyview_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
> -u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg);
> -void intel_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val);
> +int vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
> +int vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
> +int vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
> +u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg);
> +void vlv_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val);
>  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,
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index c0d7875..588fa00 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -214,7 +214,7 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  	if (IS_VALLEYVIEW(dev_priv->dev)) {
>  		u32 freq;
> -		valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &freq);
> +		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &freq);
>  		ret = vlv_gpu_freq(dev_priv->mem_freq, (freq >> 8) & 0xff);
>  	} else {
>  		ret = dev_priv->rps.cur_delay * GT_FREQUENCY_MULTIPLIER;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 87767dd..a3b507d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4245,24 +4245,24 @@ static void vlv_pllb_recal_opamp(struct drm_i915_private *dev_priv)
>  	 * PLLB opamp always calibrates to max value of 0x3f, force enable it
>  	 * and set it to a reasonable value instead.
>  	 */
> -	reg_val = intel_dpio_read(dev_priv, DPIO_IREF(1));
> +	reg_val = vlv_dpio_read(dev_priv, DPIO_IREF(1));
>  	reg_val &= 0xffffff00;
>  	reg_val |= 0x00000030;
> -	intel_dpio_write(dev_priv, DPIO_IREF(1), reg_val);
> +	vlv_dpio_write(dev_priv, DPIO_IREF(1), reg_val);
>  
> -	reg_val = intel_dpio_read(dev_priv, DPIO_CALIBRATION);
> +	reg_val = vlv_dpio_read(dev_priv, DPIO_CALIBRATION);
>  	reg_val &= 0x8cffffff;
>  	reg_val = 0x8c000000;
> -	intel_dpio_write(dev_priv, DPIO_CALIBRATION, reg_val);
> +	vlv_dpio_write(dev_priv, DPIO_CALIBRATION, reg_val);
>  
> -	reg_val = intel_dpio_read(dev_priv, DPIO_IREF(1));
> +	reg_val = vlv_dpio_read(dev_priv, DPIO_IREF(1));
>  	reg_val &= 0xffffff00;
> -	intel_dpio_write(dev_priv, DPIO_IREF(1), reg_val);
> +	vlv_dpio_write(dev_priv, DPIO_IREF(1), reg_val);
>  
> -	reg_val = intel_dpio_read(dev_priv, DPIO_CALIBRATION);
> +	reg_val = vlv_dpio_read(dev_priv, DPIO_CALIBRATION);
>  	reg_val &= 0x00ffffff;
>  	reg_val |= 0xb0000000;
> -	intel_dpio_write(dev_priv, DPIO_CALIBRATION, reg_val);
> +	vlv_dpio_write(dev_priv, DPIO_CALIBRATION, reg_val);
>  }
>  
>  static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
> @@ -4337,15 +4337,15 @@ static void vlv_update_pll(struct intel_crtc *crtc)
>  		vlv_pllb_recal_opamp(dev_priv);
>  
>  	/* Set up Tx target for periodic Rcomp update */
> -	intel_dpio_write(dev_priv, DPIO_IREF_BCAST, 0x0100000f);
> +	vlv_dpio_write(dev_priv, DPIO_IREF_BCAST, 0x0100000f);
>  
>  	/* Disable target IRef on PLL */
> -	reg_val = intel_dpio_read(dev_priv, DPIO_IREF_CTL(pipe));
> +	reg_val = vlv_dpio_read(dev_priv, DPIO_IREF_CTL(pipe));
>  	reg_val &= 0x00ffffff;
> -	intel_dpio_write(dev_priv, DPIO_IREF_CTL(pipe), reg_val);
> +	vlv_dpio_write(dev_priv, DPIO_IREF_CTL(pipe), reg_val);
>  
>  	/* Disable fast lock */
> -	intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x610);
> +	vlv_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x610);
>  
>  	/* Set idtafcrecal before PLL is enabled */
>  	mdiv = ((bestm1 << DPIO_M1DIV_SHIFT) | (bestm2 & DPIO_M2DIV_MASK));
> @@ -4359,47 +4359,47 @@ static void vlv_update_pll(struct intel_crtc *crtc)
>  	 * Note: don't use the DAC post divider as it seems unstable.
>  	 */
>  	mdiv |= (DPIO_POST_DIV_HDMIDP << DPIO_POST_DIV_SHIFT);
> -	intel_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv);
> +	vlv_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv);
>  
>  	mdiv |= DPIO_ENABLE_CALIBRATION;
> -	intel_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv);
> +	vlv_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv);
>  
>  	/* Set HBR and RBR LPF coefficients */
>  	if (adjusted_mode->clock == 162000 ||
>  	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI))
> -		intel_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
> +		vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
>  				 0x005f0021);
>  	else
> -		intel_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
> +		vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
>  				 0x00d0000f);
>  
>  	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP) ||
>  	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)) {
>  		/* Use SSC source */
>  		if (!pipe)
> -			intel_dpio_write(dev_priv, DPIO_REFSFR(pipe),
> +			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
>  					 0x0df40000);
>  		else
> -			intel_dpio_write(dev_priv, DPIO_REFSFR(pipe),
> +			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
>  					 0x0df70000);
>  	} else { /* HDMI or VGA */
>  		/* Use bend source */
>  		if (!pipe)
> -			intel_dpio_write(dev_priv, DPIO_REFSFR(pipe),
> +			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
>  					 0x0df70000);
>  		else
> -			intel_dpio_write(dev_priv, DPIO_REFSFR(pipe),
> +			vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe),
>  					 0x0df40000);
>  	}
>  
> -	coreclk = intel_dpio_read(dev_priv, DPIO_CORE_CLK(pipe));
> +	coreclk = vlv_dpio_read(dev_priv, DPIO_CORE_CLK(pipe));
>  	coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
>  	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT) ||
>  	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP))
>  		coreclk |= 0x01000000;
> -	intel_dpio_write(dev_priv, DPIO_CORE_CLK(pipe), coreclk);
> +	vlv_dpio_write(dev_priv, DPIO_CORE_CLK(pipe), coreclk);
>  
> -	intel_dpio_write(dev_priv, DPIO_PLL_CML(pipe), 0x87871000);
> +	vlv_dpio_write(dev_priv, DPIO_PLL_CML(pipe), 0x87871000);
>  
>  	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
>  		if (encoder->pre_pll_enable)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 76d950e..a146704 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1442,18 +1442,18 @@ static void intel_pre_enable_dp(struct intel_encoder *encoder)
>  		int pipe = intel_crtc->pipe;
>  		u32 val;
>  
> -		val = intel_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
> +		val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
>  		val = 0;
>  		if (pipe)
>  			val |= (1<<21);
>  		else
>  			val &= ~(1<<21);
>  		val |= 0x001000c4;
> -		intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
> +		vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
>  
> -		intel_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
> +		vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
>  				 0x00760018);
> -		intel_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
> +		vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
>  				 0x00400888);
>  	}
>  }
> @@ -1469,19 +1469,19 @@ static void intel_dp_pre_pll_enable(struct intel_encoder *encoder)
>  		return;
>  
>  	/* Program Tx lane resets to default */
> -	intel_dpio_write(dev_priv, DPIO_PCS_TX(port),
> +	vlv_dpio_write(dev_priv, DPIO_PCS_TX(port),
>  			 DPIO_PCS_TX_LANE2_RESET |
>  			 DPIO_PCS_TX_LANE1_RESET);
> -	intel_dpio_write(dev_priv, DPIO_PCS_CLK(port),
> +	vlv_dpio_write(dev_priv, DPIO_PCS_CLK(port),
>  			 DPIO_PCS_CLK_CRI_RXEB_EIOS_EN |
>  			 DPIO_PCS_CLK_CRI_RXDIGFILTSG_EN |
>  			 (1<<DPIO_PCS_CLK_DATAWIDTH_SHIFT) |
>  				 DPIO_PCS_CLK_SOFT_RESET);
>  
>  	/* Fix up inter-pair skew failure */
> -	intel_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00);
> -	intel_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x00001500);
> -	intel_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x40400000);
> +	vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00);
> +	vlv_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x00001500);
> +	vlv_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x40400000);
>  }
>  
>  /*
> @@ -1691,14 +1691,14 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp *intel_dp)
>  		return 0;
>  	}
>  
> -	intel_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x00000000);
> -	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port), demph_reg_value);
> -	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port),
> +	vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x00000000);
> +	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port), demph_reg_value);
> +	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port),
>  			 uniqtranscale_reg_value);
> -	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL3(port), 0x0C782040);
> -	intel_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x00030000);
> -	intel_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port), preemph_reg_value);
> -	intel_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x80000000);
> +	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL3(port), 0x0C782040);
> +	vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x00030000);
> +	vlv_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port), preemph_reg_value);
> +	vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x80000000);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 83b63d7..8062a92 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1019,35 +1019,35 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
>  		return;
>  
>  	/* Enable clock channels for this port */
> -	val = intel_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
> +	val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
>  	val = 0;
>  	if (pipe)
>  		val |= (1<<21);
>  	else
>  		val &= ~(1<<21);
>  	val |= 0x001000c4;
> -	intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
> +	vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
>  
>  	/* HDMI 1.0V-2dB */
> -	intel_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0);
> -	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port),
> +	vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0);
> +	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port),
>  			 0x2b245f5f);
> -	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port),
> +	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port),
>  			 0x5578b83a);
> -	intel_dpio_write(dev_priv, DPIO_TX_SWING_CTL3(port),
> +	vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL3(port),
>  			 0x0c782040);
> -	intel_dpio_write(dev_priv, DPIO_TX3_SWING_CTL4(port),
> +	vlv_dpio_write(dev_priv, DPIO_TX3_SWING_CTL4(port),
>  			 0x2b247878);
> -	intel_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x00030000);
> -	intel_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port),
> +	vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x00030000);
> +	vlv_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port),
>  			 0x00002000);
> -	intel_dpio_write(dev_priv, DPIO_TX_OCALINIT(port),
> +	vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port),
>  			 DPIO_TX_OCALINIT_EN);
>  
>  	/* Program lane clock */
> -	intel_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
> +	vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
>  			 0x00760018);
> -	intel_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
> +	vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
>  			 0x00400888);
>  }
>  
> @@ -1062,23 +1062,23 @@ static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder)
>  		return;
>  
>  	/* Program Tx lane resets to default */
> -	intel_dpio_write(dev_priv, DPIO_PCS_TX(port),
> +	vlv_dpio_write(dev_priv, DPIO_PCS_TX(port),
>  			 DPIO_PCS_TX_LANE2_RESET |
>  			 DPIO_PCS_TX_LANE1_RESET);
> -	intel_dpio_write(dev_priv, DPIO_PCS_CLK(port),
> +	vlv_dpio_write(dev_priv, DPIO_PCS_CLK(port),
>  			 DPIO_PCS_CLK_CRI_RXEB_EIOS_EN |
>  			 DPIO_PCS_CLK_CRI_RXDIGFILTSG_EN |
>  			 (1<<DPIO_PCS_CLK_DATAWIDTH_SHIFT) |
>  			 DPIO_PCS_CLK_SOFT_RESET);
>  
>  	/* Fix up inter-pair skew failure */
> -	intel_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00);
> -	intel_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x00001500);
> -	intel_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x40400000);
> +	vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00);
> +	vlv_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x00001500);
> +	vlv_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x40400000);
>  
> -	intel_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port),
> +	vlv_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port),
>  			 0x00002000);
> -	intel_dpio_write(dev_priv, DPIO_TX_OCALINIT(port),
> +	vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port),
>  			 DPIO_TX_OCALINIT_EN);
>  }
>  
> @@ -1090,8 +1090,8 @@ static void intel_hdmi_post_disable(struct intel_encoder *encoder)
>  
>  	/* Reset lanes to avoid HDMI flicker (VLV w/a) */
>  	mutex_lock(&dev_priv->dpio_lock);
> -	intel_dpio_write(dev_priv, DPIO_PCS_TX(port), 0x00000000);
> -	intel_dpio_write(dev_priv, DPIO_PCS_CLK(port), 0x00e00060);
> +	vlv_dpio_write(dev_priv, DPIO_PCS_TX(port), 0x00000000);
> +	vlv_dpio_write(dev_priv, DPIO_PCS_CLK(port), 0x00e00060);
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 19e7b3e..17285a3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2566,10 +2566,10 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
>  	if (val == dev_priv->rps.cur_delay)
>  		return;
>  
> -	valleyview_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
> +	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
>  
>  	do {
> -		valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
> +		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
>  		if (time_after(jiffies, timeout)) {
>  			DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
>  			break;
> @@ -2577,7 +2577,7 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
>  		udelay(10);
>  	} while (pval & 1);
>  
> -	valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
> +	vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
>  	if ((pval >> 8) != val)
>  		DRM_DEBUG_DRIVER("punit overrode freq: %d requested, but got %d\n",
>  			  val, pval >> 8);
> @@ -2882,7 +2882,7 @@ int valleyview_rps_max_freq(struct drm_i915_private *dev_priv)
>  {
>  	u32 val, rp0;
>  
> -	valleyview_nc_read(dev_priv, IOSF_NC_FB_GFX_FREQ_FUSE, &val);
> +	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FREQ_FUSE, &val);
>  
>  	rp0 = (val & FB_GFX_MAX_FREQ_FUSE_MASK) >> FB_GFX_MAX_FREQ_FUSE_SHIFT;
>  	/* Clamp to max */
> @@ -2895,9 +2895,9 @@ static int valleyview_rps_rpe_freq(struct drm_i915_private *dev_priv)
>  {
>  	u32 val, rpe;
>  
> -	valleyview_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_LO, &val);
> +	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_LO, &val);
>  	rpe = (val & FB_FMAX_VMIN_FREQ_LO_MASK) >> FB_FMAX_VMIN_FREQ_LO_SHIFT;
> -	valleyview_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_HI, &val);
> +	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_HI, &val);
>  	rpe |= (val & FB_FMAX_VMIN_FREQ_HI_MASK) << 5;
>  
>  	return rpe;
> @@ -2907,7 +2907,7 @@ int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
>  {
>  	u32 val;
>  
> -	valleyview_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
> +	vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
>  
>  	return val & 0xff;
>  }
> @@ -3018,7 +3018,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
>  	I915_WRITE(GEN6_RC_CONTROL,
>  		   GEN7_RC_CTL_TO_MODE);
>  
> -	valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val);
> +	vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val);
>  	switch ((val >> 6) & 3) {
>  	case 0:
>  	case 1:
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index a7c4b61..d150972 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -63,7 +63,7 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
>  	return 0;
>  }
>  
> -int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
> +int vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
>  {
>  	int ret;
>  
> @@ -77,7 +77,7 @@ int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
>  	return ret;
>  }
>  
> -int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
> +int vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
>  {
>  	int ret;
>  
> @@ -91,7 +91,7 @@ int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
>  	return ret;
>  }
>  
> -int valleyview_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
> +int vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
>  {
>  	int ret;
>  
> @@ -105,7 +105,7 @@ int valleyview_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
>  	return ret;
>  }
>  
> -u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg)
> +u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg)
>  {
>  	u32 val = 0;
>  
> @@ -115,7 +115,7 @@ u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg)
>  	return val;
>  }
>  
> -void intel_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val)
> +void vlv_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val)
>  {
>  	vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_DPIO,
>  			DPIO_OPCODE_REG_WRITE, reg, &val);

Should we change the sbi_read/write functions too?

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 5/5] drm/i915: change VLV IOSF sideband accessors to not return error code
  2013-05-23 18:06   ` Jesse Barnes
@ 2013-05-23 21:26     ` Daniel Vetter
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2013-05-23 21:26 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: Jani Nikula, intel-gfx

On Thu, May 23, 2013 at 11:06:46AM -0700, Jesse Barnes wrote:
> On Wed, 22 May 2013 15:36:20 +0300
> Jani Nikula <jani.nikula@intel.com> wrote:
> 
> > We never check the return values, and there's not much we could do on
> > errors anyway. Just simplify the signatures. No functional changes.
> > 
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c   |    7 +++----
> >  drivers/gpu/drm/i915/i915_drv.h       |    6 +++---
> >  drivers/gpu/drm/i915/i915_sysfs.c     |    2 +-
> >  drivers/gpu/drm/i915/intel_pm.c       |   18 +++++++-----------
> >  drivers/gpu/drm/i915/intel_sideband.c |   30 +++++++++++++-----------------
> >  5 files changed, 27 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 2151958..28bf3ab 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1013,16 +1013,15 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
> >  		u32 freq_sts, val;
> >  
> >  		mutex_lock(&dev_priv->rps.hw_lock);
> > -		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS,
> > -				      &freq_sts);
> > +		freq_sts = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> >  		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);
> >  
> > -		vlv_punit_read(dev_priv, PUNIT_FUSE_BUS1, &val);
> > +		val = vlv_punit_read(dev_priv, PUNIT_FUSE_BUS1);
> >  		seq_printf(m, "max GPU freq: %d MHz\n",
> >  			   vlv_gpu_freq(dev_priv->mem_freq, val));
> >  
> > -		vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
> > +		val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM);
> >  		seq_printf(m, "min GPU freq: %d MHz\n",
> >  			   vlv_gpu_freq(dev_priv->mem_freq, val));
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 56ec23e..cb84f4e9 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1919,9 +1919,9 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
> >  int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val);
> >  
> >  /* intel_sideband.c */
> > -int vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
> > -int vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
> > -int vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val);
> > +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr);
> > +void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val);
> > +u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr);
> >  u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg);
> >  void vlv_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val);
> >  u32 intel_sbi_read(struct drm_i915_private *dev_priv, u16 reg,
> > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> > index 588fa00..6875b56 100644
> > --- a/drivers/gpu/drm/i915/i915_sysfs.c
> > +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> > @@ -214,7 +214,7 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
> >  	mutex_lock(&dev_priv->rps.hw_lock);
> >  	if (IS_VALLEYVIEW(dev_priv->dev)) {
> >  		u32 freq;
> > -		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &freq);
> > +		freq = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> >  		ret = vlv_gpu_freq(dev_priv->mem_freq, (freq >> 8) & 0xff);
> >  	} else {
> >  		ret = dev_priv->rps.cur_delay * GT_FREQUENCY_MULTIPLIER;
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 17285a3..c0026d2 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2569,7 +2569,7 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
> >  	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
> >  
> >  	do {
> > -		vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
> > +		pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> >  		if (time_after(jiffies, timeout)) {
> >  			DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
> >  			break;
> > @@ -2577,7 +2577,7 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
> >  		udelay(10);
> >  	} while (pval & 1);
> >  
> > -	vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
> > +	pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> >  	if ((pval >> 8) != val)
> >  		DRM_DEBUG_DRIVER("punit overrode freq: %d requested, but got %d\n",
> >  			  val, pval >> 8);
> > @@ -2882,7 +2882,7 @@ int valleyview_rps_max_freq(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 val, rp0;
> >  
> > -	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FREQ_FUSE, &val);
> > +	val = vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FREQ_FUSE);
> >  
> >  	rp0 = (val & FB_GFX_MAX_FREQ_FUSE_MASK) >> FB_GFX_MAX_FREQ_FUSE_SHIFT;
> >  	/* Clamp to max */
> > @@ -2895,9 +2895,9 @@ static int valleyview_rps_rpe_freq(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 val, rpe;
> >  
> > -	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_LO, &val);
> > +	val = vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_LO);
> >  	rpe = (val & FB_FMAX_VMIN_FREQ_LO_MASK) >> FB_FMAX_VMIN_FREQ_LO_SHIFT;
> > -	vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_HI, &val);
> > +	val = vlv_nc_read(dev_priv, IOSF_NC_FB_GFX_FMAX_FUSE_HI);
> >  	rpe |= (val & FB_FMAX_VMIN_FREQ_HI_MASK) << 5;
> >  
> >  	return rpe;
> > @@ -2905,11 +2905,7 @@ static int valleyview_rps_rpe_freq(struct drm_i915_private *dev_priv)
> >  
> >  int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
> >  {
> > -	u32 val;
> > -
> > -	vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM, &val);
> > -
> > -	return val & 0xff;
> > +	return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
> >  }
> >  
> >  static void vlv_rps_timer_work(struct work_struct *work)
> > @@ -3018,7 +3014,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
> >  	I915_WRITE(GEN6_RC_CONTROL,
> >  		   GEN7_RC_CTL_TO_MODE);
> >  
> > -	vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &val);
> > +	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> >  	switch ((val >> 6) & 3) {
> >  	case 0:
> >  	case 1:
> > diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> > index d150972..9a0e6c5 100644
> > --- a/drivers/gpu/drm/i915/intel_sideband.c
> > +++ b/drivers/gpu/drm/i915/intel_sideband.c
> > @@ -63,46 +63,42 @@ static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
> >  	return 0;
> >  }
> >  
> > -int vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
> > +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr)
> >  {
> > -	int ret;
> > +	u32 val = 0;
> >  
> >  	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);
> > +	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;
> > +	return val;
> >  }
> >  
> > -int vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val)
> > +void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 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);
> > +	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 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val)
> > +u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr)
> >  {
> > -	int ret;
> > +	u32 val = 0;
> >  
> >  	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);
> > +	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;
> > +	return val;
> >  }
> >  
> >  u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg)
> 
> Looks fine.
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Entire series merged into dinq, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-05-23 21:26 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-22 12:36 [PATCH v2 0/5] drm/i915: sideband refactoring Jani Nikula
2013-05-22 12:36 ` [PATCH v2 1/5] drm/i915: group sideband register accessors to a new file Jani Nikula
2013-05-23 18:07   ` Jesse Barnes
2013-05-22 12:36 ` [PATCH v2 2/5] drm/i915: refactor VLV IOSF sideband accessors to use one helper Jani Nikula
2013-05-22 15:34   ` Jesse Barnes
2013-05-22 12:36 ` [PATCH v2 3/5] drm/i915: drop redundant warnings on not holding dpio_lock Jani Nikula
2013-05-23 18:07   ` Jesse Barnes
2013-05-22 12:36 ` [PATCH v2 4/5] drm/i915: rename VLV IOSF sideband functions logically Jani Nikula
2013-05-23 18:08   ` Jesse Barnes
2013-05-22 12:36 ` [PATCH v2 5/5] drm/i915: change VLV IOSF sideband accessors to not return error code Jani Nikula
2013-05-23 18:06   ` Jesse Barnes
2013-05-23 21:26     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.