diff mbox series

[2/3] drm/i915/ehl: Make icp_digital_port_connected() use phy instead of port

Message ID 20191126210732.407496-2-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/3] drm/i915: Handle SDEISR according to PCH rather than platform | expand

Commit Message

Matt Roper Nov. 26, 2019, 9:07 p.m. UTC 
When looking at SDEISR to determine the connection status of combo
outputs, we should use the phy index rather than the port index.
Although they're usually the same thing, EHL's DDI-D (port D) is
attached to PHY-A and SDEISR doesn't even have bits for a "D" output.
It's also possible that future platforms may map DDIs (the internal
display engine programming units) to PHYs (the output handling on the IO
side) in ways where port!=phy, so let's look at the PHY index by
default.

Fixes: 719d24002602 ("drm/i915/ehl: Enable DDI-D")
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Souza, Jose Nov. 26, 2019, 9:34 p.m. UTC | #1 
On Tue, 2019-11-26 at 13:07 -0800, Matt Roper wrote:
> When looking at SDEISR to determine the connection status of combo
> outputs, we should use the phy index rather than the port index.
> Although they're usually the same thing, EHL's DDI-D (port D) is
> attached to PHY-A and SDEISR doesn't even have bits for a "D" output.
> It's also possible that future platforms may map DDIs (the internal
> display engine programming units) to PHYs (the output handling on the
> IO
> side) in ways where port!=phy, so let's look at the PHY index by
> default.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Fixes: 719d24002602 ("drm/i915/ehl: Enable DDI-D")
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index ddf5bad1b969..59c5fd7bf27d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5477,14 +5477,12 @@ static bool bxt_digital_port_connected(struct
> intel_encoder *encoder)
>  }
>  
>  static bool icl_combo_port_connected(struct drm_i915_private
> *dev_priv,
> -				     struct intel_digital_port
> *intel_dig_port)
> +				     enum phy phy)
>  {
> -	enum port port = intel_dig_port->base.port;
> -
> -	if (HAS_PCH_MCC(dev_priv) && port == PORT_C)
> +	if (HAS_PCH_MCC(dev_priv) && phy == PHY_C)
>  		return I915_READ(SDEISR) &
> SDE_TC_HOTPLUG_ICP(PORT_TC1);
>  
> -	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
> +	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(phy);
>  }
>  
>  static bool icp_digital_port_connected(struct intel_encoder
> *encoder)
> @@ -5494,7 +5492,7 @@ static bool icp_digital_port_connected(struct
> intel_encoder *encoder)
>  	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>  
>  	if (intel_phy_is_combo(dev_priv, phy))
> -		return icl_combo_port_connected(dev_priv, dig_port);
> +		return icl_combo_port_connected(dev_priv, phy);
>  	else if (intel_phy_is_tc(dev_priv, phy))
>  		return intel_tc_port_connected(dig_port);
>  	else
Lucas De Marchi Nov. 27, 2019, 5:59 p.m. UTC | #2 
On Tue, Nov 26, 2019 at 01:07:31PM -0800, Matt Roper wrote:
>When looking at SDEISR to determine the connection status of combo
>outputs, we should use the phy index rather than the port index.
>Although they're usually the same thing, EHL's DDI-D (port D) is
>attached to PHY-A and SDEISR doesn't even have bits for a "D" output.
>It's also possible that future platforms may map DDIs (the internal
>display engine programming units) to PHYs (the output handling on the IO
>side) in ways where port!=phy, so let's look at the PHY index by
>default.
>
>Fixes: 719d24002602 ("drm/i915/ehl: Enable DDI-D")
>Cc: José Roberto de Souza <jose.souza@intel.com>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dp.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>index ddf5bad1b969..59c5fd7bf27d 100644
>--- a/drivers/gpu/drm/i915/display/intel_dp.c
>+++ b/drivers/gpu/drm/i915/display/intel_dp.c
>@@ -5477,14 +5477,12 @@ static bool bxt_digital_port_connected(struct intel_encoder *encoder)
> }
>
> static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,

while at it I think it would make more sense to call this
icl_combo_phy_connected() and maybe even intel_combo_phy_connected()

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>-				     struct intel_digital_port *intel_dig_port)
>+				     enum phy phy)
> {
>-	enum port port = intel_dig_port->base.port;
>-
>-	if (HAS_PCH_MCC(dev_priv) && port == PORT_C)
>+	if (HAS_PCH_MCC(dev_priv) && phy == PHY_C)
> 		return I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(PORT_TC1);
>
>-	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
>+	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(phy);
> }
>
> static bool icp_digital_port_connected(struct intel_encoder *encoder)
>@@ -5494,7 +5492,7 @@ static bool icp_digital_port_connected(struct intel_encoder *encoder)
> 	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>
> 	if (intel_phy_is_combo(dev_priv, phy))
>-		return icl_combo_port_connected(dev_priv, dig_port);
>+		return icl_combo_port_connected(dev_priv, phy);
> 	else if (intel_phy_is_tc(dev_priv, phy))
> 		return intel_tc_port_connected(dig_port);
> 	else
>-- 
>2.23.0
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index ddf5bad1b969..59c5fd7bf27d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5477,14 +5477,12 @@  static bool bxt_digital_port_connected(struct intel_encoder *encoder)
 }
 
 static bool icl_combo_port_connected(struct drm_i915_private *dev_priv,
-				     struct intel_digital_port *intel_dig_port)
+				     enum phy phy)
 {
-	enum port port = intel_dig_port->base.port;
-
-	if (HAS_PCH_MCC(dev_priv) && port == PORT_C)
+	if (HAS_PCH_MCC(dev_priv) && phy == PHY_C)
 		return I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(PORT_TC1);
 
-	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
+	return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(phy);
 }
 
 static bool icp_digital_port_connected(struct intel_encoder *encoder)
@@ -5494,7 +5492,7 @@  static bool icp_digital_port_connected(struct intel_encoder *encoder)
 	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
 
 	if (intel_phy_is_combo(dev_priv, phy))
-		return icl_combo_port_connected(dev_priv, dig_port);
+		return icl_combo_port_connected(dev_priv, phy);
 	else if (intel_phy_is_tc(dev_priv, phy))
 		return intel_tc_port_connected(dig_port);
 	else