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

[Goel, Akash]

On 6/28/2016 3:17 PM, Chris Wilson wrote:
> 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.

Yes, realized this today, that can’t get to the ‘dri’ directory until
the end of Driver load.
So will have to create the relay file after i915_driver_register().

>
>> +	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)

Yes can now use the dev->primary->debugfs_root itself.

Actually earlier 'i915_debugfs_files' were being created inside other
drm_minor directories also (i.e. dri/64 & dri/128), but now they are
restricted only to dri/0.

Best regards
Akash

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