Re: [PATCH 14/15] drm/i915: Debugfs interface for GuC submission statistics

Chris Wilson
2015-06-16T09:28:30Z

On Mon, Jun 15, 2015 at 07:36:32PM +0100, Dave Gordon wrote:
> This provides a means of reading status and counts relating
> to GuC actions and submissions.

Anything that ends to ease debugging also tends to ease
postmortem error analysis...

>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> Signed-off-by: Alex Dai <yu.dai@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 41 +++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c6e2582..e699b38 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2388,6 +2388,46 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
> return 0;
> }
>
> +static int i915_guc_info(struct seq_file *m, void *data)
> +{
> + struct drm_info_node *node = m->private;
> + struct drm_device *dev = node->minor->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_guc guc;
> + struct i915_guc_client client = { .client_obj = 0 };
> +
> + if (!HAS_GUC_SCHED(dev_priv->dev))
> + return 0;
> +
> + /* Take a local copy of the GuC data, so we can dump it at leisure */
> + spin_lock(&dev_priv->guc.host2guc_lock);
> + guc = dev_priv->guc;
> + if (guc.execbuf_client) {
> + spin_lock(&guc.execbuf_client->wq_lock);
> + client = *guc.execbuf_client;
> + spin_unlock(&guc.execbuf_client->wq_lock);
> + }
> + spin_unlock(&dev_priv->guc.host2guc_lock);
> +
> + seq_printf(m, "GuC total action count: %llu\n", guc.action_count);
> + seq_printf(m, "GuC last action command: 0x%x\n", guc.action_cmd);
> + seq_printf(m, "GuC last action status: 0x%x\n", guc.action_status);
> +
> + seq_printf(m, "GuC action failure count: %u\n", guc.action_fail);
> + seq_printf(m, "GuC last action error code: %d\n", guc.action_err);

If these had been a struct you could have minimised that copy.
Again, it would have been best if the debug inteface had been added all
at once, so we could take the extra infrastructure or leave it out
altogether.

> + seq_printf(m, "\nGuC execbuf client @ %p:\n", guc.execbuf_client);
> + seq_printf(m, "\tTotal submissions: %llu\n", client.submissions);
> + seq_printf(m, "\tFailed to queue: %u\n", client.q_fail);
> + seq_printf(m, "\tFailed doorbell: %u\n", client.b_fail);
> + seq_printf(m, "\tLast submission result: %d\n", client.retcode);
> +
> + /* Add more as required ... */
> + seq_puts(m, "\n");

Trailing newline, why?
-Chris

--
Chris Wilson, Intel Open Source Technology Centre