[Intel-gfx] [PATCH 06/11] drm/i915: Add a relay backed debugfs interface for capturing GuC logs

[Chris Wilson]

On Mon, Jun 27, 2016 at 05:46:53PM +0530, akash.goel at intel.com wrote:
> +static void guc_remove_log_relay_file(struct intel_guc *guc)
> +{
> +	relay_close(guc->log_relay_chan);
> +}
> +
> +static void guc_create_log_relay_file(struct intel_guc *guc)
> +{
> +	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +	struct drm_device *dev = dev_priv->dev;
> +	struct dentry *log_dir;
> +	struct rchan *guc_log_relay_chan;
> +	size_t n_subbufs, subbuf_size;
> +
> +	if (guc->log_relay_chan)
> +		return;
> +
> +	/* If /sys/kernel/debug/dri/0 location do not exist, then debugfs is
> +	 * not mounted and so can't create the relay file.
> +	 * The relay API seems to fit well with debugfs only.
> +	 */

Ah. dev->primary->debugfs_root does not exist until the end of driver
loading.

You need to add an intel_guc_register() to the i915_driver_register()
after we call drm_dev_rigster() (that then calls this function).

Similarly, this needs to be torn down in unregister.

> +	if (!dev->primary->debugfs_root) {
> +		/* logging will remain off */
> +		i915.guc_log_level = -1;
> +		return;
> +	}
> +
> +	/* For now create the log file in /sys/kernel/debug/dri dir. */
> +	log_dir = dev->primary->debugfs_root->d_parent;

In future, this will be something like /sys/kernel/gpu/i915/guc_log, so
I don't see a good argument for not being more canonical in the debugfs
placement and using dev->primary->debugfs_root (i.e. /.../dri/0)

At the very least, you need to explain why we don't use dri/0/
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre