public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/i915: Add extra plane information in debugfs.
@ 2015-10-27 15:49 Robert Fekete
  2015-10-27 15:55 ` Robert Fekete
  2015-10-27 17:31 ` Chris Wilson
  0 siblings, 2 replies; 4+ messages in thread
From: Robert Fekete @ 2015-10-27 15:49 UTC (permalink / raw)
  To: intel-gfx

Extends i915_display_info so that for each active crtc also print
all planes associated with the pipe. This patch shows information
about each plane wrt format, size, position, rotation, and scaling.
This is very useful when debugging user space compositors that try
to utilize several planes for a commit.

V2: Fixed comments from Maarten, Ville, and Chris. Fixed printing of
16.16 fixpoint, better rotation bitmask management and some minor fixes

Signed-off-by: Robert Fekete <robert.fekete@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 109 +++++++++++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index eca94d0e4d99..9a2265e1139a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2950,6 +2950,107 @@ static bool cursor_position(struct drm_device *dev, int pipe, int *x, int *y)
 	return cursor_active(dev, pipe);
 }
 
+static const char *plane_type(enum drm_plane_type type)
+{
+	switch (type) {
+	case DRM_PLANE_TYPE_OVERLAY:
+		return "OVL";
+	case DRM_PLANE_TYPE_PRIMARY:
+		return "PRI";
+	case DRM_PLANE_TYPE_CURSOR:
+		return "CUR";
+	/*
+	 * Deliberately omitting default: to generate compiler warnings
+	 * when a new drm_plane_type gets added.
+	 */
+	}
+
+	return "unknown";
+}
+
+static const char *plane_rotation(unsigned int rotation)
+{
+	static char buf[48];
+	/*
+	 * According to doc only one DRM_ROTATE_ is allowed but this
+	 * will print them all to visualize if the values are misused
+	 */
+	snprintf(buf, sizeof(buf),
+		 "%s%s%s%s%s%s(0x%08x)",
+		 (rotation & BIT(DRM_ROTATE_0)) ? "0 " : "",
+		 (rotation & BIT(DRM_ROTATE_90)) ? "90 " : "",
+		 (rotation & BIT(DRM_ROTATE_180)) ? "180 " : "",
+		 (rotation & BIT(DRM_ROTATE_270)) ? "270 " : "",
+		 (rotation & BIT(DRM_REFLECT_X)) ? "FLIPX " : "",
+		 (rotation & BIT(DRM_REFLECT_Y)) ? "FLIPY " : "",
+		 rotation);
+
+	return buf;
+}
+
+static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct intel_plane *intel_plane;
+
+	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+		struct drm_plane_state *state;
+		struct drm_plane *plane = &intel_plane->base;
+
+		if (!plane->state) {
+			seq_puts(m, "plane->state is NULL!\n");
+			continue;
+		}
+
+		state = plane->state;
+
+		seq_printf(m, "\t--Plane id %d: type=%s, crtc_pos=%4dx%4d, crtc_size=%4dx%4d, src_pos=%d.%04ux%d.%04u, src_size=%d.%04ux%d.%04u, format=%s, rotation=%s\n",
+			   plane->base.id,
+			   plane_type(intel_plane->base.type),
+			   state->crtc_x, state->crtc_y,
+			   state->crtc_w, state->crtc_h,
+			   (state->src_x >> 16),
+			   ((state->src_x & 0x00ff) * 15625) >> 10,
+			   (state->src_y >> 16),
+			   ((state->src_y & 0x00ff) * 15625) >> 10,
+			   (state->src_w >> 16),
+			   ((state->src_w & 0x00ff) * 15625) >> 10,
+			   (state->src_h >> 16),
+			   ((state->src_h & 0x00ff) * 15625) >> 10,
+			   state->fb ? drm_get_format_name(state->fb->pixel_format) : "N/A",
+			   plane_rotation(state->rotation));
+	}
+}
+
+static void intel_scaler_info(struct seq_file *m, struct intel_crtc *intel_crtc)
+{
+	struct intel_crtc_state *pipe_config;
+	int num_scalers = intel_crtc->num_scalers;
+	int i;
+
+	pipe_config = to_intel_crtc_state(intel_crtc->base.state);
+
+	/* Not all platformas have a scaler */
+	if (num_scalers) {
+		seq_printf(m, "\tnum_scalers=%d, scaler_users=%x scaler_id=%d",
+			   num_scalers,
+			   pipe_config->scaler_state.scaler_users,
+			   pipe_config->scaler_state.scaler_id);
+
+		for (i = 0; i < SKL_NUM_SCALERS; i++) {
+			struct intel_scaler *sc =
+					&pipe_config->scaler_state.scalers[i];
+
+			seq_printf(m, ", scalers[%d]: use=%s, mode=%x",
+				   i, yesno(sc->in_use), sc->mode);
+		}
+		seq_puts(m, "\n");
+	} else {
+		seq_puts(m, "\tNo scalers available on this platform\n");
+	}
+}
+
 static int i915_display_info(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = m->private;
@@ -2969,10 +3070,12 @@ static int i915_display_info(struct seq_file *m, void *unused)
 
 		pipe_config = to_intel_crtc_state(crtc->base.state);
 
-		seq_printf(m, "CRTC %d: pipe: %c, active=%s (size=%dx%d)\n",
+		seq_printf(m, "CRTC %d: pipe: %c, active=%s, (size=%dx%d), dither=%s, bpp=%d\n",
 			   crtc->base.base.id, pipe_name(crtc->pipe),
 			   yesno(pipe_config->base.active),
-			   pipe_config->pipe_src_w, pipe_config->pipe_src_h);
+			   pipe_config->pipe_src_w, pipe_config->pipe_src_h,
+			   yesno(pipe_config->dither), pipe_config->pipe_bpp);
+
 		if (pipe_config->base.active) {
 			intel_crtc_info(m, crtc);
 
@@ -2982,6 +3085,8 @@ static int i915_display_info(struct seq_file *m, void *unused)
 				   x, y, crtc->base.cursor->state->crtc_w,
 				   crtc->base.cursor->state->crtc_h,
 				   crtc->cursor_addr, yesno(active));
+			intel_scaler_info(m, crtc);
+			intel_plane_info(m, crtc);
 		}
 
 		seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915: Add extra plane information in debugfs.
  2015-10-27 15:49 [PATCH v2] drm/i915: Add extra plane information in debugfs Robert Fekete
@ 2015-10-27 15:55 ` Robert Fekete
  2015-10-27 17:31 ` Chris Wilson
  1 sibling, 0 replies; 4+ messages in thread
From: Robert Fekete @ 2015-10-27 15:55 UTC (permalink / raw)
  To: intel-gfx

Do not review this one!

I just found that I forgot to and(&) state->src_x with 0xffff.

V3 coming in a sec or two.

BR
/Robert F

On tis, 2015-10-27 at 16:49 +0100, Robert Fekete wrote:
> Extends i915_display_info so that for each active crtc also print
> all planes associated with the pipe. This patch shows information
> about each plane wrt format, size, position, rotation, and scaling.
> This is very useful when debugging user space compositors that try
> to utilize several planes for a commit.
> 
> V2: Fixed comments from Maarten, Ville, and Chris. Fixed printing of
> 16.16 fixpoint, better rotation bitmask management and some minor fixes
> 
> Signed-off-by: Robert Fekete <robert.fekete@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 109 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index eca94d0e4d99..9a2265e1139a 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2950,6 +2950,107 @@ static bool cursor_position(struct drm_device *dev, int pipe, int *x, int *y)
>  	return cursor_active(dev, pipe);
>  }
>  
> +static const char *plane_type(enum drm_plane_type type)
> +{
> +	switch (type) {
> +	case DRM_PLANE_TYPE_OVERLAY:
> +		return "OVL";
> +	case DRM_PLANE_TYPE_PRIMARY:
> +		return "PRI";
> +	case DRM_PLANE_TYPE_CURSOR:
> +		return "CUR";
> +	/*
> +	 * Deliberately omitting default: to generate compiler warnings
> +	 * when a new drm_plane_type gets added.
> +	 */
> +	}
> +
> +	return "unknown";
> +}
> +
> +static const char *plane_rotation(unsigned int rotation)
> +{
> +	static char buf[48];
> +	/*
> +	 * According to doc only one DRM_ROTATE_ is allowed but this
> +	 * will print them all to visualize if the values are misused
> +	 */
> +	snprintf(buf, sizeof(buf),
> +		 "%s%s%s%s%s%s(0x%08x)",
> +		 (rotation & BIT(DRM_ROTATE_0)) ? "0 " : "",
> +		 (rotation & BIT(DRM_ROTATE_90)) ? "90 " : "",
> +		 (rotation & BIT(DRM_ROTATE_180)) ? "180 " : "",
> +		 (rotation & BIT(DRM_ROTATE_270)) ? "270 " : "",
> +		 (rotation & BIT(DRM_REFLECT_X)) ? "FLIPX " : "",
> +		 (rotation & BIT(DRM_REFLECT_Y)) ? "FLIPY " : "",
> +		 rotation);
> +
> +	return buf;
> +}
> +
> +static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
> +{
> +	struct drm_info_node *node = m->private;
> +	struct drm_device *dev = node->minor->dev;
> +	struct intel_plane *intel_plane;
> +
> +	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> +		struct drm_plane_state *state;
> +		struct drm_plane *plane = &intel_plane->base;
> +
> +		if (!plane->state) {
> +			seq_puts(m, "plane->state is NULL!\n");
> +			continue;
> +		}
> +
> +		state = plane->state;
> +
> +		seq_printf(m, "\t--Plane id %d: type=%s, crtc_pos=%4dx%4d, crtc_size=%4dx%4d, src_pos=%d.%04ux%d.%04u, src_size=%d.%04ux%d.%04u, format=%s, rotation=%s\n",
> +			   plane->base.id,
> +			   plane_type(intel_plane->base.type),
> +			   state->crtc_x, state->crtc_y,
> +			   state->crtc_w, state->crtc_h,
> +			   (state->src_x >> 16),
> +			   ((state->src_x & 0x00ff) * 15625) >> 10,
> +			   (state->src_y >> 16),
> +			   ((state->src_y & 0x00ff) * 15625) >> 10,
> +			   (state->src_w >> 16),
> +			   ((state->src_w & 0x00ff) * 15625) >> 10,
> +			   (state->src_h >> 16),
> +			   ((state->src_h & 0x00ff) * 15625) >> 10,
> +			   state->fb ? drm_get_format_name(state->fb->pixel_format) : "N/A",
> +			   plane_rotation(state->rotation));
> +	}
> +}
> +
> +static void intel_scaler_info(struct seq_file *m, struct intel_crtc *intel_crtc)
> +{
> +	struct intel_crtc_state *pipe_config;
> +	int num_scalers = intel_crtc->num_scalers;
> +	int i;
> +
> +	pipe_config = to_intel_crtc_state(intel_crtc->base.state);
> +
> +	/* Not all platformas have a scaler */
> +	if (num_scalers) {
> +		seq_printf(m, "\tnum_scalers=%d, scaler_users=%x scaler_id=%d",
> +			   num_scalers,
> +			   pipe_config->scaler_state.scaler_users,
> +			   pipe_config->scaler_state.scaler_id);
> +
> +		for (i = 0; i < SKL_NUM_SCALERS; i++) {
> +			struct intel_scaler *sc =
> +					&pipe_config->scaler_state.scalers[i];
> +
> +			seq_printf(m, ", scalers[%d]: use=%s, mode=%x",
> +				   i, yesno(sc->in_use), sc->mode);
> +		}
> +		seq_puts(m, "\n");
> +	} else {
> +		seq_puts(m, "\tNo scalers available on this platform\n");
> +	}
> +}
> +
>  static int i915_display_info(struct seq_file *m, void *unused)
>  {
>  	struct drm_info_node *node = m->private;
> @@ -2969,10 +3070,12 @@ static int i915_display_info(struct seq_file *m, void *unused)
>  
>  		pipe_config = to_intel_crtc_state(crtc->base.state);
>  
> -		seq_printf(m, "CRTC %d: pipe: %c, active=%s (size=%dx%d)\n",
> +		seq_printf(m, "CRTC %d: pipe: %c, active=%s, (size=%dx%d), dither=%s, bpp=%d\n",
>  			   crtc->base.base.id, pipe_name(crtc->pipe),
>  			   yesno(pipe_config->base.active),
> -			   pipe_config->pipe_src_w, pipe_config->pipe_src_h);
> +			   pipe_config->pipe_src_w, pipe_config->pipe_src_h,
> +			   yesno(pipe_config->dither), pipe_config->pipe_bpp);
> +
>  		if (pipe_config->base.active) {
>  			intel_crtc_info(m, crtc);
>  
> @@ -2982,6 +3085,8 @@ static int i915_display_info(struct seq_file *m, void *unused)
>  				   x, y, crtc->base.cursor->state->crtc_w,
>  				   crtc->base.cursor->state->crtc_h,
>  				   crtc->cursor_addr, yesno(active));
> +			intel_scaler_info(m, crtc);
> +			intel_plane_info(m, crtc);
>  		}
>  
>  		seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",

-- 
BR
/Robert Fekete
Intel Open Source Technology Center

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915: Add extra plane information in debugfs.
  2015-10-27 15:49 [PATCH v2] drm/i915: Add extra plane information in debugfs Robert Fekete
  2015-10-27 15:55 ` Robert Fekete
@ 2015-10-27 17:31 ` Chris Wilson
  2015-10-28  9:37   ` Robert Fekete
  1 sibling, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2015-10-27 17:31 UTC (permalink / raw)
  To: Robert Fekete; +Cc: intel-gfx

On Tue, Oct 27, 2015 at 04:49:49PM +0100, Robert Fekete wrote:
> +static const char *plane_rotation(unsigned int rotation)
> +{
> +	static char buf[48];
> +	/*
> +	 * According to doc only one DRM_ROTATE_ is allowed but this
> +	 * will print them all to visualize if the values are misused
> +	 */
> +	snprintf(buf, sizeof(buf),
> +		 "%s%s%s%s%s%s(0x%08x)",
> +		 (rotation & BIT(DRM_ROTATE_0)) ? "0 " : "",
> +		 (rotation & BIT(DRM_ROTATE_90)) ? "90 " : "",
> +		 (rotation & BIT(DRM_ROTATE_180)) ? "180 " : "",
> +		 (rotation & BIT(DRM_ROTATE_270)) ? "270 " : "",
> +		 (rotation & BIT(DRM_REFLECT_X)) ? "FLIPX " : "",
> +		 (rotation & BIT(DRM_REFLECT_Y)) ? "FLIPY " : "",
> +		 rotation);

I'd do it the other away around "%x (%s...%s)", the number is the one we
all know and love, and the human readable translation second.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915: Add extra plane information in debugfs.
  2015-10-27 17:31 ` Chris Wilson
@ 2015-10-28  9:37   ` Robert Fekete
  0 siblings, 0 replies; 4+ messages in thread
From: Robert Fekete @ 2015-10-28  9:37 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On tis, 2015-10-27 at 17:31 +0000, Chris Wilson wrote:
> On Tue, Oct 27, 2015 at 04:49:49PM +0100, Robert Fekete wrote:
> > +static const char *plane_rotation(unsigned int rotation)
> > +{
> > +	static char buf[48];
> > +	/*
> > +	 * According to doc only one DRM_ROTATE_ is allowed but this
> > +	 * will print them all to visualize if the values are misused
> > +	 */
> > +	snprintf(buf, sizeof(buf),
> > +		 "%s%s%s%s%s%s(0x%08x)",
> > +		 (rotation & BIT(DRM_ROTATE_0)) ? "0 " : "",
> > +		 (rotation & BIT(DRM_ROTATE_90)) ? "90 " : "",
> > +		 (rotation & BIT(DRM_ROTATE_180)) ? "180 " : "",
> > +		 (rotation & BIT(DRM_ROTATE_270)) ? "270 " : "",
> > +		 (rotation & BIT(DRM_REFLECT_X)) ? "FLIPX " : "",
> > +		 (rotation & BIT(DRM_REFLECT_Y)) ? "FLIPY " : "",
> > +		 rotation);
> 
> I'd do it the other away around "%x (%s...%s)", the number is the one we
> all know and love, and the human readable translation second.

I fully agree but this is printed right after drm_get_format_name() in
drm_crtc.c which in turn prints it like "%s%s..(%x)" so in order to make
it look consistent I went for this solution instead. Is it acceptable to
keep for consistency or do you still want me to change it?

BR
/R

> -Chris
> 

-- 
BR
/Robert Fekete
Intel Open Source Technology Center

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2015-10-28  9:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-27 15:49 [PATCH v2] drm/i915: Add extra plane information in debugfs Robert Fekete
2015-10-27 15:55 ` Robert Fekete
2015-10-27 17:31 ` Chris Wilson
2015-10-28  9:37   ` Robert Fekete

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