mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] drm/i915: Explicit the connector name for DP link training result
@ 2017-07-18 14:25 Paul Kocialkowski
  2017-07-18 18:55 ` [Intel-gfx] " Pandiyan, Dhinakaran
  2017-07-18 19:20 ` Manasi Navare
  0 siblings, 2 replies; 6+ messages in thread
From: Paul Kocialkowski @ 2017-07-18 14:25 UTC (permalink / raw)
  To: intel-gfx, dri-devel, linux-kernel
  Cc: Manasi Navare, Daniel Vetter, Jani Nikula, David Airlie,
	Paul Kocialkowski

This adds the connector name when printing a debug message about the DP
link training result. It is useful to figure out what connector is
failing when multiple DP connectors are used.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp_link_training.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
index b79c1c0e404c..05907fa8a553 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -321,12 +321,16 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 	if (!intel_dp_link_training_channel_equalization(intel_dp))
 		goto failure_handling;
 
-	DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
+	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training Passed at Link Rate = %d, Lane count = %d",
+		      intel_connector->base.base.id,
+		      intel_connector->base.name,
 		      intel_dp->link_rate, intel_dp->lane_count);
 	return;
 
  failure_handling:
-	DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d",
+	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d",
+		      intel_connector->base.base.id,
+		      intel_connector->base.name,
 		      intel_dp->link_rate, intel_dp->lane_count);
 	if (!intel_dp_get_link_train_fallback_values(intel_dp,
 						     intel_dp->link_rate,
-- 
2.13.2

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

* Re: [Intel-gfx] [PATCH v2] drm/i915: Explicit the connector name for DP link training result
  2017-07-18 14:25 [PATCH v2] drm/i915: Explicit the connector name for DP link training result Paul Kocialkowski
@ 2017-07-18 18:55 ` Pandiyan, Dhinakaran
  2017-07-18 19:20 ` Manasi Navare
  1 sibling, 0 replies; 6+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-07-18 18:55 UTC (permalink / raw)
  To: paul.kocialkowski
  Cc: dri-devel, intel-gfx, linux-kernel, Vetter, Daniel, airlied




On Tue, 2017-07-18 at 17:25 +0300, Paul Kocialkowski wrote:
> This adds the connector name when printing a debug message about the DP
> link training result. It is useful to figure out what connector is
> failing when multiple DP connectors are used.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp_link_training.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index b79c1c0e404c..05907fa8a553 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -321,12 +321,16 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
>  	if (!intel_dp_link_training_channel_equalization(intel_dp))
>  		goto failure_handling;
>  
> -	DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training Passed at Link Rate = %d, Lane count = %d",
> +		      intel_connector->base.base.id,
> +		      intel_connector->base.name,
>  		      intel_dp->link_rate, intel_dp->lane_count);
>  	return;
>  
>   failure_handling:
> -	DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d",
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d",
> +		      intel_connector->base.base.id,
> +		      intel_connector->base.name,
>  		      intel_dp->link_rate, intel_dp->lane_count);
>  	if (!intel_dp_get_link_train_fallback_values(intel_dp,
>  						     intel_dp->link_rate,


Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

The choice of upper v/s lower case letters in these debug messages is a
bit odd.

Passed/failed
Link rate/link rate
Lane count/lane count

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

* Re: [PATCH v2] drm/i915: Explicit the connector name for DP link training result
  2017-07-18 14:25 [PATCH v2] drm/i915: Explicit the connector name for DP link training result Paul Kocialkowski
  2017-07-18 18:55 ` [Intel-gfx] " Pandiyan, Dhinakaran
@ 2017-07-18 19:20 ` Manasi Navare
  2017-07-19  6:33   ` Daniel Vetter
  2017-07-19  8:33   ` Paul Kocialkowski
  1 sibling, 2 replies; 6+ messages in thread
From: Manasi Navare @ 2017-07-18 19:20 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: intel-gfx, dri-devel, linux-kernel, Daniel Vetter, Jani Nikula,
	David Airlie

On Tue, Jul 18, 2017 at 05:25:36PM +0300, Paul Kocialkowski wrote:
> This adds the connector name when printing a debug message about the DP
> link training result. It is useful to figure out what connector is
> failing when multiple DP connectors are used.
>

Thanks for the patch, this does make sense during the link training
failure debugging to know the connector name.
While at it feel free to change "Failed", "Link Rate, Lane Count", to upper case
in the failure_handling case to be consistent with the pass case.

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp_link_training.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index b79c1c0e404c..05907fa8a553 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -321,12 +321,16 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
>  	if (!intel_dp_link_training_channel_equalization(intel_dp))
>  		goto failure_handling;
>  
> -	DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training Passed at Link Rate = %d, Lane count = %d",
> +		      intel_connector->base.base.id,
> +		      intel_connector->base.name,
>  		      intel_dp->link_rate, intel_dp->lane_count);
>  	return;
>  
>   failure_handling:
> -	DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d",
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d",
> +		      intel_connector->base.base.id,
> +		      intel_connector->base.name,
>  		      intel_dp->link_rate, intel_dp->lane_count);
>  	if (!intel_dp_get_link_train_fallback_values(intel_dp,
>  						     intel_dp->link_rate,
> -- 
> 2.13.2
> 

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

* Re: [PATCH v2] drm/i915: Explicit the connector name for DP link training result
  2017-07-18 19:20 ` Manasi Navare
@ 2017-07-19  6:33   ` Daniel Vetter
  2017-07-19  8:33   ` Paul Kocialkowski
  1 sibling, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2017-07-19  6:33 UTC (permalink / raw)
  To: Manasi Navare
  Cc: Paul Kocialkowski, intel-gfx, linux-kernel, dri-devel, Daniel Vetter

On Tue, Jul 18, 2017 at 12:20:36PM -0700, Manasi Navare wrote:
> On Tue, Jul 18, 2017 at 05:25:36PM +0300, Paul Kocialkowski wrote:
> > This adds the connector name when printing a debug message about the DP
> > link training result. It is useful to figure out what connector is
> > failing when multiple DP connectors are used.
> >
> 
> Thanks for the patch, this does make sense during the link training
> failure debugging to know the connector name.
> While at it feel free to change "Failed", "Link Rate, Lane Count", to upper case
> in the failure_handling case to be consistent with the pass case.
> 
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Applied, thanks.
-Daniel

> 
> Manasi
> 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp_link_training.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > index b79c1c0e404c..05907fa8a553 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > @@ -321,12 +321,16 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
> >  	if (!intel_dp_link_training_channel_equalization(intel_dp))
> >  		goto failure_handling;
> >  
> > -	DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
> > +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training Passed at Link Rate = %d, Lane count = %d",
> > +		      intel_connector->base.base.id,
> > +		      intel_connector->base.name,
> >  		      intel_dp->link_rate, intel_dp->lane_count);
> >  	return;
> >  
> >   failure_handling:
> > -	DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d",
> > +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d",
> > +		      intel_connector->base.base.id,
> > +		      intel_connector->base.name,
> >  		      intel_dp->link_rate, intel_dp->lane_count);
> >  	if (!intel_dp_get_link_train_fallback_values(intel_dp,
> >  						     intel_dp->link_rate,
> > -- 
> > 2.13.2
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH v2] drm/i915: Explicit the connector name for DP link training result
  2017-07-18 19:20 ` Manasi Navare
  2017-07-19  6:33   ` Daniel Vetter
@ 2017-07-19  8:33   ` Paul Kocialkowski
  2017-07-19  8:49     ` Jani Nikula
  1 sibling, 1 reply; 6+ messages in thread
From: Paul Kocialkowski @ 2017-07-19  8:33 UTC (permalink / raw)
  To: Manasi Navare
  Cc: intel-gfx, dri-devel, linux-kernel, Daniel Vetter, Jani Nikula,
	David Airlie

On Tue, 2017-07-18 at 12:20 -0700, Manasi Navare wrote:
> On Tue, Jul 18, 2017 at 05:25:36PM +0300, Paul Kocialkowski wrote:
> > This adds the connector name when printing a debug message about the
> > DP
> > link training result. It is useful to figure out what connector is
> > failing when multiple DP connectors are used.
> > 
> 
> Thanks for the patch, this does make sense during the link training
> failure debugging to know the connector name.
> While at it feel free to change "Failed", "Link Rate, Lane Count", to
> upper case
> in the failure_handling case to be consistent with the pass case.

Thanks for the review!

Now that the patch was merged, do you feel like I should make a follow-
up patch to fix consistency in the upper case use or is it enough of a
detail that we can just forget about it?

> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> 
> Manasi
> 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp_link_training.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > index b79c1c0e404c..05907fa8a553 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > @@ -321,12 +321,16 @@ intel_dp_start_link_train(struct intel_dp
> > *intel_dp)
> >  	if (!intel_dp_link_training_channel_equalization(intel_dp))
> >  		goto failure_handling;
> >  
> > -	DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane
> > count = %d",
> > +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training Passed at
> > Link Rate = %d, Lane count = %d",
> > +		      intel_connector->base.base.id,
> > +		      intel_connector->base.name,
> >  		      intel_dp->link_rate, intel_dp->lane_count);
> >  	return;
> >  
> >   failure_handling:
> > -	DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane
> > count = %d",
> > +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at
> > link rate = %d, lane count = %d",
> > +		      intel_connector->base.base.id,
> > +		      intel_connector->base.name,
> >  		      intel_dp->link_rate, intel_dp->lane_count);
> >  	if (!intel_dp_get_link_train_fallback_values(intel_dp,
> >  						     intel_dp-
> > >link_rate,
> > -- 
> > 2.13.2
> > 
-- 
Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland

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

* Re: [PATCH v2] drm/i915: Explicit the connector name for DP link training result
  2017-07-19  8:33   ` Paul Kocialkowski
@ 2017-07-19  8:49     ` Jani Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2017-07-19  8:49 UTC (permalink / raw)
  To: Paul Kocialkowski, Manasi Navare
  Cc: intel-gfx, dri-devel, linux-kernel, Daniel Vetter, David Airlie

On Wed, 19 Jul 2017, Paul Kocialkowski <paul.kocialkowski@linux.intel.com> wrote:
> On Tue, 2017-07-18 at 12:20 -0700, Manasi Navare wrote:
>> On Tue, Jul 18, 2017 at 05:25:36PM +0300, Paul Kocialkowski wrote:
>> > This adds the connector name when printing a debug message about the
>> > DP
>> > link training result. It is useful to figure out what connector is
>> > failing when multiple DP connectors are used.
>> > 
>> 
>> Thanks for the patch, this does make sense during the link training
>> failure debugging to know the connector name.
>> While at it feel free to change "Failed", "Link Rate, Lane Count", to
>> upper case
>> in the failure_handling case to be consistent with the pass case.
>
> Thanks for the review!
>
> Now that the patch was merged, do you feel like I should make a follow-
> up patch to fix consistency in the upper case use or is it enough of a
> detail that we can just forget about it?

If it bothers you, send a patch, otherwise wait for it to bother someone
else enough to send a patch. ;)

But please avoid the overuse of all caps for regular words, or
capitalization except at the beginning of sentences. I'd go with
something like this all over the place:

"Link training failed, link rate %d, lane count %d\n"

But it hasn't bothered me enough to send a patch. ;)

BR,
Jani.



>
>> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
>> 
>> Manasi
>> 
>> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_dp_link_training.c | 8 ++++++--
>> >  1 file changed, 6 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
>> > b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> > index b79c1c0e404c..05907fa8a553 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> > @@ -321,12 +321,16 @@ intel_dp_start_link_train(struct intel_dp
>> > *intel_dp)
>> >  	if (!intel_dp_link_training_channel_equalization(intel_dp))
>> >  		goto failure_handling;
>> >  
>> > -	DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane
>> > count = %d",
>> > +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training Passed at
>> > Link Rate = %d, Lane count = %d",
>> > +		      intel_connector->base.base.id,
>> > +		      intel_connector->base.name,
>> >  		      intel_dp->link_rate, intel_dp->lane_count);
>> >  	return;
>> >  
>> >   failure_handling:
>> > -	DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane
>> > count = %d",
>> > +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at
>> > link rate = %d, lane count = %d",
>> > +		      intel_connector->base.base.id,
>> > +		      intel_connector->base.name,
>> >  		      intel_dp->link_rate, intel_dp->lane_count);
>> >  	if (!intel_dp_get_link_train_fallback_values(intel_dp,
>> >  						     intel_dp-
>> > >link_rate,
>> > -- 
>> > 2.13.2
>> > 

-- 
Jani Nikula, Intel Open Source Technology Center

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

end of thread, other threads:[~2017-07-19  8:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-18 14:25 [PATCH v2] drm/i915: Explicit the connector name for DP link training result Paul Kocialkowski
2017-07-18 18:55 ` [Intel-gfx] " Pandiyan, Dhinakaran
2017-07-18 19:20 ` Manasi Navare
2017-07-19  6:33   ` Daniel Vetter
2017-07-19  8:33   ` Paul Kocialkowski
2017-07-19  8:49     ` Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome