diff mbox

drm/i915/dp: Debug log MST active links explicitly

Message ID 1476836328-7759-1-git-send-email-dhinakaran.pandiyan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dhinakaran Pandiyan Oct. 19, 2016, 12:18 a.m. UTC 
From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>

No functional change. Just printing the number of active links without
stating what the number means is not very useful. So, add relevant text.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Jim Bride <jim.bride@linux.intel.com>
---
No changes, sending with Jim's R-B

 drivers/gpu/drm/i915/intel_dp_mst.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Chris Wilson Oct. 19, 2016, 7:47 a.m. UTC | #1 
On Tue, Oct 18, 2016 at 05:18:48PM -0700, Dhinakaran Pandiyan wrote:
> From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> 
> No functional change. Just printing the number of active links without
> stating what the number means is not very useful. So, add relevant text.

If you are going to add information, consider also noting which
connector/encoder you are talking about. Even before we get concurrent
handling of multiple channels, this is useful!
-Chris
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Jim Bride <jim.bride@linux.intel.com>
> ---
> No changes, sending with Jim's R-B
> 
>  drivers/gpu/drm/i915/intel_dp_mst.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 3ffbd69..62a69b5 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -86,7 +86,7 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,
>  		to_intel_connector(old_conn_state->connector);
>  	int ret;
>  
> -	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
> +	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);
>  
>  	drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, connector->port);
>  
> @@ -106,7 +106,7 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
>  	struct intel_connector *connector =
>  		to_intel_connector(old_conn_state->connector);
>  
> -	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
> +	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);
>  
>  	/* this can fail */
>  	drm_dp_check_act_status(&intel_dp->mst_mgr);
> @@ -147,7 +147,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
>  	connector->encoder = encoder;
>  	intel_mst->connector = connector;
>  
> -	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
> +	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);
>  
>  	if (intel_dp->active_mst_links == 0) {
>  		intel_ddi_clk_select(&intel_dig_port->base,
> @@ -194,7 +194,7 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder,
>  	enum port port = intel_dig_port->port;
>  	int ret;
>  
> -	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
> +	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);
>  
>  	if (intel_wait_for_register(dev_priv,
>  				    DP_TP_STATUS(port),
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dhinakaran Pandiyan Oct. 20, 2016, 12:33 a.m. UTC | #2 
On Wed, 2016-10-19 at 08:47 +0100, Chris Wilson wrote:
> On Tue, Oct 18, 2016 at 05:18:48PM -0700, Dhinakaran Pandiyan wrote:

> > From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>

> > 

> > No functional change. Just printing the number of active links without

> > stating what the number means is not very useful. So, add relevant text.

> 

> If you are going to add information, consider also noting which

> connector/encoder you are talking about. Even before we get concurrent

> handling of multiple channels, this is useful!

> -Chris


Yeah, connector and encoder info must be included. But, now I think of
it, printing active links twice while enabling and twice while disabling
is excessive. The value we print in post_disable() is not intuitive too.

-DK 

> > 

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

> > Reviewed-by: Jim Bride <jim.bride@linux.intel.com>

> > ---

> > No changes, sending with Jim's R-B

> > 

> >  drivers/gpu/drm/i915/intel_dp_mst.c | 8 ++++----

> >  1 file changed, 4 insertions(+), 4 deletions(-)

> > 

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

> > index 3ffbd69..62a69b5 100644

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

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

> > @@ -86,7 +86,7 @@ static void intel_mst_disable_dp(struct intel_encoder *encoder,

> >  		to_intel_connector(old_conn_state->connector);

> >  	int ret;

> >  

> > -	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);

> > +	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);

> >  

> >  	drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, connector->port);

> >  

> > @@ -106,7 +106,7 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,

> >  	struct intel_connector *connector =

> >  		to_intel_connector(old_conn_state->connector);

> >  

> > -	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);

> > +	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);

> >  

> >  	/* this can fail */

> >  	drm_dp_check_act_status(&intel_dp->mst_mgr);

> > @@ -147,7 +147,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,

> >  	connector->encoder = encoder;

> >  	intel_mst->connector = connector;

> >  

> > -	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);

> > +	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);

> >  

> >  	if (intel_dp->active_mst_links == 0) {

> >  		intel_ddi_clk_select(&intel_dig_port->base,

> > @@ -194,7 +194,7 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder,

> >  	enum port port = intel_dig_port->port;

> >  	int ret;

> >  

> > -	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);

> > +	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);

> >  

> >  	if (intel_wait_for_register(dev_priv,

> >  				    DP_TP_STATUS(port),

> > -- 

> > 2.7.4

> > 

> > _______________________________________________

> > Intel-gfx mailing list

> > Intel-gfx@lists.freedesktop.org

> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 3ffbd69..62a69b5 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -86,7 +86,7 @@  static void intel_mst_disable_dp(struct intel_encoder *encoder,
 		to_intel_connector(old_conn_state->connector);
 	int ret;
 
-	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
+	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);
 
 	drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, connector->port);
 
@@ -106,7 +106,7 @@  static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
 	struct intel_connector *connector =
 		to_intel_connector(old_conn_state->connector);
 
-	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
+	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);
 
 	/* this can fail */
 	drm_dp_check_act_status(&intel_dp->mst_mgr);
@@ -147,7 +147,7 @@  static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
 	connector->encoder = encoder;
 	intel_mst->connector = connector;
 
-	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
+	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);
 
 	if (intel_dp->active_mst_links == 0) {
 		intel_ddi_clk_select(&intel_dig_port->base,
@@ -194,7 +194,7 @@  static void intel_mst_enable_dp(struct intel_encoder *encoder,
 	enum port port = intel_dig_port->port;
 	int ret;
 
-	DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links);
+	DRM_DEBUG_KMS("active MST links: %d\n", intel_dp->active_mst_links);
 
 	if (intel_wait_for_register(dev_priv,
 				    DP_TP_STATUS(port),