Hi,

Thanks for review.


On Wednesday 09 August 2017 03:03 PM, Maarten Lankhorst wrote:
Op 18-07-17 om 14:49 schreef Mahesh Kumar:
From: "Kumar, Mahesh" <mahesh1.ku...@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.ku...@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
hmm, I switched HAS_IPC macro in this version of patch only. will fix this. thanks for catching it.
+
+       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().
ok, will remove this check.
+       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. :)
kstrtobool_from_user sounds good, will use this macro :)

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


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

Reply via email to