Re: [Intel-gfx] [PATCH v4 17/26] drm/i915/slpc: Add enable/disable debugfs for slpc

[Kamble, Sagar A]

On 9/9/2016 10:24 PM, Chris Wilson wrote:

On Fri, Sep 09, 2016 at 06:21:36PM +0530, Sagar Arun Kamble wrote:

+static ssize_t 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_param_write(m, ubuf, len, SLPC_PARAM_TASK_ENABLE_DCC,
+  SLPC_PARAM_TASK_DISABLE_DCC);
+   if (ret)
+   return (size_t) ret;

What value is (ssize_t)(size_t)-1 as seen by userspace? Is it negative?

Will fix this. casting is not needed.

-Chris



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 17/26] drm/i915/slpc: Add enable/disable debugfs for slpc

[Chris Wilson]

On Fri, Sep 09, 2016 at 06:21:36PM +0530, Sagar Arun Kamble wrote:
> +static ssize_t 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_param_write(m, ubuf, len, SLPC_PARAM_TASK_ENABLE_DCC,
> +SLPC_PARAM_TASK_DISABLE_DCC);
> + if (ret)
> + return (size_t) ret;

What value is (ssize_t)(size_t)-1 as seen by userspace? Is it negative?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v4 17/26] drm/i915/slpc: Add enable/disable debugfs for slpc

[Sagar Arun Kamble]

From: Tom O'Rourke 

Adds debugfs hooks for each slpc task.

The enable/disable debugfs files are
i915_slpc_gtperf, i915_slpc_balancer, and i915_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.

Signed-off-by: Tom O'Rourke 
Signed-off-by: Sagar Arun Kamble 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 252 
 drivers/gpu/drm/i915/intel_slpc.h   |   5 +
 2 files changed, 257 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 0956d1f..4fde685 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1075,6 +1075,255 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_next_seqno_fops,
i915_next_seqno_get, i915_next_seqno_set,
"0x%llx\n");
 
+static int slpc_enable_disable_get(struct drm_device *dev, u64 *val,
+  enum slpc_param_id enable_id,
+  enum slpc_param_id disable_id)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int override_enable, override_disable;
+   u32 value_enable, value_disable;
+   int ret = 0;
+
+   if (!intel_slpc_active(dev_priv)) {
+   ret = -ENODEV;
+   } else if (val) {
+   intel_slpc_get_param(dev_priv, enable_id, _enable,
+_enable);
+   intel_slpc_get_param(dev_priv, disable_id, _disable,
+_disable);
+
+   /* set the output value:
+   * 0: default
+   * 1: enabled
+   * 2: disabled
+   * 3: unknown (should not happen)
+   */
+   if (override_disable && (value_disable == 1))
+   *val = SLPC_PARAM_TASK_DISABLED;
+   else if (override_enable && (value_enable == 1))
+   *val = SLPC_PARAM_TASK_ENABLED;
+   else if (!override_enable && !override_disable)
+   *val = SLPC_PARAM_TASK_DEFAULT;
+   else
+   *val = SLPC_PARAM_TASK_UNKNOWN;
+
+   } else {
+   ret = -EINVAL;
+   }
+
+   return ret;
+}
+
+static int slpc_enable_disable_set(struct drm_device *dev, u64 val,
+  enum slpc_param_id enable_id,
+  enum slpc_param_id disable_id)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int ret = 0;
+
+   if (!intel_slpc_active(dev_priv)) {
+   ret = -ENODEV;
+   } else if (val == SLPC_PARAM_TASK_DEFAULT) {
+   /* set default */
+   intel_slpc_unset_param(dev_priv, enable_id);
+   intel_slpc_unset_param(dev_priv, disable_id);
+   } else if (val == SLPC_PARAM_TASK_ENABLED) {
+   /* set enable */
+   intel_slpc_set_param(dev_priv, enable_id, 1);
+   intel_slpc_unset_param(dev_priv, disable_id);
+   } else if (val == SLPC_PARAM_TASK_DISABLED) {
+   /* set disable */
+   intel_slpc_set_param(dev_priv, disable_id, 1);
+   intel_slpc_unset_param(dev_priv, enable_id);
+   } else {
+   ret = -EINVAL;
+   }
+
+   return ret;
+}
+
+static void slpc_param_show(struct seq_file *m, enum slpc_param_id enable_id,
+   enum slpc_param_id disable_id)
+{
+   struct drm_device *dev = m->private;
+   const char *status;
+   u64 val;
+   int ret;
+
+   ret = slpc_enable_disable_get(dev, , enable_id, disable_id);
+
+   if (ret) {
+   seq_printf(m, "error %d\n", ret);
+   } else {
+   switch (val) {
+   case SLPC_PARAM_TASK_DEFAULT:
+   status = "default\n";
+   break;
+
+   case SLPC_PARAM_TASK_ENABLED:
+   status = "enabled\n";
+   break;
+
+   case SLPC_PARAM_TASK_DISABLED:
+   status = "disabled\n";
+   break;
+
+   default:
+   status = "unknown\n";
+   break;
+   }
+
+   seq_puts(m, status);
+   }
+}
+
+static int slpc_param_write(struct seq_file *m, const char __user *ubuf,
+   size_t len, enum slpc_param_id enable_id,
+   enum slpc_param_id disable_id)
+{
+   struct drm_device *dev = m->private;
+   u64 val;
+   int ret = 0;
+   char buf[10];
+
+   if (len >= sizeof(buf))
+   ret = -EINVAL;
+   else if (copy_from_user(buf,