drm/i915: clean up *_digital_port_connected

-----Original Message-----
From: Nikula, Jani
Sent: Thursday, August 20, 2015 1:18 PM
To: [email protected]
Cc: R, Durgadoss; Ville Syrjälä; Nikula, Jani
Subject: [PATCH v2 2/7] drm/i915: make g4x_digital_port_connected return boolean status

We should not be hitting any of the default cases in g4x_digital_port_connected, so add MISSING_CASE annotation and return boolean status. The current behaviour is just cargo culting from the days of yonder when the display port support was added to i915.

Signed-off-by Jani Nikula <[email protected]>

---
drivers/gpu/drm/i915/intel_dp.c | 26 +++++++++++---------------

1 file changed, 11 insertions(+), 15 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +9 -9

index 4aa3d664765b..d35dc0711f9b 100644
@@ -4518,14 +4518,14 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
 	return I915_READ(SDEISR) & bit;
 }
 
-static int g4x_digital_port_connected(struct drm_device *dev,
-				       struct intel_digital_port *intel_dig_port)
+static bool g4x_digital_port_connected(struct drm_device *dev,
+				       struct intel_digital_port *port)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t bit;
 
 	if (IS_VALLEYVIEW(dev)) {
-		switch (intel_dig_port->port) {
+		switch (port->port) {
 		case PORT_B:
 			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;
 			break;

@@ -4536,10 +4536,11 @@ static int g4x_digital_port_connected(struct drm_device *dev,
 			bit = PORTD_HOTPLUG_LIVE_STATUS_VLV;
 			break;
 		default:
-			return -EINVAL;
+			MISSING_CASE(port->port);
+			return false;
 		}
 	} else {
-		switch (intel_dig_port->port) {
+		switch (port->port) {
 		case PORT_B:
 			bit = PORTB_HOTPLUG_LIVE_STATUS_G4X;
 			break;

@@ -4550,13 +4551,12 @@ static int g4x_digital_port_connected(struct drm_device *dev,
 			bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
 			break;
 		default:
-			return -EINVAL;
+			MISSING_CASE(port->port);
+			return false;
 		}
 	}
 
-	if ((I915_READ(PORT_HOTPLUG_STAT) & bit) == 0)
-		return 0;
-	return 1;
+	return I915_READ(PORT_HOTPLUG_STAT) & bit;
 }
 

static enum drm_connector_status

drivers/gpu/drm/i915/intel_dp.c +2 -7

@@ -4577,7 +4577,6 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	int ret;
 
 	/* Can't disconnect eDP, but you can close the lid... */
 	if (is_edp(intel_dp)) {

@@ -4589,10 +4588,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 		return status;
 	}
 
-	ret = g4x_digital_port_connected(dev, intel_dig_port);
-	if (ret == -EINVAL)
-		return connector_status_unknown;
-	else if (ret == 0)
+	if (!g4x_digital_port_connected(dev, intel_dig_port))
 		return connector_status_disconnected;
 
 	return intel_dp_detect_dpcd(intel_dp);

@@ -5059,7 +5055,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 			if (!ibx_digital_port_connected(dev_priv, intel_dig_port))
 				goto mst_fail;
 		} else {
-			if (g4x_digital_port_connected(dev, intel_dig_port) != 1)
+			if (!g4x_digital_port_connected(dev, intel_dig_port))
 				goto mst_fail;
 		}
 
--

2.1.4

-----Original Message-----
From: Nikula, Jani
Sent: Thursday, August 20, 2015 1:18 PM
To: [email protected]
Cc: R, Durgadoss; Ville Syrjälä; Nikula, Jani
Subject: [PATCH v2 3/7] drm/i915: add MISSING_CASE annotation to ibx_digital_port_connected

With the case added for eDP on port A (always connected from this function's point of view), we should not be hitting any of the default cases in ibx_digital_port_connected, so add MISSING_CASE annotation.

Signed-off-by Jani Nikula <[email protected]>

---
drivers/gpu/drm/i915/intel_dp.c | 10 ++++++++--

1 file changed, 8 insertions(+), 2 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +8 -3

index d35dc0711f9b..f08859471841 100644
@@ -4487,6 +4487,8 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
 
 	if (HAS_PCH_IBX(dev_priv->dev)) {
 		switch (port->port) {
+		case PORT_A:
+			return true;
 		case PORT_B:
 			bit = SDE_PORTB_HOTPLUG;
 			break;

@@ -4497,10 +4499,13 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
 			bit = SDE_PORTD_HOTPLUG;
 			break;
 		default:
-			return true;
+			MISSING_CASE(port->port);
+			return false;
 		}
 	} else {
 		switch (port->port) {
+		case PORT_A:
+			return true;
 		case PORT_B:
 			bit = SDE_PORTB_HOTPLUG_CPT;
 			break;

@@ -4511,7 +4516,8 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
 			bit = SDE_PORTD_HOTPLUG_CPT;
 			break;
 		default:
-			return true;
+			MISSING_CASE(port->port);
+			return false;
 		}
 	}
 
--

2.1.4

Oh, bxt is here..:) Spoke too soon..
But will need to make it based upon intel_encoder->hpd_pin because of the A0/A1 WA for BXT.

Also, we need to make intel_digital_port_connected global to be accessed from intel_hdmi.c

Regards,
Sonika

-----Original Message-----
From: Intel-gfx [mailto:[email protected]] On Behalf Of Jani Nikula
Sent: Thursday, August 20, 2015 1:18 PM
To: [email protected]
Cc: Nikula, Jani
Subject: [Intel-gfx] [PATCH v2 7/7] drm/i915/bxt: Use correct live status register for BXT platform

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 <[email protected]>, from whom the commit message is shamelessly copy pasted.

Reported-by Durgadoss R <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

drivers/gpu/drm/i915/intel_dp.c +23 -1

@@ -4569,6 +4569,29 @@ static bool vlv_digital_port_connected(struct drm_i915_private *dev_priv,
 	return I915_READ(PORT_HOTPLUG_STAT) & bit;  }
 
+static bool 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:
+		MISSING_CASE(port->port);
+		return false;
+	}
+
+	return I915_READ(GEN8_DE_PORT_ISR) & bit; }
+
 /*
  * intel_digital_port_connected - is the specified port connected?
  * @dev_priv: i915 private structure

@@ -4583,6 +4606,8 @@ static bool 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_BROXTON(dev_priv))
+		return bxt_digital_port_connected(dev_priv, port);
 	else if (IS_VALLEYVIEW(dev_priv))
 		return vlv_digital_port_connected(dev_priv, port);
 	else
--

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

v2 with missing cases handled and intel_digital_port_connected return value changed to bool. Mostly it's just the addition of patches 2 and 3, and rebase of the rest.

BR,
Jani.

Jani Nikula (7):
drm/i915: move ibx_digital_port_connected to intel_dp.c drm/i915: make g4x_digital_port_connected return boolean status drm/i915: add MISSING_CASE annotation to ibx_digital_port_connected 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      | 199 +++++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h     |   2 -
 3 files changed, 145 insertions(+), 101 deletions(-)

Signature · 6 lines

2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

v2: make the function return a boolean

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/intel_dp.c | 41 ++++++++++++++++++++++-------------------
  1 file changed, 22 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +0 -7

index f08859471841..f947951a01d4 100644
@@ -4473,13 +4473,6 @@ 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)
  {

@@ -4524,13 +4517,12 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
  	return I915_READ(SDEISR) & bit;
  }

drivers/gpu/drm/i915/intel_dp.c +20 -9

  		switch (port->port) {
  		case PORT_B:
  			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;

@@ -4565,6 +4557,22 @@ static bool g4x_digital_port_connected(struct drm_device *dev,
  	return I915_READ(PORT_HOTPLUG_STAT) & bit;
  }
 
+/*
+ * intel_digital_port_connected - is the specified port connected?
+ * @dev_priv: i915 private structure
+ * @port: the port to test
+ *
+ * Return %true if @port is connected, %false otherwise.
+ */
+static bool 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)
  {

@@ -4572,7 +4580,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))
  		return connector_status_disconnected;
 
  	return intel_dp_detect_dpcd(intel_dp);

@@ -4594,7 +4602,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
  		return status;
  	}
 
-	if (!g4x_digital_port_connected(dev, intel_dig_port))
+	if (!intel_digital_port_connected(dev->dev_private, intel_dig_port))
  		return connector_status_disconnected;
 
  	return intel_dp_detect_dpcd(intel_dp);

@@ -5057,13 +5065,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))
-				goto mst_fail;
-		}
+		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
+			goto mst_fail;
 
  		if (!intel_dp_get_dpcd(intel_dp)) {
  			goto mst_fail;

Quoted 39 lines · Jani Nikula

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

v2: make the function return a boolean

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/intel_dp.c | 41 ++++++++++++++++++++++-------------------
  1 file changed, 22 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +1 -9

index f08859471841..f947951a01d4 100644
@@ -4473,13 +4473,6 @@ 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)
  {

@@ -4524,13 +4517,12 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
  	return I915_READ(SDEISR) & bit;
  }
 
-static bool g4x_digital_port_connected(struct drm_device *dev,
+static bool g4x_digital_port_connected(struct drm_i915_private *dev_priv,
  				       struct intel_digital_port *port)
  {
-	struct drm_i915_private *dev_priv = dev->dev_private;
  	uint32_t bit;
 

drivers/gpu/drm/i915/intel_dp.c static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv, · line 4525

Quoted 2 lines

drivers/gpu/drm/i915/intel_dp.c +1 -1

-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_VALLEYVIEW(dev_priv)) {

this might not work :(. just noted this as part of another review. please fix this since
it is merged.

Quoted 60 lines

drivers/gpu/drm/i915/intel_dp.c +20 -9

  		switch (port->port) {
  		case PORT_B:
  			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;

@@ -4565,6 +4557,22 @@ static bool g4x_digital_port_connected(struct drm_device *dev,
  	return I915_READ(PORT_HOTPLUG_STAT) & bit;
  }
 
+/*
+ * intel_digital_port_connected - is the specified port connected?
+ * @dev_priv: i915 private structure
+ * @port: the port to test
+ *
+ * Return %true if @port is connected, %false otherwise.
+ */
+static bool 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)
  {

@@ -4572,7 +4580,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))
  		return connector_status_disconnected;
 
  	return intel_dp_detect_dpcd(intel_dp);

@@ -4594,7 +4602,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
  		return status;
  	}
 
-	if (!g4x_digital_port_connected(dev, intel_dig_port))
+	if (!intel_digital_port_connected(dev->dev_private, intel_dig_port))
  		return connector_status_disconnected;
 
  	return intel_dp_detect_dpcd(intel_dp);

@@ -5057,13 +5065,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))
-				goto mst_fail;
-		}
+		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
+			goto mst_fail;
 
  		if (!intel_dp_get_dpcd(intel_dp)) {
  			goto mst_fail;

Signature · 2 lines

regards,
Sivakumar

Quoted 40 lines · Sivakumar Thulasimani

Quoted 39 lines · Jani Nikula

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

v2: make the function return a boolean

Signed-off-by Jani Nikula <[email protected]>

---
   drivers/gpu/drm/i915/intel_dp.c | 41 ++++++++++++++++++++++-------------------
   1 file changed, 22 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +1 -9

index f08859471841..f947951a01d4 100644
@@ -4473,13 +4473,6 @@ 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)
   {

@@ -4524,13 +4517,12 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
   	return I915_READ(SDEISR) & bit;
   }
 
-static bool g4x_digital_port_connected(struct drm_device *dev,
+static bool g4x_digital_port_connected(struct drm_i915_private *dev_priv,
   				       struct intel_digital_port *port)
   {
-	struct drm_i915_private *dev_priv = dev->dev_private;
   	uint32_t bit;
 

Quoted 5 lines

drivers/gpu/drm/i915/intel_dp.c static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv, · line 4525

Quoted 2 lines

drivers/gpu/drm/i915/intel_dp.c +1 -1

-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_VALLEYVIEW(dev_priv)) {

this might not work :(. just noted this as part of another review. please fix this since
it is merged.

Sorry, what might not work?

Jani.

Quoted 65 lines

Quoted 61 lines

drivers/gpu/drm/i915/intel_dp.c +20 -9

   		switch (port->port) {
   		case PORT_B:
   			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;

@@ -4565,6 +4557,22 @@ static bool g4x_digital_port_connected(struct drm_device *dev,
   	return I915_READ(PORT_HOTPLUG_STAT) & bit;
   }
 
+/*
+ * intel_digital_port_connected - is the specified port connected?
+ * @dev_priv: i915 private structure
+ * @port: the port to test
+ *
+ * Return %true if @port is connected, %false otherwise.
+ */
+static bool 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)
   {

@@ -4572,7 +4580,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))
   		return connector_status_disconnected;
 
   	return intel_dp_detect_dpcd(intel_dp);

@@ -4594,7 +4602,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
   		return status;
   	}
 
-	if (!g4x_digital_port_connected(dev, intel_dig_port))
+	if (!intel_digital_port_connected(dev->dev_private, intel_dig_port))
   		return connector_status_disconnected;
 
   	return intel_dp_detect_dpcd(intel_dp);

@@ -5057,13 +5065,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))
-				goto mst_fail;
-		}
+		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
+			goto mst_fail;
 
   		if (!intel_dp_get_dpcd(intel_dp)) {
   			goto mst_fail;

Signature · 2 lines

regards,
Sivakumar

Quoted 49 lines · Jani Nikula

Quoted 40 lines · Sivakumar Thulasimani

Quoted 39 lines · Jani Nikula

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

v2: make the function return a boolean

Signed-off-by Jani Nikula <[email protected]>

---
   drivers/gpu/drm/i915/intel_dp.c | 41 ++++++++++++++++++++++-------------------
   1 file changed, 22 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +1 -9

index f08859471841..f947951a01d4 100644
@@ -4473,13 +4473,6 @@ 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)
   {

@@ -4524,13 +4517,12 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
   	return I915_READ(SDEISR) & bit;
   }
 
-static bool g4x_digital_port_connected(struct drm_device *dev,
+static bool g4x_digital_port_connected(struct drm_i915_private *dev_priv,
   				       struct intel_digital_port *port)
   {
-	struct drm_i915_private *dev_priv = dev->dev_private;
   	uint32_t bit;
 

Quoted 5 lines

drivers/gpu/drm/i915/intel_dp.c static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv, · line 4525

Quoted 2 lines

drivers/gpu/drm/i915/intel_dp.c +1 -1

-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_VALLEYVIEW(dev_priv)) {

this might not work :(. just noted this as part of another review. please fix this since
it is merged.

Sorry, what might not work?

Jani.

(quoted diff) +1 -1

-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_VALLEYVIEW(dev_priv)) { <----- dev_priv is used instead of dev

should be IS_VALLEYVIEW(dev_priv->dev)

Quoted 66 lines

Quoted 66 lines

Quoted 62 lines

drivers/gpu/drm/i915/intel_dp.c +20 -9

   		switch (port->port) {
   		case PORT_B:
   			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;

@@ -4565,6 +4557,22 @@ static bool g4x_digital_port_connected(struct drm_device *dev,
   	return I915_READ(PORT_HOTPLUG_STAT) & bit;
   }
 
+/*
+ * intel_digital_port_connected - is the specified port connected?
+ * @dev_priv: i915 private structure
+ * @port: the port to test
+ *
+ * Return %true if @port is connected, %false otherwise.
+ */
+static bool 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)
   {

@@ -4572,7 +4580,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))
   		return connector_status_disconnected;
 
   	return intel_dp_detect_dpcd(intel_dp);

@@ -4594,7 +4602,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
   		return status;
   	}
 
-	if (!g4x_digital_port_connected(dev, intel_dig_port))
+	if (!intel_digital_port_connected(dev->dev_private, intel_dig_port))
   		return connector_status_disconnected;
 
   	return intel_dp_detect_dpcd(intel_dp);

@@ -5057,13 +5065,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))
-				goto mst_fail;
-		}
+		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
+			goto mst_fail;
 
   		if (!intel_dp_get_dpcd(intel_dp)) {
   			goto mst_fail;

Signature · 2 lines

regards,
Sivakumar

Signature · 2 lines

regards,
Sivakumar

Quoted 52 lines · Sivakumar Thulasimani

Quoted 49 lines · Jani Nikula

Quoted 40 lines · Sivakumar Thulasimani

Quoted 39 lines · Jani Nikula

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

v2: make the function return a boolean

Signed-off-by Jani Nikula <[email protected]>

---
    drivers/gpu/drm/i915/intel_dp.c | 41 ++++++++++++++++++++++-------------------
    1 file changed, 22 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +1 -9

index f08859471841..f947951a01d4 100644
@@ -4473,13 +4473,6 @@ 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)
    {

@@ -4524,13 +4517,12 @@ static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
    	return I915_READ(SDEISR) & bit;
    }
 
-static bool g4x_digital_port_connected(struct drm_device *dev,
+static bool g4x_digital_port_connected(struct drm_i915_private *dev_priv,
    				       struct intel_digital_port *port)
    {
-	struct drm_i915_private *dev_priv = dev->dev_private;
    	uint32_t bit;
 

Quoted 5 lines

drivers/gpu/drm/i915/intel_dp.c static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv, · line 4525

Quoted 2 lines

drivers/gpu/drm/i915/intel_dp.c +1 -1

-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_VALLEYVIEW(dev_priv)) {

this might not work :(. just noted this as part of another review. please fix this since
it is merged.

Sorry, what might not work?

Jani.

(quoted diff) +1 -1

-	if (IS_VALLEYVIEW(dev)) {
+	if (IS_VALLEYVIEW(dev_priv)) { <----- dev_priv is used instead of dev

Quoted 1 line

should be IS_VALLEYVIEW(dev_priv->dev)

Ah, that may be confusing. We're transitioning towards preferring dev_priv, and instead of switching everything at once (which would not work) Chris put together some magic to have these macros accept either drm_i915_private or drm_device pointer. See __I915__ in i915_drv.h.

BR,
Jani.

Quoted 69 lines

Quoted 65 lines

Quoted 65 lines

Quoted 61 lines

drivers/gpu/drm/i915/intel_dp.c +20 -9

    		switch (port->port) {
    		case PORT_B:
    			bit = PORTB_HOTPLUG_LIVE_STATUS_VLV;

@@ -4565,6 +4557,22 @@ static bool g4x_digital_port_connected(struct drm_device *dev,
    	return I915_READ(PORT_HOTPLUG_STAT) & bit;
    }
 
+/*
+ * intel_digital_port_connected - is the specified port connected?
+ * @dev_priv: i915 private structure
+ * @port: the port to test
+ *
+ * Return %true if @port is connected, %false otherwise.
+ */
+static bool 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)
    {

@@ -4572,7 +4580,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))
    		return connector_status_disconnected;
 
    	return intel_dp_detect_dpcd(intel_dp);

@@ -4594,7 +4602,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
    		return status;
    	}
 
-	if (!g4x_digital_port_connected(dev, intel_dig_port))
+	if (!intel_digital_port_connected(dev->dev_private, intel_dig_port))
    		return connector_status_disconnected;
 
    	return intel_dp_detect_dpcd(intel_dp);

@@ -5057,13 +5065,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))
-				goto mst_fail;
-		}
+		if (!intel_digital_port_connected(dev_priv, intel_dig_port))
+			goto mst_fail;
 
    		if (!intel_dp_get_dpcd(intel_dp)) {
    			goto mst_fail;

Signature · 2 lines

regards,
Sivakumar

Signature · 2 lines

regards,
Sivakumar