[08/23] drm/i915: Unify the TypeC port notation in debug/error messages

Submitted by Imre Deak on June 4, 2019, 2:58 p.m.

Details

Message ID 20190604145826.16424-9-imre.deak@intel.com
State New
Headers show
Series "drm/i915: Fix TypeC port mode switching" ( rev: 2 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Imre Deak June 4, 2019, 2:58 p.m.
Unify the TypeC port notation in log messages, so that it matches the
spec. For instance the first ICL TypeC port will read as 'Port C/TC#1'.

Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_tc.c | 41 +++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_tc.c b/drivers/gpu/drm/i915/intel_tc.c
index a3057c44bec6..07488235b67a 100644
--- a/drivers/gpu/drm/i915/intel_tc.c
+++ b/drivers/gpu/drm/i915/intel_tc.c
@@ -6,6 +6,29 @@ 
 #include "i915_drv.h"
 #include "intel_tc.h"
 
+static enum port intel_tc_port_to_port(struct drm_i915_private *dev_priv,
+				       enum tc_port tc_port)
+{
+	return tc_port + PORT_C;
+}
+
+static const char *tc_port_name(struct drm_i915_private *dev_priv,
+				enum tc_port tc_port)
+{
+	static char port_names[I915_MAX_TC_PORTS][8];
+
+	if (WARN_ON(INTEL_GEN(dev_priv) < 11 ||
+	    (unsigned int)tc_port >= I915_MAX_TC_PORTS))
+		tc_port = PORT_TC1;
+
+	snprintf(&port_names[tc_port][0], sizeof(port_names[tc_port]),
+		 "%c/TC#%d",
+		 port_name(intel_tc_port_to_port(dev_priv, tc_port)),
+		 tc_port + 1);
+
+	return port_names[tc_port];
+}
+
 static const char *tc_port_mode_name(enum tc_port_mode mode)
 {
 	static const char * const names[] = {
@@ -85,7 +108,8 @@  static bool icl_tc_phy_connect(struct intel_digital_port *dig_port)
 
 	val = I915_READ(PORT_TX_DFLEXDPPMS);
 	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
-		DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port);
+		DRM_DEBUG_KMS("Port %s: PHY not ready\n",
+			      tc_port_name(dev_priv, tc_port));
 		WARN_ON(dig_port->tc_legacy_port);
 		return false;
 	}
@@ -106,7 +130,8 @@  static bool icl_tc_phy_connect(struct intel_digital_port *dig_port)
 	 */
 	if (dig_port->tc_mode == TC_PORT_DP_ALT &&
 	    !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) {
-		DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port);
+		DRM_DEBUG_KMS("Port %s: PHY sudden disconnect\n",
+			      tc_port_name(dev_priv, tc_port));
 		icl_tc_phy_disconnect(dig_port);
 		return false;
 	}
@@ -136,8 +161,8 @@  void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)
 		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
 	}
 
-	DRM_DEBUG_KMS("Port %c TC type %s disconnected\n",
-		      port_name(dig_port->base.port),
+	DRM_DEBUG_KMS("Port %s: mode %s disconnected\n",
+		      tc_port_name(dev_priv, tc_port),
 		      tc_port_mode_name(dig_port->tc_mode));
 
 	dig_port->tc_mode = TC_PORT_TBT_ALT;
@@ -162,7 +187,9 @@  static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
 		return;
 
 	if (old_mode != intel_dig_port->tc_mode)
-		DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port),
+		DRM_DEBUG_KMS("Port %s: port has mode %s\n",
+			      tc_port_name(dev_priv,
+					   intel_port_to_tc(dev_priv, port)),
 			      tc_port_mode_name(intel_dig_port->tc_mode));
 }
 
@@ -191,8 +218,8 @@  bool intel_tc_port_connected(struct intel_digital_port *dig_port)
 	 */
 	if (!dig_port->tc_legacy_port &&
 	    I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {
-		DRM_ERROR("VBT incorrectly claims port %c is not TypeC legacy\n",
-			  port_name(port));
+		DRM_ERROR("Port %s: VBT incorrectly claims port is not TypeC legacy\n",
+			  tc_port_name(dev_priv, tc_port));
 		dig_port->tc_legacy_port = true;
 	}
 	is_legacy = dig_port->tc_legacy_port;

Comments

On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> Unify the TypeC port notation in log messages, so that it matches the

> spec. For instance the first ICL TypeC port will read as 'Port

> C/TC#1'.

> 

> Cc: José Roberto de Souza <jose.souza@intel.com>

> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

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

> Signed-off-by: Imre Deak <imre.deak@intel.com>

> ---

>  drivers/gpu/drm/i915/intel_tc.c | 41 +++++++++++++++++++++++++++--

> ----

>  1 file changed, 34 insertions(+), 7 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/intel_tc.c

> b/drivers/gpu/drm/i915/intel_tc.c

> index a3057c44bec6..07488235b67a 100644

> --- a/drivers/gpu/drm/i915/intel_tc.c

> +++ b/drivers/gpu/drm/i915/intel_tc.c

> @@ -6,6 +6,29 @@

>  #include "i915_drv.h"

>  #include "intel_tc.h"

>  

> +static enum port intel_tc_port_to_port(struct drm_i915_private

> *dev_priv,

> +				       enum tc_port tc_port)

> +{

> +	return tc_port + PORT_C;

> +}

> +

> +static const char *tc_port_name(struct drm_i915_private *dev_priv,

> +				enum tc_port tc_port)

> +{

> +	static char port_names[I915_MAX_TC_PORTS][8];

> +

> +	if (WARN_ON(INTEL_GEN(dev_priv) < 11 ||

> +	    (unsigned int)tc_port >= I915_MAX_TC_PORTS))

> +		tc_port = PORT_TC1;


Why no WARN_ON on the tc_port >= I915_MAX_TC_PORTS?

> +

> +	snprintf(&port_names[tc_port][0], sizeof(port_names[tc_port]),

> +		 "%c/TC#%d",

> +		 port_name(intel_tc_port_to_port(dev_priv, tc_port)),

> +		 tc_port + 1);


Maybe do it only once for each port?

if (port_names[tc_port][0])
	return port_names[tc_port];

snprintf(&port_names[tc_port], sizeof....

Other the above:

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


> +

> +	return port_names[tc_port];

> +}

> +

>  static const char *tc_port_mode_name(enum tc_port_mode mode)

>  {

>  	static const char * const names[] = {

> @@ -85,7 +108,8 @@ static bool icl_tc_phy_connect(struct

> intel_digital_port *dig_port)

>  

>  	val = I915_READ(PORT_TX_DFLEXDPPMS);

>  	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {

> -		DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n",

> tc_port);

> +		DRM_DEBUG_KMS("Port %s: PHY not ready\n",

> +			      tc_port_name(dev_priv, tc_port));

>  		WARN_ON(dig_port->tc_legacy_port);

>  		return false;

>  	}

> @@ -106,7 +130,8 @@ static bool icl_tc_phy_connect(struct

> intel_digital_port *dig_port)

>  	 */

>  	if (dig_port->tc_mode == TC_PORT_DP_ALT &&

>  	    !(I915_READ(PORT_TX_DFLEXDPSP) &

> TC_LIVE_STATE_TC(tc_port))) {

> -		DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n",

> tc_port);

> +		DRM_DEBUG_KMS("Port %s: PHY sudden disconnect\n",

> +			      tc_port_name(dev_priv, tc_port));

>  		icl_tc_phy_disconnect(dig_port);

>  		return false;

>  	}

> @@ -136,8 +161,8 @@ void icl_tc_phy_disconnect(struct

> intel_digital_port *dig_port)

>  		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);

>  	}

>  

> -	DRM_DEBUG_KMS("Port %c TC type %s disconnected\n",

> -		      port_name(dig_port->base.port),

> +	DRM_DEBUG_KMS("Port %s: mode %s disconnected\n",

> +		      tc_port_name(dev_priv, tc_port),

>  		      tc_port_mode_name(dig_port->tc_mode));

>  

>  	dig_port->tc_mode = TC_PORT_TBT_ALT;

> @@ -162,7 +187,9 @@ static void icl_update_tc_port_type(struct

> drm_i915_private *dev_priv,

>  		return;

>  

>  	if (old_mode != intel_dig_port->tc_mode)

> -		DRM_DEBUG_KMS("Port %c has TC type %s\n",

> port_name(port),

> +		DRM_DEBUG_KMS("Port %s: port has mode %s\n",

> +			      tc_port_name(dev_priv,

> +					   intel_port_to_tc(dev_priv,

> port)),

>  			      tc_port_mode_name(intel_dig_port-

> >tc_mode));

>  }

>  

> @@ -191,8 +218,8 @@ bool intel_tc_port_connected(struct

> intel_digital_port *dig_port)

>  	 */

>  	if (!dig_port->tc_legacy_port &&

>  	    I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {

> -		DRM_ERROR("VBT incorrectly claims port %c is not TypeC

> legacy\n",

> -			  port_name(port));

> +		DRM_ERROR("Port %s: VBT incorrectly claims port is not

> TypeC legacy\n",

> +			  tc_port_name(dev_priv, tc_port));

>  		dig_port->tc_legacy_port = true;

>  	}

>  	is_legacy = dig_port->tc_legacy_port;
On Fri, Jun 07, 2019 at 11:21:51PM +0300, Souza, Jose wrote:
> On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> > Unify the TypeC port notation in log messages, so that it matches the
> > spec. For instance the first ICL TypeC port will read as 'Port
> > C/TC#1'.
> > 
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_tc.c | 41 +++++++++++++++++++++++++++--
> > ----
> >  1 file changed, 34 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_tc.c
> > b/drivers/gpu/drm/i915/intel_tc.c
> > index a3057c44bec6..07488235b67a 100644
> > --- a/drivers/gpu/drm/i915/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/intel_tc.c
> > @@ -6,6 +6,29 @@
> >  #include "i915_drv.h"
> >  #include "intel_tc.h"
> >  
> > +static enum port intel_tc_port_to_port(struct drm_i915_private
> > *dev_priv,
> > +				       enum tc_port tc_port)
> > +{
> > +	return tc_port + PORT_C;
> > +}
> > +
> > +static const char *tc_port_name(struct drm_i915_private *dev_priv,
> > +				enum tc_port tc_port)
> > +{
> > +	static char port_names[I915_MAX_TC_PORTS][8];
> > +
> > +	if (WARN_ON(INTEL_GEN(dev_priv) < 11 ||
> > +	    (unsigned int)tc_port >= I915_MAX_TC_PORTS))
> > +		tc_port = PORT_TC1;
> 
> Why no WARN_ON on the tc_port >= I915_MAX_TC_PORTS?

Hm, do you mean a seaparate WARN_ON()?

> 
> > +
> > +	snprintf(&port_names[tc_port][0], sizeof(port_names[tc_port]),
> > +		 "%c/TC#%d",
> > +		 port_name(intel_tc_port_to_port(dev_priv, tc_port)),
> > +		 tc_port + 1);
> 
> Maybe do it only once for each port?
> 
> if (port_names[tc_port][0])
> 	return port_names[tc_port];

I thought why not keep it as simple as possible (not really performance
critical), but your version makes it clearer to the reader what the
logic is (static array), so can change it.

> 
> snprintf(&port_names[tc_port], sizeof....
> 
> Other the above:
> 
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> > +
> > +	return port_names[tc_port];
> > +}
> > +
> >  static const char *tc_port_mode_name(enum tc_port_mode mode)
> >  {
> >  	static const char * const names[] = {
> > @@ -85,7 +108,8 @@ static bool icl_tc_phy_connect(struct
> > intel_digital_port *dig_port)
> >  
> >  	val = I915_READ(PORT_TX_DFLEXDPPMS);
> >  	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
> > -		DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n",
> > tc_port);
> > +		DRM_DEBUG_KMS("Port %s: PHY not ready\n",
> > +			      tc_port_name(dev_priv, tc_port));
> >  		WARN_ON(dig_port->tc_legacy_port);
> >  		return false;
> >  	}
> > @@ -106,7 +130,8 @@ static bool icl_tc_phy_connect(struct
> > intel_digital_port *dig_port)
> >  	 */
> >  	if (dig_port->tc_mode == TC_PORT_DP_ALT &&
> >  	    !(I915_READ(PORT_TX_DFLEXDPSP) &
> > TC_LIVE_STATE_TC(tc_port))) {
> > -		DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n",
> > tc_port);
> > +		DRM_DEBUG_KMS("Port %s: PHY sudden disconnect\n",
> > +			      tc_port_name(dev_priv, tc_port));
> >  		icl_tc_phy_disconnect(dig_port);
> >  		return false;
> >  	}
> > @@ -136,8 +161,8 @@ void icl_tc_phy_disconnect(struct
> > intel_digital_port *dig_port)
> >  		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
> >  	}
> >  
> > -	DRM_DEBUG_KMS("Port %c TC type %s disconnected\n",
> > -		      port_name(dig_port->base.port),
> > +	DRM_DEBUG_KMS("Port %s: mode %s disconnected\n",
> > +		      tc_port_name(dev_priv, tc_port),
> >  		      tc_port_mode_name(dig_port->tc_mode));
> >  
> >  	dig_port->tc_mode = TC_PORT_TBT_ALT;
> > @@ -162,7 +187,9 @@ static void icl_update_tc_port_type(struct
> > drm_i915_private *dev_priv,
> >  		return;
> >  
> >  	if (old_mode != intel_dig_port->tc_mode)
> > -		DRM_DEBUG_KMS("Port %c has TC type %s\n",
> > port_name(port),
> > +		DRM_DEBUG_KMS("Port %s: port has mode %s\n",
> > +			      tc_port_name(dev_priv,
> > +					   intel_port_to_tc(dev_priv,
> > port)),
> >  			      tc_port_mode_name(intel_dig_port-
> > >tc_mode));
> >  }
> >  
> > @@ -191,8 +218,8 @@ bool intel_tc_port_connected(struct
> > intel_digital_port *dig_port)
> >  	 */
> >  	if (!dig_port->tc_legacy_port &&
> >  	    I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {
> > -		DRM_ERROR("VBT incorrectly claims port %c is not TypeC
> > legacy\n",
> > -			  port_name(port));
> > +		DRM_ERROR("Port %s: VBT incorrectly claims port is not
> > TypeC legacy\n",
> > +			  tc_port_name(dev_priv, tc_port));
> >  		dig_port->tc_legacy_port = true;
> >  	}
> >  	is_legacy = dig_port->tc_legacy_port;
On Fri, 2019-06-07 at 23:42 +0300, Imre Deak wrote:
> On Fri, Jun 07, 2019 at 11:21:51PM +0300, Souza, Jose wrote:

> > On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:

> > > Unify the TypeC port notation in log messages, so that it matches

> > > the

> > > spec. For instance the first ICL TypeC port will read as 'Port

> > > C/TC#1'.

> > > 

> > > Cc: José Roberto de Souza <jose.souza@intel.com>

> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

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

> > > Signed-off-by: Imre Deak <imre.deak@intel.com>

> > > ---

> > >  drivers/gpu/drm/i915/intel_tc.c | 41

> > > +++++++++++++++++++++++++++--

> > > ----

> > >  1 file changed, 34 insertions(+), 7 deletions(-)

> > > 

> > > diff --git a/drivers/gpu/drm/i915/intel_tc.c

> > > b/drivers/gpu/drm/i915/intel_tc.c

> > > index a3057c44bec6..07488235b67a 100644

> > > --- a/drivers/gpu/drm/i915/intel_tc.c

> > > +++ b/drivers/gpu/drm/i915/intel_tc.c

> > > @@ -6,6 +6,29 @@

> > >  #include "i915_drv.h"

> > >  #include "intel_tc.h"

> > >  

> > > +static enum port intel_tc_port_to_port(struct drm_i915_private

> > > *dev_priv,

> > > +				       enum tc_port tc_port)

> > > +{

> > > +	return tc_port + PORT_C;

> > > +}

> > > +

> > > +static const char *tc_port_name(struct drm_i915_private

> > > *dev_priv,

> > > +				enum tc_port tc_port)

> > > +{

> > > +	static char port_names[I915_MAX_TC_PORTS][8];

> > > +

> > > +	if (WARN_ON(INTEL_GEN(dev_priv) < 11 ||

> > > +	    (unsigned int)tc_port >= I915_MAX_TC_PORTS))

> > > +		tc_port = PORT_TC1;

> > 

> > Why no WARN_ON on the tc_port >= I915_MAX_TC_PORTS?

> 

> Hm, do you mean a seaparate WARN_ON()?


Oh I misread the parentheses, it is right.

> 

> > > +

> > > +	snprintf(&port_names[tc_port][0], sizeof(port_names[tc_port]),

> > > +		 "%c/TC#%d",

> > > +		 port_name(intel_tc_port_to_port(dev_priv, tc_port)),

> > > +		 tc_port + 1);

> > 

> > Maybe do it only once for each port?

> > 

> > if (port_names[tc_port][0])

> > 	return port_names[tc_port];

> 

> I thought why not keep it as simple as possible (not really

> performance

> critical), but your version makes it clearer to the reader what the

> logic is (static array), so can change it.

> 

> > snprintf(&port_names[tc_port], sizeof....

> > 

> > Other the above:

> > 

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

> > 

> > > +

> > > +	return port_names[tc_port];

> > > +}

> > > +

> > >  static const char *tc_port_mode_name(enum tc_port_mode mode)

> > >  {

> > >  	static const char * const names[] = {

> > > @@ -85,7 +108,8 @@ static bool icl_tc_phy_connect(struct

> > > intel_digital_port *dig_port)

> > >  

> > >  	val = I915_READ(PORT_TX_DFLEXDPPMS);

> > >  	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {

> > > -		DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n",

> > > tc_port);

> > > +		DRM_DEBUG_KMS("Port %s: PHY not ready\n",

> > > +			      tc_port_name(dev_priv, tc_port));

> > >  		WARN_ON(dig_port->tc_legacy_port);

> > >  		return false;

> > >  	}

> > > @@ -106,7 +130,8 @@ static bool icl_tc_phy_connect(struct

> > > intel_digital_port *dig_port)

> > >  	 */

> > >  	if (dig_port->tc_mode == TC_PORT_DP_ALT &&

> > >  	    !(I915_READ(PORT_TX_DFLEXDPSP) &

> > > TC_LIVE_STATE_TC(tc_port))) {

> > > -		DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n",

> > > tc_port);

> > > +		DRM_DEBUG_KMS("Port %s: PHY sudden disconnect\n",

> > > +			      tc_port_name(dev_priv, tc_port));

> > >  		icl_tc_phy_disconnect(dig_port);

> > >  		return false;

> > >  	}

> > > @@ -136,8 +161,8 @@ void icl_tc_phy_disconnect(struct

> > > intel_digital_port *dig_port)

> > >  		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);

> > >  	}

> > >  

> > > -	DRM_DEBUG_KMS("Port %c TC type %s disconnected\n",

> > > -		      port_name(dig_port->base.port),

> > > +	DRM_DEBUG_KMS("Port %s: mode %s disconnected\n",

> > > +		      tc_port_name(dev_priv, tc_port),

> > >  		      tc_port_mode_name(dig_port->tc_mode));

> > >  

> > >  	dig_port->tc_mode = TC_PORT_TBT_ALT;

> > > @@ -162,7 +187,9 @@ static void icl_update_tc_port_type(struct

> > > drm_i915_private *dev_priv,

> > >  		return;

> > >  

> > >  	if (old_mode != intel_dig_port->tc_mode)

> > > -		DRM_DEBUG_KMS("Port %c has TC type %s\n",

> > > port_name(port),

> > > +		DRM_DEBUG_KMS("Port %s: port has mode %s\n",

> > > +			      tc_port_name(dev_priv,

> > > +					   intel_port_to_tc(dev_priv,

> > > port)),

> > >  			      tc_port_mode_name(intel_dig_port-

> > > > tc_mode));

> > >  }

> > >  

> > > @@ -191,8 +218,8 @@ bool intel_tc_port_connected(struct

> > > intel_digital_port *dig_port)

> > >  	 */

> > >  	if (!dig_port->tc_legacy_port &&

> > >  	    I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {

> > > -		DRM_ERROR("VBT incorrectly claims port %c is not TypeC

> > > legacy\n",

> > > -			  port_name(port));

> > > +		DRM_ERROR("Port %s: VBT incorrectly claims port is not

> > > TypeC legacy\n",

> > > +			  tc_port_name(dev_priv, tc_port));

> > >  		dig_port->tc_legacy_port = true;

> > >  	}

> > >  	is_legacy = dig_port->tc_legacy_port;
On Fri, Jun 07, 2019 at 11:42:31PM +0300, Imre Deak wrote:
> On Fri, Jun 07, 2019 at 11:21:51PM +0300, Souza, Jose wrote:
> > On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> > > Unify the TypeC port notation in log messages, so that it matches the
> > > spec. For instance the first ICL TypeC port will read as 'Port
> > > C/TC#1'.
> > > 
> > > Cc: José Roberto de Souza <jose.souza@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_tc.c | 41 +++++++++++++++++++++++++++--
> > > ----
> > >  1 file changed, 34 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_tc.c
> > > b/drivers/gpu/drm/i915/intel_tc.c
> > > index a3057c44bec6..07488235b67a 100644
> > > --- a/drivers/gpu/drm/i915/intel_tc.c
> > > +++ b/drivers/gpu/drm/i915/intel_tc.c
> > > @@ -6,6 +6,29 @@
> > >  #include "i915_drv.h"
> > >  #include "intel_tc.h"
> > >  
> > > +static enum port intel_tc_port_to_port(struct drm_i915_private
> > > *dev_priv,
> > > +				       enum tc_port tc_port)
> > > +{
> > > +	return tc_port + PORT_C;
> > > +}
> > > +
> > > +static const char *tc_port_name(struct drm_i915_private *dev_priv,
> > > +				enum tc_port tc_port)
> > > +{
> > > +	static char port_names[I915_MAX_TC_PORTS][8];
> > > +
> > > +	if (WARN_ON(INTEL_GEN(dev_priv) < 11 ||
> > > +	    (unsigned int)tc_port >= I915_MAX_TC_PORTS))
> > > +		tc_port = PORT_TC1;
> > 
> > Why no WARN_ON on the tc_port >= I915_MAX_TC_PORTS?
> 
> Hm, do you mean a seaparate WARN_ON()?

Well, at least it's misindented, I can fix that.

> 
> > 
> > > +
> > > +	snprintf(&port_names[tc_port][0], sizeof(port_names[tc_port]),
> > > +		 "%c/TC#%d",
> > > +		 port_name(intel_tc_port_to_port(dev_priv, tc_port)),
> > > +		 tc_port + 1);
> > 
> > Maybe do it only once for each port?
> > 
> > if (port_names[tc_port][0])
> > 	return port_names[tc_port];
> 
> I thought why not keep it as simple as possible (not really performance
> critical), but your version makes it clearer to the reader what the
> logic is (static array), so can change it.
> 
> > 
> > snprintf(&port_names[tc_port], sizeof....
> > 
> > Other the above:
> > 
> > Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> > 
> > > +
> > > +	return port_names[tc_port];
> > > +}
> > > +
> > >  static const char *tc_port_mode_name(enum tc_port_mode mode)
> > >  {
> > >  	static const char * const names[] = {
> > > @@ -85,7 +108,8 @@ static bool icl_tc_phy_connect(struct
> > > intel_digital_port *dig_port)
> > >  
> > >  	val = I915_READ(PORT_TX_DFLEXDPPMS);
> > >  	if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
> > > -		DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n",
> > > tc_port);
> > > +		DRM_DEBUG_KMS("Port %s: PHY not ready\n",
> > > +			      tc_port_name(dev_priv, tc_port));
> > >  		WARN_ON(dig_port->tc_legacy_port);
> > >  		return false;
> > >  	}
> > > @@ -106,7 +130,8 @@ static bool icl_tc_phy_connect(struct
> > > intel_digital_port *dig_port)
> > >  	 */
> > >  	if (dig_port->tc_mode == TC_PORT_DP_ALT &&
> > >  	    !(I915_READ(PORT_TX_DFLEXDPSP) &
> > > TC_LIVE_STATE_TC(tc_port))) {
> > > -		DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n",
> > > tc_port);
> > > +		DRM_DEBUG_KMS("Port %s: PHY sudden disconnect\n",
> > > +			      tc_port_name(dev_priv, tc_port));
> > >  		icl_tc_phy_disconnect(dig_port);
> > >  		return false;
> > >  	}
> > > @@ -136,8 +161,8 @@ void icl_tc_phy_disconnect(struct
> > > intel_digital_port *dig_port)
> > >  		I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
> > >  	}
> > >  
> > > -	DRM_DEBUG_KMS("Port %c TC type %s disconnected\n",
> > > -		      port_name(dig_port->base.port),
> > > +	DRM_DEBUG_KMS("Port %s: mode %s disconnected\n",
> > > +		      tc_port_name(dev_priv, tc_port),
> > >  		      tc_port_mode_name(dig_port->tc_mode));
> > >  
> > >  	dig_port->tc_mode = TC_PORT_TBT_ALT;
> > > @@ -162,7 +187,9 @@ static void icl_update_tc_port_type(struct
> > > drm_i915_private *dev_priv,
> > >  		return;
> > >  
> > >  	if (old_mode != intel_dig_port->tc_mode)
> > > -		DRM_DEBUG_KMS("Port %c has TC type %s\n",
> > > port_name(port),
> > > +		DRM_DEBUG_KMS("Port %s: port has mode %s\n",
> > > +			      tc_port_name(dev_priv,
> > > +					   intel_port_to_tc(dev_priv,
> > > port)),
> > >  			      tc_port_mode_name(intel_dig_port-
> > > >tc_mode));
> > >  }
> > >  
> > > @@ -191,8 +218,8 @@ bool intel_tc_port_connected(struct
> > > intel_digital_port *dig_port)
> > >  	 */
> > >  	if (!dig_port->tc_legacy_port &&
> > >  	    I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {
> > > -		DRM_ERROR("VBT incorrectly claims port %c is not TypeC
> > > legacy\n",
> > > -			  port_name(port));
> > > +		DRM_ERROR("Port %s: VBT incorrectly claims port is not
> > > TypeC legacy\n",
> > > +			  tc_port_name(dev_priv, tc_port));
> > >  		dig_port->tc_legacy_port = true;
> > >  	}
> > >  	is_legacy = dig_port->tc_legacy_port;
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx