[Intel-gfx] [PATCH 8/8] drm/i915/skl+: debugfs entry to control IPC

[Maarten Lankhorst]

Op 18-07-17 om 14:49 schreef Mahesh Kumar:
> From: "Kumar, Mahesh" <mahesh1.kumar at intel.com>
>
> This patch creates an entry in debugfs to check the status of IPC.
> This can also be used to enable/disable IPC in supported platforms.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 73 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2ef75c1a6119..368f64de0fdc 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3592,6 +3592,76 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
>  	return 0;
>  }
>  
> +static int i915_ipc_status_show(struct seq_file *m, void *data)
> +{
> +	struct drm_i915_private *dev_priv = m->private;
> +
> +	seq_printf(m, "Isochronous Priority Control: %s\n",
> +			enableddisabled(dev_priv->ipc_enabled));
> +	return 0;
> +}
> +
> +static int i915_ipc_status_open(struct inode *inode, struct file *file)
> +{
> +	struct drm_i915_private *dev_priv = inode->i_private;
> +
> +	if (HAS_IPC(dev_priv))
> +		return -ENODEV;
I take it you didn't test this version of the patch? :p
> +
> +	return single_open(file, i915_ipc_status_show, dev_priv);
> +}
> +
> +static ssize_t i915_ipc_status_write(struct file *file, const char __user *ubuf,
> +				     size_t len, loff_t *offp)
> +{
> +	struct seq_file *m = file->private_data;
> +	struct drm_i915_private *dev_priv = m->private;
> +	char *newline;
> +	char tmp[16];
> +	bool enable;
> +
> +	if (HAS_IPC(dev_priv))
> +		return -ENODEV;
Again, though I would remove this check since you already test in open().
> +	if (len >= sizeof(tmp))
> +		return -EINVAL;
> +
> +	if (copy_from_user(tmp, ubuf, len))
> +		return -EFAULT;
> +
> +	tmp[len] = '\0';
> +
> +	/* Strip newline, if any */
> +	newline = strchr(tmp, '\n');
> +	if (newline)
> +		*newline = '\0';
> +
> +	if (strcmp(tmp, "0") == 0 || strcmp(tmp, "disable") == 0 ||
> +	    strcmp(tmp, "off") == 0 || strcmp(tmp, "dis") == 0)
> +		enable = false;
> +	else if (strcmp(tmp, "1") == 0 || strcmp(tmp, "enable") == 0 ||
> +	    strcmp(tmp, "on") == 0 || strcmp(tmp, "en") == 0)
> +		enable = true;
> +	else
> +		return -EINVAL;
Maybe replace with kstrtobool_from_user, and use yesno for ipc_enabled in show()? That way you won't have to do all these special cases here. :)
> +
> +	intel_runtime_pm_get(dev_priv);
> +	dev_priv->ipc_enabled = enable;
> +	intel_enable_ipc(dev_priv);
> +	intel_runtime_pm_put(dev_priv);
> +
> +	return len;
> +}
> +
> +static const struct file_operations i915_ipc_status_fops = {
> +	.owner = THIS_MODULE,
> +	.open = i915_ipc_status_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +	.write = i915_ipc_status_write
> +};
> +
>  static int i915_ddb_info(struct seq_file *m, void *unused)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> @@ -4929,7 +4999,8 @@ static const struct i915_debugfs_files {
>  	{"i915_dp_test_type", &i915_displayport_test_type_fops},
>  	{"i915_dp_test_active", &i915_displayport_test_active_fops},
>  	{"i915_guc_log_control", &i915_guc_log_control_fops},
> -	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}
> +	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
> +	{"i915_ipc_status", &i915_ipc_status_fops}
>  };
>  
>  int i915_debugfs_register(struct drm_i915_private *dev_priv)