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

[Mahesh Kumar]

Hi,


On Wednesday 09 August 2017 03:16 PM, Maarten Lankhorst wrote:
> 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;
>> +
>> +	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;
>> +
>> +	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;
>> +
>> +	intel_runtime_pm_get(dev_priv);
>> +	dev_priv->ipc_enabled = enable;
>> +	intel_enable_ipc(dev_priv);
>> +	intel_runtime_pm_put(dev_priv);
> Hm, intel_enable_ipc should take a bool on whether to enable ipc.
intel_enable_ipc takes decision to enable/disable ipc based on 
dev_priv->ipc_enabled value. which is anyway input to function.
>
> Forcefully enabling IPC here might give weird effects until the next plane update. The transition watermarks are not updated yet until the next commit. This could probably be handled here, but that would be overkill..
yes, agree, but I don't wanted to call a commit internally. Will add a 
drm_info when changing IPC disable => enable as suggested.
-Mahesh
>
> Perhaps add a drm_info or something about it when setting enable = true, when it was previously false?
>> +
>> +	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)
>