[RFC,09/12] drm/i915: Add a char device file interface to capture GuC ukernel logs

Commit Message

[e-mail] May 27, 2016, 7:43 p.m. UTC

From: Akash Goel <[e-mail]>

This patch provides a new character device file interface '/dev/dri/guc_log'
for the User to capture the GuC ukernel logs.

Signed-off-by: Akash Goel <[e-mail]>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 51 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h           |  2 ++
 2 files changed, 53 insertions(+)

Comments

Chris Wilson May 27, 2016, 7:48 p.m. UTC | #1

On Sat, May 28, 2016 at 01:13:00AM +0530, [e-mail] wrote:
> From: Akash Goel <[e-mail]>
> 
> This patch provides a new character device file interface '/dev/dri/guc_log'
> for the User to capture the GuC ukernel logs.

Do not make the device conditional on !CONFIG_DEBUGFS.
With a stable interface like the device, you do not need a debugfs
entry.
-Chris

[e-mail] May 28, 2016, 8:51 a.m. UTC | #2

On 5/28/2016 1:18 AM, Chris Wilson wrote:
> On Sat, May 28, 2016 at 01:13:00AM +0530, [e-mail] wrote:
>> From: Akash Goel <[e-mail]>
>>
>> This patch provides a new character device file interface '/dev/dri/guc_log'
>> for the User to capture the GuC ukernel logs.
>
> Do not make the device conditional on !CONFIG_DEBUGFS.
> With a stable interface like the device, you do not need a debugfs
> entry.

Sorry, actually I was unsure, don't know which interface would be most 
appropriate here.
Thought if CONFIG_DEBUG_FS is defined, then relay backed debugfs file 
might be the most preferred interface, in which case device file 
interface may not be required.

If device file interface is most suitable, then may not have a relay 
backed debugfs interface.

Please suggest.

Best regards
Akash
> -Chris
>

Patch

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 149826a..8a79b6d 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -25,6 +25,7 @@ 
 #include <linux/circ_buf.h>
 #include <linux/debugfs.h>
 #include <linux/relay.h>
+#include <linux/miscdevice.h>
 #include "i915_drv.h"
 #include "intel_guc.h"
 
@@ -983,6 +984,54 @@  static void guc_create_log_relay_file(struct intel_guc *guc)
 	if (i915.guc_log_level >= 0)
 		gen8_enable_guc_interrupts(dev);
 }
+#else
+static ssize_t guc_log_read(struct file *file, char __user *buf,
+			   size_t len, loff_t *pos)
+{
+	return 0;
+}
+
+static int guc_log_release(struct inode *inode, struct file *file)
+{
+	return 0;
+}
+
+static int guc_log_open(struct inode *inode, struct file *file)
+{
+	struct miscdevice *miscdev = file->private_data;
+	struct intel_guc_log *guc_log =
+		container_of(miscdev, struct intel_guc_log, misc_dev);
+	struct intel_guc *guc =
+		container_of(guc_log, struct intel_guc, log);
+
+	return 0;
+}
+
+static const struct file_operations guc_log_fops = {
+	.owner          = THIS_MODULE,
+	.open           = guc_log_open,
+	.release        = guc_log_release,
+	.read		= guc_log_read,
+	.llseek		= no_llseek,
+};
+
+static void guc_remove_log_device_file(struct intel_guc *guc)
+{
+	misc_deregister(&guc->log.misc_dev);
+}
+
+static void guc_create_log_device_file(struct intel_guc *guc)
+{
+	guc->log.misc_dev.minor = MISC_DYNAMIC_MINOR;
+	guc->log.misc_dev.name = "guc_log";
+	guc->log.misc_dev.nodename = "dri/guc_log";
+	guc->log.misc_dev.fops = &guc_log_fops;
+
+	if (misc_register(&guc->log.misc_dev)) {
+		DRM_ERROR("failed to register misc device for guc logs!\n");
+		return;
+	}
+}
 #endif
 
 static void guc_logging_fini(struct intel_guc *guc)
@@ -991,6 +1040,7 @@  static void guc_logging_fini(struct intel_guc *guc)
         guc_remove_log_relay_file(guc);
 #else
 	if (guc->log.buf_obj) {
+		guc_remove_log_device_file(guc);
 		i915_gem_object_unpin_map(guc->log.buf_obj);
 		drm_gem_object_unreference(&guc->log.buf_obj->base);
 		guc->log.buf_obj = NULL;
@@ -1031,6 +1081,7 @@  static void guc_logging_init(struct intel_guc *guc)
 	}
 
 	guc->log.buf_obj = obj;
+	guc_create_log_device_file(guc);
 
 	/* Enable the flush interrupt, we have a buffer to store GuC logs */
 	if (i915.guc_log_level >= 0)
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index bdc3e63..13810d0 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -127,6 +127,8 @@  struct intel_guc_log {
 	wait_queue_head_t wq;
 	uint64_t first_seq;
 	uint64_t next_seq;
+	/* For Userspace clients to pull logs via /dev/dri/guc_log */
+	struct miscdevice misc_dev;
 };
 
 struct intel_guc {