diff mbox

drm/i915: Use the connector name in fbdev debug messages

Message ID 1399987599-10950-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson May 13, 2014, 1:26 p.m. UTC
During initial probing of the modes to assign to the fbdev console, we
use the CRTC and connector ids. These are much harder for us to
understand than if we used their actual names (or pipe in the CRTC
case). Similarly, we want to manually print the mode size rather than
rely on mode->name being set.

v2: Use '%c' for pipe_name()

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_fbdev.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

Comments

Daniel Vetter May 13, 2014, 2:02 p.m. UTC | #1
On Tue, May 13, 2014 at 02:26:39PM +0100, Chris Wilson wrote:
> During initial probing of the modes to assign to the fbdev console, we
> use the CRTC and connector ids. These are much harder for us to
> understand than if we used their actual names (or pipe in the CRTC
> case). Similarly, we want to manually print the mode size rather than
> rely on mode->name being set.
> 
> v2: Use '%c' for pipe_name()
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index fbe7941f88c8..1d3f0a9cce31 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -344,14 +344,14 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
>  			num_connectors_detected++;
>  
>  		if (!enabled[i]) {
> -			DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
> -				      connector->base.id);
> +			DRM_DEBUG_KMS("connector %s not enabled, skipping\n",
> +				      drm_get_connector_name(connector));
>  			continue;
>  		}
>  
>  		if (connector->force == DRM_FORCE_OFF) {

I seem to be lacking the patch to obey the connector->force stuff you have
here. Instead I have an early bail-out at the top of fb_initial_config.

Do I miss some crucial patch or can you please rebase on top of dinq?

Thanks, Daniel

> -			DRM_DEBUG_KMS("connector %d is disabled by user, skipping\n",
> -				      connector->base.id);
> +			DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",
> +				      drm_get_connector_name(connector));
>  			enabled[i] = false;
>  			continue;
>  		}
> @@ -361,8 +361,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
>  			if (connector->force > DRM_FORCE_OFF)
>  				goto bail;
>  
> -			DRM_DEBUG_KMS("connector %d has no encoder or crtc, skipping\n",
> -				      connector->base.id);
> +			DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",
> +				      drm_get_connector_name(connector));
>  			enabled[i] = false;
>  			continue;
>  		}
> @@ -383,16 +383,16 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
>  			}
>  		}
>  
> -		DRM_DEBUG_KMS("looking for cmdline mode on connector %d\n",
> -			      fb_conn->connector->base.id);
> +		DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n",
> +			      drm_get_connector_name(connector));
>  
>  		/* go for command line mode first */
>  		modes[i] = drm_pick_cmdline_mode(fb_conn, width, height);
>  
>  		/* try for preferred next */
>  		if (!modes[i]) {
> -			DRM_DEBUG_KMS("looking for preferred mode on connector %d\n",
> -				      fb_conn->connector->base.id);
> +			DRM_DEBUG_KMS("looking for preferred mode on connector %s\n",
> +				      drm_get_connector_name(connector));
>  			modes[i] = drm_has_preferred_mode(fb_conn, width,
>  							  height);
>  		}
> @@ -410,16 +410,20 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
>  			 * since the fb helper layer wants a pointer to
>  			 * something we own.
>  			 */
> +			DRM_DEBUG_KMS("looking for current mode on connector %s\n",
> +				      drm_get_connector_name(connector));
>  			intel_mode_from_pipe_config(&encoder->crtc->hwmode,
>  						    &to_intel_crtc(encoder->crtc)->config);
>  			modes[i] = &encoder->crtc->hwmode;
>  		}
>  		crtcs[i] = new_crtc;
>  
> -		DRM_DEBUG_KMS("connector %s on crtc %d: %s\n",
> +		DRM_DEBUG_KMS("connector %s on pipe %c [CRTC:%d]: %dx%d%s\n",
>  			      drm_get_connector_name(connector),
> +			      pipe_name(to_intel_crtc(encoder->crtc)->pipe),
>  			      encoder->crtc->base.id,
> -			      modes[i]->name);
> +			      modes[i]->hdisplay, modes[i]->vdisplay,
> +			      modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" :"");
>  
>  		fallback = false;
>  	}
> -- 
> 2.0.0.rc2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Chris Wilson May 13, 2014, 2:11 p.m. UTC | #2
On Tue, May 13, 2014 at 04:02:40PM +0200, Daniel Vetter wrote:
> On Tue, May 13, 2014 at 02:26:39PM +0100, Chris Wilson wrote:
> > During initial probing of the modes to assign to the fbdev console, we
> > use the CRTC and connector ids. These are much harder for us to
> > understand than if we used their actual names (or pipe in the CRTC
> > case). Similarly, we want to manually print the mode size rather than
> > rely on mode->name being set.
> > 
> > v2: Use '%c' for pipe_name()
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_fbdev.c | 28 ++++++++++++++++------------
> >  1 file changed, 16 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > index fbe7941f88c8..1d3f0a9cce31 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -344,14 +344,14 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> >  			num_connectors_detected++;
> >  
> >  		if (!enabled[i]) {
> > -			DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
> > -				      connector->base.id);
> > +			DRM_DEBUG_KMS("connector %s not enabled, skipping\n",
> > +				      drm_get_connector_name(connector));
> >  			continue;
> >  		}
> >  
> >  		if (connector->force == DRM_FORCE_OFF) {
> 
> I seem to be lacking the patch to obey the connector->force stuff you have
> here. Instead I have an early bail-out at the top of fb_initial_config.
> 
> Do I miss some crucial patch or can you please rebase on top of dinq?

It's not crucial, just a patch you missed to fix up some of Jesse's
shortcomings.
-Chris
Daniel Vetter May 14, 2014, 10:07 a.m. UTC | #3
On Tue, May 13, 2014 at 03:11:45PM +0100, Chris Wilson wrote:
> On Tue, May 13, 2014 at 04:02:40PM +0200, Daniel Vetter wrote:
> > On Tue, May 13, 2014 at 02:26:39PM +0100, Chris Wilson wrote:
> > > During initial probing of the modes to assign to the fbdev console, we
> > > use the CRTC and connector ids. These are much harder for us to
> > > understand than if we used their actual names (or pipe in the CRTC
> > > case). Similarly, we want to manually print the mode size rather than
> > > rely on mode->name being set.
> > > 
> > > v2: Use '%c' for pipe_name()
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_fbdev.c | 28 ++++++++++++++++------------
> > >  1 file changed, 16 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index fbe7941f88c8..1d3f0a9cce31 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -344,14 +344,14 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> > >  			num_connectors_detected++;
> > >  
> > >  		if (!enabled[i]) {
> > > -			DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
> > > -				      connector->base.id);
> > > +			DRM_DEBUG_KMS("connector %s not enabled, skipping\n",
> > > +				      drm_get_connector_name(connector));
> > >  			continue;
> > >  		}
> > >  
> > >  		if (connector->force == DRM_FORCE_OFF) {
> > 
> > I seem to be lacking the patch to obey the connector->force stuff you have
> > here. Instead I have an early bail-out at the top of fb_initial_config.
> > 
> > Do I miss some crucial patch or can you please rebase on top of dinq?
> 
> It's not crucial, just a patch you missed to fix up some of Jesse's
> shortcomings.

Hm, looked at that patch again and I guess that needs to be rebased to
take out the logic from Jesse's patch. So merged this one for now and
resolved the conflict.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index fbe7941f88c8..1d3f0a9cce31 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -344,14 +344,14 @@  static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 			num_connectors_detected++;
 
 		if (!enabled[i]) {
-			DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
-				      connector->base.id);
+			DRM_DEBUG_KMS("connector %s not enabled, skipping\n",
+				      drm_get_connector_name(connector));
 			continue;
 		}
 
 		if (connector->force == DRM_FORCE_OFF) {
-			DRM_DEBUG_KMS("connector %d is disabled by user, skipping\n",
-				      connector->base.id);
+			DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",
+				      drm_get_connector_name(connector));
 			enabled[i] = false;
 			continue;
 		}
@@ -361,8 +361,8 @@  static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 			if (connector->force > DRM_FORCE_OFF)
 				goto bail;
 
-			DRM_DEBUG_KMS("connector %d has no encoder or crtc, skipping\n",
-				      connector->base.id);
+			DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",
+				      drm_get_connector_name(connector));
 			enabled[i] = false;
 			continue;
 		}
@@ -383,16 +383,16 @@  static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 			}
 		}
 
-		DRM_DEBUG_KMS("looking for cmdline mode on connector %d\n",
-			      fb_conn->connector->base.id);
+		DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n",
+			      drm_get_connector_name(connector));
 
 		/* go for command line mode first */
 		modes[i] = drm_pick_cmdline_mode(fb_conn, width, height);
 
 		/* try for preferred next */
 		if (!modes[i]) {
-			DRM_DEBUG_KMS("looking for preferred mode on connector %d\n",
-				      fb_conn->connector->base.id);
+			DRM_DEBUG_KMS("looking for preferred mode on connector %s\n",
+				      drm_get_connector_name(connector));
 			modes[i] = drm_has_preferred_mode(fb_conn, width,
 							  height);
 		}
@@ -410,16 +410,20 @@  static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 			 * since the fb helper layer wants a pointer to
 			 * something we own.
 			 */
+			DRM_DEBUG_KMS("looking for current mode on connector %s\n",
+				      drm_get_connector_name(connector));
 			intel_mode_from_pipe_config(&encoder->crtc->hwmode,
 						    &to_intel_crtc(encoder->crtc)->config);
 			modes[i] = &encoder->crtc->hwmode;
 		}
 		crtcs[i] = new_crtc;
 
-		DRM_DEBUG_KMS("connector %s on crtc %d: %s\n",
+		DRM_DEBUG_KMS("connector %s on pipe %c [CRTC:%d]: %dx%d%s\n",
 			      drm_get_connector_name(connector),
+			      pipe_name(to_intel_crtc(encoder->crtc)->pipe),
 			      encoder->crtc->base.id,
-			      modes[i]->name);
+			      modes[i]->hdisplay, modes[i]->vdisplay,
+			      modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" :"");
 
 		fallback = false;
 	}