intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915/bxt: Use correct live status register for BXT platform
@ 2015-08-19 10:58 Durgadoss R
  2015-08-19 12:30 ` [PATCH 0/5] drm/i915: clean up *_digital_port_connected Jani Nikula
  2015-08-28 10:30 ` [PATCH] " shuang.he
  0 siblings, 2 replies; 13+ messages in thread
From: Durgadoss R @ 2015-08-19 10:58 UTC (permalink / raw)
  To: intel-gfx

BXT platform uses live status bits from 0x44440 register to
obtain DP status on hotplug. The existing g4x_digital_port_connected()
uses a different register and hence misses DP hotplug events on
BXT platform. This patch fixes it by using the appropriate
register(0x44440) and live status bits(3:5).

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a1dac9c..821d770 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4444,6 +4444,7 @@ static int g4x_digital_port_connected(struct drm_device *dev,
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t bit;
+	uint32_t reg = IS_BROXTON(dev) ? GEN8_DE_PORT_ISR : PORT_HOTPLUG_STAT;
 
 	if (IS_VALLEYVIEW(dev)) {
 		switch (intel_dig_port->port) {
@@ -4459,6 +4460,20 @@ static int g4x_digital_port_connected(struct drm_device *dev,
 		default:
 			return -EINVAL;
 		}
+	} else if (IS_BROXTON(dev)) {
+		switch (intel_dig_port->port) {
+		case PORT_A:
+			bit = BXT_DE_PORT_HP_DDIA;
+			break;
+		case PORT_B:
+			bit = BXT_DE_PORT_HP_DDIB;
+			break;
+		case PORT_C:
+			bit = BXT_DE_PORT_HP_DDIC;
+			break;
+		default:
+			return -EINVAL;
+		}
 	} else {
 		switch (intel_dig_port->port) {
 		case PORT_B:
@@ -4475,7 +4490,7 @@ static int g4x_digital_port_connected(struct drm_device *dev,
 		}
 	}
 
-	if ((I915_READ(PORT_HOTPLUG_STAT) & bit) == 0)
+	if ((I915_READ(reg) & bit) == 0)
 		return 0;
 	return 1;
 }
-- 
1.9.1

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

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

* [PATCH 0/5] drm/i915: clean up *_digital_port_connected
  2015-08-19 10:58 [PATCH] drm/i915/bxt: Use correct live status register for BXT platform Durgadoss R
@ 2015-08-19 12:30 ` Jani Nikula
  2015-08-19 12:30   ` [PATCH 1/5] drm/i915: move ibx_digital_port_connected to intel_dp.c Jani Nikula
                     ` (4 more replies)
  2015-08-28 10:30 ` [PATCH] " shuang.he
  1 sibling, 5 replies; 13+ messages in thread
From: Jani Nikula @ 2015-08-19 12:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Durgadoss' patch made me look at the {ibx,g4x}_digital_port_connected
functions, and I decided those need some love. No point in having the
platform specific code split both to functions and if ladders within
those functions. So add one common top level
intel_digital_port_connected function with one if ladder splitting out
to purely platform specific functions.

BR,
Jani.


Jani Nikula (5):
  drm/i915: move ibx_digital_port_connected to intel_dp.c
  drm/i915: add common intel_digital_port_connected function
  drm/i915: split ibx_digital_port_connected to ibx and cpt variants
  drm/i915: split g4x_digital_port_connected to g4x and vlv variants
  drm/i915/bxt: Use correct live status register for BXT platform

 drivers/gpu/drm/i915/intel_display.c |  45 ---------
 drivers/gpu/drm/i915/intel_dp.c      | 186 +++++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h     |   2 -
 3 files changed, 136 insertions(+), 97 deletions(-)

-- 
2.1.4

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

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

* [PATCH 1/5] drm/i915: move ibx_digital_port_connected to intel_dp.c
  2015-08-19 12:30 ` [PATCH 0/5] drm/i915: clean up *_digital_port_connected Jani Nikula
@ 2015-08-19 12:30   ` Jani Nikula
  2015-08-19 17:42     ` R, Durgadoss
  2015-08-19 12:30   ` [PATCH 2/5] drm/i915: add common intel_digital_port_connected function Jani Nikula
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2015-08-19 12:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

The function can be made static there. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 45 --------------------------
 drivers/gpu/drm/i915/intel_dp.c      | 61 +++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h     |  2 --
 3 files changed, 53 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f604ce1c528b..1a0670259cdf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1061,51 +1061,6 @@ static void intel_wait_for_pipe_off(struct intel_crtc *crtc)
 	}
 }
 
-/*
- * ibx_digital_port_connected - is the specified port connected?
- * @dev_priv: i915 private structure
- * @port: the port to test
- *
- * Returns true if @port is connected, false otherwise.
- */
-bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
-				struct intel_digital_port *port)
-{
-	u32 bit;
-
-	if (HAS_PCH_IBX(dev_priv->dev)) {
-		switch (port->port) {
-		case PORT_B:
-			bit = SDE_PORTB_HOTPLUG;
-			break;
-		case PORT_C:
-			bit = SDE_PORTC_HOTPLUG;
-			break;
-		case PORT_D:
-			bit = SDE_PORTD_HOTPLUG;
-			break;
-		default:
-			return true;
-		}
-	} else {
-		switch (port->port) {
-		case PORT_B:
-			bit = SDE_PORTB_HOTPLUG_CPT;
-			break;
-		case PORT_C:
-			bit = SDE_PORTC_HOTPLUG_CPT;
-			break;
-		case PORT_D:
-			bit = SDE_PORTD_HOTPLUG_CPT;
-			break;
-		default:
-			return true;
-		}
-	}
-
-	return I915_READ(SDEISR) & bit;
-}
-
 static const char *state_string(bool enabled)
 {
 	return enabled ? "on" : "off";
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d32ce4841654..4aa3d664765b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4473,17 +4473,49 @@ edp_detect(struct intel_dp *intel_dp)
 	return status;
 }
 
-static enum drm_connector_status
-ironlake_dp_detect(struct intel_dp *intel_dp)
+/*
+ * ibx_digital_port_connected - is the specified port connected?
+ * @dev_priv: i915 private structure
+ * @port: the port to test
+ *
+ * Returns true if @port is connected, false otherwise.
+ */
+static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
+				       struct intel_digital_port *port)
 {
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	u32 bit;
 
-	if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
-		return connector_status_disconnected;
+	if (HAS_PCH_IBX(dev_priv->dev)) {
+		switch (port->port) {
+		case PORT_B:
+			bit = SDE_PORTB_HOTPLUG;
+			break;
+		case PORT_C:
+			bit = SDE_PORTC_HOTPLUG;
+			break;
+		case PORT_D:
+			bit = SDE_PORTD_HOTPLUG;
+			break;
+		default:
+			return true;
+		}
+	} else {
+		switch (port->port) {
+		case PORT_B:
+			bit = SDE_PORTB_HOTPLUG_CPT;
+			break;
+		case PORT_C:
+			bit = SDE_PORTC_HOTPLUG_CPT;
+			break;
+		case PORT_D:
+			bit = SDE_PORTD_HOTPLUG_CPT;
+			break;
+		default:
+			return true;
+		}
+	}
 
-	return intel_dp_detect_dpcd(intel_dp);
+	return I915_READ(SDEISR) & bit;
 }
 
 static int g4x_digital_port_connected(struct drm_device *dev,
@@ -4528,6 +4560,19 @@ static int g4x_digital_port_connected(struct drm_device *dev,
 }
 
 static enum drm_connector_status
+ironlake_dp_detect(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+
+	if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
+		return connector_status_disconnected;
+
+	return intel_dp_detect_dpcd(intel_dp);
+}
+
+static enum drm_connector_status
 g4x_dp_detect(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 81b7d77a3c8b..a9e6c2789ea9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1001,8 +1001,6 @@ void intel_encoder_destroy(struct drm_encoder *encoder);
 int intel_connector_init(struct intel_connector *);
 struct intel_connector *intel_connector_alloc(void);
 bool intel_connector_get_hw_state(struct intel_connector *connector);
-bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
-				struct intel_digital_port *port);
 void intel_connector_attach_encoder(struct intel_connector *connector,
 				    struct intel_encoder *encoder);
 struct drm_encoder *intel_best_encoder(struct drm_connector *connector);
-- 
2.1.4

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

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

* [PATCH 2/5] drm/i915: add common intel_digital_port_connected function
  2015-08-19 12:30 ` [PATCH 0/5] drm/i915: clean up *_digital_port_connected Jani Nikula
  2015-08-19 12:30   ` [PATCH 1/5] drm/i915: move ibx_digital_port_connected to intel_dp.c Jani Nikula
@ 2015-08-19 12:30   ` Jani Nikula
  2015-08-19 12:34     ` Jani Nikula
  2015-08-19 12:30   ` [PATCH 3/5] drm/i915: split ibx_digital_port_connected to ibx and cpt variants Jani Nikula
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2015-08-19 12:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Add a common intel_digital_port_connected() that splits out to functions
for different platforms. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 53 ++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4aa3d664765b..d48af8b0c84e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4473,15 +4473,8 @@ edp_detect(struct intel_dp *intel_dp)
 	return status;
 }
 
-/*
- * ibx_digital_port_connected - is the specified port connected?
- * @dev_priv: i915 private structure
- * @port: the port to test
- *
- * Returns true if @port is connected, false otherwise.
- */
-static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
-				       struct intel_digital_port *port)
+static int ibx_digital_port_connected(struct drm_i915_private *dev_priv,
+				      struct intel_digital_port *port)
 {
 	u32 bit;
 
@@ -4497,7 +4490,7 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
 			bit = SDE_PORTD_HOTPLUG;
 			break;
 		default:
-			return true;
+			return 1;
 		}
 	} else {
 		switch (port->port) {
@@ -4511,20 +4504,19 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
 			bit = SDE_PORTD_HOTPLUG_CPT;
 			break;
 		default:
-			return true;
+			return 1;
 		}
 	}
 
-	return I915_READ(SDEISR) & bit;
+	return I915_READ(SDEISR) & bit ? 1 : 0;
 }
 
-static int g4x_digital_port_connected(struct drm_device *dev,
-				       struct intel_digital_port *intel_dig_port)
+static int g4x_digital_port_connected(struct drm_i915_private *dev_priv,
+				      struct intel_digital_port *intel_dig_port)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t bit;
 
-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_VALLEYVIEW(dev_priv)) {
 		switch (intel_dig_port->port) {
 		case PORT_B:
 			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
@@ -4559,6 +4551,22 @@ static int g4x_digital_port_connected(struct drm_device *dev,
 	return 1;
 }
 
+/*
+ * intel_digital_port_connected - is the specified port connected?
+ * @dev_priv: i915 private structure
+ * @port: the port to test
+ *
+ * Returns a negative error code on errors, 1 for connected, 0 for disconnected.
+ */
+static int intel_digital_port_connected(struct drm_i915_private *dev_priv,
+					struct intel_digital_port *port)
+{
+	if (HAS_PCH_SPLIT(dev_priv))
+		return ibx_digital_port_connected(dev_priv, port);
+	else
+		return g4x_digital_port_connected(dev_priv, port);
+}
+
 static enum drm_connector_status
 ironlake_dp_detect(struct intel_dp *intel_dp)
 {
@@ -4566,7 +4574,7 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 
-	if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
+	if (intel_digital_port_connected(dev_priv, intel_dig_port) != 1)
 		return connector_status_disconnected;
 
 	return intel_dp_detect_dpcd(intel_dp);
@@ -4589,7 +4597,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 		return status;
 	}
 
-	ret = g4x_digital_port_connected(dev, intel_dig_port);
+	ret = intel_digital_port_connected(dev->dev_private, intel_dig_port);
 	if (ret == -EINVAL)
 		return connector_status_unknown;
 	else if (ret == 0)
@@ -5055,13 +5063,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		/* indicate that we need to restart link training */
 		intel_dp->train_set_valid = false;
 
-		if (HAS_PCH_SPLIT(dev)) {
-			if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
-				goto mst_fail;
-		} else {
-			if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
-				goto mst_fail;
-		}
+		if (intel_digital_port_connected(dev_priv, intel_dig_port) != 1)
+			goto mst_fail;
 
 		if (!intel_dp_get_dpcd(intel_dp)) {
 			goto mst_fail;
-- 
2.1.4

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

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

* [PATCH 3/5] drm/i915: split ibx_digital_port_connected to ibx and cpt variants
  2015-08-19 12:30 ` [PATCH 0/5] drm/i915: clean up *_digital_port_connected Jani Nikula
  2015-08-19 12:30   ` [PATCH 1/5] drm/i915: move ibx_digital_port_connected to intel_dp.c Jani Nikula
  2015-08-19 12:30   ` [PATCH 2/5] drm/i915: add common intel_digital_port_connected function Jani Nikula
@ 2015-08-19 12:30   ` Jani Nikula
  2015-08-19 12:30   ` [PATCH 4/5] drm/i915: split g4x_digital_port_connected to g4x and vlv variants Jani Nikula
  2015-08-19 12:30   ` [PATCH 5/5] drm/i915/bxt: Use correct live status register for BXT platform Jani Nikula
  4 siblings, 0 replies; 13+ messages in thread
From: Jani Nikula @ 2015-08-19 12:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Choose the right function at the intel_digital_port_connected level.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 66 +++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d48af8b0c84e..2cb95dc88ba7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4478,34 +4478,40 @@ static int ibx_digital_port_connected(struct drm_i915_private *dev_priv,
 {
 	u32 bit;
 
-	if (HAS_PCH_IBX(dev_priv->dev)) {
-		switch (port->port) {
-		case PORT_B:
-			bit = SDE_PORTB_HOTPLUG;
-			break;
-		case PORT_C:
-			bit = SDE_PORTC_HOTPLUG;
-			break;
-		case PORT_D:
-			bit = SDE_PORTD_HOTPLUG;
-			break;
-		default:
-			return 1;
-		}
-	} else {
-		switch (port->port) {
-		case PORT_B:
-			bit = SDE_PORTB_HOTPLUG_CPT;
-			break;
-		case PORT_C:
-			bit = SDE_PORTC_HOTPLUG_CPT;
-			break;
-		case PORT_D:
-			bit = SDE_PORTD_HOTPLUG_CPT;
-			break;
-		default:
-			return 1;
-		}
+	switch (port->port) {
+	case PORT_B:
+		bit = SDE_PORTB_HOTPLUG;
+		break;
+	case PORT_C:
+		bit = SDE_PORTC_HOTPLUG;
+		break;
+	case PORT_D:
+		bit = SDE_PORTD_HOTPLUG;
+		break;
+	default:
+		return 1;
+	}
+
+	return I915_READ(SDEISR) & bit ? 1 : 0;
+}
+
+static int cpt_digital_port_connected(struct drm_i915_private *dev_priv,
+				      struct intel_digital_port *port)
+{
+	u32 bit;
+
+	switch (port->port) {
+	case PORT_B:
+		bit = SDE_PORTB_HOTPLUG_CPT;
+		break;
+	case PORT_C:
+		bit = SDE_PORTC_HOTPLUG_CPT;
+		break;
+	case PORT_D:
+		bit = SDE_PORTD_HOTPLUG_CPT;
+		break;
+	default:
+		return 1;
 	}
 
 	return I915_READ(SDEISR) & bit ? 1 : 0;
@@ -4561,8 +4567,10 @@ static int g4x_digital_port_connected(struct drm_i915_private *dev_priv,
 static int intel_digital_port_connected(struct drm_i915_private *dev_priv,
 					struct intel_digital_port *port)
 {
-	if (HAS_PCH_SPLIT(dev_priv))
+	if (HAS_PCH_IBX(dev_priv))
 		return ibx_digital_port_connected(dev_priv, port);
+	if (HAS_PCH_SPLIT(dev_priv))
+		return cpt_digital_port_connected(dev_priv, port);
 	else
 		return g4x_digital_port_connected(dev_priv, port);
 }
-- 
2.1.4

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

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

* [PATCH 4/5] drm/i915: split g4x_digital_port_connected to g4x and vlv variants
  2015-08-19 12:30 ` [PATCH 0/5] drm/i915: clean up *_digital_port_connected Jani Nikula
                     ` (2 preceding siblings ...)
  2015-08-19 12:30   ` [PATCH 3/5] drm/i915: split ibx_digital_port_connected to ibx and cpt variants Jani Nikula
@ 2015-08-19 12:30   ` Jani Nikula
  2015-08-19 12:30   ` [PATCH 5/5] drm/i915/bxt: Use correct live status register for BXT platform Jani Nikula
  4 siblings, 0 replies; 13+ messages in thread
From: Jani Nikula @ 2015-08-19 12:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Choose the right function at the intel_digital_port_connected level.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 72 ++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2cb95dc88ba7..bc3b94fa15cb 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4518,43 +4518,47 @@ static int cpt_digital_port_connected(struct drm_i915_private *dev_priv,
 }
 
 static int g4x_digital_port_connected(struct drm_i915_private *dev_priv,
-				      struct intel_digital_port *intel_dig_port)
+				      struct intel_digital_port *port)
 {
-	uint32_t bit;
+	u32 bit;
 
-	if (IS_VALLEYVIEW(dev_priv)) {
-		switch (intel_dig_port->port) {
-		case PORT_B:
-			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
-			break;
-		case PORT_C:
-			bit = PORTC_HOTPLUG_LIVE_STATUS_VLV;
-			break;
-		case PORT_D:
-			bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
-			break;
-		default:
-			return -EINVAL;
-		}
-	} else {
-		switch (intel_dig_port->port) {
-		case PORT_B:
-			bit = PORTB_HOTPLUG_LIVE_STATUS_G4X;
-			break;
-		case PORT_C:
-			bit = PORTC_HOTPLUG_LIVE_STATUS_G4X;
-			break;
-		case PORT_D:
-			bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
-			break;
-		default:
-			return -EINVAL;
-		}
+	switch (port->port) {
+	case PORT_B:
+		bit = PORTB_HOTPLUG_LIVE_STATUS_G4X;
+		break;
+	case PORT_C:
+		bit = PORTC_HOTPLUG_LIVE_STATUS_G4X;
+		break;
+	case PORT_D:
+		bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
+		break;
+	default:
+		return -EINVAL;
 	}
 
-	if ((I915_READ(PORT_HOTPLUG_STAT) & bit) == 0)
-		return 0;
-	return 1;
+	return I915_READ(PORT_HOTPLUG_STAT) & bit ? 1 : 0;
+}
+
+static int vlv_digital_port_connected(struct drm_i915_private *dev_priv,
+				      struct intel_digital_port *port)
+{
+	u32 bit;
+
+	switch (port->port) {
+	case PORT_B:
+		bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
+		break;
+	case PORT_C:
+		bit = PORTC_HOTPLUG_LIVE_STATUS_VLV;
+		break;
+	case PORT_D:
+		bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return I915_READ(PORT_HOTPLUG_STAT) & bit ? 1 : 0;
 }
 
 /*
@@ -4571,6 +4575,8 @@ static int intel_digital_port_connected(struct drm_i915_private *dev_priv,
 		return ibx_digital_port_connected(dev_priv, port);
 	if (HAS_PCH_SPLIT(dev_priv))
 		return cpt_digital_port_connected(dev_priv, port);
+	else if (IS_VALLEYVIEW(dev_priv))
+		return vlv_digital_port_connected(dev_priv, port);
 	else
 		return g4x_digital_port_connected(dev_priv, port);
 }
-- 
2.1.4

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

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

* [PATCH 5/5] drm/i915/bxt: Use correct live status register for BXT platform
  2015-08-19 12:30 ` [PATCH 0/5] drm/i915: clean up *_digital_port_connected Jani Nikula
                     ` (3 preceding siblings ...)
  2015-08-19 12:30   ` [PATCH 4/5] drm/i915: split g4x_digital_port_connected to g4x and vlv variants Jani Nikula
@ 2015-08-19 12:30   ` Jani Nikula
  4 siblings, 0 replies; 13+ messages in thread
From: Jani Nikula @ 2015-08-19 12:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

BXT platform uses live status bits from 0x44440 register to obtain DP
status on hotplug. The existing g4x_digital_port_connected() uses a
different register and hence misses DP hotplug events on BXT
platform. This patch fixes it by using the appropriate register(0x44440)
and live status bits(3:5).

Based on a patch by Durgadoss R <durgadoss.r@intel.com>, from whom the
commit message is shamelessly copy pasted.

Reported-by: Durgadoss R <durgadoss.r@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bc3b94fa15cb..c6d4bb62e83a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4561,6 +4561,28 @@ static int vlv_digital_port_connected(struct drm_i915_private *dev_priv,
 	return I915_READ(PORT_HOTPLUG_STAT) & bit ? 1 : 0;
 }
 
+static int bxt_digital_port_connected(struct drm_i915_private *dev_priv,
+				      struct intel_digital_port *port)
+{
+	u32 bit;
+
+	switch (port->port) {
+	case PORT_A:
+		bit = BXT_DE_PORT_HP_DDIA;
+		break;
+	case PORT_B:
+		bit = BXT_DE_PORT_HP_DDIB;
+		break;
+	case PORT_C:
+		bit = BXT_DE_PORT_HP_DDIC;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return I915_READ(GEN8_DE_PORT_ISR) & bit ? 1 : 0;
+}
+
 /*
  * intel_digital_port_connected - is the specified port connected?
  * @dev_priv: i915 private structure
@@ -4577,6 +4599,8 @@ static int intel_digital_port_connected(struct drm_i915_private *dev_priv,
 		return cpt_digital_port_connected(dev_priv, port);
 	else if (IS_VALLEYVIEW(dev_priv))
 		return vlv_digital_port_connected(dev_priv, port);
+	else if (IS_BROXTON(dev_priv))
+		return bxt_digital_port_connected(dev_priv, port);
 	else
 		return g4x_digital_port_connected(dev_priv, port);
 }
-- 
2.1.4

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

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

* Re: [PATCH 2/5] drm/i915: add common intel_digital_port_connected function
  2015-08-19 12:30   ` [PATCH 2/5] drm/i915: add common intel_digital_port_connected function Jani Nikula
@ 2015-08-19 12:34     ` Jani Nikula
  2015-08-19 17:50       ` R, Durgadoss
  0 siblings, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2015-08-19 12:34 UTC (permalink / raw)
  To: intel-gfx

On Wed, 19 Aug 2015, Jani Nikula <jani.nikula@intel.com> wrote:
> Add a common intel_digital_port_connected() that splits out to functions
> for different platforms. No functional changes.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 53 ++++++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4aa3d664765b..d48af8b0c84e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4473,15 +4473,8 @@ edp_detect(struct intel_dp *intel_dp)
>  	return status;
>  }
>  
> -/*
> - * ibx_digital_port_connected - is the specified port connected?
> - * @dev_priv: i915 private structure
> - * @port: the port to test
> - *
> - * Returns true if @port is connected, false otherwise.
> - */
> -static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
> -				       struct intel_digital_port *port)
> +static int ibx_digital_port_connected(struct drm_i915_private *dev_priv,
> +				      struct intel_digital_port *port)
>  {
>  	u32 bit;
>  
> @@ -4497,7 +4490,7 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>  			bit = SDE_PORTD_HOTPLUG;
>  			break;
>  		default:
> -			return true;
> +			return 1;
>  		}
>  	} else {
>  		switch (port->port) {
> @@ -4511,20 +4504,19 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>  			bit = SDE_PORTD_HOTPLUG_CPT;
>  			break;
>  		default:
> -			return true;
> +			return 1;

BTW a follow-up could look into making the ibx/cpt default cases return
-EINVAL like the g4x/vlv/bxt cases do.

BR,
Jani.


>  		}
>  	}
>  
> -	return I915_READ(SDEISR) & bit;
> +	return I915_READ(SDEISR) & bit ? 1 : 0;
>  }
>  
> -static int g4x_digital_port_connected(struct drm_device *dev,
> -				       struct intel_digital_port *intel_dig_port)
> +static int g4x_digital_port_connected(struct drm_i915_private *dev_priv,
> +				      struct intel_digital_port *intel_dig_port)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t bit;
>  
> -	if (IS_VALLEYVIEW(dev)) {
> +	if (IS_VALLEYVIEW(dev_priv)) {
>  		switch (intel_dig_port->port) {
>  		case PORT_B:
>  			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
> @@ -4559,6 +4551,22 @@ static int g4x_digital_port_connected(struct drm_device *dev,
>  	return 1;
>  }
>  
> +/*
> + * intel_digital_port_connected - is the specified port connected?
> + * @dev_priv: i915 private structure
> + * @port: the port to test
> + *
> + * Returns a negative error code on errors, 1 for connected, 0 for disconnected.
> + */
> +static int intel_digital_port_connected(struct drm_i915_private *dev_priv,
> +					struct intel_digital_port *port)
> +{
> +	if (HAS_PCH_SPLIT(dev_priv))
> +		return ibx_digital_port_connected(dev_priv, port);
> +	else
> +		return g4x_digital_port_connected(dev_priv, port);
> +}
> +
>  static enum drm_connector_status
>  ironlake_dp_detect(struct intel_dp *intel_dp)
>  {
> @@ -4566,7 +4574,7 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  
> -	if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
> +	if (intel_digital_port_connected(dev_priv, intel_dig_port) != 1)
>  		return connector_status_disconnected;
>  
>  	return intel_dp_detect_dpcd(intel_dp);
> @@ -4589,7 +4597,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
>  		return status;
>  	}
>  
> -	ret = g4x_digital_port_connected(dev, intel_dig_port);
> +	ret = intel_digital_port_connected(dev->dev_private, intel_dig_port);
>  	if (ret == -EINVAL)
>  		return connector_status_unknown;
>  	else if (ret == 0)
> @@ -5055,13 +5063,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  		/* indicate that we need to restart link training */
>  		intel_dp->train_set_valid = false;
>  
> -		if (HAS_PCH_SPLIT(dev)) {
> -			if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
> -				goto mst_fail;
> -		} else {
> -			if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
> -				goto mst_fail;
> -		}
> +		if (intel_digital_port_connected(dev_priv, intel_dig_port) != 1)
> +			goto mst_fail;
>  
>  		if (!intel_dp_get_dpcd(intel_dp)) {
>  			goto mst_fail;
> -- 
> 2.1.4
>

-- 
Jani Nikula, 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] 13+ messages in thread

* Re: [PATCH 1/5] drm/i915: move ibx_digital_port_connected to intel_dp.c
  2015-08-19 12:30   ` [PATCH 1/5] drm/i915: move ibx_digital_port_connected to intel_dp.c Jani Nikula
@ 2015-08-19 17:42     ` R, Durgadoss
  0 siblings, 0 replies; 13+ messages in thread
From: R, Durgadoss @ 2015-08-19 17:42 UTC (permalink / raw)
  To: Nikula, Jani, intel-gfx@lists.freedesktop.org

>-----Original Message-----
>From: Nikula, Jani
>Sent: Wednesday, August 19, 2015 6:01 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: R, Durgadoss; Nikula, Jani
>Subject: [PATCH 1/5] drm/i915: move ibx_digital_port_connected to intel_dp.c
>
>The function can be made static there. No functional changes.
>

This clean up series looks good to me.

Reviewed-by: Durgadoss R <durgadoss.r@intel.com>

Thanks,
Durga

>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
> drivers/gpu/drm/i915/intel_display.c | 45 --------------------------
> drivers/gpu/drm/i915/intel_dp.c      | 61 +++++++++++++++++++++++++++++++-----
> drivers/gpu/drm/i915/intel_drv.h     |  2 --
> 3 files changed, 53 insertions(+), 55 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>index f604ce1c528b..1a0670259cdf 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -1061,51 +1061,6 @@ static void intel_wait_for_pipe_off(struct intel_crtc *crtc)
> 	}
> }
>
>-/*
>- * ibx_digital_port_connected - is the specified port connected?
>- * @dev_priv: i915 private structure
>- * @port: the port to test
>- *
>- * Returns true if @port is connected, false otherwise.
>- */
>-bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>-				struct intel_digital_port *port)
>-{
>-	u32 bit;
>-
>-	if (HAS_PCH_IBX(dev_priv->dev)) {
>-		switch (port->port) {
>-		case PORT_B:
>-			bit = SDE_PORTB_HOTPLUG;
>-			break;
>-		case PORT_C:
>-			bit = SDE_PORTC_HOTPLUG;
>-			break;
>-		case PORT_D:
>-			bit = SDE_PORTD_HOTPLUG;
>-			break;
>-		default:
>-			return true;
>-		}
>-	} else {
>-		switch (port->port) {
>-		case PORT_B:
>-			bit = SDE_PORTB_HOTPLUG_CPT;
>-			break;
>-		case PORT_C:
>-			bit = SDE_PORTC_HOTPLUG_CPT;
>-			break;
>-		case PORT_D:
>-			bit = SDE_PORTD_HOTPLUG_CPT;
>-			break;
>-		default:
>-			return true;
>-		}
>-	}
>-
>-	return I915_READ(SDEISR) & bit;
>-}
>-
> static const char *state_string(bool enabled)
> {
> 	return enabled ? "on" : "off";
>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>index d32ce4841654..4aa3d664765b 100644
>--- a/drivers/gpu/drm/i915/intel_dp.c
>+++ b/drivers/gpu/drm/i915/intel_dp.c
>@@ -4473,17 +4473,49 @@ edp_detect(struct intel_dp *intel_dp)
> 	return status;
> }
>
>-static enum drm_connector_status
>-ironlake_dp_detect(struct intel_dp *intel_dp)
>+/*
>+ * ibx_digital_port_connected - is the specified port connected?
>+ * @dev_priv: i915 private structure
>+ * @port: the port to test
>+ *
>+ * Returns true if @port is connected, false otherwise.
>+ */
>+static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>+				       struct intel_digital_port *port)
> {
>-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>-	struct drm_i915_private *dev_priv = dev->dev_private;
>-	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>+	u32 bit;
>
>-	if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
>-		return connector_status_disconnected;
>+	if (HAS_PCH_IBX(dev_priv->dev)) {
>+		switch (port->port) {
>+		case PORT_B:
>+			bit = SDE_PORTB_HOTPLUG;
>+			break;
>+		case PORT_C:
>+			bit = SDE_PORTC_HOTPLUG;
>+			break;
>+		case PORT_D:
>+			bit = SDE_PORTD_HOTPLUG;
>+			break;
>+		default:
>+			return true;
>+		}
>+	} else {
>+		switch (port->port) {
>+		case PORT_B:
>+			bit = SDE_PORTB_HOTPLUG_CPT;
>+			break;
>+		case PORT_C:
>+			bit = SDE_PORTC_HOTPLUG_CPT;
>+			break;
>+		case PORT_D:
>+			bit = SDE_PORTD_HOTPLUG_CPT;
>+			break;
>+		default:
>+			return true;
>+		}
>+	}
>
>-	return intel_dp_detect_dpcd(intel_dp);
>+	return I915_READ(SDEISR) & bit;
> }
>
> static int g4x_digital_port_connected(struct drm_device *dev,
>@@ -4528,6 +4560,19 @@ static int g4x_digital_port_connected(struct drm_device *dev,
> }
>
> static enum drm_connector_status
>+ironlake_dp_detect(struct intel_dp *intel_dp)
>+{
>+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>+	struct drm_i915_private *dev_priv = dev->dev_private;
>+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>+
>+	if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
>+		return connector_status_disconnected;
>+
>+	return intel_dp_detect_dpcd(intel_dp);
>+}
>+
>+static enum drm_connector_status
> g4x_dp_detect(struct intel_dp *intel_dp)
> {
> 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>index 81b7d77a3c8b..a9e6c2789ea9 100644
>--- a/drivers/gpu/drm/i915/intel_drv.h
>+++ b/drivers/gpu/drm/i915/intel_drv.h
>@@ -1001,8 +1001,6 @@ void intel_encoder_destroy(struct drm_encoder *encoder);
> int intel_connector_init(struct intel_connector *);
> struct intel_connector *intel_connector_alloc(void);
> bool intel_connector_get_hw_state(struct intel_connector *connector);
>-bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>-				struct intel_digital_port *port);
> void intel_connector_attach_encoder(struct intel_connector *connector,
> 				    struct intel_encoder *encoder);
> struct drm_encoder *intel_best_encoder(struct drm_connector *connector);
>--
>2.1.4

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

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

* Re: [PATCH 2/5] drm/i915: add common intel_digital_port_connected function
  2015-08-19 12:34     ` Jani Nikula
@ 2015-08-19 17:50       ` R, Durgadoss
  2015-08-19 18:10         ` Jani Nikula
  2015-08-19 18:12         ` Ville Syrjälä
  0 siblings, 2 replies; 13+ messages in thread
From: R, Durgadoss @ 2015-08-19 17:50 UTC (permalink / raw)
  To: Nikula, Jani, intel-gfx@lists.freedesktop.org

>-----Original Message-----
>From: Nikula, Jani
>Sent: Wednesday, August 19, 2015 6:04 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: R, Durgadoss
>Subject: Re: [PATCH 2/5] drm/i915: add common intel_digital_port_connected function
>
>On Wed, 19 Aug 2015, Jani Nikula <jani.nikula@intel.com> wrote:
>> Add a common intel_digital_port_connected() that splits out to functions
>> for different platforms. No functional changes.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 53 ++++++++++++++++++++++-------------------
>>  1 file changed, 28 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 4aa3d664765b..d48af8b0c84e 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4473,15 +4473,8 @@ edp_detect(struct intel_dp *intel_dp)
>>  	return status;
>>  }
>>
>> -/*
>> - * ibx_digital_port_connected - is the specified port connected?
>> - * @dev_priv: i915 private structure
>> - * @port: the port to test
>> - *
>> - * Returns true if @port is connected, false otherwise.
>> - */
>> -static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>> -				       struct intel_digital_port *port)
>> +static int ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>> +				      struct intel_digital_port *port)
>>  {
>>  	u32 bit;
>>
>> @@ -4497,7 +4490,7 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>>  			bit = SDE_PORTD_HOTPLUG;
>>  			break;
>>  		default:
>> -			return true;
>> +			return 1;
>>  		}
>>  	} else {
>>  		switch (port->port) {
>> @@ -4511,20 +4504,19 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>>  			bit = SDE_PORTD_HOTPLUG_CPT;
>>  			break;
>>  		default:
>> -			return true;
>> +			return 1;
>
>BTW a follow-up could look into making the ibx/cpt default cases return
>-EINVAL like the g4x/vlv/bxt cases do.

But the default case for ibx/cpt currently return 1 .. meaning "connected"
as default result.. In this case, by returning -EINVAL,
won't we break existing users ?

or,
Can we add a WARN_ON()/MISSING_CASE() and then return -EINVAL ?

Thanks,
Durga

>
>BR,
>Jani.
>
>
>>  		}
>>  	}
>>
>> -	return I915_READ(SDEISR) & bit;
>> +	return I915_READ(SDEISR) & bit ? 1 : 0;
>>  }
>>
>> -static int g4x_digital_port_connected(struct drm_device *dev,
>> -				       struct intel_digital_port *intel_dig_port)
>> +static int g4x_digital_port_connected(struct drm_i915_private *dev_priv,
>> +				      struct intel_digital_port *intel_dig_port)
>>  {
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	uint32_t bit;
>>
>> -	if (IS_VALLEYVIEW(dev)) {
>> +	if (IS_VALLEYVIEW(dev_priv)) {
>>  		switch (intel_dig_port->port) {
>>  		case PORT_B:
>>  			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
>> @@ -4559,6 +4551,22 @@ static int g4x_digital_port_connected(struct drm_device *dev,
>>  	return 1;
>>  }
>>
>> +/*
>> + * intel_digital_port_connected - is the specified port connected?
>> + * @dev_priv: i915 private structure
>> + * @port: the port to test
>> + *
>> + * Returns a negative error code on errors, 1 for connected, 0 for disconnected.
>> + */
>> +static int intel_digital_port_connected(struct drm_i915_private *dev_priv,
>> +					struct intel_digital_port *port)
>> +{
>> +	if (HAS_PCH_SPLIT(dev_priv))
>> +		return ibx_digital_port_connected(dev_priv, port);
>> +	else
>> +		return g4x_digital_port_connected(dev_priv, port);
>> +}
>> +
>>  static enum drm_connector_status
>>  ironlake_dp_detect(struct intel_dp *intel_dp)
>>  {
>> @@ -4566,7 +4574,7 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>
>> -	if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
>> +	if (intel_digital_port_connected(dev_priv, intel_dig_port) != 1)
>>  		return connector_status_disconnected;
>>
>>  	return intel_dp_detect_dpcd(intel_dp);
>> @@ -4589,7 +4597,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
>>  		return status;
>>  	}
>>
>> -	ret = g4x_digital_port_connected(dev, intel_dig_port);
>> +	ret = intel_digital_port_connected(dev->dev_private, intel_dig_port);
>>  	if (ret == -EINVAL)
>>  		return connector_status_unknown;
>>  	else if (ret == 0)
>> @@ -5055,13 +5063,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool
>long_hpd)
>>  		/* indicate that we need to restart link training */
>>  		intel_dp->train_set_valid = false;
>>
>> -		if (HAS_PCH_SPLIT(dev)) {
>> -			if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
>> -				goto mst_fail;
>> -		} else {
>> -			if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
>> -				goto mst_fail;
>> -		}
>> +		if (intel_digital_port_connected(dev_priv, intel_dig_port) != 1)
>> +			goto mst_fail;
>>
>>  		if (!intel_dp_get_dpcd(intel_dp)) {
>>  			goto mst_fail;
>> --
>> 2.1.4
>>
>
>--
>Jani Nikula, 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] 13+ messages in thread

* Re: [PATCH 2/5] drm/i915: add common intel_digital_port_connected function
  2015-08-19 17:50       ` R, Durgadoss
@ 2015-08-19 18:10         ` Jani Nikula
  2015-08-19 18:12         ` Ville Syrjälä
  1 sibling, 0 replies; 13+ messages in thread
From: Jani Nikula @ 2015-08-19 18:10 UTC (permalink / raw)
  To: R, Durgadoss, intel-gfx@lists.freedesktop.org

On Wed, 19 Aug 2015, "R, Durgadoss" <durgadoss.r@intel.com> wrote:
>>-----Original Message-----
>>From: Nikula, Jani
>>Sent: Wednesday, August 19, 2015 6:04 PM
>>To: intel-gfx@lists.freedesktop.org
>>Cc: R, Durgadoss
>>Subject: Re: [PATCH 2/5] drm/i915: add common intel_digital_port_connected function
>>
>>On Wed, 19 Aug 2015, Jani Nikula <jani.nikula@intel.com> wrote:
>>> Add a common intel_digital_port_connected() that splits out to functions
>>> for different platforms. No functional changes.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_dp.c | 53 ++++++++++++++++++++++-------------------
>>>  1 file changed, 28 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 4aa3d664765b..d48af8b0c84e 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -4473,15 +4473,8 @@ edp_detect(struct intel_dp *intel_dp)
>>>  	return status;
>>>  }
>>>
>>> -/*
>>> - * ibx_digital_port_connected - is the specified port connected?
>>> - * @dev_priv: i915 private structure
>>> - * @port: the port to test
>>> - *
>>> - * Returns true if @port is connected, false otherwise.
>>> - */
>>> -static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>>> -				       struct intel_digital_port *port)
>>> +static int ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>>> +				      struct intel_digital_port *port)
>>>  {
>>>  	u32 bit;
>>>
>>> @@ -4497,7 +4490,7 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>>>  			bit = SDE_PORTD_HOTPLUG;
>>>  			break;
>>>  		default:
>>> -			return true;
>>> +			return 1;
>>>  		}
>>>  	} else {
>>>  		switch (port->port) {
>>> @@ -4511,20 +4504,19 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
>>>  			bit = SDE_PORTD_HOTPLUG_CPT;
>>>  			break;
>>>  		default:
>>> -			return true;
>>> +			return 1;
>>
>>BTW a follow-up could look into making the ibx/cpt default cases return
>>-EINVAL like the g4x/vlv/bxt cases do.
>
> But the default case for ibx/cpt currently return 1 .. meaning "connected"
> as default result.. In this case, by returning -EINVAL,
> won't we break existing users ?

By "looking into" I really meant figuring out why pch split platforms
are different, and what, if anything, would break. ;)

Thanks for the review.

BR,
Jani.


>
> or,
> Can we add a WARN_ON()/MISSING_CASE() and then return -EINVAL ?
>
> Thanks,
> Durga
>
>>
>>BR,
>>Jani.
>>
>>
>>>  		}
>>>  	}
>>>
>>> -	return I915_READ(SDEISR) & bit;
>>> +	return I915_READ(SDEISR) & bit ? 1 : 0;
>>>  }
>>>
>>> -static int g4x_digital_port_connected(struct drm_device *dev,
>>> -				       struct intel_digital_port *intel_dig_port)
>>> +static int g4x_digital_port_connected(struct drm_i915_private *dev_priv,
>>> +				      struct intel_digital_port *intel_dig_port)
>>>  {
>>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>>>  	uint32_t bit;
>>>
>>> -	if (IS_VALLEYVIEW(dev)) {
>>> +	if (IS_VALLEYVIEW(dev_priv)) {
>>>  		switch (intel_dig_port->port) {
>>>  		case PORT_B:
>>>  			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
>>> @@ -4559,6 +4551,22 @@ static int g4x_digital_port_connected(struct drm_device *dev,
>>>  	return 1;
>>>  }
>>>
>>> +/*
>>> + * intel_digital_port_connected - is the specified port connected?
>>> + * @dev_priv: i915 private structure
>>> + * @port: the port to test
>>> + *
>>> + * Returns a negative error code on errors, 1 for connected, 0 for disconnected.
>>> + */
>>> +static int intel_digital_port_connected(struct drm_i915_private *dev_priv,
>>> +					struct intel_digital_port *port)
>>> +{
>>> +	if (HAS_PCH_SPLIT(dev_priv))
>>> +		return ibx_digital_port_connected(dev_priv, port);
>>> +	else
>>> +		return g4x_digital_port_connected(dev_priv, port);
>>> +}
>>> +
>>>  static enum drm_connector_status
>>>  ironlake_dp_detect(struct intel_dp *intel_dp)
>>>  {
>>> @@ -4566,7 +4574,7 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
>>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>>
>>> -	if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
>>> +	if (intel_digital_port_connected(dev_priv, intel_dig_port) != 1)
>>>  		return connector_status_disconnected;
>>>
>>>  	return intel_dp_detect_dpcd(intel_dp);
>>> @@ -4589,7 +4597,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
>>>  		return status;
>>>  	}
>>>
>>> -	ret = g4x_digital_port_connected(dev, intel_dig_port);
>>> +	ret = intel_digital_port_connected(dev->dev_private, intel_dig_port);
>>>  	if (ret == -EINVAL)
>>>  		return connector_status_unknown;
>>>  	else if (ret == 0)
>>> @@ -5055,13 +5063,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool
>>long_hpd)
>>>  		/* indicate that we need to restart link training */
>>>  		intel_dp->train_set_valid = false;
>>>
>>> -		if (HAS_PCH_SPLIT(dev)) {
>>> -			if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
>>> -				goto mst_fail;
>>> -		} else {
>>> -			if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
>>> -				goto mst_fail;
>>> -		}
>>> +		if (intel_digital_port_connected(dev_priv, intel_dig_port) != 1)
>>> +			goto mst_fail;
>>>
>>>  		if (!intel_dp_get_dpcd(intel_dp)) {
>>>  			goto mst_fail;
>>> --
>>> 2.1.4
>>>
>>
>>--
>>Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, 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] 13+ messages in thread

* Re: [PATCH 2/5] drm/i915: add common intel_digital_port_connected function
  2015-08-19 17:50       ` R, Durgadoss
  2015-08-19 18:10         ` Jani Nikula
@ 2015-08-19 18:12         ` Ville Syrjälä
  1 sibling, 0 replies; 13+ messages in thread
From: Ville Syrjälä @ 2015-08-19 18:12 UTC (permalink / raw)
  To: R, Durgadoss; +Cc: Nikula, Jani, intel-gfx@lists.freedesktop.org

On Wed, Aug 19, 2015 at 05:50:52PM +0000, R, Durgadoss wrote:
> >-----Original Message-----
> >From: Nikula, Jani
> >Sent: Wednesday, August 19, 2015 6:04 PM
> >To: intel-gfx@lists.freedesktop.org
> >Cc: R, Durgadoss
> >Subject: Re: [PATCH 2/5] drm/i915: add common intel_digital_port_connected function
> >
> >On Wed, 19 Aug 2015, Jani Nikula <jani.nikula@intel.com> wrote:
> >> Add a common intel_digital_port_connected() that splits out to functions
> >> for different platforms. No functional changes.
> >>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_dp.c | 53 ++++++++++++++++++++++-------------------
> >>  1 file changed, 28 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 4aa3d664765b..d48af8b0c84e 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -4473,15 +4473,8 @@ edp_detect(struct intel_dp *intel_dp)
> >>  	return status;
> >>  }
> >>
> >> -/*
> >> - * ibx_digital_port_connected - is the specified port connected?
> >> - * @dev_priv: i915 private structure
> >> - * @port: the port to test
> >> - *
> >> - * Returns true if @port is connected, false otherwise.
> >> - */
> >> -static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
> >> -				       struct intel_digital_port *port)
> >> +static int ibx_digital_port_connected(struct drm_i915_private *dev_priv,
> >> +				      struct intel_digital_port *port)
> >>  {
> >>  	u32 bit;
> >>
> >> @@ -4497,7 +4490,7 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
> >>  			bit = SDE_PORTD_HOTPLUG;
> >>  			break;
> >>  		default:
> >> -			return true;
> >> +			return 1;
> >>  		}
> >>  	} else {
> >>  		switch (port->port) {
> >> @@ -4511,20 +4504,19 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
> >>  			bit = SDE_PORTD_HOTPLUG_CPT;
> >>  			break;
> >>  		default:
> >> -			return true;
> >> +			return 1;
> >
> >BTW a follow-up could look into making the ibx/cpt default cases return
> >-EINVAL like the g4x/vlv/bxt cases do.
> 
> But the default case for ibx/cpt currently return 1 .. meaning "connected"
> as default result.. In this case, by returning -EINVAL,
> won't we break existing users ?
> 
> or,
> Can we add a WARN_ON()/MISSING_CASE() and then return -EINVAL ?

Can we ever hit the default case except due to bugs somewhere else?
If not, then I suggest we just ignore it and make the thing return
bool.

> 
> Thanks,
> Durga
> 
> >
> >BR,
> >Jani.
> >
> >
> >>  		}
> >>  	}
> >>
> >> -	return I915_READ(SDEISR) & bit;
> >> +	return I915_READ(SDEISR) & bit ? 1 : 0;
> >>  }
> >>
> >> -static int g4x_digital_port_connected(struct drm_device *dev,
> >> -				       struct intel_digital_port *intel_dig_port)
> >> +static int g4x_digital_port_connected(struct drm_i915_private *dev_priv,
> >> +				      struct intel_digital_port *intel_dig_port)
> >>  {
> >> -	struct drm_i915_private *dev_priv = dev->dev_private;
> >>  	uint32_t bit;
> >>
> >> -	if (IS_VALLEYVIEW(dev)) {
> >> +	if (IS_VALLEYVIEW(dev_priv)) {
> >>  		switch (intel_dig_port->port) {
> >>  		case PORT_B:
> >>  			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
> >> @@ -4559,6 +4551,22 @@ static int g4x_digital_port_connected(struct drm_device *dev,
> >>  	return 1;
> >>  }
> >>
> >> +/*
> >> + * intel_digital_port_connected - is the specified port connected?
> >> + * @dev_priv: i915 private structure
> >> + * @port: the port to test
> >> + *
> >> + * Returns a negative error code on errors, 1 for connected, 0 for disconnected.
> >> + */
> >> +static int intel_digital_port_connected(struct drm_i915_private *dev_priv,
> >> +					struct intel_digital_port *port)
> >> +{
> >> +	if (HAS_PCH_SPLIT(dev_priv))
> >> +		return ibx_digital_port_connected(dev_priv, port);
> >> +	else
> >> +		return g4x_digital_port_connected(dev_priv, port);
> >> +}
> >> +
> >>  static enum drm_connector_status
> >>  ironlake_dp_detect(struct intel_dp *intel_dp)
> >>  {
> >> @@ -4566,7 +4574,7 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>
> >> -	if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
> >> +	if (intel_digital_port_connected(dev_priv, intel_dig_port) != 1)
> >>  		return connector_status_disconnected;
> >>
> >>  	return intel_dp_detect_dpcd(intel_dp);
> >> @@ -4589,7 +4597,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
> >>  		return status;
> >>  	}
> >>
> >> -	ret = g4x_digital_port_connected(dev, intel_dig_port);
> >> +	ret = intel_digital_port_connected(dev->dev_private, intel_dig_port);
> >>  	if (ret == -EINVAL)
> >>  		return connector_status_unknown;
> >>  	else if (ret == 0)
> >> @@ -5055,13 +5063,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool
> >long_hpd)
> >>  		/* indicate that we need to restart link training */
> >>  		intel_dp->train_set_valid = false;
> >>
> >> -		if (HAS_PCH_SPLIT(dev)) {
> >> -			if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
> >> -				goto mst_fail;
> >> -		} else {
> >> -			if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
> >> -				goto mst_fail;
> >> -		}
> >> +		if (intel_digital_port_connected(dev_priv, intel_dig_port) != 1)
> >> +			goto mst_fail;
> >>
> >>  		if (!intel_dp_get_dpcd(intel_dp)) {
> >>  			goto mst_fail;
> >> --
> >> 2.1.4
> >>
> >
> >--
> >Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/bxt: Use correct live status register for BXT platform
  2015-08-19 10:58 [PATCH] drm/i915/bxt: Use correct live status register for BXT platform Durgadoss R
  2015-08-19 12:30 ` [PATCH 0/5] drm/i915: clean up *_digital_port_connected Jani Nikula
@ 2015-08-28 10:30 ` shuang.he
  1 sibling, 0 replies; 13+ messages in thread
From: shuang.he @ 2015-08-28 10:30 UTC (permalink / raw)
  To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu, intel-gfx,
	durgadoss.r

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7232
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                 -1              302/302              301/302
SNB                                  315/315              315/315
IVB                                  336/336              336/336
BYT                 -1              283/283              282/283
HSW                                  378/378              378/378
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@kms_flip@flip-vs-dpms-interruptible      PASS(1)      DMESG_WARN(1)
*BYT  igt@gem_tiled_partial_pwrite_pread@reads      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2015-08-28 10:30 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-19 10:58 [PATCH] drm/i915/bxt: Use correct live status register for BXT platform Durgadoss R
2015-08-19 12:30 ` [PATCH 0/5] drm/i915: clean up *_digital_port_connected Jani Nikula
2015-08-19 12:30   ` [PATCH 1/5] drm/i915: move ibx_digital_port_connected to intel_dp.c Jani Nikula
2015-08-19 17:42     ` R, Durgadoss
2015-08-19 12:30   ` [PATCH 2/5] drm/i915: add common intel_digital_port_connected function Jani Nikula
2015-08-19 12:34     ` Jani Nikula
2015-08-19 17:50       ` R, Durgadoss
2015-08-19 18:10         ` Jani Nikula
2015-08-19 18:12         ` Ville Syrjälä
2015-08-19 12:30   ` [PATCH 3/5] drm/i915: split ibx_digital_port_connected to ibx and cpt variants Jani Nikula
2015-08-19 12:30   ` [PATCH 4/5] drm/i915: split g4x_digital_port_connected to g4x and vlv variants Jani Nikula
2015-08-19 12:30   ` [PATCH 5/5] drm/i915/bxt: Use correct live status register for BXT platform Jani Nikula
2015-08-28 10:30 ` [PATCH] " shuang.he

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).