[v12,12/17] drm/i915/guc/slpc: Add enable/disable controls for SLPC tasks

Commit Message

[e-mail] March 30, 2018, 8:31 a.m. UTC

From: Tom O'Rourke <Tom.O'[e-mail]>

Adds debugfs hooks for enabling/disabling each SLPC task.

The enable/disable debugfs files are:
i915_guc_slpc_gtperf, i915_guc_slpc_balancer, and i915_guc_slpc_dcc.

Each of these can take the values: "default", "enabled", or "disabled"

v1: update for SLPC v2015.2.4
    dfps and turbo merged and renamed "gtperf"
    ibc split out and renamed "balancer"
    Avoid magic numbers (Jon Bloomfield)

v2-v3: Rebase.

v5: Moved slpc_enable_disable_set and slpc_enable_disable_get to
    intel_slpc.c. s/slpc_enable_disable_get/intel_slpc_task_status
    and s/slpc_enable_disable_set/intel_slpc_task_control. Prepared
    separate functions to update the task status only in the SLPC
    shared memory. Passing dev_priv as parameter.

v6: Rebase. s/slpc_param_show|write/slpc_task_param_show|write.
    Moved functions to intel_slpc.c. RPM Get/Put added before setting
    parameters and sending RESET event explicitly. (Sagar)

v7: Rebase.

Signed-off-by: Tom O'Rourke <Tom.O'[e-mail]>
Signed-off-by: Sagar Arun Kamble <[e-mail]>
Cc: Chris Wilson <[e-mail]>
Cc: Joonas Lahtinen <[e-mail]>
Cc: Radoslaw Szwichtenberg <[e-mail]>
Cc: Michal Wajdeczko <[e-mail]>
Cc: Sujaritha Sundaresan <[e-mail]>
Cc: Jeff McGee <[e-mail]>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 200 ++++++++++++++++++++++++++++++++++++
 1 file changed, 200 insertions(+)

Comments

Michal Wajdeczko May 14, 2018, 11:52 a.m. UTC | #1

On Fri, 30 Mar 2018 10:31:57 +0200, Sagar Arun Kamble  
<[e-mail]> wrote:

> From: Tom O'Rourke <Tom.O'[e-mail]>
>
> Adds debugfs hooks for enabling/disabling each SLPC task.
>
> The enable/disable debugfs files are:
> i915_guc_slpc_gtperf, i915_guc_slpc_balancer, and i915_guc_slpc_dcc.
>
> Each of these can take the values: "default", "enabled", or "disabled"
>
> v1: update for SLPC v2015.2.4
>     dfps and turbo merged and renamed "gtperf"
>     ibc split out and renamed "balancer"
>     Avoid magic numbers (Jon Bloomfield)
>
> v2-v3: Rebase.
>
> v5: Moved slpc_enable_disable_set and slpc_enable_disable_get to
>     intel_slpc.c. s/slpc_enable_disable_get/intel_slpc_task_status
>     and s/slpc_enable_disable_set/intel_slpc_task_control. Prepared
>     separate functions to update the task status only in the SLPC
>     shared memory. Passing dev_priv as parameter.
>
> v6: Rebase. s/slpc_param_show|write/slpc_task_param_show|write.
>     Moved functions to intel_slpc.c. RPM Get/Put added before setting
>     parameters and sending RESET event explicitly. (Sagar)
>
> v7: Rebase.
>
> Signed-off-by: Tom O'Rourke <Tom.O'[e-mail]>
> Signed-off-by: Sagar Arun Kamble <[e-mail]>
> Cc: Chris Wilson <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Radoslaw Szwichtenberg <[e-mail]>
> Cc: Michal Wajdeczko <[e-mail]>
> Cc: Sujaritha Sundaresan <[e-mail]>
> Cc: Jeff McGee <[e-mail]>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 200  
> ++++++++++++++++++++++++++++++++++++
>  1 file changed, 200 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c  
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index ff90577..d646a04 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2580,6 +2580,203 @@ static const struct file_operations  
> i915_guc_log_relay_fops = {
>  	.release = i915_guc_log_relay_release,
>  };
> +static const char *slpc_task_status_stringify(int state)
> +{
> +	const char *str = NULL;
> +
> +	switch (state) {
> +	case SLPC_PARAM_TASK_DEFAULT:
> +		str = "default\n";

no \n please

> +		break;
> +
> +	case SLPC_PARAM_TASK_ENABLED:
> +		str = "enabled\n";
> +		break;
> +
> +	case SLPC_PARAM_TASK_DISABLED:
> +		str = "disabled\n";
> +		break;
> +
> +	default:
> +		str = "unknown\n";
> +		break;
> +	}
> +

btw, stringify_* functions can be as simple as:

switch(state)
{
case A:
	return "A";
case B:
	return "B";
default:
	return "<invalid>";
}

> +	return str;
> +}
> +
> +static int slpc_task_status_show(struct seq_file *m,
> +				 u32 enable_id,
> +				 u32 disable_id)
> +{
> +	struct drm_i915_private *dev_priv = m->private;
> +	struct intel_guc_slpc *slpc = &dev_priv->guc.slpc;
> +	const char *status = NULL;
> +	u64 val;
> +	int ret;
> +
> +	mutex_lock(&slpc->lock);
> +	ret = intel_guc_slpc_task_status(slpc, &val, enable_id, disable_id);

s/intel_guc_slpc_task_status/intel_guc_slpc_get_task_status

> +	mutex_unlock(&slpc->lock);
> +
> +	if (!ret)
> +		status = slpc_task_status_stringify(val);

hmm, I'm not sure if status "null" is something that user want to see
if we hit non zero 'ret'

> +
> +	seq_printf(m, "%s", status);

hmm, as val is returned by guc fw, what if its value will not
match predefined 0..3 ? showing just "unknown" without actual
bad value may be insufficient

> +
> +	return 0;
> +}
> +
> +static int slpc_task_status_write(struct seq_file *m,
> +				  const char __user *ubuf,
> +				  size_t len,
> +				  u32 enable_id,
> +				  u32 disable_id)
> +{
> +	struct drm_i915_private *dev_priv = m->private;
> +	struct intel_guc_slpc *slpc = &dev_priv->guc.slpc;
> +	int ret = 0;
> +	char status[10];
> +	u64 val;
> +
> +	if (len >= sizeof(status))
> +		ret = -EINVAL;
> +	else if (copy_from_user(status, ubuf, len))
> +		ret = -EFAULT;
> +	else
> +		status[len] = '\0';
> +
> +	if (ret)
> +		return ret;
> +
> +	if (!strncmp(status, "default", 7))
> +		val = SLPC_PARAM_TASK_DEFAULT;
> +	else if (!strncmp(status, "enabled", 7))
> +		val = SLPC_PARAM_TASK_ENABLED;
> +	else if (!strncmp(status, "disabled", 8))
> +		val = SLPC_PARAM_TASK_DISABLED;
> +	else
> +		return -EINVAL;
> +
> +	mutex_lock(&slpc->lock);
> +	ret = intel_guc_slpc_task_control(slpc, val, enable_id, disable_id);
> +	mutex_unlock(&slpc->lock);
> +
> +	return ret;
> +}
> +
> +static int slpc_gtperf_show(struct seq_file *m, void *data)
> +{
> +	return slpc_task_status_show(m,
> +				     SLPC_PARAM_TASK_ENABLE_GTPERF,
> +				     SLPC_PARAM_TASK_DISABLE_GTPERF);
> +}
> +
> +static int i915_guc_slpc_gtperf_open(struct inode *inode, struct file  
> *file)
> +{
> +	struct drm_i915_private *dev_priv = inode->i_private;
> +
> +	return single_open(file, slpc_gtperf_show, dev_priv);
> +}
> +
> +static ssize_t i915_guc_slpc_gtperf_write(struct file *file,
> +					  const char __user *ubuf,
> +					  size_t len,
> +					  loff_t *offp)
> +{
> +	struct seq_file *m = file->private_data;
> +	int ret = 0;
> +
> +	ret = slpc_task_status_write(m, ubuf, len,
> +				     SLPC_PARAM_TASK_ENABLE_GTPERF,
> +				     SLPC_PARAM_TASK_DISABLE_GTPERF);
> +
> +	return ret ?: len;
> +}
> +
> +const struct file_operations i915_guc_slpc_gtperf_fops = {
> +	.owner	 = THIS_MODULE,
> +	.open	 = i915_guc_slpc_gtperf_open,
> +	.release = single_release,
> +	.read	 = seq_read,
> +	.write	 = i915_guc_slpc_gtperf_write,
> +	.llseek	 = seq_lseek
> +};
> +
> +static int slpc_balancer_show(struct seq_file *m, void *data)
> +{
> +	return slpc_task_status_show(m,
> +				     SLPC_PARAM_TASK_ENABLE_BALANCER,
> +				     SLPC_PARAM_TASK_DISABLE_BALANCER);
> +}
> +
> +static int i915_guc_slpc_balancer_open(struct inode *inode, struct file  
> *file)
> +{
> +	struct drm_i915_private *dev_priv = inode->i_private;
> +
> +	return single_open(file, slpc_balancer_show, dev_priv);
> +}
> +
> +static ssize_t i915_guc_slpc_balancer_write(struct file *file,
> +					    const char __user *ubuf,
> +					    size_t len,
> +					    loff_t *offp)
> +{
> +	struct seq_file *m = file->private_data;
> +	int ret = 0;
> +
> +	ret = slpc_task_status_write(m, ubuf, len,
> +				     SLPC_PARAM_TASK_ENABLE_BALANCER,
> +				     SLPC_PARAM_TASK_DISABLE_BALANCER);
> +	return ret ?: len;
> +}
> +
> +const struct file_operations i915_guc_slpc_balancer_fops = {
> +	.owner	 = THIS_MODULE,
> +	.open	 = i915_guc_slpc_balancer_open,
> +	.release = single_release,
> +	.read	 = seq_read,
> +	.write	 = i915_guc_slpc_balancer_write,
> +	.llseek	 = seq_lseek
> +};
> +
> +static int slpc_dcc_show(struct seq_file *m, void *data)
> +{
> +	return slpc_task_status_show(m,
> +				     SLPC_PARAM_TASK_ENABLE_DCC,
> +				     SLPC_PARAM_TASK_DISABLE_DCC);
> +}
> +
> +static int i915_guc_slpc_dcc_open(struct inode *inode, struct file  
> *file)
> +{
> +	struct drm_i915_private *dev_priv = inode->i_private;
> +
> +	return single_open(file, slpc_dcc_show, dev_priv);
> +}
> +
> +static ssize_t i915_guc_slpc_dcc_write(struct file *file,
> +				       const char __user *ubuf,
> +				       size_t len,
> +				       loff_t *offp)
> +{
> +	struct seq_file *m = file->private_data;
> +	int ret = 0;
> +
> +	ret = slpc_task_status_write(m, ubuf, len,
> +				     SLPC_PARAM_TASK_ENABLE_DCC,
> +				     SLPC_PARAM_TASK_DISABLE_DCC);
> +	return ret ?: len;
> +}
> +
> +const struct file_operations i915_guc_slpc_dcc_fops = {
> +	.owner	 = THIS_MODULE,
> +	.open	 = i915_guc_slpc_dcc_open,
> +	.release = single_release,
> +	.read	 = seq_read,
> +	.write	 = i915_guc_slpc_dcc_write,
> +	.llseek	 = seq_lseek
> +};
> +

it would be good if you move all above code to intel_guc_slpc.c
to keep all slpc related stuff in one file

/michal

>  static const char *psr2_live_status(u32 val)
>  {
>  	static const char * const live_status[] = {
> @@ -4810,6 +5007,9 @@ static const struct i915_debugfs_files {
>  	{"i915_dp_test_active", &i915_displayport_test_active_fops},
>  	{"i915_guc_log_level", &i915_guc_log_level_fops},
>  	{"i915_guc_log_relay", &i915_guc_log_relay_fops},
> +	{"i915_guc_slpc_gtperf", &i915_guc_slpc_gtperf_fops},
> +	{"i915_guc_slpc_balancer", &i915_guc_slpc_balancer_fops},
> +	{"i915_guc_slpc_dcc", &i915_guc_slpc_dcc_fops},
>  	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
>  	{"i915_ipc_status", &i915_ipc_status_fops},
>  	{"i915_drrs_ctl", &i915_drrs_ctl_fops}

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ff90577..d646a04 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2580,6 +2580,203 @@  static const struct file_operations i915_guc_log_relay_fops = {
 	.release = i915_guc_log_relay_release,
 };
 
+static const char *slpc_task_status_stringify(int state)
+{
+	const char *str = NULL;
+
+	switch (state) {
+	case SLPC_PARAM_TASK_DEFAULT:
+		str = "default\n";
+		break;
+
+	case SLPC_PARAM_TASK_ENABLED:
+		str = "enabled\n";
+		break;
+
+	case SLPC_PARAM_TASK_DISABLED:
+		str = "disabled\n";
+		break;
+
+	default:
+		str = "unknown\n";
+		break;
+	}
+
+	return str;
+}
+
+static int slpc_task_status_show(struct seq_file *m,
+				 u32 enable_id,
+				 u32 disable_id)
+{
+	struct drm_i915_private *dev_priv = m->private;
+	struct intel_guc_slpc *slpc = &dev_priv->guc.slpc;
+	const char *status = NULL;
+	u64 val;
+	int ret;
+
+	mutex_lock(&slpc->lock);
+	ret = intel_guc_slpc_task_status(slpc, &val, enable_id, disable_id);
+	mutex_unlock(&slpc->lock);
+
+	if (!ret)
+		status = slpc_task_status_stringify(val);
+
+	seq_printf(m, "%s", status);
+
+	return 0;
+}
+
+static int slpc_task_status_write(struct seq_file *m,
+				  const char __user *ubuf,
+				  size_t len,
+				  u32 enable_id,
+				  u32 disable_id)
+{
+	struct drm_i915_private *dev_priv = m->private;
+	struct intel_guc_slpc *slpc = &dev_priv->guc.slpc;
+	int ret = 0;
+	char status[10];
+	u64 val;
+
+	if (len >= sizeof(status))
+		ret = -EINVAL;
+	else if (copy_from_user(status, ubuf, len))
+		ret = -EFAULT;
+	else
+		status[len] = '\0';
+
+	if (ret)
+		return ret;
+
+	if (!strncmp(status, "default", 7))
+		val = SLPC_PARAM_TASK_DEFAULT;
+	else if (!strncmp(status, "enabled", 7))
+		val = SLPC_PARAM_TASK_ENABLED;
+	else if (!strncmp(status, "disabled", 8))
+		val = SLPC_PARAM_TASK_DISABLED;
+	else
+		return -EINVAL;
+
+	mutex_lock(&slpc->lock);
+	ret = intel_guc_slpc_task_control(slpc, val, enable_id, disable_id);
+	mutex_unlock(&slpc->lock);
+
+	return ret;
+}
+
+static int slpc_gtperf_show(struct seq_file *m, void *data)
+{
+	return slpc_task_status_show(m,
+				     SLPC_PARAM_TASK_ENABLE_GTPERF,
+				     SLPC_PARAM_TASK_DISABLE_GTPERF);
+}
+
+static int i915_guc_slpc_gtperf_open(struct inode *inode, struct file *file)
+{
+	struct drm_i915_private *dev_priv = inode->i_private;
+
+	return single_open(file, slpc_gtperf_show, dev_priv);
+}
+
+static ssize_t i915_guc_slpc_gtperf_write(struct file *file,
+					  const char __user *ubuf,
+					  size_t len,
+					  loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	int ret = 0;
+
+	ret = slpc_task_status_write(m, ubuf, len,
+				     SLPC_PARAM_TASK_ENABLE_GTPERF,
+				     SLPC_PARAM_TASK_DISABLE_GTPERF);
+
+	return ret ?: len;
+}
+
+const struct file_operations i915_guc_slpc_gtperf_fops = {
+	.owner	 = THIS_MODULE,
+	.open	 = i915_guc_slpc_gtperf_open,
+	.release = single_release,
+	.read	 = seq_read,
+	.write	 = i915_guc_slpc_gtperf_write,
+	.llseek	 = seq_lseek
+};
+
+static int slpc_balancer_show(struct seq_file *m, void *data)
+{
+	return slpc_task_status_show(m,
+				     SLPC_PARAM_TASK_ENABLE_BALANCER,
+				     SLPC_PARAM_TASK_DISABLE_BALANCER);
+}
+
+static int i915_guc_slpc_balancer_open(struct inode *inode, struct file *file)
+{
+	struct drm_i915_private *dev_priv = inode->i_private;
+
+	return single_open(file, slpc_balancer_show, dev_priv);
+}
+
+static ssize_t i915_guc_slpc_balancer_write(struct file *file,
+					    const char __user *ubuf,
+					    size_t len,
+					    loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	int ret = 0;
+
+	ret = slpc_task_status_write(m, ubuf, len,
+				     SLPC_PARAM_TASK_ENABLE_BALANCER,
+				     SLPC_PARAM_TASK_DISABLE_BALANCER);
+	return ret ?: len;
+}
+
+const struct file_operations i915_guc_slpc_balancer_fops = {
+	.owner	 = THIS_MODULE,
+	.open	 = i915_guc_slpc_balancer_open,
+	.release = single_release,
+	.read	 = seq_read,
+	.write	 = i915_guc_slpc_balancer_write,
+	.llseek	 = seq_lseek
+};
+
+static int slpc_dcc_show(struct seq_file *m, void *data)
+{
+	return slpc_task_status_show(m,
+				     SLPC_PARAM_TASK_ENABLE_DCC,
+				     SLPC_PARAM_TASK_DISABLE_DCC);
+}
+
+static int i915_guc_slpc_dcc_open(struct inode *inode, struct file *file)
+{
+	struct drm_i915_private *dev_priv = inode->i_private;
+
+	return single_open(file, slpc_dcc_show, dev_priv);
+}
+
+static ssize_t i915_guc_slpc_dcc_write(struct file *file,
+				       const char __user *ubuf,
+				       size_t len,
+				       loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	int ret = 0;
+
+	ret = slpc_task_status_write(m, ubuf, len,
+				     SLPC_PARAM_TASK_ENABLE_DCC,
+				     SLPC_PARAM_TASK_DISABLE_DCC);
+	return ret ?: len;
+}
+
+const struct file_operations i915_guc_slpc_dcc_fops = {
+	.owner	 = THIS_MODULE,
+	.open	 = i915_guc_slpc_dcc_open,
+	.release = single_release,
+	.read	 = seq_read,
+	.write	 = i915_guc_slpc_dcc_write,
+	.llseek	 = seq_lseek
+};
+
 static const char *psr2_live_status(u32 val)
 {
 	static const char * const live_status[] = {
@@ -4810,6 +5007,9 @@  static const struct i915_debugfs_files {
 	{"i915_dp_test_active", &i915_displayport_test_active_fops},
 	{"i915_guc_log_level", &i915_guc_log_level_fops},
 	{"i915_guc_log_relay", &i915_guc_log_relay_fops},
+	{"i915_guc_slpc_gtperf", &i915_guc_slpc_gtperf_fops},
+	{"i915_guc_slpc_balancer", &i915_guc_slpc_balancer_fops},
+	{"i915_guc_slpc_dcc", &i915_guc_slpc_dcc_fops},
 	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
 	{"i915_ipc_status", &i915_ipc_status_fops},
 	{"i915_drrs_ctl", &i915_drrs_ctl_fops}