[PATCH 0/8] Small refactorings v2

[PATCH 0/8] Small refactorings v2

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Hi

All the patches on this series have already been sent to this mailing list
(maybe with different titles) and received comments, except for patch 5, which
was posted on pastebin by Daniel. This series contains new versions of the
patches with all the bikesheds applied.

Please notice that even though patches 6 and 8 are only coding style changes,
they were created because we already had regressions caused by the confusing
code.

Thanks,
Paulo


Paulo Zanoni (8):
  drm/i915: create functions for the "unclaimed register" checks
  drm/i915: use FPGA_DBG for the "unclaimed register" checks
  drm/i915: clear the FPGA_DBG_RM_NOCLAIM bit at driver init
  drm/i915: use HAS_DDI on intel_hdmi.c and intel_display.c
  drm/i915: wait_event_timeout's timeout is in jiffies
  drm/i915: add aux_ch_ctl_reg to struct intel_dp
  drm/i915: rename sdvox_reg to hdmi_reg on HDMI context
  drm/i915: clarify confusion between SDVO and HDMI registers

 drivers/gpu/drm/i915/i915_dma.c      |    4 ++
 drivers/gpu/drm/i915/i915_drv.c      |   31 ++++++++++----
 drivers/gpu/drm/i915/i915_reg.h      |   22 +++++-----
 drivers/gpu/drm/i915/intel_ddi.c     |    4 +-
 drivers/gpu/drm/i915/intel_display.c |   42 ++++++++++---------
 drivers/gpu/drm/i915/intel_dp.c      |   77 ++++++++++++----------------------
 drivers/gpu/drm/i915/intel_drv.h     |    5 ++-
 drivers/gpu/drm/i915/intel_hdmi.c    |   74 ++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_sdvo.c    |   22 +++++-----
 9 files changed, 140 insertions(+), 141 deletions(-)

-- 
1.7.10.4

[PATCH 1/8] drm/i915: create functions for the "unclaimed register" checks

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

This avoids polluting i915_write##x and also allows us to reuse code
on i915_read##x.

v2: Rebase
v3: Convert the macros to static functions

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |   31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c5b8c81..b820c26 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1131,6 +1131,27 @@ ilk_dummy_write(struct drm_i915_private *dev_priv)
 	I915_WRITE_NOTRACE(MI_MODE, 0);
 }
 
+static void
+hsw_unclaimed_reg_clear(struct drm_i915_private *dev_priv, u32 reg)
+{
+	if (IS_HASWELL(dev_priv->dev) &&
+	    (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) {
+		DRM_ERROR("Unknown unclaimed register before writing to %x\n",
+			  reg);
+		I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED);
+	}
+}
+
+static void
+hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
+{
+	if (IS_HASWELL(dev_priv->dev) &&
+	    (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) {
+		DRM_ERROR("Unclaimed write to %x\n", reg);
+		writel(ERR_INT_MMIO_UNCLAIMED, dev_priv->regs + GEN7_ERR_INT);
+	}
+}
+
 #define __i915_read(x, y) \
 u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
 	u##x val = 0; \
@@ -1167,18 +1188,12 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
 	} \
 	if (IS_GEN5(dev_priv->dev)) \
 		ilk_dummy_write(dev_priv); \
-	if (IS_HASWELL(dev_priv->dev) && (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \
-		DRM_ERROR("Unknown unclaimed register before writing to %x\n", reg); \
-		I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED); \
-	} \
+	hsw_unclaimed_reg_clear(dev_priv, reg); \
 	write##y(val, dev_priv->regs + reg); \
 	if (unlikely(__fifo_ret)) { \
 		gen6_gt_check_fifodbg(dev_priv); \
 	} \
-	if (IS_HASWELL(dev_priv->dev) && (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) { \
-		DRM_ERROR("Unclaimed write to %x\n", reg); \
-		writel(ERR_INT_MMIO_UNCLAIMED, dev_priv->regs + GEN7_ERR_INT);	\
-	} \
+	hsw_unclaimed_reg_check(dev_priv, reg); \
 }
 __i915_write(8, b)
 __i915_write(16, w)
-- 
1.7.10.4

[PATCH 2/8] drm/i915: use FPGA_DBG for the "unclaimed register" checks

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We plan to treat GEN7_ERR_INT as an interrupt, so use this register
for the checks inside I915_WRITE. This way we can have the best of
both worlds: the error message with a register address and the

V2: Split in 2 patches: one for the macro, one for changing the
register, as requested by Ben.
V3: Rebase.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |    8 ++++----
 drivers/gpu/drm/i915/i915_reg.h |    3 +++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b820c26..b916d23 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1135,10 +1135,10 @@ static void
 hsw_unclaimed_reg_clear(struct drm_i915_private *dev_priv, u32 reg)
 {
 	if (IS_HASWELL(dev_priv->dev) &&
-	    (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) {
+	    (I915_READ_NOTRACE(FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
 		DRM_ERROR("Unknown unclaimed register before writing to %x\n",
 			  reg);
-		I915_WRITE_NOTRACE(GEN7_ERR_INT, ERR_INT_MMIO_UNCLAIMED);
+		I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
 	}
 }
 
@@ -1146,9 +1146,9 @@ static void
 hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
 {
 	if (IS_HASWELL(dev_priv->dev) &&
-	    (I915_READ_NOTRACE(GEN7_ERR_INT) & ERR_INT_MMIO_UNCLAIMED)) {
+	    (I915_READ_NOTRACE(FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
 		DRM_ERROR("Unclaimed write to %x\n", reg);
-		writel(ERR_INT_MMIO_UNCLAIMED, dev_priv->regs + GEN7_ERR_INT);
+		I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 527b664..9e5844b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -522,6 +522,9 @@
 #define GEN7_ERR_INT	0x44040
 #define   ERR_INT_MMIO_UNCLAIMED (1<<13)
 
+#define FPGA_DBG		0x42300
+#define   FPGA_DBG_RM_NOCLAIM	(1<<31)
+
 #define DERRMR		0x44050
 
 /* GM45+ chicken bits -- debug workaround bits that may be required
-- 
1.7.10.4

[PATCH 3/8] drm/i915: clear the FPGA_DBG_RM_NOCLAIM bit at driver init

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Otherwise, if the BIOS did anything wrong, our first I915_{WRITE,READ}
will give us "unclaimed register"  messages.

V2: Even earlier.

Bugzilla: http://bugs.freedesktop.org/show_bug.cgi?id=58897
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4fa6beb..6d8672e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1542,6 +1542,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		goto put_gmch;
 	}
 
+	/* This must happen before any I915_{READ,WRITE}: */
+	if (IS_HASWELL(dev))
+		I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+
 	aperture_size = dev_priv->gtt.mappable_end;
 
 	dev_priv->gtt.mappable =
-- 
1.7.10.4

[PATCH 4/8] drm/i915: use HAS_DDI on intel_hdmi.c and intel_display.c

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Since basically every code called on these places comes from
intel_ddi.c

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c   |   12 ++++++------
 drivers/gpu/drm/i915/intel_hdmi.c |    2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7b8bfe8..770ec90 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -332,7 +332,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	uint32_t status;
 	bool done;
 
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		switch (intel_dig_port->port) {
 		case PORT_A:
 			ch_ctl = DPA_AUX_CH_CTL;
@@ -387,7 +387,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	 */
 	pm_qos_update_request(&dev_priv->pm_qos, 0);
 
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		switch (intel_dig_port->port) {
 		case PORT_A:
 			ch_ctl = DPA_AUX_CH_CTL;
@@ -842,7 +842,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	intel_link_compute_m_n(intel_crtc->bpp, lane_count,
 			       mode->clock, adjusted_mode->clock, &m_n);
 
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
 			   TU_SIZE(m_n.tu) | m_n.gmch_m);
 		I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
@@ -1537,7 +1537,7 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
 		case DP_TRAIN_VOLTAGE_SWING_400:
 			return DP_TRAIN_PRE_EMPHASIS_9_5;
@@ -1745,7 +1745,7 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp, uint32_t *DP)
 	uint32_t signal_levels, mask;
 	uint8_t train_set = intel_dp->train_set[0];
 
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		signal_levels = intel_hsw_signal_levels(train_set);
 		mask = DDI_BUF_EMP_MASK;
 	} else if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
@@ -1776,7 +1776,7 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 	int ret;
 	uint32_t temp;
 
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		temp = I915_READ(DP_TP_CTL(port));
 
 		if (dp_train_pat & DP_LINK_SCRAMBLING_DISABLE)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 5a6138c..ed65c6d 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1044,7 +1044,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	} else if (IS_VALLEYVIEW(dev)) {
 		intel_hdmi->write_infoframe = vlv_write_infoframe;
 		intel_hdmi->set_infoframes = vlv_set_infoframes;
-	} else if (IS_HASWELL(dev)) {
+	} else if (HAS_DDI(dev)) {
 		intel_hdmi->write_infoframe = hsw_write_infoframe;
 		intel_hdmi->set_infoframes = hsw_set_infoframes;
 	} else if (HAS_PCH_IBX(dev)) {
-- 
1.7.10.4

[PATCH 5/8] drm/i915: wait_event_timeout's timeout is in jiffies

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

So use msecs_to_jiffies(10) to make the timeout the same as in the
"!has_aux_irq" case.

This patch was initially written by Daniel Vetter and posted on
pastebin a few weeks ago. I'm just bringing it to the mailing list.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 770ec90..00bc79f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -353,7 +353,8 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 
 #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
 	if (has_aux_irq)
-		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
+		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
+					  msecs_to_jiffies(10));
 	else
 		done = wait_for_atomic(C, 10) == 0;
 	if (!done)
-- 
1.7.10.4

[PATCH 6/8] drm/i915: add aux_ch_ctl_reg to struct intel_dp

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

This way we can remove some duplicated code and avoid more mistakes
and regressions with these registers in the future.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |   66 ++++++++++++--------------------------
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 2 files changed, 22 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 00bc79f..0e2750c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -328,29 +328,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint32_t ch_ctl = intel_dp->output_reg + 0x10;
+	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
 	uint32_t status;
 	bool done;
 
-	if (HAS_DDI(dev)) {
-		switch (intel_dig_port->port) {
-		case PORT_A:
-			ch_ctl = DPA_AUX_CH_CTL;
-			break;
-		case PORT_B:
-			ch_ctl = PCH_DPB_AUX_CH_CTL;
-			break;
-		case PORT_C:
-			ch_ctl = PCH_DPC_AUX_CH_CTL;
-			break;
-		case PORT_D:
-			ch_ctl = PCH_DPD_AUX_CH_CTL;
-			break;
-		default:
-			BUG();
-		}
-	}
-
 #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
 	if (has_aux_irq)
 		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
@@ -370,11 +351,10 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 		uint8_t *send, int send_bytes,
 		uint8_t *recv, int recv_size)
 {
-	uint32_t output_reg = intel_dp->output_reg;
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint32_t ch_ctl = output_reg + 0x10;
+	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
 	uint32_t ch_data = ch_ctl + 4;
 	int i, ret, recv_bytes;
 	uint32_t status;
@@ -388,29 +368,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	 */
 	pm_qos_update_request(&dev_priv->pm_qos, 0);
 
-	if (HAS_DDI(dev)) {
-		switch (intel_dig_port->port) {
-		case PORT_A:
-			ch_ctl = DPA_AUX_CH_CTL;
-			ch_data = DPA_AUX_CH_DATA1;
-			break;
-		case PORT_B:
-			ch_ctl = PCH_DPB_AUX_CH_CTL;
-			ch_data = PCH_DPB_AUX_CH_DATA1;
-			break;
-		case PORT_C:
-			ch_ctl = PCH_DPC_AUX_CH_CTL;
-			ch_data = PCH_DPC_AUX_CH_DATA1;
-			break;
-		case PORT_D:
-			ch_ctl = PCH_DPD_AUX_CH_CTL;
-			ch_data = PCH_DPD_AUX_CH_DATA1;
-			break;
-		default:
-			BUG();
-		}
-	}
-
 	intel_dp_check_edp(intel_dp);
 	/* The clock divider is based off the hrawclk,
 	 * and would like to run at 2MHz. So, take the
@@ -2832,6 +2789,25 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	else
 		intel_connector->get_hw_state = intel_connector_get_hw_state;
 
+	intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
+	if (HAS_DDI(dev)) {
+		switch (intel_dig_port->port) {
+		case PORT_A:
+			intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
+			break;
+		case PORT_B:
+			intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
+			break;
+		case PORT_C:
+			intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
+			break;
+		case PORT_D:
+			intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
+			break;
+		default:
+			BUG();
+		}
+	}
 
 	/* Set up the DDC bus. */
 	switch (port) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 005a91f..6f41a07 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -366,6 +366,7 @@ struct intel_hdmi {
 
 struct intel_dp {
 	uint32_t output_reg;
+	uint32_t aux_ch_ctl_reg;
 	uint32_t DP;
 	uint8_t  link_configuration[DP_LINK_CONFIGURATION_SIZE];
 	bool has_audio;
-- 
1.7.10.4

[PATCH 7/8] drm/i915: rename sdvox_reg to hdmi_reg on HDMI context

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Some (but not all) of the HDMI registers can be used to control sDVO,
so those registers have two names. IMHO, when we're talking about
HDMI, we really should call the HDMI control register "hdmi_reg"
instead of "sdvox_reg", otherwise we'll just confuse people reading
our code (we now have platforms with HDMI but without SDVO). So now
"struct intel_hdmi" has a member called "hdmi_reg" instead of
"sdvox_reg".

Also, don't worry: "struct intel_sdvo" still has a member called
"sdvo_reg".

v2: Rebase (v1 was sent in May 2012).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  |    4 +--
 drivers/gpu/drm/i915/intel_drv.h  |    4 +--
 drivers/gpu/drm/i915/intel_hdmi.c |   72 ++++++++++++++++++-------------------
 3 files changed, 39 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 816c45c..56bb7cb 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1538,9 +1538,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 	intel_dig_port->port_reversal = I915_READ(DDI_BUF_CTL(port)) &
 					DDI_BUF_PORT_REVERSAL;
 	if (hdmi_connector)
-		intel_dig_port->hdmi.sdvox_reg = DDI_BUF_CTL(port);
-	else
-		intel_dig_port->hdmi.sdvox_reg = 0;
+		intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
 	intel_dig_port->dp.output_reg = DDI_BUF_CTL(port);
 
 	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6f41a07..8659df2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -347,7 +347,7 @@ struct dip_infoframe {
 } __attribute__((packed));
 
 struct intel_hdmi {
-	u32 sdvox_reg;
+	u32 hdmi_reg;
 	int ddc_bus;
 	uint32_t color_range;
 	bool color_range_auto;
@@ -444,7 +444,7 @@ extern void intel_attach_broadcast_rgb_property(struct drm_connector *connector)
 
 extern void intel_crt_init(struct drm_device *dev);
 extern void intel_hdmi_init(struct drm_device *dev,
-			    int sdvox_reg, enum port port);
+			    int hdmi_reg, enum port port);
 extern void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 				      struct intel_connector *intel_connector);
 extern struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ed65c6d..fcb36c6 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -50,7 +50,7 @@ assert_hdmi_port_disabled(struct intel_hdmi *intel_hdmi)
 
 	enabled_bits = HAS_DDI(dev) ? DDI_BUF_CTL_ENABLE : SDVO_ENABLE;
 
-	WARN(I915_READ(intel_hdmi->sdvox_reg) & enabled_bits,
+	WARN(I915_READ(intel_hdmi->hdmi_reg) & enabled_bits,
 	     "HDMI port enabled, expecting disabled\n");
 }
 
@@ -597,40 +597,40 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
-	u32 sdvox;
+	u32 hdmi_val;
 
-	sdvox = SDVO_ENCODING_HDMI;
+	hdmi_val = SDVO_ENCODING_HDMI;
 	if (!HAS_PCH_SPLIT(dev))
-		sdvox |= intel_hdmi->color_range;
+		hdmi_val |= intel_hdmi->color_range;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
-		sdvox |= SDVO_VSYNC_ACTIVE_HIGH;
+		hdmi_val |= SDVO_VSYNC_ACTIVE_HIGH;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
-		sdvox |= SDVO_HSYNC_ACTIVE_HIGH;
+		hdmi_val |= SDVO_HSYNC_ACTIVE_HIGH;
 
 	if (intel_crtc->bpp > 24)
-		sdvox |= COLOR_FORMAT_12bpc;
+		hdmi_val |= COLOR_FORMAT_12bpc;
 	else
-		sdvox |= COLOR_FORMAT_8bpc;
+		hdmi_val |= COLOR_FORMAT_8bpc;
 
 	/* Required on CPT */
 	if (intel_hdmi->has_hdmi_sink && HAS_PCH_CPT(dev))
-		sdvox |= HDMI_MODE_SELECT;
+		hdmi_val |= HDMI_MODE_SELECT;
 
 	if (intel_hdmi->has_audio) {
 		DRM_DEBUG_DRIVER("Enabling HDMI audio on pipe %c\n",
 				 pipe_name(intel_crtc->pipe));
-		sdvox |= SDVO_AUDIO_ENABLE;
-		sdvox |= SDVO_NULL_PACKETS_DURING_VSYNC;
+		hdmi_val |= SDVO_AUDIO_ENABLE;
+		hdmi_val |= SDVO_NULL_PACKETS_DURING_VSYNC;
 		intel_write_eld(encoder, adjusted_mode);
 	}
 
 	if (HAS_PCH_CPT(dev))
-		sdvox |= PORT_TRANS_SEL_CPT(intel_crtc->pipe);
+		hdmi_val |= PORT_TRANS_SEL_CPT(intel_crtc->pipe);
 	else if (intel_crtc->pipe == PIPE_B)
-		sdvox |= SDVO_PIPE_B_SELECT;
+		hdmi_val |= SDVO_PIPE_B_SELECT;
 
-	I915_WRITE(intel_hdmi->sdvox_reg, sdvox);
-	POSTING_READ(intel_hdmi->sdvox_reg);
+	I915_WRITE(intel_hdmi->hdmi_reg, hdmi_val);
+	POSTING_READ(intel_hdmi->hdmi_reg);
 
 	intel_hdmi->set_infoframes(encoder, adjusted_mode);
 }
@@ -643,7 +643,7 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	u32 tmp;
 
-	tmp = I915_READ(intel_hdmi->sdvox_reg);
+	tmp = I915_READ(intel_hdmi->hdmi_reg);
 
 	if (!(tmp & SDVO_ENABLE))
 		return false;
@@ -667,7 +667,7 @@ static void intel_enable_hdmi(struct intel_encoder *encoder)
 	if (intel_hdmi->has_audio)
 		enable_bits |= SDVO_AUDIO_ENABLE;
 
-	temp = I915_READ(intel_hdmi->sdvox_reg);
+	temp = I915_READ(intel_hdmi->hdmi_reg);
 
 	/* HW workaround for IBX, we need to move the port to transcoder A
 	 * before disabling it. */
@@ -684,21 +684,21 @@ static void intel_enable_hdmi(struct intel_encoder *encoder)
 	 * we do this anyway which shows more stable in testing.
 	 */
 	if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(intel_hdmi->sdvox_reg, temp & ~SDVO_ENABLE);
-		POSTING_READ(intel_hdmi->sdvox_reg);
+		I915_WRITE(intel_hdmi->hdmi_reg, temp & ~SDVO_ENABLE);
+		POSTING_READ(intel_hdmi->hdmi_reg);
 	}
 
 	temp |= enable_bits;
 
-	I915_WRITE(intel_hdmi->sdvox_reg, temp);
-	POSTING_READ(intel_hdmi->sdvox_reg);
+	I915_WRITE(intel_hdmi->hdmi_reg, temp);
+	POSTING_READ(intel_hdmi->hdmi_reg);
 
 	/* HW workaround, need to write this twice for issue that may result
 	 * in first write getting masked.
 	 */
 	if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(intel_hdmi->sdvox_reg, temp);
-		POSTING_READ(intel_hdmi->sdvox_reg);
+		I915_WRITE(intel_hdmi->hdmi_reg, temp);
+		POSTING_READ(intel_hdmi->hdmi_reg);
 	}
 }
 
@@ -710,7 +710,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
 	u32 temp;
 	u32 enable_bits = SDVO_ENABLE | SDVO_AUDIO_ENABLE;
 
-	temp = I915_READ(intel_hdmi->sdvox_reg);
+	temp = I915_READ(intel_hdmi->hdmi_reg);
 
 	/* HW workaround for IBX, we need to move the port to transcoder A
 	 * before disabling it. */
@@ -720,12 +720,12 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
 
 		if (temp & SDVO_PIPE_B_SELECT) {
 			temp &= ~SDVO_PIPE_B_SELECT;
-			I915_WRITE(intel_hdmi->sdvox_reg, temp);
-			POSTING_READ(intel_hdmi->sdvox_reg);
+			I915_WRITE(intel_hdmi->hdmi_reg, temp);
+			POSTING_READ(intel_hdmi->hdmi_reg);
 
 			/* Again we need to write this twice. */
-			I915_WRITE(intel_hdmi->sdvox_reg, temp);
-			POSTING_READ(intel_hdmi->sdvox_reg);
+			I915_WRITE(intel_hdmi->hdmi_reg, temp);
+			POSTING_READ(intel_hdmi->hdmi_reg);
 
 			/* Transcoder selection bits only update
 			 * effectively on vblank. */
@@ -740,21 +740,21 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
 	 * we do this anyway which shows more stable in testing.
 	 */
 	if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(intel_hdmi->sdvox_reg, temp & ~SDVO_ENABLE);
-		POSTING_READ(intel_hdmi->sdvox_reg);
+		I915_WRITE(intel_hdmi->hdmi_reg, temp & ~SDVO_ENABLE);
+		POSTING_READ(intel_hdmi->hdmi_reg);
 	}
 
 	temp &= ~enable_bits;
 
-	I915_WRITE(intel_hdmi->sdvox_reg, temp);
-	POSTING_READ(intel_hdmi->sdvox_reg);
+	I915_WRITE(intel_hdmi->hdmi_reg, temp);
+	POSTING_READ(intel_hdmi->hdmi_reg);
 
 	/* HW workaround, need to write this twice for issue that may result
 	 * in first write getting masked.
 	 */
 	if (HAS_PCH_SPLIT(dev)) {
-		I915_WRITE(intel_hdmi->sdvox_reg, temp);
-		POSTING_READ(intel_hdmi->sdvox_reg);
+		I915_WRITE(intel_hdmi->hdmi_reg, temp);
+		POSTING_READ(intel_hdmi->hdmi_reg);
 	}
 }
 
@@ -1075,7 +1075,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	}
 }
 
-void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
+void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 {
 	struct intel_digital_port *intel_dig_port;
 	struct intel_encoder *intel_encoder;
@@ -1108,7 +1108,7 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
 	intel_encoder->cloneable = false;
 
 	intel_dig_port->port = port;
-	intel_dig_port->hdmi.sdvox_reg = sdvox_reg;
+	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
 	intel_dig_port->dp.output_reg = 0;
 
 	intel_hdmi_init_connector(intel_dig_port, intel_connector);
-- 
1.7.10.4

[PATCH 8/8] drm/i915: clarify confusion between SDVO and HDMI registers

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Some HDMI registers can be used for SDVO, so saying "HDMIB" should be
the same as saying "SDVOB" for a given HW generation. This was not
true and led to confusions and even a regression.

Previously we had:
  - SDVO{B,C} defined as the Gen3+ registers
  - HDMI{B,C,D} and PCH_SDVOB defined as the PCH registers

But now:
  - SDVO{B,C} became GEN3_SDVO{B,C} on SDVO code
  - SDVO{B,C} became GEN4_HDMI{B,C} on HDMI code
  - HDMI{B,C,D} became PCH_HDMI{B,C,D}
  - PCH_SDVOB is still the same thing

v2: Rebase (v1 was sent in May 2012).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |   19 ++++++++-------
 drivers/gpu/drm/i915/intel_display.c |   42 ++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_sdvo.c    |   22 +++++++++---------
 3 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9e5844b..cd31af2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1680,8 +1680,9 @@
 #define   SDVOB_HOTPLUG_INT_STATUS_I915		(1 << 6)
 
 /* SDVO port control */
-#define SDVOB			0x61140
-#define SDVOC			0x61160
+#define GEN3_SDVOB		0x61140
+#define GEN3_SDVOC		0x61160
+#define PCH_SDVOB		0xe1140
 #define   SDVO_ENABLE		(1 << 31)
 #define   SDVO_PIPE_B_SELECT	(1 << 30)
 #define   SDVO_STALL_SELECT	(1 << 29)
@@ -3979,8 +3980,12 @@
 #define FDI_PLL_CTL_1           0xfe000
 #define FDI_PLL_CTL_2           0xfe004
 
-/* or SDVOB */
-#define HDMIB   0xe1140
+/* The same register may be used for SDVO or HDMI */
+#define GEN4_HDMIB	GEN3_SDVOB
+#define GEN4_HDMIC	GEN3_SDVOC
+#define PCH_HDMIB	PCH_SDVOB
+#define PCH_HDMIC	0xe1150
+#define PCH_HDMID	0xe1160
 #define  PORT_ENABLE    (1 << 31)
 #define  TRANSCODER(pipe)       ((pipe) << 30)
 #define  TRANSCODER_CPT(pipe)   ((pipe) << 29)
@@ -4001,12 +4006,6 @@
 #define  HSYNC_ACTIVE_HIGH      (1 << 3)
 #define  PORT_DETECTED          (1 << 2)
 
-/* PCH SDVOB multiplex with HDMIB */
-#define PCH_SDVOB	HDMIB
-
-#define HDMIC   0xe1150
-#define HDMID   0xe1160
-
 #define PCH_LVDS	0xe1180
 #define  LVDS_DETECTED	(1 << 1)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6eb3882..744db70 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1419,9 +1419,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 	     "PCH LVDS enabled on transcoder %c, should be disabled\n",
 	     pipe_name(pipe));
 
-	assert_pch_hdmi_disabled(dev_priv, pipe, HDMIB);
-	assert_pch_hdmi_disabled(dev_priv, pipe, HDMIC);
-	assert_pch_hdmi_disabled(dev_priv, pipe, HDMID);
+	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIB);
+	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIC);
+	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
 }
 
 /**
@@ -8323,20 +8323,20 @@ static void intel_setup_outputs(struct drm_device *dev)
 		if (has_edp_a(dev))
 			intel_dp_init(dev, DP_A, PORT_A);
 
-		if (I915_READ(HDMIB) & PORT_DETECTED) {
+		if (I915_READ(PCH_HDMIB) & PORT_DETECTED) {
 			/* PCH SDVOB multiplex with HDMIB */
 			found = intel_sdvo_init(dev, PCH_SDVOB, true);
 			if (!found)
-				intel_hdmi_init(dev, HDMIB, PORT_B);
+				intel_hdmi_init(dev, PCH_HDMIB, PORT_B);
 			if (!found && (I915_READ(PCH_DP_B) & DP_DETECTED))
 				intel_dp_init(dev, PCH_DP_B, PORT_B);
 		}
 
-		if (I915_READ(HDMIC) & PORT_DETECTED)
-			intel_hdmi_init(dev, HDMIC, PORT_C);
+		if (I915_READ(PCH_HDMIC) & PORT_DETECTED)
+			intel_hdmi_init(dev, PCH_HDMIC, PORT_C);
 
-		if (!dpd_is_edp && I915_READ(HDMID) & PORT_DETECTED)
-			intel_hdmi_init(dev, HDMID, PORT_D);
+		if (!dpd_is_edp && I915_READ(PCH_HDMID) & PORT_DETECTED)
+			intel_hdmi_init(dev, PCH_HDMID, PORT_D);
 
 		if (I915_READ(PCH_DP_C) & DP_DETECTED)
 			intel_dp_init(dev, PCH_DP_C, PORT_C);
@@ -8348,24 +8348,26 @@ static void intel_setup_outputs(struct drm_device *dev)
 		if (I915_READ(VLV_DISPLAY_BASE + DP_C) & DP_DETECTED)
 			intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C, PORT_C);
 
-		if (I915_READ(VLV_DISPLAY_BASE + SDVOB) & PORT_DETECTED) {
-			intel_hdmi_init(dev, VLV_DISPLAY_BASE + SDVOB, PORT_B);
+		if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIB) & PORT_DETECTED) {
+			intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIB,
+					PORT_B);
 			if (I915_READ(VLV_DISPLAY_BASE + DP_B) & DP_DETECTED)
 				intel_dp_init(dev, VLV_DISPLAY_BASE + DP_B, PORT_B);
 		}
 
-		if (I915_READ(VLV_DISPLAY_BASE + SDVOC) & PORT_DETECTED)
-			intel_hdmi_init(dev, VLV_DISPLAY_BASE + SDVOC, PORT_C);
+		if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) & PORT_DETECTED)
+			intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC,
+					PORT_C);
 
 	} else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
 		bool found = false;
 
-		if (I915_READ(SDVOB) & SDVO_DETECTED) {
+		if (I915_READ(GEN3_SDVOB) & SDVO_DETECTED) {
 			DRM_DEBUG_KMS("probing SDVOB\n");
-			found = intel_sdvo_init(dev, SDVOB, true);
+			found = intel_sdvo_init(dev, GEN3_SDVOB, true);
 			if (!found && SUPPORTS_INTEGRATED_HDMI(dev)) {
 				DRM_DEBUG_KMS("probing HDMI on SDVOB\n");
-				intel_hdmi_init(dev, SDVOB, PORT_B);
+				intel_hdmi_init(dev, GEN4_HDMIB, PORT_B);
 			}
 
 			if (!found && SUPPORTS_INTEGRATED_DP(dev)) {
@@ -8376,16 +8378,16 @@ static void intel_setup_outputs(struct drm_device *dev)
 
 		/* Before G4X SDVOC doesn't have its own detect register */
 
-		if (I915_READ(SDVOB) & SDVO_DETECTED) {
+		if (I915_READ(GEN3_SDVOB) & SDVO_DETECTED) {
 			DRM_DEBUG_KMS("probing SDVOC\n");
-			found = intel_sdvo_init(dev, SDVOC, false);
+			found = intel_sdvo_init(dev, GEN3_SDVOC, false);
 		}
 
-		if (!found && (I915_READ(SDVOC) & SDVO_DETECTED)) {
+		if (!found && (I915_READ(GEN3_SDVOC) & SDVO_DETECTED)) {
 
 			if (SUPPORTS_INTEGRATED_HDMI(dev)) {
 				DRM_DEBUG_KMS("probing HDMI on SDVOC\n");
-				intel_hdmi_init(dev, SDVOC, PORT_C);
+				intel_hdmi_init(dev, GEN4_HDMIC, PORT_C);
 			}
 			if (SUPPORTS_INTEGRATED_DP(dev)) {
 				DRM_DEBUG_KMS("probing DP_C\n");
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index f01063a..7d94db8 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -246,11 +246,11 @@ static void intel_sdvo_write_sdvox(struct intel_sdvo *intel_sdvo, u32 val)
 		return;
 	}
 
-	if (intel_sdvo->sdvo_reg == SDVOB) {
-		cval = I915_READ(SDVOC);
-	} else {
-		bval = I915_READ(SDVOB);
-	}
+	if (intel_sdvo->sdvo_reg == GEN3_SDVOB)
+		cval = I915_READ(GEN3_SDVOC);
+	else
+		bval = I915_READ(GEN3_SDVOB);
+
 	/*
 	 * Write the registers twice for luck. Sometimes,
 	 * writing them only once doesn't appear to 'stick'.
@@ -258,10 +258,10 @@ static void intel_sdvo_write_sdvox(struct intel_sdvo *intel_sdvo, u32 val)
 	 */
 	for (i = 0; i < 2; i++)
 	{
-		I915_WRITE(SDVOB, bval);
-		I915_READ(SDVOB);
-		I915_WRITE(SDVOC, cval);
-		I915_READ(SDVOC);
+		I915_WRITE(GEN3_SDVOB, bval);
+		I915_READ(GEN3_SDVOB);
+		I915_WRITE(GEN3_SDVOC, cval);
+		I915_READ(GEN3_SDVOC);
 	}
 }
 
@@ -1182,10 +1182,10 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 	} else {
 		sdvox = I915_READ(intel_sdvo->sdvo_reg);
 		switch (intel_sdvo->sdvo_reg) {
-		case SDVOB:
+		case GEN3_SDVOB:
 			sdvox &= SDVOB_PRESERVE_MASK;
 			break;
-		case SDVOC:
+		case GEN3_SDVOC:
 			sdvox &= SDVOC_PRESERVE_MASK;
 			break;
 		}
-- 
1.7.10.4

Re: [PATCH 3/8] drm/i915: clear the FPGA_DBG_RM_NOCLAIM bit at driver init

[Chris Wilson]

On Mon, Feb 18, 2013 at 07:00:22PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Otherwise, if the BIOS did anything wrong, our first I915_{WRITE,READ}
> will give us "unclaimed register"  messages.
> 
> V2: Even earlier.

Call it intel_early_sanitize_regs() as it seems like a function that
will only grow with time as more warts are discovered.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 7/8] drm/i915: rename sdvox_reg to hdmi_reg on HDMI context

[Daniel Vetter]

On Mon, Feb 18, 2013 at 07:00:26PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Some (but not all) of the HDMI registers can be used to control sDVO,
> so those registers have two names. IMHO, when we're talking about
> HDMI, we really should call the HDMI control register "hdmi_reg"
> instead of "sdvox_reg", otherwise we'll just confuse people reading
> our code (we now have platforms with HDMI but without SDVO). So now
> "struct intel_hdmi" has a member called "hdmi_reg" instead of
> "sdvox_reg".
> 
> Also, don't worry: "struct intel_sdvo" still has a member called
> "sdvo_reg".
> 
> v2: Rebase (v1 was sent in May 2012).
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I've merged patches 4-7 from this series, thanks.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_ddi.c  |    4 +--
>  drivers/gpu/drm/i915/intel_drv.h  |    4 +--
>  drivers/gpu/drm/i915/intel_hdmi.c |   72 ++++++++++++++++++-------------------
>  3 files changed, 39 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 816c45c..56bb7cb 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1538,9 +1538,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  	intel_dig_port->port_reversal = I915_READ(DDI_BUF_CTL(port)) &
>  					DDI_BUF_PORT_REVERSAL;
>  	if (hdmi_connector)
> -		intel_dig_port->hdmi.sdvox_reg = DDI_BUF_CTL(port);
> -	else
> -		intel_dig_port->hdmi.sdvox_reg = 0;
> +		intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
>  	intel_dig_port->dp.output_reg = DDI_BUF_CTL(port);
>  
>  	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6f41a07..8659df2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -347,7 +347,7 @@ struct dip_infoframe {
>  } __attribute__((packed));
>  
>  struct intel_hdmi {
> -	u32 sdvox_reg;
> +	u32 hdmi_reg;
>  	int ddc_bus;
>  	uint32_t color_range;
>  	bool color_range_auto;
> @@ -444,7 +444,7 @@ extern void intel_attach_broadcast_rgb_property(struct drm_connector *connector)
>  
>  extern void intel_crt_init(struct drm_device *dev);
>  extern void intel_hdmi_init(struct drm_device *dev,
> -			    int sdvox_reg, enum port port);
> +			    int hdmi_reg, enum port port);
>  extern void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  				      struct intel_connector *intel_connector);
>  extern struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index ed65c6d..fcb36c6 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -50,7 +50,7 @@ assert_hdmi_port_disabled(struct intel_hdmi *intel_hdmi)
>  
>  	enabled_bits = HAS_DDI(dev) ? DDI_BUF_CTL_ENABLE : SDVO_ENABLE;
>  
> -	WARN(I915_READ(intel_hdmi->sdvox_reg) & enabled_bits,
> +	WARN(I915_READ(intel_hdmi->hdmi_reg) & enabled_bits,
>  	     "HDMI port enabled, expecting disabled\n");
>  }
>  
> @@ -597,40 +597,40 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> -	u32 sdvox;
> +	u32 hdmi_val;
>  
> -	sdvox = SDVO_ENCODING_HDMI;
> +	hdmi_val = SDVO_ENCODING_HDMI;
>  	if (!HAS_PCH_SPLIT(dev))
> -		sdvox |= intel_hdmi->color_range;
> +		hdmi_val |= intel_hdmi->color_range;
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
> -		sdvox |= SDVO_VSYNC_ACTIVE_HIGH;
> +		hdmi_val |= SDVO_VSYNC_ACTIVE_HIGH;
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
> -		sdvox |= SDVO_HSYNC_ACTIVE_HIGH;
> +		hdmi_val |= SDVO_HSYNC_ACTIVE_HIGH;
>  
>  	if (intel_crtc->bpp > 24)
> -		sdvox |= COLOR_FORMAT_12bpc;
> +		hdmi_val |= COLOR_FORMAT_12bpc;
>  	else
> -		sdvox |= COLOR_FORMAT_8bpc;
> +		hdmi_val |= COLOR_FORMAT_8bpc;
>  
>  	/* Required on CPT */
>  	if (intel_hdmi->has_hdmi_sink && HAS_PCH_CPT(dev))
> -		sdvox |= HDMI_MODE_SELECT;
> +		hdmi_val |= HDMI_MODE_SELECT;
>  
>  	if (intel_hdmi->has_audio) {
>  		DRM_DEBUG_DRIVER("Enabling HDMI audio on pipe %c\n",
>  				 pipe_name(intel_crtc->pipe));
> -		sdvox |= SDVO_AUDIO_ENABLE;
> -		sdvox |= SDVO_NULL_PACKETS_DURING_VSYNC;
> +		hdmi_val |= SDVO_AUDIO_ENABLE;
> +		hdmi_val |= SDVO_NULL_PACKETS_DURING_VSYNC;
>  		intel_write_eld(encoder, adjusted_mode);
>  	}
>  
>  	if (HAS_PCH_CPT(dev))
> -		sdvox |= PORT_TRANS_SEL_CPT(intel_crtc->pipe);
> +		hdmi_val |= PORT_TRANS_SEL_CPT(intel_crtc->pipe);
>  	else if (intel_crtc->pipe == PIPE_B)
> -		sdvox |= SDVO_PIPE_B_SELECT;
> +		hdmi_val |= SDVO_PIPE_B_SELECT;
>  
> -	I915_WRITE(intel_hdmi->sdvox_reg, sdvox);
> -	POSTING_READ(intel_hdmi->sdvox_reg);
> +	I915_WRITE(intel_hdmi->hdmi_reg, hdmi_val);
> +	POSTING_READ(intel_hdmi->hdmi_reg);
>  
>  	intel_hdmi->set_infoframes(encoder, adjusted_mode);
>  }
> @@ -643,7 +643,7 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	u32 tmp;
>  
> -	tmp = I915_READ(intel_hdmi->sdvox_reg);
> +	tmp = I915_READ(intel_hdmi->hdmi_reg);
>  
>  	if (!(tmp & SDVO_ENABLE))
>  		return false;
> @@ -667,7 +667,7 @@ static void intel_enable_hdmi(struct intel_encoder *encoder)
>  	if (intel_hdmi->has_audio)
>  		enable_bits |= SDVO_AUDIO_ENABLE;
>  
> -	temp = I915_READ(intel_hdmi->sdvox_reg);
> +	temp = I915_READ(intel_hdmi->hdmi_reg);
>  
>  	/* HW workaround for IBX, we need to move the port to transcoder A
>  	 * before disabling it. */
> @@ -684,21 +684,21 @@ static void intel_enable_hdmi(struct intel_encoder *encoder)
>  	 * we do this anyway which shows more stable in testing.
>  	 */
>  	if (HAS_PCH_SPLIT(dev)) {
> -		I915_WRITE(intel_hdmi->sdvox_reg, temp & ~SDVO_ENABLE);
> -		POSTING_READ(intel_hdmi->sdvox_reg);
> +		I915_WRITE(intel_hdmi->hdmi_reg, temp & ~SDVO_ENABLE);
> +		POSTING_READ(intel_hdmi->hdmi_reg);
>  	}
>  
>  	temp |= enable_bits;
>  
> -	I915_WRITE(intel_hdmi->sdvox_reg, temp);
> -	POSTING_READ(intel_hdmi->sdvox_reg);
> +	I915_WRITE(intel_hdmi->hdmi_reg, temp);
> +	POSTING_READ(intel_hdmi->hdmi_reg);
>  
>  	/* HW workaround, need to write this twice for issue that may result
>  	 * in first write getting masked.
>  	 */
>  	if (HAS_PCH_SPLIT(dev)) {
> -		I915_WRITE(intel_hdmi->sdvox_reg, temp);
> -		POSTING_READ(intel_hdmi->sdvox_reg);
> +		I915_WRITE(intel_hdmi->hdmi_reg, temp);
> +		POSTING_READ(intel_hdmi->hdmi_reg);
>  	}
>  }
>  
> @@ -710,7 +710,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
>  	u32 temp;
>  	u32 enable_bits = SDVO_ENABLE | SDVO_AUDIO_ENABLE;
>  
> -	temp = I915_READ(intel_hdmi->sdvox_reg);
> +	temp = I915_READ(intel_hdmi->hdmi_reg);
>  
>  	/* HW workaround for IBX, we need to move the port to transcoder A
>  	 * before disabling it. */
> @@ -720,12 +720,12 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
>  
>  		if (temp & SDVO_PIPE_B_SELECT) {
>  			temp &= ~SDVO_PIPE_B_SELECT;
> -			I915_WRITE(intel_hdmi->sdvox_reg, temp);
> -			POSTING_READ(intel_hdmi->sdvox_reg);
> +			I915_WRITE(intel_hdmi->hdmi_reg, temp);
> +			POSTING_READ(intel_hdmi->hdmi_reg);
>  
>  			/* Again we need to write this twice. */
> -			I915_WRITE(intel_hdmi->sdvox_reg, temp);
> -			POSTING_READ(intel_hdmi->sdvox_reg);
> +			I915_WRITE(intel_hdmi->hdmi_reg, temp);
> +			POSTING_READ(intel_hdmi->hdmi_reg);
>  
>  			/* Transcoder selection bits only update
>  			 * effectively on vblank. */
> @@ -740,21 +740,21 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
>  	 * we do this anyway which shows more stable in testing.
>  	 */
>  	if (HAS_PCH_SPLIT(dev)) {
> -		I915_WRITE(intel_hdmi->sdvox_reg, temp & ~SDVO_ENABLE);
> -		POSTING_READ(intel_hdmi->sdvox_reg);
> +		I915_WRITE(intel_hdmi->hdmi_reg, temp & ~SDVO_ENABLE);
> +		POSTING_READ(intel_hdmi->hdmi_reg);
>  	}
>  
>  	temp &= ~enable_bits;
>  
> -	I915_WRITE(intel_hdmi->sdvox_reg, temp);
> -	POSTING_READ(intel_hdmi->sdvox_reg);
> +	I915_WRITE(intel_hdmi->hdmi_reg, temp);
> +	POSTING_READ(intel_hdmi->hdmi_reg);
>  
>  	/* HW workaround, need to write this twice for issue that may result
>  	 * in first write getting masked.
>  	 */
>  	if (HAS_PCH_SPLIT(dev)) {
> -		I915_WRITE(intel_hdmi->sdvox_reg, temp);
> -		POSTING_READ(intel_hdmi->sdvox_reg);
> +		I915_WRITE(intel_hdmi->hdmi_reg, temp);
> +		POSTING_READ(intel_hdmi->hdmi_reg);
>  	}
>  }
>  
> @@ -1075,7 +1075,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  	}
>  }
>  
> -void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
> +void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>  {
>  	struct intel_digital_port *intel_dig_port;
>  	struct intel_encoder *intel_encoder;
> @@ -1108,7 +1108,7 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
>  	intel_encoder->cloneable = false;
>  
>  	intel_dig_port->port = port;
> -	intel_dig_port->hdmi.sdvox_reg = sdvox_reg;
> +	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
>  	intel_dig_port->dp.output_reg = 0;
>  
>  	intel_hdmi_init_connector(intel_dig_port, intel_connector);
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 8/8] drm/i915: clarify confusion between SDVO and HDMI registers

[Daniel Vetter]

On Mon, Feb 18, 2013 at 07:00:27PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Some HDMI registers can be used for SDVO, so saying "HDMIB" should be
> the same as saying "SDVOB" for a given HW generation. This was not
> true and led to confusions and even a regression.
> 
> Previously we had:
>   - SDVO{B,C} defined as the Gen3+ registers
>   - HDMI{B,C,D} and PCH_SDVOB defined as the PCH registers
> 
> But now:
>   - SDVO{B,C} became GEN3_SDVO{B,C} on SDVO code
>   - SDVO{B,C} became GEN4_HDMI{B,C} on HDMI code
>   - HDMI{B,C,D} became PCH_HDMI{B,C,D}
>   - PCH_SDVOB is still the same thing
> 
> v2: Rebase (v1 was sent in May 2012).
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I think we still have a bit of ugly left in here, especially that the
register bit definitions are splattered all over irks me a bit. What about
moving the HDMI stuff up to the SDVO definitions and giving the HDMI bits
consisten HDMI_ prefixes? Imo there's no point in adding duplicate
#defines for all the SDVO_ bits we use in intel_hdmi.c ...
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   19 ++++++++-------
>  drivers/gpu/drm/i915/intel_display.c |   42 ++++++++++++++++++----------------
>  drivers/gpu/drm/i915/intel_sdvo.c    |   22 +++++++++---------
>  3 files changed, 42 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9e5844b..cd31af2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1680,8 +1680,9 @@
>  #define   SDVOB_HOTPLUG_INT_STATUS_I915		(1 << 6)
>  
>  /* SDVO port control */
> -#define SDVOB			0x61140
> -#define SDVOC			0x61160
> +#define GEN3_SDVOB		0x61140
> +#define GEN3_SDVOC		0x61160
> +#define PCH_SDVOB		0xe1140
>  #define   SDVO_ENABLE		(1 << 31)
>  #define   SDVO_PIPE_B_SELECT	(1 << 30)
>  #define   SDVO_STALL_SELECT	(1 << 29)
> @@ -3979,8 +3980,12 @@
>  #define FDI_PLL_CTL_1           0xfe000
>  #define FDI_PLL_CTL_2           0xfe004
>  
> -/* or SDVOB */
> -#define HDMIB   0xe1140
> +/* The same register may be used for SDVO or HDMI */
> +#define GEN4_HDMIB	GEN3_SDVOB
> +#define GEN4_HDMIC	GEN3_SDVOC
> +#define PCH_HDMIB	PCH_SDVOB
> +#define PCH_HDMIC	0xe1150
> +#define PCH_HDMID	0xe1160
>  #define  PORT_ENABLE    (1 << 31)
>  #define  TRANSCODER(pipe)       ((pipe) << 30)
>  #define  TRANSCODER_CPT(pipe)   ((pipe) << 29)
> @@ -4001,12 +4006,6 @@
>  #define  HSYNC_ACTIVE_HIGH      (1 << 3)
>  #define  PORT_DETECTED          (1 << 2)
>  
> -/* PCH SDVOB multiplex with HDMIB */
> -#define PCH_SDVOB	HDMIB
> -
> -#define HDMIC   0xe1150
> -#define HDMID   0xe1160
> -
>  #define PCH_LVDS	0xe1180
>  #define  LVDS_DETECTED	(1 << 1)
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6eb3882..744db70 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1419,9 +1419,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
>  	     "PCH LVDS enabled on transcoder %c, should be disabled\n",
>  	     pipe_name(pipe));
>  
> -	assert_pch_hdmi_disabled(dev_priv, pipe, HDMIB);
> -	assert_pch_hdmi_disabled(dev_priv, pipe, HDMIC);
> -	assert_pch_hdmi_disabled(dev_priv, pipe, HDMID);
> +	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIB);
> +	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIC);
> +	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
>  }
>  
>  /**
> @@ -8323,20 +8323,20 @@ static void intel_setup_outputs(struct drm_device *dev)
>  		if (has_edp_a(dev))
>  			intel_dp_init(dev, DP_A, PORT_A);
>  
> -		if (I915_READ(HDMIB) & PORT_DETECTED) {
> +		if (I915_READ(PCH_HDMIB) & PORT_DETECTED) {
>  			/* PCH SDVOB multiplex with HDMIB */
>  			found = intel_sdvo_init(dev, PCH_SDVOB, true);
>  			if (!found)
> -				intel_hdmi_init(dev, HDMIB, PORT_B);
> +				intel_hdmi_init(dev, PCH_HDMIB, PORT_B);
>  			if (!found && (I915_READ(PCH_DP_B) & DP_DETECTED))
>  				intel_dp_init(dev, PCH_DP_B, PORT_B);
>  		}
>  
> -		if (I915_READ(HDMIC) & PORT_DETECTED)
> -			intel_hdmi_init(dev, HDMIC, PORT_C);
> +		if (I915_READ(PCH_HDMIC) & PORT_DETECTED)
> +			intel_hdmi_init(dev, PCH_HDMIC, PORT_C);
>  
> -		if (!dpd_is_edp && I915_READ(HDMID) & PORT_DETECTED)
> -			intel_hdmi_init(dev, HDMID, PORT_D);
> +		if (!dpd_is_edp && I915_READ(PCH_HDMID) & PORT_DETECTED)
> +			intel_hdmi_init(dev, PCH_HDMID, PORT_D);
>  
>  		if (I915_READ(PCH_DP_C) & DP_DETECTED)
>  			intel_dp_init(dev, PCH_DP_C, PORT_C);
> @@ -8348,24 +8348,26 @@ static void intel_setup_outputs(struct drm_device *dev)
>  		if (I915_READ(VLV_DISPLAY_BASE + DP_C) & DP_DETECTED)
>  			intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C, PORT_C);
>  
> -		if (I915_READ(VLV_DISPLAY_BASE + SDVOB) & PORT_DETECTED) {
> -			intel_hdmi_init(dev, VLV_DISPLAY_BASE + SDVOB, PORT_B);
> +		if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIB) & PORT_DETECTED) {
> +			intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIB,
> +					PORT_B);
>  			if (I915_READ(VLV_DISPLAY_BASE + DP_B) & DP_DETECTED)
>  				intel_dp_init(dev, VLV_DISPLAY_BASE + DP_B, PORT_B);
>  		}
>  
> -		if (I915_READ(VLV_DISPLAY_BASE + SDVOC) & PORT_DETECTED)
> -			intel_hdmi_init(dev, VLV_DISPLAY_BASE + SDVOC, PORT_C);
> +		if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) & PORT_DETECTED)
> +			intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC,
> +					PORT_C);
>  
>  	} else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
>  		bool found = false;
>  
> -		if (I915_READ(SDVOB) & SDVO_DETECTED) {
> +		if (I915_READ(GEN3_SDVOB) & SDVO_DETECTED) {
>  			DRM_DEBUG_KMS("probing SDVOB\n");
> -			found = intel_sdvo_init(dev, SDVOB, true);
> +			found = intel_sdvo_init(dev, GEN3_SDVOB, true);
>  			if (!found && SUPPORTS_INTEGRATED_HDMI(dev)) {
>  				DRM_DEBUG_KMS("probing HDMI on SDVOB\n");
> -				intel_hdmi_init(dev, SDVOB, PORT_B);
> +				intel_hdmi_init(dev, GEN4_HDMIB, PORT_B);
>  			}
>  
>  			if (!found && SUPPORTS_INTEGRATED_DP(dev)) {
> @@ -8376,16 +8378,16 @@ static void intel_setup_outputs(struct drm_device *dev)
>  
>  		/* Before G4X SDVOC doesn't have its own detect register */
>  
> -		if (I915_READ(SDVOB) & SDVO_DETECTED) {
> +		if (I915_READ(GEN3_SDVOB) & SDVO_DETECTED) {
>  			DRM_DEBUG_KMS("probing SDVOC\n");
> -			found = intel_sdvo_init(dev, SDVOC, false);
> +			found = intel_sdvo_init(dev, GEN3_SDVOC, false);
>  		}
>  
> -		if (!found && (I915_READ(SDVOC) & SDVO_DETECTED)) {
> +		if (!found && (I915_READ(GEN3_SDVOC) & SDVO_DETECTED)) {
>  
>  			if (SUPPORTS_INTEGRATED_HDMI(dev)) {
>  				DRM_DEBUG_KMS("probing HDMI on SDVOC\n");
> -				intel_hdmi_init(dev, SDVOC, PORT_C);
> +				intel_hdmi_init(dev, GEN4_HDMIC, PORT_C);
>  			}
>  			if (SUPPORTS_INTEGRATED_DP(dev)) {
>  				DRM_DEBUG_KMS("probing DP_C\n");
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index f01063a..7d94db8 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -246,11 +246,11 @@ static void intel_sdvo_write_sdvox(struct intel_sdvo *intel_sdvo, u32 val)
>  		return;
>  	}
>  
> -	if (intel_sdvo->sdvo_reg == SDVOB) {
> -		cval = I915_READ(SDVOC);
> -	} else {
> -		bval = I915_READ(SDVOB);
> -	}
> +	if (intel_sdvo->sdvo_reg == GEN3_SDVOB)
> +		cval = I915_READ(GEN3_SDVOC);
> +	else
> +		bval = I915_READ(GEN3_SDVOB);
> +
>  	/*
>  	 * Write the registers twice for luck. Sometimes,
>  	 * writing them only once doesn't appear to 'stick'.
> @@ -258,10 +258,10 @@ static void intel_sdvo_write_sdvox(struct intel_sdvo *intel_sdvo, u32 val)
>  	 */
>  	for (i = 0; i < 2; i++)
>  	{
> -		I915_WRITE(SDVOB, bval);
> -		I915_READ(SDVOB);
> -		I915_WRITE(SDVOC, cval);
> -		I915_READ(SDVOC);
> +		I915_WRITE(GEN3_SDVOB, bval);
> +		I915_READ(GEN3_SDVOB);
> +		I915_WRITE(GEN3_SDVOC, cval);
> +		I915_READ(GEN3_SDVOC);
>  	}
>  }
>  
> @@ -1182,10 +1182,10 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
>  	} else {
>  		sdvox = I915_READ(intel_sdvo->sdvo_reg);
>  		switch (intel_sdvo->sdvo_reg) {
> -		case SDVOB:
> +		case GEN3_SDVOB:
>  			sdvox &= SDVOB_PRESERVE_MASK;
>  			break;
> -		case SDVOC:
> +		case GEN3_SDVOC:
>  			sdvox &= SDVOC_PRESERVE_MASK;
>  			break;
>  		}
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 3/7] drm/i915: clear the FPGA_DBG_RM_NOCLAIM bit at driver init

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Otherwise, if the BIOS did anything wrong, our first I915_{WRITE,READ}
will give us "unclaimed register"  messages.

V2: Even earlier.
V3: Move it to intel_early_sanitize_regs.

Bugzilla: http://bugs.freedesktop.org/show_bug.cgi?id=58897
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4fa6beb..e16099b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1453,6 +1453,22 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
 }
 
 /**
+ * intel_early_sanitize_regs - clean up BIOS state
+ * @dev: DRM device
+ *
+ * This function must be called before we do any I915_READ or I915_WRITE. Its
+ * purpose is to clean up any state left by the BIOS that may affect us when
+ * reading and/or writing registers.
+ */
+static void intel_early_sanitize_regs(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (IS_HASWELL(dev))
+		I915_WRITE_NOTRACE(FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+}
+
+/**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
  * @flags: startup flags
@@ -1542,6 +1558,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		goto put_gmch;
 	}
 
+	intel_early_sanitize_regs(dev);
+
 	aperture_size = dev_priv->gtt.mappable_end;
 
 	dev_priv->gtt.mappable =
-- 
1.7.10.4

Re: [PATCH 8/8] drm/i915: clarify confusion between SDVO and HDMI registers

[Paulo Zanoni]

Hi

2013/2/19 Daniel Vetter <daniel@ffwll.ch>:
> On Mon, Feb 18, 2013 at 07:00:27PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> Some HDMI registers can be used for SDVO, so saying "HDMIB" should be
>> the same as saying "SDVOB" for a given HW generation. This was not
>> true and led to confusions and even a regression.
>>
>> Previously we had:
>>   - SDVO{B,C} defined as the Gen3+ registers
>>   - HDMI{B,C,D} and PCH_SDVOB defined as the PCH registers
>>
>> But now:
>>   - SDVO{B,C} became GEN3_SDVO{B,C} on SDVO code
>>   - SDVO{B,C} became GEN4_HDMI{B,C} on HDMI code
>>   - HDMI{B,C,D} became PCH_HDMI{B,C,D}
>>   - PCH_SDVOB is still the same thing
>>
>> v2: Rebase (v1 was sent in May 2012).
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> I think we still have a bit of ugly left in here, especially that the
> register bit definitions are splattered all over irks me a bit. What about
> moving the HDMI stuff up to the SDVO definitions and giving the HDMI bits
> consisten HDMI_ prefixes? Imo there's no point in adding duplicate
> #defines for all the SDVO_ bits we use in intel_hdmi.c ...

I agree, there's more to clean up. I thought about amending your
suggestions to this patch, but I don't think this will be a good idea,
so I will send 3 additional patches on top of this one. Feel free to
merge them as a single patch if you want.

> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h      |   19 ++++++++-------
>>  drivers/gpu/drm/i915/intel_display.c |   42 ++++++++++++++++++----------------
>>  drivers/gpu/drm/i915/intel_sdvo.c    |   22 +++++++++---------
>>  3 files changed, 42 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 9e5844b..cd31af2 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1680,8 +1680,9 @@
>>  #define   SDVOB_HOTPLUG_INT_STATUS_I915              (1 << 6)
>>
>>  /* SDVO port control */
>> -#define SDVOB                        0x61140
>> -#define SDVOC                        0x61160
>> +#define GEN3_SDVOB           0x61140
>> +#define GEN3_SDVOC           0x61160
>> +#define PCH_SDVOB            0xe1140
>>  #define   SDVO_ENABLE                (1 << 31)
>>  #define   SDVO_PIPE_B_SELECT (1 << 30)
>>  #define   SDVO_STALL_SELECT  (1 << 29)
>> @@ -3979,8 +3980,12 @@
>>  #define FDI_PLL_CTL_1           0xfe000
>>  #define FDI_PLL_CTL_2           0xfe004
>>
>> -/* or SDVOB */
>> -#define HDMIB   0xe1140
>> +/* The same register may be used for SDVO or HDMI */
>> +#define GEN4_HDMIB   GEN3_SDVOB
>> +#define GEN4_HDMIC   GEN3_SDVOC
>> +#define PCH_HDMIB    PCH_SDVOB
>> +#define PCH_HDMIC    0xe1150
>> +#define PCH_HDMID    0xe1160
>>  #define  PORT_ENABLE    (1 << 31)
>>  #define  TRANSCODER(pipe)       ((pipe) << 30)
>>  #define  TRANSCODER_CPT(pipe)   ((pipe) << 29)
>> @@ -4001,12 +4006,6 @@
>>  #define  HSYNC_ACTIVE_HIGH      (1 << 3)
>>  #define  PORT_DETECTED          (1 << 2)
>>
>> -/* PCH SDVOB multiplex with HDMIB */
>> -#define PCH_SDVOB    HDMIB
>> -
>> -#define HDMIC   0xe1150
>> -#define HDMID   0xe1160
>> -
>>  #define PCH_LVDS     0xe1180
>>  #define  LVDS_DETECTED       (1 << 1)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 6eb3882..744db70 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -1419,9 +1419,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
>>            "PCH LVDS enabled on transcoder %c, should be disabled\n",
>>            pipe_name(pipe));
>>
>> -     assert_pch_hdmi_disabled(dev_priv, pipe, HDMIB);
>> -     assert_pch_hdmi_disabled(dev_priv, pipe, HDMIC);
>> -     assert_pch_hdmi_disabled(dev_priv, pipe, HDMID);
>> +     assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIB);
>> +     assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIC);
>> +     assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
>>  }
>>
>>  /**
>> @@ -8323,20 +8323,20 @@ static void intel_setup_outputs(struct drm_device *dev)
>>               if (has_edp_a(dev))
>>                       intel_dp_init(dev, DP_A, PORT_A);
>>
>> -             if (I915_READ(HDMIB) & PORT_DETECTED) {
>> +             if (I915_READ(PCH_HDMIB) & PORT_DETECTED) {
>>                       /* PCH SDVOB multiplex with HDMIB */
>>                       found = intel_sdvo_init(dev, PCH_SDVOB, true);
>>                       if (!found)
>> -                             intel_hdmi_init(dev, HDMIB, PORT_B);
>> +                             intel_hdmi_init(dev, PCH_HDMIB, PORT_B);
>>                       if (!found && (I915_READ(PCH_DP_B) & DP_DETECTED))
>>                               intel_dp_init(dev, PCH_DP_B, PORT_B);
>>               }
>>
>> -             if (I915_READ(HDMIC) & PORT_DETECTED)
>> -                     intel_hdmi_init(dev, HDMIC, PORT_C);
>> +             if (I915_READ(PCH_HDMIC) & PORT_DETECTED)
>> +                     intel_hdmi_init(dev, PCH_HDMIC, PORT_C);
>>
>> -             if (!dpd_is_edp && I915_READ(HDMID) & PORT_DETECTED)
>> -                     intel_hdmi_init(dev, HDMID, PORT_D);
>> +             if (!dpd_is_edp && I915_READ(PCH_HDMID) & PORT_DETECTED)
>> +                     intel_hdmi_init(dev, PCH_HDMID, PORT_D);
>>
>>               if (I915_READ(PCH_DP_C) & DP_DETECTED)
>>                       intel_dp_init(dev, PCH_DP_C, PORT_C);
>> @@ -8348,24 +8348,26 @@ static void intel_setup_outputs(struct drm_device *dev)
>>               if (I915_READ(VLV_DISPLAY_BASE + DP_C) & DP_DETECTED)
>>                       intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C, PORT_C);
>>
>> -             if (I915_READ(VLV_DISPLAY_BASE + SDVOB) & PORT_DETECTED) {
>> -                     intel_hdmi_init(dev, VLV_DISPLAY_BASE + SDVOB, PORT_B);
>> +             if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIB) & PORT_DETECTED) {
>> +                     intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIB,
>> +                                     PORT_B);
>>                       if (I915_READ(VLV_DISPLAY_BASE + DP_B) & DP_DETECTED)
>>                               intel_dp_init(dev, VLV_DISPLAY_BASE + DP_B, PORT_B);
>>               }
>>
>> -             if (I915_READ(VLV_DISPLAY_BASE + SDVOC) & PORT_DETECTED)
>> -                     intel_hdmi_init(dev, VLV_DISPLAY_BASE + SDVOC, PORT_C);
>> +             if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) & PORT_DETECTED)
>> +                     intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC,
>> +                                     PORT_C);
>>
>>       } else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
>>               bool found = false;
>>
>> -             if (I915_READ(SDVOB) & SDVO_DETECTED) {
>> +             if (I915_READ(GEN3_SDVOB) & SDVO_DETECTED) {
>>                       DRM_DEBUG_KMS("probing SDVOB\n");
>> -                     found = intel_sdvo_init(dev, SDVOB, true);
>> +                     found = intel_sdvo_init(dev, GEN3_SDVOB, true);
>>                       if (!found && SUPPORTS_INTEGRATED_HDMI(dev)) {
>>                               DRM_DEBUG_KMS("probing HDMI on SDVOB\n");
>> -                             intel_hdmi_init(dev, SDVOB, PORT_B);
>> +                             intel_hdmi_init(dev, GEN4_HDMIB, PORT_B);
>>                       }
>>
>>                       if (!found && SUPPORTS_INTEGRATED_DP(dev)) {
>> @@ -8376,16 +8378,16 @@ static void intel_setup_outputs(struct drm_device *dev)
>>
>>               /* Before G4X SDVOC doesn't have its own detect register */
>>
>> -             if (I915_READ(SDVOB) & SDVO_DETECTED) {
>> +             if (I915_READ(GEN3_SDVOB) & SDVO_DETECTED) {
>>                       DRM_DEBUG_KMS("probing SDVOC\n");
>> -                     found = intel_sdvo_init(dev, SDVOC, false);
>> +                     found = intel_sdvo_init(dev, GEN3_SDVOC, false);
>>               }
>>
>> -             if (!found && (I915_READ(SDVOC) & SDVO_DETECTED)) {
>> +             if (!found && (I915_READ(GEN3_SDVOC) & SDVO_DETECTED)) {
>>
>>                       if (SUPPORTS_INTEGRATED_HDMI(dev)) {
>>                               DRM_DEBUG_KMS("probing HDMI on SDVOC\n");
>> -                             intel_hdmi_init(dev, SDVOC, PORT_C);
>> +                             intel_hdmi_init(dev, GEN4_HDMIC, PORT_C);
>>                       }
>>                       if (SUPPORTS_INTEGRATED_DP(dev)) {
>>                               DRM_DEBUG_KMS("probing DP_C\n");
>> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>> index f01063a..7d94db8 100644
>> --- a/drivers/gpu/drm/i915/intel_sdvo.c
>> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
>> @@ -246,11 +246,11 @@ static void intel_sdvo_write_sdvox(struct intel_sdvo *intel_sdvo, u32 val)
>>               return;
>>       }
>>
>> -     if (intel_sdvo->sdvo_reg == SDVOB) {
>> -             cval = I915_READ(SDVOC);
>> -     } else {
>> -             bval = I915_READ(SDVOB);
>> -     }
>> +     if (intel_sdvo->sdvo_reg == GEN3_SDVOB)
>> +             cval = I915_READ(GEN3_SDVOC);
>> +     else
>> +             bval = I915_READ(GEN3_SDVOB);
>> +
>>       /*
>>        * Write the registers twice for luck. Sometimes,
>>        * writing them only once doesn't appear to 'stick'.
>> @@ -258,10 +258,10 @@ static void intel_sdvo_write_sdvox(struct intel_sdvo *intel_sdvo, u32 val)
>>        */
>>       for (i = 0; i < 2; i++)
>>       {
>> -             I915_WRITE(SDVOB, bval);
>> -             I915_READ(SDVOB);
>> -             I915_WRITE(SDVOC, cval);
>> -             I915_READ(SDVOC);
>> +             I915_WRITE(GEN3_SDVOB, bval);
>> +             I915_READ(GEN3_SDVOB);
>> +             I915_WRITE(GEN3_SDVOC, cval);
>> +             I915_READ(GEN3_SDVOC);
>>       }
>>  }
>>
>> @@ -1182,10 +1182,10 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
>>       } else {
>>               sdvox = I915_READ(intel_sdvo->sdvo_reg);
>>               switch (intel_sdvo->sdvo_reg) {
>> -             case SDVOB:
>> +             case GEN3_SDVOB:
>>                       sdvox &= SDVOB_PRESERVE_MASK;
>>                       break;
>> -             case SDVOC:
>> +             case GEN3_SDVOC:
>>                       sdvox &= SDVOC_PRESERVE_MASK;
>>                       break;
>>               }
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Paulo Zanoni

[PATCH 9/8] drm/i915: unify the definitions of the HDMI/SDVO register

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Since they're all the same register, leave all the #defines at the
same place, organized by Gen and also specify which bits are used by
only a specific port or encoding.

Also remove a few unused duplicates and adjust indentation.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  111 +++++++++++++++++++--------------------
 1 file changed, 55 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cd31af2..f35b28c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1679,43 +1679,68 @@
 #define   SDVOC_HOTPLUG_INT_STATUS_I915		(1 << 7)
 #define   SDVOB_HOTPLUG_INT_STATUS_I915		(1 << 6)
 
-/* SDVO port control */
-#define GEN3_SDVOB		0x61140
-#define GEN3_SDVOC		0x61160
-#define PCH_SDVOB		0xe1140
-#define   SDVO_ENABLE		(1 << 31)
-#define   SDVO_PIPE_B_SELECT	(1 << 30)
-#define   SDVO_STALL_SELECT	(1 << 29)
-#define   SDVO_INTERRUPT_ENABLE	(1 << 26)
+/* SDVO and HDMI port control.
+ * The same register may be used for SDVO or HDMI */
+#define GEN3_SDVOB	0x61140
+#define GEN3_SDVOC	0x61160
+#define GEN4_HDMIB	GEN3_SDVOB
+#define GEN4_HDMIC	GEN3_SDVOC
+#define PCH_SDVOB	0xe1140
+#define PCH_HDMIB	PCH_SDVOB
+#define PCH_HDMIC	0xe1150
+#define PCH_HDMID	0xe1160
+
+/* Gen 3 SDVO bits: */
+#define   SDVO_ENABLE				(1 << 31)
+#define   SDVO_PIPE_B_SELECT			(1 << 30)
+#define   SDVO_STALL_SELECT			(1 << 29)
+#define   SDVO_INTERRUPT_ENABLE			(1 << 26)
 /**
  * 915G/GM SDVO pixel multiplier.
- *
  * Programmed value is multiplier - 1, up to 5x.
- *
  * \sa DPLL_MD_UDI_MULTIPLIER_MASK
  */
-#define   SDVO_PORT_MULTIPLY_MASK	(7 << 23)
+#define   SDVO_PORT_MULTIPLY_MASK		(7 << 23)
 #define   SDVO_PORT_MULTIPLY_SHIFT		23
-#define   SDVO_PHASE_SELECT_MASK	(15 << 19)
-#define   SDVO_PHASE_SELECT_DEFAULT	(6 << 19)
-#define   SDVO_CLOCK_OUTPUT_INVERT	(1 << 18)
-#define   SDVOC_GANG_MODE		(1 << 16)
-#define   SDVO_ENCODING_SDVO		(0x0 << 10)
-#define   SDVO_ENCODING_HDMI		(0x2 << 10)
-/** Requird for HDMI operation */
-#define   SDVO_NULL_PACKETS_DURING_VSYNC (1 << 9)
-#define   SDVO_COLOR_RANGE_16_235	(1 << 8)
-#define   SDVO_BORDER_ENABLE		(1 << 7)
-#define   SDVO_AUDIO_ENABLE		(1 << 6)
-/** New with 965, default is to be set */
-#define   SDVO_VSYNC_ACTIVE_HIGH	(1 << 4)
-/** New with 965, default is to be set */
-#define   SDVO_HSYNC_ACTIVE_HIGH	(1 << 3)
-#define   SDVOB_PCIE_CONCURRENCY	(1 << 3)
-#define   SDVO_DETECTED			(1 << 2)
+#define   SDVO_PHASE_SELECT_MASK		(15 << 19)
+#define   SDVO_PHASE_SELECT_DEFAULT		(6 << 19)
+#define   SDVO_CLOCK_OUTPUT_INVERT		(1 << 18)
+#define   SDVOC_GANG_MODE			(1 << 16) /* Port C only */
+#define   SDVO_BORDER_ENABLE			(1 << 7) /* SDVO only */
+#define   SDVOB_PCIE_CONCURRENCY		(1 << 3) /* Port B only */
+#define   SDVO_DETECTED				(1 << 2)
 /* Bits to be preserved when writing */
-#define   SDVOB_PRESERVE_MASK ((1 << 17) | (1 << 16) | (1 << 14) | (1 << 26))
-#define   SDVOC_PRESERVE_MASK ((1 << 17) | (1 << 26))
+#define   SDVOB_PRESERVE_MASK ((1 << 17) | (1 << 16) | (1 << 14) | \
+			       SDVO_INTERRUPT_ENABLE)
+#define   SDVOC_PRESERVE_MASK ((1 << 17) | SDVO_INTERRUPT_ENABLE)
+
+/* Gen 4 SDVO/HDMI bits: */
+#define   COLOR_FORMAT_8bpc			(0 << 26)
+#define   SDVO_ENCODING_SDVO			(0 << 10)
+#define   SDVO_ENCODING_HDMI			(2 << 10)
+#define   SDVO_NULL_PACKETS_DURING_VSYNC	(1 << 9) /* HDMI only */
+#define   SDVO_COLOR_RANGE_16_235		(1 << 8) /* HDMI only */
+#define   SDVO_AUDIO_ENABLE			(1 << 6)
+/* VSYNC/HSYNC bits new with 965, default is to be set */
+#define   SDVO_VSYNC_ACTIVE_HIGH		(1 << 4)
+#define   SDVO_HSYNC_ACTIVE_HIGH		(1 << 3)
+
+/* Gen 5 (IBX) SDVO/HDMI bits: */
+#define   COLOR_FORMAT_12bpc			(3 << 26) /* HDMI only */
+#define   SDVOB_HOTPLUG_ENABLE			(1 << 23) /* SDVO only */
+
+/* Gen 6 (CPT) SDVO/HDMI bits: */
+#define   TRANSCODER_CPT(pipe)			((pipe) << 29)
+#define   TRANSCODER_MASK_CPT			(3 << 29)
+
+/* Repeated but still used bits: */
+#define   PORT_ENABLE				(1 << 31)
+#define   TRANSCODER(pipe)			((pipe) << 30)
+#define   TRANSCODER_MASK			(1 << 30)
+#define   HDMI_MODE_SELECT			(1 << 9)
+#define   DVI_MODE_SELECT			(0 << 9)
+#define   PORT_DETECTED				(1 << 2)
+
 
 /* DVO port control */
 #define DVOA			0x61120
@@ -3980,32 +4005,6 @@
 #define FDI_PLL_CTL_1           0xfe000
 #define FDI_PLL_CTL_2           0xfe004
 
-/* The same register may be used for SDVO or HDMI */
-#define GEN4_HDMIB	GEN3_SDVOB
-#define GEN4_HDMIC	GEN3_SDVOC
-#define PCH_HDMIB	PCH_SDVOB
-#define PCH_HDMIC	0xe1150
-#define PCH_HDMID	0xe1160
-#define  PORT_ENABLE    (1 << 31)
-#define  TRANSCODER(pipe)       ((pipe) << 30)
-#define  TRANSCODER_CPT(pipe)   ((pipe) << 29)
-#define  TRANSCODER_MASK        (1 << 30)
-#define  TRANSCODER_MASK_CPT    (3 << 29)
-#define  COLOR_FORMAT_8bpc      (0)
-#define  COLOR_FORMAT_12bpc     (3 << 26)
-#define  SDVOB_HOTPLUG_ENABLE   (1 << 23)
-#define  SDVO_ENCODING          (0)
-#define  TMDS_ENCODING          (2 << 10)
-#define  NULL_PACKET_VSYNC_ENABLE       (1 << 9)
-/* CPT */
-#define  HDMI_MODE_SELECT	(1 << 9)
-#define  DVI_MODE_SELECT	(0)
-#define  SDVOB_BORDER_ENABLE    (1 << 7)
-#define  AUDIO_ENABLE           (1 << 6)
-#define  VSYNC_ACTIVE_HIGH      (1 << 4)
-#define  HSYNC_ACTIVE_HIGH      (1 << 3)
-#define  PORT_DETECTED          (1 << 2)
-
 #define PCH_LVDS	0xe1180
 #define  LVDS_DETECTED	(1 << 1)
 
-- 
1.7.10.4

[PATCH 10/8] drm/i915: remove duplicated SDVO/HDMI bit definitions

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |   17 ++++++-----------
 drivers/gpu/drm/i915/intel_display.c |   18 +++++++++---------
 drivers/gpu/drm/i915/intel_hdmi.c    |   23 +++++++++--------------
 drivers/gpu/drm/i915/intel_sdvo.c    |   16 +++++-----------
 4 files changed, 29 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f35b28c..aad8eee 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1692,6 +1692,8 @@
 
 /* Gen 3 SDVO bits: */
 #define   SDVO_ENABLE				(1 << 31)
+#define   SDVO_PIPE_SEL(pipe)			((pipe) << 30)
+#define   SDVO_PIPE_SEL_MASK			(1 << 30)
 #define   SDVO_PIPE_B_SELECT			(1 << 30)
 #define   SDVO_STALL_SELECT			(1 << 29)
 #define   SDVO_INTERRUPT_ENABLE			(1 << 26)
@@ -1718,7 +1720,8 @@
 #define   COLOR_FORMAT_8bpc			(0 << 26)
 #define   SDVO_ENCODING_SDVO			(0 << 10)
 #define   SDVO_ENCODING_HDMI			(2 << 10)
-#define   SDVO_NULL_PACKETS_DURING_VSYNC	(1 << 9) /* HDMI only */
+#define   HDMI_MODE_SELECT_HDMI			(1 << 9) /* HDMI only */
+#define   HDMI_MODE_SELECT_DVI			(0 << 9) /* HDMI only */
 #define   SDVO_COLOR_RANGE_16_235		(1 << 8) /* HDMI only */
 #define   SDVO_AUDIO_ENABLE			(1 << 6)
 /* VSYNC/HSYNC bits new with 965, default is to be set */
@@ -1730,16 +1733,8 @@
 #define   SDVOB_HOTPLUG_ENABLE			(1 << 23) /* SDVO only */
 
 /* Gen 6 (CPT) SDVO/HDMI bits: */
-#define   TRANSCODER_CPT(pipe)			((pipe) << 29)
-#define   TRANSCODER_MASK_CPT			(3 << 29)
-
-/* Repeated but still used bits: */
-#define   PORT_ENABLE				(1 << 31)
-#define   TRANSCODER(pipe)			((pipe) << 30)
-#define   TRANSCODER_MASK			(1 << 30)
-#define   HDMI_MODE_SELECT			(1 << 9)
-#define   DVI_MODE_SELECT			(0 << 9)
-#define   PORT_DETECTED				(1 << 2)
+#define   SDVO_PIPE_SEL_CPT(pipe)		((pipe) << 29)
+#define   SDVO_PIPE_SEL_MASK_CPT		(3 << 29)
 
 
 /* DVO port control */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d32ac72..b381fcd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1327,14 +1327,14 @@ static bool dp_pipe_enabled(struct drm_i915_private *dev_priv,
 static bool hdmi_pipe_enabled(struct drm_i915_private *dev_priv,
 			      enum pipe pipe, u32 val)
 {
-	if ((val & PORT_ENABLE) == 0)
+	if ((val & SDVO_ENABLE) == 0)
 		return false;
 
 	if (HAS_PCH_CPT(dev_priv->dev)) {
-		if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
+		if ((val & SDVO_PIPE_SEL_MASK_CPT) != SDVO_PIPE_SEL_CPT(pipe))
 			return false;
 	} else {
-		if ((val & TRANSCODER_MASK) != TRANSCODER(pipe))
+		if ((val & SDVO_PIPE_SEL_MASK) != SDVO_PIPE_SEL(pipe))
 			return false;
 	}
 	return true;
@@ -1392,7 +1392,7 @@ static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv,
 	     "PCH HDMI (0x%08x) enabled on transcoder %c, should be disabled\n",
 	     reg, pipe_name(pipe));
 
-	WARN(HAS_PCH_IBX(dev_priv->dev) && (val & PORT_ENABLE) == 0
+	WARN(HAS_PCH_IBX(dev_priv->dev) && (val & SDVO_ENABLE) == 0
 	     && (val & SDVO_PIPE_B_SELECT),
 	     "IBX PCH hdmi port still using transcoder B\n");
 }
@@ -8357,7 +8357,7 @@ static void intel_setup_outputs(struct drm_device *dev)
 		if (has_edp_a(dev))
 			intel_dp_init(dev, DP_A, PORT_A);
 
-		if (I915_READ(PCH_HDMIB) & PORT_DETECTED) {
+		if (I915_READ(PCH_HDMIB) & SDVO_DETECTED) {
 			/* PCH SDVOB multiplex with HDMIB */
 			found = intel_sdvo_init(dev, PCH_SDVOB, true);
 			if (!found)
@@ -8366,10 +8366,10 @@ static void intel_setup_outputs(struct drm_device *dev)
 				intel_dp_init(dev, PCH_DP_B, PORT_B);
 		}
 
-		if (I915_READ(PCH_HDMIC) & PORT_DETECTED)
+		if (I915_READ(PCH_HDMIC) & SDVO_DETECTED)
 			intel_hdmi_init(dev, PCH_HDMIC, PORT_C);
 
-		if (!dpd_is_edp && I915_READ(PCH_HDMID) & PORT_DETECTED)
+		if (!dpd_is_edp && I915_READ(PCH_HDMID) & SDVO_DETECTED)
 			intel_hdmi_init(dev, PCH_HDMID, PORT_D);
 
 		if (I915_READ(PCH_DP_C) & DP_DETECTED)
@@ -8382,14 +8382,14 @@ static void intel_setup_outputs(struct drm_device *dev)
 		if (I915_READ(VLV_DISPLAY_BASE + DP_C) & DP_DETECTED)
 			intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C, PORT_C);
 
-		if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIB) & PORT_DETECTED) {
+		if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIB) & SDVO_DETECTED) {
 			intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIB,
 					PORT_B);
 			if (I915_READ(VLV_DISPLAY_BASE + DP_B) & DP_DETECTED)
 				intel_dp_init(dev, VLV_DISPLAY_BASE + DP_B, PORT_B);
 		}
 
-		if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) & PORT_DETECTED)
+		if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) & SDVO_DETECTED)
 			intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC,
 					PORT_C);
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index fcb36c6..4b953cf 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -614,20 +614,20 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 
 	/* Required on CPT */
 	if (intel_hdmi->has_hdmi_sink && HAS_PCH_CPT(dev))
-		hdmi_val |= HDMI_MODE_SELECT;
+		hdmi_val |= HDMI_MODE_SELECT_HDMI;
 
 	if (intel_hdmi->has_audio) {
 		DRM_DEBUG_DRIVER("Enabling HDMI audio on pipe %c\n",
 				 pipe_name(intel_crtc->pipe));
 		hdmi_val |= SDVO_AUDIO_ENABLE;
-		hdmi_val |= SDVO_NULL_PACKETS_DURING_VSYNC;
+		hdmi_val |= HDMI_MODE_SELECT_HDMI;
 		intel_write_eld(encoder, adjusted_mode);
 	}
 
 	if (HAS_PCH_CPT(dev))
-		hdmi_val |= PORT_TRANS_SEL_CPT(intel_crtc->pipe);
-	else if (intel_crtc->pipe == PIPE_B)
-		hdmi_val |= SDVO_PIPE_B_SELECT;
+		hdmi_val |= SDVO_PIPE_SEL_CPT(intel_crtc->pipe);
+	else
+		hdmi_val |= SDVO_PIPE_SEL(intel_crtc->pipe);
 
 	I915_WRITE(intel_hdmi->hdmi_reg, hdmi_val);
 	POSTING_READ(intel_hdmi->hdmi_reg);
@@ -660,6 +660,7 @@ static void intel_enable_hdmi(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	u32 temp;
 	u32 enable_bits = SDVO_ENABLE;
@@ -670,15 +671,9 @@ static void intel_enable_hdmi(struct intel_encoder *encoder)
 	temp = I915_READ(intel_hdmi->hdmi_reg);
 
 	/* HW workaround for IBX, we need to move the port to transcoder A
-	 * before disabling it. */
-	if (HAS_PCH_IBX(dev)) {
-		struct drm_crtc *crtc = encoder->base.crtc;
-		int pipe = crtc ? to_intel_crtc(crtc)->pipe : -1;
-
-		/* Restore the transcoder select bit. */
-		if (pipe == PIPE_B)
-			enable_bits |= SDVO_PIPE_B_SELECT;
-	}
+	 * before disabling it, so restore the transcoder select bit here. */
+	if (HAS_PCH_IBX(dev))
+		enable_bits |= SDVO_PIPE_SEL(intel_crtc->pipe);
 
 	/* HW workaround, need to toggle enable bit off and on for 12bpc, but
 	 * we do this anyway which shows more stable in testing.
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 7d94db8..eef0731 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1193,9 +1193,9 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 	}
 
 	if (INTEL_PCH_TYPE(dev) >= PCH_CPT)
-		sdvox |= TRANSCODER_CPT(intel_crtc->pipe);
+		sdvox |= SDVO_PIPE_SEL_CPT(intel_crtc->pipe);
 	else
-		sdvox |= TRANSCODER(intel_crtc->pipe);
+		sdvox |= SDVO_PIPE_SEL(intel_crtc->pipe);
 
 	if (intel_sdvo->has_hdmi_audio)
 		sdvox |= SDVO_AUDIO_ENABLE;
@@ -1305,15 +1305,9 @@ static void intel_enable_sdvo(struct intel_encoder *encoder)
 	temp = I915_READ(intel_sdvo->sdvo_reg);
 	if ((temp & SDVO_ENABLE) == 0) {
 		/* HW workaround for IBX, we need to move the port
-		 * to transcoder A before disabling it. */
-		if (HAS_PCH_IBX(dev)) {
-			struct drm_crtc *crtc = encoder->base.crtc;
-			int pipe = crtc ? to_intel_crtc(crtc)->pipe : -1;
-
-			/* Restore the transcoder select bit. */
-			if (pipe == PIPE_B)
-				temp |= SDVO_PIPE_B_SELECT;
-		}
+		 * to transcoder A before disabling it, so restore it here. */
+		if (HAS_PCH_IBX(dev))
+			temp |= SDVO_PIPE_SEL(intel_crtc->pipe);
 
 		intel_sdvo_write_sdvox(intel_sdvo, temp | SDVO_ENABLE);
 	}
-- 
1.7.10.4

[PATCH 11/8] drm/i915: rename some HDMI bit definitions

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Bits used only on HDMI mode now have HDMI_ prefix instead of SDVO_.
The COLOR_FORMAT bits now have prefixes (and the 12bpc bit is for HDMI
only).

Notice that this patch uncovers a bug on the SDVO code: the
COLOR_RANGE_16_235 bit can only be used if the port is in TMDS mode,
not SDVO mode. This will have to be fixed in a later patch.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h   |    6 +++---
 drivers/gpu/drm/i915/intel_hdmi.c |    8 ++++----
 drivers/gpu/drm/i915/intel_sdvo.c |    8 ++++++--
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index aad8eee..4c6bab7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1717,19 +1717,19 @@
 #define   SDVOC_PRESERVE_MASK ((1 << 17) | SDVO_INTERRUPT_ENABLE)
 
 /* Gen 4 SDVO/HDMI bits: */
-#define   COLOR_FORMAT_8bpc			(0 << 26)
+#define   SDVO_COLOR_FORMAT_8bpc		(0 << 26)
 #define   SDVO_ENCODING_SDVO			(0 << 10)
 #define   SDVO_ENCODING_HDMI			(2 << 10)
 #define   HDMI_MODE_SELECT_HDMI			(1 << 9) /* HDMI only */
 #define   HDMI_MODE_SELECT_DVI			(0 << 9) /* HDMI only */
-#define   SDVO_COLOR_RANGE_16_235		(1 << 8) /* HDMI only */
+#define   HDMI_COLOR_RANGE_16_235		(1 << 8) /* HDMI only */
 #define   SDVO_AUDIO_ENABLE			(1 << 6)
 /* VSYNC/HSYNC bits new with 965, default is to be set */
 #define   SDVO_VSYNC_ACTIVE_HIGH		(1 << 4)
 #define   SDVO_HSYNC_ACTIVE_HIGH		(1 << 3)
 
 /* Gen 5 (IBX) SDVO/HDMI bits: */
-#define   COLOR_FORMAT_12bpc			(3 << 26) /* HDMI only */
+#define   HDMI_COLOR_FORMAT_12bpc		(3 << 26) /* HDMI only */
 #define   SDVOB_HOTPLUG_ENABLE			(1 << 23) /* SDVO only */
 
 /* Gen 6 (CPT) SDVO/HDMI bits: */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 4b953cf..c1b5809 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -608,9 +608,9 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 		hdmi_val |= SDVO_HSYNC_ACTIVE_HIGH;
 
 	if (intel_crtc->bpp > 24)
-		hdmi_val |= COLOR_FORMAT_12bpc;
+		hdmi_val |= HDMI_COLOR_FORMAT_12bpc;
 	else
-		hdmi_val |= COLOR_FORMAT_8bpc;
+		hdmi_val |= SDVO_COLOR_FORMAT_8bpc;
 
 	/* Required on CPT */
 	if (intel_hdmi->has_hdmi_sink && HAS_PCH_CPT(dev))
@@ -777,7 +777,7 @@ bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
 		if (intel_hdmi->has_hdmi_sink &&
 		    drm_mode_cea_vic(adjusted_mode) > 1)
-			intel_hdmi->color_range = SDVO_COLOR_RANGE_16_235;
+			intel_hdmi->color_range = HDMI_COLOR_RANGE_16_235;
 		else
 			intel_hdmi->color_range = 0;
 	}
@@ -940,7 +940,7 @@ intel_hdmi_set_property(struct drm_connector *connector,
 			break;
 		case INTEL_BROADCAST_RGB_LIMITED:
 			intel_hdmi->color_range_auto = false;
-			intel_hdmi->color_range = SDVO_COLOR_RANGE_16_235;
+			intel_hdmi->color_range = HDMI_COLOR_RANGE_16_235;
 			break;
 		default:
 			return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index eef0731..63dcb76 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1076,9 +1076,11 @@ static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
 
 	if (intel_sdvo->color_range_auto) {
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
+		/* FIXME: This bit is only valid when using TMDS encoding and 8
+		 * bit per color mode. */
 		if (intel_sdvo->has_hdmi_monitor &&
 		    drm_mode_cea_vic(adjusted_mode) > 1)
-			intel_sdvo->color_range = SDVO_COLOR_RANGE_16_235;
+			intel_sdvo->color_range = HDMI_COLOR_RANGE_16_235;
 		else
 			intel_sdvo->color_range = 0;
 	}
@@ -1926,7 +1928,9 @@ intel_sdvo_set_property(struct drm_connector *connector,
 			break;
 		case INTEL_BROADCAST_RGB_LIMITED:
 			intel_sdvo->color_range_auto = false;
-			intel_sdvo->color_range = SDVO_COLOR_RANGE_16_235;
+			/* FIXME: this bit is only valid when using TMDS
+			 * encoding and 8 bit per color mode. */
+			intel_sdvo->color_range = HDMI_COLOR_RANGE_16_235;
 			break;
 		default:
 			return -EINVAL;
-- 
1.7.10.4

Re: [PATCH 3/7] drm/i915: clear the FPGA_DBG_RM_NOCLAIM bit at driver init

[Daniel Vetter]

On Tue, Feb 19, 2013 at 04:13:35PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Otherwise, if the BIOS did anything wrong, our first I915_{WRITE,READ}
> will give us "unclaimed register"  messages.
> 
> V2: Even earlier.
> V3: Move it to intel_early_sanitize_regs.
> 
> Bugzilla: http://bugs.freedesktop.org/show_bug.cgi?id=58897
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Slurped in the first 3 patches of this series, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 5/8] drm/i915: wait_event_timeout's timeout is in jiffies

[Daniel Vetter]

On Mon, Feb 18, 2013 at 07:00:24PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> So use msecs_to_jiffies(10) to make the timeout the same as in the
> "!has_aux_irq" case.
> 
> This patch was initially written by Daniel Vetter and posted on
> pastebin a few weeks ago. I'm just bringing it to the mailing list.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Picked up for -fixes, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 8/8] drm/i915: clarify confusion between SDVO and HDMI registers

[Daniel Vetter]

On Tue, Feb 19, 2013 at 04:19:52PM -0300, Paulo Zanoni wrote:
> Hi
> 
> 2013/2/19 Daniel Vetter <daniel@ffwll.ch>:
> > On Mon, Feb 18, 2013 at 07:00:27PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> Some HDMI registers can be used for SDVO, so saying "HDMIB" should be
> >> the same as saying "SDVOB" for a given HW generation. This was not
> >> true and led to confusions and even a regression.
> >>
> >> Previously we had:
> >>   - SDVO{B,C} defined as the Gen3+ registers
> >>   - HDMI{B,C,D} and PCH_SDVOB defined as the PCH registers
> >>
> >> But now:
> >>   - SDVO{B,C} became GEN3_SDVO{B,C} on SDVO code
> >>   - SDVO{B,C} became GEN4_HDMI{B,C} on HDMI code
> >>   - HDMI{B,C,D} became PCH_HDMI{B,C,D}
> >>   - PCH_SDVOB is still the same thing
> >>
> >> v2: Rebase (v1 was sent in May 2012).
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > I think we still have a bit of ugly left in here, especially that the
> > register bit definitions are splattered all over irks me a bit. What about
> > moving the HDMI stuff up to the SDVO definitions and giving the HDMI bits
> > consisten HDMI_ prefixes? Imo there's no point in adding duplicate
> > #defines for all the SDVO_ bits we use in intel_hdmi.c ...
> 
> I agree, there's more to clean up. I thought about amending your
> suggestions to this patch, but I don't think this will be a good idea,
> so I will send 3 additional patches on top of this one. Feel free to
> merge them as a single patch if you want.

I've merged all four patches to dinq, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch