[Intel-gfx] [PATCH 14/16] drm/i915: Implement blocking read for pipe CRC files

Damien Lespiau damien.lespiau at intel.com
Tue Oct 15 19:55:40 CEST 2013


seq_file is not quite the right interface for these ones. We have a
circular buffer with a new entry per vblank on one side and a process
wanting to dequeue the CRC with a read().

It's quite racy to wait for vblank in user land and then try to read a
pipe_crc file, sometimes the CRC interrupt hasn't been fired and we end
up with an EOF.

So, let's have the read on the pipe_crc file block until the interrupt
gives us a new entry. At that point we can wake the reading process.

Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 164 +++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/i915_dma.c     |   2 +
 drivers/gpu/drm/i915/i915_drv.h     |   6 ++
 drivers/gpu/drm/i915/i915_irq.c     |   2 +
 4 files changed, 155 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index baa2e43..5137f8f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1760,37 +1760,138 @@ static int i915_pc8_status(struct seq_file *m, void *unused)
 	return 0;
 }
 
-static int i915_pipe_crc(struct seq_file *m, void *data)
+struct pipe_crc_info {
+	const char *name;
+	struct drm_device *dev;
+	enum pipe pipe;
+};
+
+static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
+{
+	filep->private_data = inode->i_private;
+
+	return 0;
+}
+
+static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
+{
+	return 0;
+}
+
+/* (6 fields, 8 chars each, space separated (5) + '\n') */
+#define PIPE_CRC_LINE_LEN	(6 * 8 + 5 + 1)
+/* account for \'0' */
+#define PIPE_CRC_BUFFER_LEN	(PIPE_CRC_LINE_LEN + 1)
+
+static int pipe_crc_data_count(struct intel_pipe_crc *pipe_crc)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_device *dev = node->minor->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum pipe pipe = (enum pipe)node->info_ent->data;
-	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
 	int head, tail;
 
-	if (dev_priv->pipe_crc[pipe].source == INTEL_PIPE_CRC_SOURCE_NONE) {
-		seq_puts(m, "none\n");
+	head = atomic_read(&pipe_crc->head);
+	tail = atomic_read(&pipe_crc->tail);
+
+	return CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR);
+}
+
+static ssize_t
+i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
+		   loff_t *pos)
+{
+	struct pipe_crc_info *info = filep->private_data;
+	struct drm_device *dev = info->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
+	char buf[PIPE_CRC_BUFFER_LEN];
+	int head, tail, n_entries, n;
+	ssize_t bytes_read;
+
+	/*
+	 * Don't allow user space to provide buffers not big enough to hold
+	 * a line of data.
+	 */
+	if (count < PIPE_CRC_LINE_LEN)
+		return -EINVAL;
+
+	if (pipe_crc->source == INTEL_PIPE_CRC_SOURCE_NONE)
 		return 0;
+
+	/* nothing to read */
+	while (pipe_crc_data_count(pipe_crc) == 0) {
+		if (filep->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+
+		if (wait_event_interruptible(pipe_crc->wq,
+					     pipe_crc_data_count(pipe_crc)))
+			 return -ERESTARTSYS;
 	}
 
-	seq_puts(m, "  frame    CRC1     CRC2     CRC3     CRC4     CRC5\n");
+	/* We now have one or more entries to read */
 	head = atomic_read(&pipe_crc->head);
 	tail = atomic_read(&pipe_crc->tail);
-
-	while (CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) >= 1) {
+	n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR),
+			count / PIPE_CRC_LINE_LEN);
+	bytes_read = 0;
+	n = 0;
+	do {
 		struct intel_pipe_crc_entry *entry = &pipe_crc->entries[tail];
+		int ret;
 
-		seq_printf(m, "%8u %8x %8x %8x %8x %8x\n", entry->frame,
-			   entry->crc[0], entry->crc[1], entry->crc[2],
-			   entry->crc[3], entry->crc[4]);
+		bytes_read += snprintf(buf, PIPE_CRC_BUFFER_LEN,
+				       "%8u %8x %8x %8x %8x %8x\n",
+				       entry->frame, entry->crc[0],
+				       entry->crc[1], entry->crc[2],
+				       entry->crc[3], entry->crc[4]);
+
+		ret = copy_to_user(user_buf + n * PIPE_CRC_LINE_LEN,
+				   buf, PIPE_CRC_LINE_LEN);
+		if (ret == PIPE_CRC_LINE_LEN)
+			return -EFAULT;
 
 		BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
 		tail = (tail + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
 		atomic_set(&pipe_crc->tail, tail);
-	}
+		n++;
+	} while (--n_entries);
 
-	return 0;
+	return bytes_read;
+}
+
+static const struct file_operations i915_pipe_crc_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_pipe_crc_open,
+	.read = i915_pipe_crc_read,
+	.release = i915_pipe_crc_release,
+};
+
+static struct pipe_crc_info i915_pipe_crc_data[I915_MAX_PIPES] = {
+	{
+		.name = "i915_pipe_A_crc",
+		.pipe = PIPE_A,
+	},
+	{
+		.name = "i915_pipe_B_crc",
+		.pipe = PIPE_B,
+	},
+	{
+		.name = "i915_pipe_C_crc",
+		.pipe = PIPE_C,
+	},
+};
+
+static int i915_pipe_crc_create(struct dentry *root, struct drm_minor *minor,
+				enum pipe pipe)
+{
+	struct drm_device *dev = minor->dev;
+	struct dentry *ent;
+	struct pipe_crc_info *info = &i915_pipe_crc_data[pipe];
+
+	info->dev = dev;
+	ent = debugfs_create_file(info->name, S_IRUGO, root, info,
+				  &i915_pipe_crc_fops);
+	if (IS_ERR(ent))
+		return PTR_ERR(ent);
+
+	return drm_add_fake_info_node(minor, ent, info);
 }
 
 static const char *pipe_crc_sources[] = {
@@ -2555,9 +2656,6 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_edp_psr_status", i915_edp_psr_status, 0},
 	{"i915_energy_uJ", i915_energy_uJ, 0},
 	{"i915_pc8_status", i915_pc8_status, 0},
-	{"i915_pipe_A_crc", i915_pipe_crc, 0, (void *)PIPE_A},
-	{"i915_pipe_B_crc", i915_pipe_crc, 0, (void *)PIPE_B},
-	{"i915_pipe_C_crc", i915_pipe_crc, 0, (void *)PIPE_C},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
@@ -2578,6 +2676,18 @@ static struct i915_debugfs_files {
 	{"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
 };
 
+void intel_display_crc_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int i;
+
+	for (i = 0; i < INTEL_INFO(dev)->num_pipes; i++) {
+		struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[i];
+
+		init_waitqueue_head(&pipe_crc->wq);
+	}
+}
+
 int i915_debugfs_init(struct drm_minor *minor)
 {
 	int ret, i;
@@ -2586,6 +2696,12 @@ int i915_debugfs_init(struct drm_minor *minor)
 	if (ret)
 		return ret;
 
+	for (i = 0; i < ARRAY_SIZE(i915_pipe_crc_data); i++) {
+		ret = i915_pipe_crc_create(minor->debugfs_root, minor, i);
+		if (ret)
+			return ret;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) {
 		ret = i915_debugfs_create(minor->debugfs_root, minor,
 					  i915_debugfs_files[i].name,
@@ -2601,12 +2717,22 @@ int i915_debugfs_init(struct drm_minor *minor)
 
 void i915_debugfs_cleanup(struct drm_minor *minor)
 {
+	struct drm_device *dev = minor->dev;
 	int i;
 
 	drm_debugfs_remove_files(i915_debugfs_list,
 				 I915_DEBUGFS_ENTRIES, minor);
+
 	drm_debugfs_remove_files((struct drm_info_list *) &i915_forcewake_fops,
 				 1, minor);
+
+	for (i = 0; i < INTEL_INFO(dev)->num_pipes; i++) {
+		struct drm_info_list *info_list =
+			(struct drm_info_list *)&i915_pipe_crc_data[i];
+
+		drm_debugfs_remove_files(info_list, 1, minor);
+	}
+
 	for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) {
 		struct drm_info_list *info_list =
 			(struct drm_info_list *) i915_debugfs_files[i].fops;
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b8bc887..4a57a7c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1501,6 +1501,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	dev_priv->pc8.disable_count = 2; /* requirements_met + gpu_idle */
 	INIT_DELAYED_WORK(&dev_priv->pc8.enable_work, hsw_enable_pc8_work);
 
+	intel_display_crc_init(dev);
+
 	i915_dump_device_info(dev_priv);
 
 	/* Not all pre-production machines fall into this category, only the
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5171a69..e16cf1e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1237,6 +1237,7 @@ struct intel_pipe_crc {
 	struct intel_pipe_crc_entry *entries;
 	enum intel_pipe_crc_source source;
 	atomic_t head, tail;
+	wait_queue_head_t wq;
 };
 
 typedef struct drm_i915_private {
@@ -2231,6 +2232,11 @@ int i915_verify_lists(struct drm_device *dev);
 /* i915_debugfs.c */
 int i915_debugfs_init(struct drm_minor *minor);
 void i915_debugfs_cleanup(struct drm_minor *minor);
+#if defined(CONFIG_DEBUG_FS)
+void intel_display_crc_init(struct drm_device *dev);
+#else
+void intel_display_crc_init(struct drm_device *dev) {}
+#endif
 
 /* i915_gpu_error.c */
 __printf(2, 3)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b201a21..b2be057 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1221,6 +1221,8 @@ static void ivb_pipe_crc_update(struct drm_device *dev, enum pipe pipe)
 
 	head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
 	atomic_set(&pipe_crc->head, head);
+
+	wake_up_interruptible(&pipe_crc->wq);
 }
 #else
 static void ivb_pipe_crc_update(struct drm_device *dev, int pipe) {}
-- 
1.8.3.1




More information about the Intel-gfx mailing list