* [PATCH 1/2] drm: share drm_add_fake_info_node
@ 2014-01-14 14:14 Ben Widawsky
2014-01-14 14:14 ` [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file Ben Widawsky
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Ben Widawsky @ 2014-01-14 14:14 UTC (permalink / raw)
To: DRI Development; +Cc: intel-gfx, Ben Widawsky, linux-arm-kernel, Ben Widawsky
Both i915 and Armada had the exact same implementation. For an upcoming
patch, I'd like to call this function from two different source files in
i915, and having it available externally helps there too.
While moving, add 'debugfs_' to the name in order to match the other drm
debugfs helper functions.
Cc: linux-arm-kernel@lists.infradead.org
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/armada/armada_debugfs.c | 24 +-----------------------
drivers/gpu/drm/drm_debugfs.c | 24 ++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_debugfs.c | 32 +++-----------------------------
include/drm/drmP.h | 9 +++++++++
4 files changed, 37 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_debugfs.c b/drivers/gpu/drm/armada/armada_debugfs.c
index 471e456..02f2978 100644
--- a/drivers/gpu/drm/armada/armada_debugfs.c
+++ b/drivers/gpu/drm/armada/armada_debugfs.c
@@ -107,28 +107,6 @@ static struct drm_info_list armada_debugfs_list[] = {
};
#define ARMADA_DEBUGFS_ENTRIES ARRAY_SIZE(armada_debugfs_list)
-static int drm_add_fake_info_node(struct drm_minor *minor, struct dentry *ent,
- const void *key)
-{
- struct drm_info_node *node;
-
- node = kmalloc(sizeof(struct drm_info_node), GFP_KERNEL);
- if (node == NULL) {
- debugfs_remove(ent);
- return -ENOMEM;
- }
-
- node->minor = minor;
- node->dent = ent;
- node->info_ent = (void *) key;
-
- mutex_lock(&minor->debugfs_lock);
- list_add(&node->list, &minor->debugfs_list);
- mutex_unlock(&minor->debugfs_lock);
-
- return 0;
-}
-
static int armada_debugfs_create(struct dentry *root, struct drm_minor *minor,
const char *name, umode_t mode, const struct file_operations *fops)
{
@@ -136,7 +114,7 @@ static int armada_debugfs_create(struct dentry *root, struct drm_minor *minor,
de = debugfs_create_file(name, mode, root, minor->dev, fops);
- return drm_add_fake_info_node(minor, de, fops);
+ return drm_debugfs_add_fake_info_node(minor, de, fops);
}
int armada_drm_debugfs_init(struct drm_minor *minor)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index b4b51d4..1edaac0 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -237,5 +237,29 @@ int drm_debugfs_cleanup(struct drm_minor *minor)
return 0;
}
+int drm_debugfs_add_fake_info_node(struct drm_minor *minor,
+ struct dentry *ent,
+ const void *key)
+{
+ struct drm_info_node *node;
+
+ node = kmalloc(sizeof(*node), GFP_KERNEL);
+ if (node == NULL) {
+ debugfs_remove(ent);
+ return -ENOMEM;
+ }
+
+ node->minor = minor;
+ node->dent = ent;
+ node->info_ent = (void *) key;
+
+ mutex_lock(&minor->debugfs_lock);
+ list_add(&node->list, &minor->debugfs_list);
+ mutex_unlock(&minor->debugfs_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_debugfs_add_fake_info_node);
+
#endif /* CONFIG_DEBUG_FS */
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 430eb3e..f2ef30c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -51,32 +51,6 @@ static const char *yesno(int v)
return v ? "yes" : "no";
}
-/* As the drm_debugfs_init() routines are called before dev->dev_private is
- * allocated we need to hook into the minor for release. */
-static int
-drm_add_fake_info_node(struct drm_minor *minor,
- struct dentry *ent,
- const void *key)
-{
- struct drm_info_node *node;
-
- node = kmalloc(sizeof(*node), GFP_KERNEL);
- if (node == NULL) {
- debugfs_remove(ent);
- return -ENOMEM;
- }
-
- node->minor = minor;
- node->dent = ent;
- node->info_ent = (void *) key;
-
- mutex_lock(&minor->debugfs_lock);
- list_add(&node->list, &minor->debugfs_list);
- mutex_unlock(&minor->debugfs_lock);
-
- return 0;
-}
-
static int i915_capabilities(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -2148,7 +2122,7 @@ static int i915_pipe_crc_create(struct dentry *root, struct drm_minor *minor,
if (!ent)
return -ENOMEM;
- return drm_add_fake_info_node(minor, ent, info);
+ return drm_debugfs_add_fake_info_node(minor, ent, info);
}
static const char * const pipe_crc_sources[] = {
@@ -3164,7 +3138,7 @@ static int i915_forcewake_create(struct dentry *root, struct drm_minor *minor)
if (!ent)
return -ENOMEM;
- return drm_add_fake_info_node(minor, ent, &i915_forcewake_fops);
+ return drm_debugfs_add_fake_info_node(minor, ent, &i915_forcewake_fops);
}
static int i915_debugfs_create(struct dentry *root,
@@ -3182,7 +3156,7 @@ static int i915_debugfs_create(struct dentry *root,
if (!ent)
return -ENOMEM;
- return drm_add_fake_info_node(minor, ent, fops);
+ return drm_debugfs_add_fake_info_node(minor, ent, fops);
}
static const struct drm_info_list i915_debugfs_list[] = {
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2fe9b5d..5788fb1 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1471,6 +1471,9 @@ extern int drm_debugfs_create_files(const struct drm_info_list *files,
extern int drm_debugfs_remove_files(const struct drm_info_list *files,
int count, struct drm_minor *minor);
extern int drm_debugfs_cleanup(struct drm_minor *minor);
+extern int drm_debugfs_add_fake_info_node(struct drm_minor *minor,
+ struct dentry *ent,
+ const void *key);
#else
static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
struct dentry *root)
@@ -1495,6 +1498,12 @@ static inline int drm_debugfs_cleanup(struct drm_minor *minor)
{
return 0;
}
+static int drm_debugfs_add_fake_info_node(struct drm_minor *minor,
+ struct dentry *ent,
+ const void *key)
+{
+ return 0;
+}
#endif
/* Info file support */
--
1.8.5.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file 2014-01-14 14:14 [PATCH 1/2] drm: share drm_add_fake_info_node Ben Widawsky @ 2014-01-14 14:14 ` Ben Widawsky 2014-01-14 23:28 ` Daniel Vetter 2014-01-15 1:22 ` Damien Lespiau 2014-01-14 23:25 ` [Intel-gfx] [PATCH 1/2] drm: share drm_add_fake_info_node Daniel Vetter 2014-01-21 20:33 ` [PATCH 1/6] drm: Create a debugfs file creation helper Ben Widawsky 2 siblings, 2 replies; 19+ messages in thread From: Ben Widawsky @ 2014-01-14 14:14 UTC (permalink / raw) To: Intel GFX; +Cc: Ben Widawsky, Ben Widawsky At almost 800 lines of code, with almost a function per platform, this code was cluttering up the existing debugfs file, which has historically had fairly small functions. Patch should have no functional changes. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/Makefile | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 764 ---------------------------------- drivers/gpu/drm/i915/i915_drv.h | 12 + drivers/gpu/drm/i915/i915_pipecrc.c | 790 ++++++++++++++++++++++++++++++++++++ 4 files changed, 803 insertions(+), 765 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_pipecrc.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index da682cb..432d231 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -54,7 +54,7 @@ i915-$(CONFIG_ACPI) += intel_acpi.o i915-$(CONFIG_DRM_I915_FBDEV) += intel_fbdev.o -i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o +i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o i915_pipecrc.o obj-$(CONFIG_DRM_I915) += i915.o diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index f2ef30c..c98f91b 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -27,8 +27,6 @@ */ #include <linux/seq_file.h> -#include <linux/circ_buf.h> -#include <linux/ctype.h> #include <linux/debugfs.h> #include <linux/slab.h> #include <linux/export.h> @@ -1956,754 +1954,6 @@ static int i915_power_domain_info(struct seq_file *m, void *unused) return 0; } -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) -{ - struct pipe_crc_info *info = inode->i_private; - struct drm_i915_private *dev_priv = info->dev->dev_private; - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; - - if (info->pipe >= INTEL_INFO(info->dev)->num_pipes) - return -ENODEV; - - spin_lock_irq(&pipe_crc->lock); - - if (pipe_crc->opened) { - spin_unlock_irq(&pipe_crc->lock); - return -EBUSY; /* already open */ - } - - pipe_crc->opened = true; - filep->private_data = inode->i_private; - - spin_unlock_irq(&pipe_crc->lock); - - return 0; -} - -static int i915_pipe_crc_release(struct inode *inode, struct file *filep) -{ - struct pipe_crc_info *info = inode->i_private; - struct drm_i915_private *dev_priv = info->dev->dev_private; - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; - - spin_lock_irq(&pipe_crc->lock); - pipe_crc->opened = false; - spin_unlock_irq(&pipe_crc->lock); - - 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) -{ - assert_spin_locked(&pipe_crc->lock); - return CIRC_CNT(pipe_crc->head, pipe_crc->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 */ - spin_lock_irq(&pipe_crc->lock); - while (pipe_crc_data_count(pipe_crc) == 0) { - int ret; - - if (filep->f_flags & O_NONBLOCK) { - spin_unlock_irq(&pipe_crc->lock); - return -EAGAIN; - } - - ret = wait_event_interruptible_lock_irq(pipe_crc->wq, - pipe_crc_data_count(pipe_crc), pipe_crc->lock); - if (ret) { - spin_unlock_irq(&pipe_crc->lock); - return ret; - } - } - - /* We now have one or more entries to read */ - head = pipe_crc->head; - tail = pipe_crc->tail; - n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR), - count / PIPE_CRC_LINE_LEN); - spin_unlock_irq(&pipe_crc->lock); - - bytes_read = 0; - n = 0; - do { - struct intel_pipe_crc_entry *entry = &pipe_crc->entries[tail]; - int ret; - - 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); - n++; - } while (--n_entries); - - spin_lock_irq(&pipe_crc->lock); - pipe_crc->tail = tail; - spin_unlock_irq(&pipe_crc->lock); - - 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 (!ent) - return -ENOMEM; - - return drm_debugfs_add_fake_info_node(minor, ent, info); -} - -static const char * const pipe_crc_sources[] = { - "none", - "plane1", - "plane2", - "pf", - "pipe", - "TV", - "DP-B", - "DP-C", - "DP-D", - "auto", -}; - -static const char *pipe_crc_source_name(enum intel_pipe_crc_source source) -{ - BUILD_BUG_ON(ARRAY_SIZE(pipe_crc_sources) != INTEL_PIPE_CRC_SOURCE_MAX); - return pipe_crc_sources[source]; -} - -static int display_crc_ctl_show(struct seq_file *m, void *data) -{ - struct drm_device *dev = m->private; - struct drm_i915_private *dev_priv = dev->dev_private; - int i; - - for (i = 0; i < I915_MAX_PIPES; i++) - seq_printf(m, "%c %s\n", pipe_name(i), - pipe_crc_source_name(dev_priv->pipe_crc[i].source)); - - return 0; -} - -static int display_crc_ctl_open(struct inode *inode, struct file *file) -{ - struct drm_device *dev = inode->i_private; - - return single_open(file, display_crc_ctl_show, dev); -} - -static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, - uint32_t *val) -{ - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) - *source = INTEL_PIPE_CRC_SOURCE_PIPE; - - switch (*source) { - case INTEL_PIPE_CRC_SOURCE_PIPE: - *val = PIPE_CRC_ENABLE | PIPE_CRC_INCLUDE_BORDER_I8XX; - break; - case INTEL_PIPE_CRC_SOURCE_NONE: - *val = 0; - break; - default: - return -EINVAL; - } - - return 0; -} - -static int i9xx_pipe_crc_auto_source(struct drm_device *dev, enum pipe pipe, - enum intel_pipe_crc_source *source) -{ - struct intel_encoder *encoder; - struct intel_crtc *crtc; - struct intel_digital_port *dig_port; - int ret = 0; - - *source = INTEL_PIPE_CRC_SOURCE_PIPE; - - mutex_lock(&dev->mode_config.mutex); - list_for_each_entry(encoder, &dev->mode_config.encoder_list, - base.head) { - if (!encoder->base.crtc) - continue; - - crtc = to_intel_crtc(encoder->base.crtc); - - if (crtc->pipe != pipe) - continue; - - switch (encoder->type) { - case INTEL_OUTPUT_TVOUT: - *source = INTEL_PIPE_CRC_SOURCE_TV; - break; - case INTEL_OUTPUT_DISPLAYPORT: - case INTEL_OUTPUT_EDP: - dig_port = enc_to_dig_port(&encoder->base); - switch (dig_port->port) { - case PORT_B: - *source = INTEL_PIPE_CRC_SOURCE_DP_B; - break; - case PORT_C: - *source = INTEL_PIPE_CRC_SOURCE_DP_C; - break; - case PORT_D: - *source = INTEL_PIPE_CRC_SOURCE_DP_D; - break; - default: - WARN(1, "nonexisting DP port %c\n", - port_name(dig_port->port)); - break; - } - break; - } - } - mutex_unlock(&dev->mode_config.mutex); - - return ret; -} - -static int vlv_pipe_crc_ctl_reg(struct drm_device *dev, - enum pipe pipe, - enum intel_pipe_crc_source *source, - uint32_t *val) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - bool need_stable_symbols = false; - - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { - int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); - if (ret) - return ret; - } - - switch (*source) { - case INTEL_PIPE_CRC_SOURCE_PIPE: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_VLV; - break; - case INTEL_PIPE_CRC_SOURCE_DP_B: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_VLV; - need_stable_symbols = true; - break; - case INTEL_PIPE_CRC_SOURCE_DP_C: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_VLV; - need_stable_symbols = true; - break; - case INTEL_PIPE_CRC_SOURCE_NONE: - *val = 0; - break; - default: - return -EINVAL; - } - - /* - * When the pipe CRC tap point is after the transcoders we need - * to tweak symbol-level features to produce a deterministic series of - * symbols for a given frame. We need to reset those features only once - * a frame (instead of every nth symbol): - * - DC-balance: used to ensure a better clock recovery from the data - * link (SDVO) - * - DisplayPort scrambling: used for EMI reduction - */ - if (need_stable_symbols) { - uint32_t tmp = I915_READ(PORT_DFT2_G4X); - - WARN_ON(!IS_G4X(dev)); - - tmp |= DC_BALANCE_RESET_VLV; - if (pipe == PIPE_A) - tmp |= PIPE_A_SCRAMBLE_RESET; - else - tmp |= PIPE_B_SCRAMBLE_RESET; - - I915_WRITE(PORT_DFT2_G4X, tmp); - } - - return 0; -} - -static int i9xx_pipe_crc_ctl_reg(struct drm_device *dev, - enum pipe pipe, - enum intel_pipe_crc_source *source, - uint32_t *val) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - bool need_stable_symbols = false; - - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { - int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); - if (ret) - return ret; - } - - switch (*source) { - case INTEL_PIPE_CRC_SOURCE_PIPE: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_I9XX; - break; - case INTEL_PIPE_CRC_SOURCE_TV: - if (!SUPPORTS_TV(dev)) - return -EINVAL; - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_TV_PRE; - break; - case INTEL_PIPE_CRC_SOURCE_DP_B: - if (!IS_G4X(dev)) - return -EINVAL; - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_G4X; - need_stable_symbols = true; - break; - case INTEL_PIPE_CRC_SOURCE_DP_C: - if (!IS_G4X(dev)) - return -EINVAL; - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_G4X; - need_stable_symbols = true; - break; - case INTEL_PIPE_CRC_SOURCE_DP_D: - if (!IS_G4X(dev)) - return -EINVAL; - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_D_G4X; - need_stable_symbols = true; - break; - case INTEL_PIPE_CRC_SOURCE_NONE: - *val = 0; - break; - default: - return -EINVAL; - } - - /* - * When the pipe CRC tap point is after the transcoders we need - * to tweak symbol-level features to produce a deterministic series of - * symbols for a given frame. We need to reset those features only once - * a frame (instead of every nth symbol): - * - DC-balance: used to ensure a better clock recovery from the data - * link (SDVO) - * - DisplayPort scrambling: used for EMI reduction - */ - if (need_stable_symbols) { - uint32_t tmp = I915_READ(PORT_DFT2_G4X); - - WARN_ON(!IS_G4X(dev)); - - I915_WRITE(PORT_DFT_I9XX, - I915_READ(PORT_DFT_I9XX) | DC_BALANCE_RESET); - - if (pipe == PIPE_A) - tmp |= PIPE_A_SCRAMBLE_RESET; - else - tmp |= PIPE_B_SCRAMBLE_RESET; - - I915_WRITE(PORT_DFT2_G4X, tmp); - } - - return 0; -} - -static void vlv_undo_pipe_scramble_reset(struct drm_device *dev, - enum pipe pipe) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - uint32_t tmp = I915_READ(PORT_DFT2_G4X); - - if (pipe == PIPE_A) - tmp &= ~PIPE_A_SCRAMBLE_RESET; - else - tmp &= ~PIPE_B_SCRAMBLE_RESET; - if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) - tmp &= ~DC_BALANCE_RESET_VLV; - I915_WRITE(PORT_DFT2_G4X, tmp); - -} - -static void g4x_undo_pipe_scramble_reset(struct drm_device *dev, - enum pipe pipe) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - uint32_t tmp = I915_READ(PORT_DFT2_G4X); - - if (pipe == PIPE_A) - tmp &= ~PIPE_A_SCRAMBLE_RESET; - else - tmp &= ~PIPE_B_SCRAMBLE_RESET; - I915_WRITE(PORT_DFT2_G4X, tmp); - - if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) { - I915_WRITE(PORT_DFT_I9XX, - I915_READ(PORT_DFT_I9XX) & ~DC_BALANCE_RESET); - } -} - -static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, - uint32_t *val) -{ - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) - *source = INTEL_PIPE_CRC_SOURCE_PIPE; - - switch (*source) { - case INTEL_PIPE_CRC_SOURCE_PLANE1: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_ILK; - break; - case INTEL_PIPE_CRC_SOURCE_PLANE2: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_ILK; - break; - case INTEL_PIPE_CRC_SOURCE_PIPE: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_ILK; - break; - case INTEL_PIPE_CRC_SOURCE_NONE: - *val = 0; - break; - default: - return -EINVAL; - } - - return 0; -} - -static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, - uint32_t *val) -{ - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) - *source = INTEL_PIPE_CRC_SOURCE_PF; - - switch (*source) { - case INTEL_PIPE_CRC_SOURCE_PLANE1: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_IVB; - break; - case INTEL_PIPE_CRC_SOURCE_PLANE2: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB; - break; - case INTEL_PIPE_CRC_SOURCE_PF: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB; - break; - case INTEL_PIPE_CRC_SOURCE_NONE: - *val = 0; - break; - default: - return -EINVAL; - } - - return 0; -} - -static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, - enum intel_pipe_crc_source source) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; - u32 val = 0; /* shut up gcc */ - int ret; - - if (pipe_crc->source == source) - return 0; - - /* forbid changing the source without going back to 'none' */ - if (pipe_crc->source && source) - return -EINVAL; - - if (IS_GEN2(dev)) - ret = i8xx_pipe_crc_ctl_reg(&source, &val); - else if (INTEL_INFO(dev)->gen < 5) - ret = i9xx_pipe_crc_ctl_reg(dev, pipe, &source, &val); - else if (IS_VALLEYVIEW(dev)) - ret = vlv_pipe_crc_ctl_reg(dev,pipe, &source, &val); - else if (IS_GEN5(dev) || IS_GEN6(dev)) - ret = ilk_pipe_crc_ctl_reg(&source, &val); - else - ret = ivb_pipe_crc_ctl_reg(&source, &val); - - if (ret != 0) - return ret; - - /* none -> real source transition */ - if (source) { - DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n", - pipe_name(pipe), pipe_crc_source_name(source)); - - pipe_crc->entries = kzalloc(sizeof(*pipe_crc->entries) * - INTEL_PIPE_CRC_ENTRIES_NR, - GFP_KERNEL); - if (!pipe_crc->entries) - return -ENOMEM; - - spin_lock_irq(&pipe_crc->lock); - pipe_crc->head = 0; - pipe_crc->tail = 0; - spin_unlock_irq(&pipe_crc->lock); - } - - pipe_crc->source = source; - - I915_WRITE(PIPE_CRC_CTL(pipe), val); - POSTING_READ(PIPE_CRC_CTL(pipe)); - - /* real source -> none transition */ - if (source == INTEL_PIPE_CRC_SOURCE_NONE) { - struct intel_pipe_crc_entry *entries; - - DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n", - pipe_name(pipe)); - - intel_wait_for_vblank(dev, pipe); - - spin_lock_irq(&pipe_crc->lock); - entries = pipe_crc->entries; - pipe_crc->entries = NULL; - spin_unlock_irq(&pipe_crc->lock); - - kfree(entries); - - if (IS_G4X(dev)) - g4x_undo_pipe_scramble_reset(dev, pipe); - else if (IS_VALLEYVIEW(dev)) - vlv_undo_pipe_scramble_reset(dev, pipe); - } - - return 0; -} - -/* - * Parse pipe CRC command strings: - * command: wsp* object wsp+ name wsp+ source wsp* - * object: 'pipe' - * name: (A | B | C) - * source: (none | plane1 | plane2 | pf) - * wsp: (#0x20 | #0x9 | #0xA)+ - * - * eg.: - * "pipe A plane1" -> Start CRC computations on plane1 of pipe A - * "pipe A none" -> Stop CRC - */ -static int display_crc_ctl_tokenize(char *buf, char *words[], int max_words) -{ - int n_words = 0; - - while (*buf) { - char *end; - - /* skip leading white space */ - buf = skip_spaces(buf); - if (!*buf) - break; /* end of buffer */ - - /* find end of word */ - for (end = buf; *end && !isspace(*end); end++) - ; - - if (n_words == max_words) { - DRM_DEBUG_DRIVER("too many words, allowed <= %d\n", - max_words); - return -EINVAL; /* ran out of words[] before bytes */ - } - - if (*end) - *end++ = '\0'; - words[n_words++] = buf; - buf = end; - } - - return n_words; -} - -enum intel_pipe_crc_object { - PIPE_CRC_OBJECT_PIPE, -}; - -static const char * const pipe_crc_objects[] = { - "pipe", -}; - -static int -display_crc_ctl_parse_object(const char *buf, enum intel_pipe_crc_object *o) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(pipe_crc_objects); i++) - if (!strcmp(buf, pipe_crc_objects[i])) { - *o = i; - return 0; - } - - return -EINVAL; -} - -static int display_crc_ctl_parse_pipe(const char *buf, enum pipe *pipe) -{ - const char name = buf[0]; - - if (name < 'A' || name >= pipe_name(I915_MAX_PIPES)) - return -EINVAL; - - *pipe = name - 'A'; - - return 0; -} - -static int -display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(pipe_crc_sources); i++) - if (!strcmp(buf, pipe_crc_sources[i])) { - *s = i; - return 0; - } - - return -EINVAL; -} - -static int display_crc_ctl_parse(struct drm_device *dev, char *buf, size_t len) -{ -#define N_WORDS 3 - int n_words; - char *words[N_WORDS]; - enum pipe pipe; - enum intel_pipe_crc_object object; - enum intel_pipe_crc_source source; - - n_words = display_crc_ctl_tokenize(buf, words, N_WORDS); - if (n_words != N_WORDS) { - DRM_DEBUG_DRIVER("tokenize failed, a command is %d words\n", - N_WORDS); - return -EINVAL; - } - - if (display_crc_ctl_parse_object(words[0], &object) < 0) { - DRM_DEBUG_DRIVER("unknown object %s\n", words[0]); - return -EINVAL; - } - - if (display_crc_ctl_parse_pipe(words[1], &pipe) < 0) { - DRM_DEBUG_DRIVER("unknown pipe %s\n", words[1]); - return -EINVAL; - } - - if (display_crc_ctl_parse_source(words[2], &source) < 0) { - DRM_DEBUG_DRIVER("unknown source %s\n", words[2]); - return -EINVAL; - } - - return pipe_crc_set_source(dev, pipe, source); -} - -static ssize_t display_crc_ctl_write(struct file *file, const char __user *ubuf, - size_t len, loff_t *offp) -{ - struct seq_file *m = file->private_data; - struct drm_device *dev = m->private; - char *tmpbuf; - int ret; - - if (len == 0) - return 0; - - if (len > PAGE_SIZE - 1) { - DRM_DEBUG_DRIVER("expected <%lu bytes into pipe crc control\n", - PAGE_SIZE); - return -E2BIG; - } - - tmpbuf = kmalloc(len + 1, GFP_KERNEL); - if (!tmpbuf) - return -ENOMEM; - - if (copy_from_user(tmpbuf, ubuf, len)) { - ret = -EFAULT; - goto out; - } - tmpbuf[len] = '\0'; - - ret = display_crc_ctl_parse(dev, tmpbuf, len); - -out: - kfree(tmpbuf); - if (ret < 0) - return ret; - - *offp += len; - return len; -} - -static const struct file_operations i915_display_crc_ctl_fops = { - .owner = THIS_MODULE, - .open = display_crc_ctl_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, - .write = display_crc_ctl_write -}; - static int i915_wedged_get(void *data, u64 *val) { @@ -3219,20 +2469,6 @@ static const 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; - enum pipe pipe; - - for_each_pipe(pipe) { - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; - - pipe_crc->opened = false; - spin_lock_init(&pipe_crc->lock); - init_waitqueue_head(&pipe_crc->wq); - } -} - int i915_debugfs_init(struct drm_minor *minor) { int ret, i; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index cc8afff..7ded723 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1356,6 +1356,18 @@ struct intel_pipe_crc { wait_queue_head_t wq; }; +struct pipe_crc_info { + const char *name; + struct drm_device *dev; + enum pipe pipe; +}; + +extern struct pipe_crc_info i915_pipe_crc_data[I915_MAX_PIPES]; +extern const struct file_operations i915_display_crc_ctl_fops; + +int i915_pipe_crc_create(struct dentry *root, struct drm_minor *minor, + enum pipe pipe); + typedef struct drm_i915_private { struct drm_device *dev; struct kmem_cache *slab; diff --git a/drivers/gpu/drm/i915/i915_pipecrc.c b/drivers/gpu/drm/i915/i915_pipecrc.c new file mode 100644 index 0000000..7b0bec6 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_pipecrc.c @@ -0,0 +1,790 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include <linux/debugfs.h> +#include <linux/fs.h> +#include <linux/seq_file.h> +#include <linux/circ_buf.h> +#include <linux/ctype.h> +#if 0 +#include <drm/drm_crtc.h> +#include <drm/drmP.h> +#endif +#include "i915_drv.h" +#include "intel_drv.h" + +static int i915_pipe_crc_open(struct inode *inode, struct file *filep) +{ + struct pipe_crc_info *info = inode->i_private; + struct drm_i915_private *dev_priv = info->dev->dev_private; + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; + + if (info->pipe >= INTEL_INFO(info->dev)->num_pipes) + return -ENODEV; + + spin_lock_irq(&pipe_crc->lock); + + if (pipe_crc->opened) { + spin_unlock_irq(&pipe_crc->lock); + return -EBUSY; /* already open */ + } + + pipe_crc->opened = true; + filep->private_data = inode->i_private; + + spin_unlock_irq(&pipe_crc->lock); + + return 0; +} + +static int i915_pipe_crc_release(struct inode *inode, struct file *filep) +{ + struct pipe_crc_info *info = inode->i_private; + struct drm_i915_private *dev_priv = info->dev->dev_private; + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; + + spin_lock_irq(&pipe_crc->lock); + pipe_crc->opened = false; + spin_unlock_irq(&pipe_crc->lock); + + 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) +{ + assert_spin_locked(&pipe_crc->lock); + return CIRC_CNT(pipe_crc->head, pipe_crc->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 */ + spin_lock_irq(&pipe_crc->lock); + while (pipe_crc_data_count(pipe_crc) == 0) { + int ret; + + if (filep->f_flags & O_NONBLOCK) { + spin_unlock_irq(&pipe_crc->lock); + return -EAGAIN; + } + + ret = wait_event_interruptible_lock_irq(pipe_crc->wq, + pipe_crc_data_count(pipe_crc), pipe_crc->lock); + if (ret) { + spin_unlock_irq(&pipe_crc->lock); + return ret; + } + } + + /* We now have one or more entries to read */ + head = pipe_crc->head; + tail = pipe_crc->tail; + n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR), + count / PIPE_CRC_LINE_LEN); + spin_unlock_irq(&pipe_crc->lock); + + bytes_read = 0; + n = 0; + do { + struct intel_pipe_crc_entry *entry = &pipe_crc->entries[tail]; + int ret; + + 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); + n++; + } while (--n_entries); + + spin_lock_irq(&pipe_crc->lock); + pipe_crc->tail = tail; + spin_unlock_irq(&pipe_crc->lock); + + return bytes_read; +} + +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, +}; + +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, + }, +}; + +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 (!ent) + return -ENOMEM; + + return drm_debugfs_add_fake_info_node(minor, ent, info); +} + +static const char * const pipe_crc_sources[] = { + "none", + "plane1", + "plane2", + "pf", + "pipe", + "TV", + "DP-B", + "DP-C", + "DP-D", + "auto", +}; + +static const char *pipe_crc_source_name(enum intel_pipe_crc_source source) +{ + BUILD_BUG_ON(ARRAY_SIZE(pipe_crc_sources) != INTEL_PIPE_CRC_SOURCE_MAX); + return pipe_crc_sources[source]; +} + +static int display_crc_ctl_show(struct seq_file *m, void *data) +{ + struct drm_device *dev = m->private; + struct drm_i915_private *dev_priv = dev->dev_private; + int i; + + for (i = 0; i < I915_MAX_PIPES; i++) + seq_printf(m, "%c %s\n", pipe_name(i), + pipe_crc_source_name(dev_priv->pipe_crc[i].source)); + + return 0; +} + +static int display_crc_ctl_open(struct inode *inode, struct file *file) +{ + struct drm_device *dev = inode->i_private; + + return single_open(file, display_crc_ctl_show, dev); +} + +static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, + uint32_t *val) +{ + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) + *source = INTEL_PIPE_CRC_SOURCE_PIPE; + + switch (*source) { + case INTEL_PIPE_CRC_SOURCE_PIPE: + *val = PIPE_CRC_ENABLE | PIPE_CRC_INCLUDE_BORDER_I8XX; + break; + case INTEL_PIPE_CRC_SOURCE_NONE: + *val = 0; + break; + default: + return -EINVAL; + } + + return 0; +} + +static int i9xx_pipe_crc_auto_source(struct drm_device *dev, enum pipe pipe, + enum intel_pipe_crc_source *source) +{ + struct intel_encoder *encoder; + struct intel_crtc *crtc; + struct intel_digital_port *dig_port; + int ret = 0; + + *source = INTEL_PIPE_CRC_SOURCE_PIPE; + + mutex_lock(&dev->mode_config.mutex); + list_for_each_entry(encoder, &dev->mode_config.encoder_list, + base.head) { + if (!encoder->base.crtc) + continue; + + crtc = to_intel_crtc(encoder->base.crtc); + + if (crtc->pipe != pipe) + continue; + + switch (encoder->type) { + case INTEL_OUTPUT_TVOUT: + *source = INTEL_PIPE_CRC_SOURCE_TV; + break; + case INTEL_OUTPUT_DISPLAYPORT: + case INTEL_OUTPUT_EDP: + dig_port = enc_to_dig_port(&encoder->base); + switch (dig_port->port) { + case PORT_B: + *source = INTEL_PIPE_CRC_SOURCE_DP_B; + break; + case PORT_C: + *source = INTEL_PIPE_CRC_SOURCE_DP_C; + break; + case PORT_D: + *source = INTEL_PIPE_CRC_SOURCE_DP_D; + break; + default: + WARN(1, "nonexisting DP port %c\n", + port_name(dig_port->port)); + break; + } + break; + } + } + mutex_unlock(&dev->mode_config.mutex); + + return ret; +} + +static int vlv_pipe_crc_ctl_reg(struct drm_device *dev, + enum pipe pipe, + enum intel_pipe_crc_source *source, + uint32_t *val) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + bool need_stable_symbols = false; + + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { + int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); + if (ret) + return ret; + } + + switch (*source) { + case INTEL_PIPE_CRC_SOURCE_PIPE: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_VLV; + break; + case INTEL_PIPE_CRC_SOURCE_DP_B: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_VLV; + need_stable_symbols = true; + break; + case INTEL_PIPE_CRC_SOURCE_DP_C: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_VLV; + need_stable_symbols = true; + break; + case INTEL_PIPE_CRC_SOURCE_NONE: + *val = 0; + break; + default: + return -EINVAL; + } + + /* + * When the pipe CRC tap point is after the transcoders we need + * to tweak symbol-level features to produce a deterministic series of + * symbols for a given frame. We need to reset those features only once + * a frame (instead of every nth symbol): + * - DC-balance: used to ensure a better clock recovery from the data + * link (SDVO) + * - DisplayPort scrambling: used for EMI reduction + */ + if (need_stable_symbols) { + uint32_t tmp = I915_READ(PORT_DFT2_G4X); + + WARN_ON(!IS_G4X(dev)); + + tmp |= DC_BALANCE_RESET_VLV; + if (pipe == PIPE_A) + tmp |= PIPE_A_SCRAMBLE_RESET; + else + tmp |= PIPE_B_SCRAMBLE_RESET; + + I915_WRITE(PORT_DFT2_G4X, tmp); + } + + return 0; +} + +static int i9xx_pipe_crc_ctl_reg(struct drm_device *dev, + enum pipe pipe, + enum intel_pipe_crc_source *source, + uint32_t *val) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + bool need_stable_symbols = false; + + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { + int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); + if (ret) + return ret; + } + + switch (*source) { + case INTEL_PIPE_CRC_SOURCE_PIPE: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_I9XX; + break; + case INTEL_PIPE_CRC_SOURCE_TV: + if (!SUPPORTS_TV(dev)) + return -EINVAL; + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_TV_PRE; + break; + case INTEL_PIPE_CRC_SOURCE_DP_B: + if (!IS_G4X(dev)) + return -EINVAL; + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_G4X; + need_stable_symbols = true; + break; + case INTEL_PIPE_CRC_SOURCE_DP_C: + if (!IS_G4X(dev)) + return -EINVAL; + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_G4X; + need_stable_symbols = true; + break; + case INTEL_PIPE_CRC_SOURCE_DP_D: + if (!IS_G4X(dev)) + return -EINVAL; + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_D_G4X; + need_stable_symbols = true; + break; + case INTEL_PIPE_CRC_SOURCE_NONE: + *val = 0; + break; + default: + return -EINVAL; + } + + /* + * When the pipe CRC tap point is after the transcoders we need + * to tweak symbol-level features to produce a deterministic series of + * symbols for a given frame. We need to reset those features only once + * a frame (instead of every nth symbol): + * - DC-balance: used to ensure a better clock recovery from the data + * link (SDVO) + * - DisplayPort scrambling: used for EMI reduction + */ + if (need_stable_symbols) { + uint32_t tmp = I915_READ(PORT_DFT2_G4X); + + WARN_ON(!IS_G4X(dev)); + + I915_WRITE(PORT_DFT_I9XX, + I915_READ(PORT_DFT_I9XX) | DC_BALANCE_RESET); + + if (pipe == PIPE_A) + tmp |= PIPE_A_SCRAMBLE_RESET; + else + tmp |= PIPE_B_SCRAMBLE_RESET; + + I915_WRITE(PORT_DFT2_G4X, tmp); + } + + return 0; +} + +static void vlv_undo_pipe_scramble_reset(struct drm_device *dev, + enum pipe pipe) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + uint32_t tmp = I915_READ(PORT_DFT2_G4X); + + if (pipe == PIPE_A) + tmp &= ~PIPE_A_SCRAMBLE_RESET; + else + tmp &= ~PIPE_B_SCRAMBLE_RESET; + if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) + tmp &= ~DC_BALANCE_RESET_VLV; + I915_WRITE(PORT_DFT2_G4X, tmp); + +} + +static void g4x_undo_pipe_scramble_reset(struct drm_device *dev, + enum pipe pipe) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + uint32_t tmp = I915_READ(PORT_DFT2_G4X); + + if (pipe == PIPE_A) + tmp &= ~PIPE_A_SCRAMBLE_RESET; + else + tmp &= ~PIPE_B_SCRAMBLE_RESET; + I915_WRITE(PORT_DFT2_G4X, tmp); + + if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) { + I915_WRITE(PORT_DFT_I9XX, + I915_READ(PORT_DFT_I9XX) & ~DC_BALANCE_RESET); + } +} + +static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, + uint32_t *val) +{ + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) + *source = INTEL_PIPE_CRC_SOURCE_PIPE; + + switch (*source) { + case INTEL_PIPE_CRC_SOURCE_PLANE1: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_ILK; + break; + case INTEL_PIPE_CRC_SOURCE_PLANE2: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_ILK; + break; + case INTEL_PIPE_CRC_SOURCE_PIPE: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_ILK; + break; + case INTEL_PIPE_CRC_SOURCE_NONE: + *val = 0; + break; + default: + return -EINVAL; + } + + return 0; +} + +static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, + uint32_t *val) +{ + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) + *source = INTEL_PIPE_CRC_SOURCE_PF; + + switch (*source) { + case INTEL_PIPE_CRC_SOURCE_PLANE1: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_IVB; + break; + case INTEL_PIPE_CRC_SOURCE_PLANE2: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB; + break; + case INTEL_PIPE_CRC_SOURCE_PF: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB; + break; + case INTEL_PIPE_CRC_SOURCE_NONE: + *val = 0; + break; + default: + return -EINVAL; + } + + return 0; +} + +static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, + enum intel_pipe_crc_source source) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; + u32 val = 0; /* shut up gcc */ + int ret; + + if (pipe_crc->source == source) + return 0; + + /* forbid changing the source without going back to 'none' */ + if (pipe_crc->source && source) + return -EINVAL; + + if (IS_GEN2(dev)) + ret = i8xx_pipe_crc_ctl_reg(&source, &val); + else if (INTEL_INFO(dev)->gen < 5) + ret = i9xx_pipe_crc_ctl_reg(dev, pipe, &source, &val); + else if (IS_VALLEYVIEW(dev)) + ret = vlv_pipe_crc_ctl_reg(dev,pipe, &source, &val); + else if (IS_GEN5(dev) || IS_GEN6(dev)) + ret = ilk_pipe_crc_ctl_reg(&source, &val); + else + ret = ivb_pipe_crc_ctl_reg(&source, &val); + + if (ret != 0) + return ret; + + /* none -> real source transition */ + if (source) { + DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n", + pipe_name(pipe), pipe_crc_source_name(source)); + + pipe_crc->entries = kzalloc(sizeof(*pipe_crc->entries) * + INTEL_PIPE_CRC_ENTRIES_NR, + GFP_KERNEL); + if (!pipe_crc->entries) + return -ENOMEM; + + spin_lock_irq(&pipe_crc->lock); + pipe_crc->head = 0; + pipe_crc->tail = 0; + spin_unlock_irq(&pipe_crc->lock); + } + + pipe_crc->source = source; + + I915_WRITE(PIPE_CRC_CTL(pipe), val); + POSTING_READ(PIPE_CRC_CTL(pipe)); + + /* real source -> none transition */ + if (source == INTEL_PIPE_CRC_SOURCE_NONE) { + struct intel_pipe_crc_entry *entries; + + DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n", + pipe_name(pipe)); + + intel_wait_for_vblank(dev, pipe); + + spin_lock_irq(&pipe_crc->lock); + entries = pipe_crc->entries; + pipe_crc->entries = NULL; + spin_unlock_irq(&pipe_crc->lock); + + kfree(entries); + + if (IS_G4X(dev)) + g4x_undo_pipe_scramble_reset(dev, pipe); + else if (IS_VALLEYVIEW(dev)) + vlv_undo_pipe_scramble_reset(dev, pipe); + } + + return 0; +} + +/* + * Parse pipe CRC command strings: + * command: wsp* object wsp+ name wsp+ source wsp* + * object: 'pipe' + * name: (A | B | C) + * source: (none | plane1 | plane2 | pf) + * wsp: (#0x20 | #0x9 | #0xA)+ + * + * eg.: + * "pipe A plane1" -> Start CRC computations on plane1 of pipe A + * "pipe A none" -> Stop CRC + */ +static int display_crc_ctl_tokenize(char *buf, char *words[], int max_words) +{ + int n_words = 0; + + while (*buf) { + char *end; + + /* skip leading white space */ + buf = skip_spaces(buf); + if (!*buf) + break; /* end of buffer */ + + /* find end of word */ + for (end = buf; *end && !isspace(*end); end++) + ; + + if (n_words == max_words) { + DRM_DEBUG_DRIVER("too many words, allowed <= %d\n", + max_words); + return -EINVAL; /* ran out of words[] before bytes */ + } + + if (*end) + *end++ = '\0'; + words[n_words++] = buf; + buf = end; + } + + return n_words; +} + +enum intel_pipe_crc_object { + PIPE_CRC_OBJECT_PIPE, +}; + +static const char * const pipe_crc_objects[] = { + "pipe", +}; + +static int +display_crc_ctl_parse_object(const char *buf, enum intel_pipe_crc_object *o) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(pipe_crc_objects); i++) + if (!strcmp(buf, pipe_crc_objects[i])) { + *o = i; + return 0; + } + + return -EINVAL; +} + +static int display_crc_ctl_parse_pipe(const char *buf, enum pipe *pipe) +{ + const char name = buf[0]; + + if (name < 'A' || name >= pipe_name(I915_MAX_PIPES)) + return -EINVAL; + + *pipe = name - 'A'; + + return 0; +} + +static int +display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(pipe_crc_sources); i++) + if (!strcmp(buf, pipe_crc_sources[i])) { + *s = i; + return 0; + } + + return -EINVAL; +} + +static int display_crc_ctl_parse(struct drm_device *dev, char *buf, size_t len) +{ +#define N_WORDS 3 + int n_words; + char *words[N_WORDS]; + enum pipe pipe; + enum intel_pipe_crc_object object; + enum intel_pipe_crc_source source; + + n_words = display_crc_ctl_tokenize(buf, words, N_WORDS); + if (n_words != N_WORDS) { + DRM_DEBUG_DRIVER("tokenize failed, a command is %d words\n", + N_WORDS); + return -EINVAL; + } + + if (display_crc_ctl_parse_object(words[0], &object) < 0) { + DRM_DEBUG_DRIVER("unknown object %s\n", words[0]); + return -EINVAL; + } + + if (display_crc_ctl_parse_pipe(words[1], &pipe) < 0) { + DRM_DEBUG_DRIVER("unknown pipe %s\n", words[1]); + return -EINVAL; + } + + if (display_crc_ctl_parse_source(words[2], &source) < 0) { + DRM_DEBUG_DRIVER("unknown source %s\n", words[2]); + return -EINVAL; + } + + return pipe_crc_set_source(dev, pipe, source); +} + +static ssize_t display_crc_ctl_write(struct file *file, const char __user *ubuf, + size_t len, loff_t *offp) +{ + struct seq_file *m = file->private_data; + struct drm_device *dev = m->private; + char *tmpbuf; + int ret; + + if (len == 0) + return 0; + + if (len > PAGE_SIZE - 1) { + DRM_DEBUG_DRIVER("expected <%lu bytes into pipe crc control\n", + PAGE_SIZE); + return -E2BIG; + } + + tmpbuf = kmalloc(len + 1, GFP_KERNEL); + if (!tmpbuf) + return -ENOMEM; + + if (copy_from_user(tmpbuf, ubuf, len)) { + ret = -EFAULT; + goto out; + } + tmpbuf[len] = '\0'; + + ret = display_crc_ctl_parse(dev, tmpbuf, len); + +out: + kfree(tmpbuf); + if (ret < 0) + return ret; + + *offp += len; + return len; +} + +void intel_display_crc_init(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + enum pipe pipe; + + for_each_pipe(pipe) { + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; + + pipe_crc->opened = false; + spin_lock_init(&pipe_crc->lock); + init_waitqueue_head(&pipe_crc->wq); + } +} + +const struct file_operations i915_display_crc_ctl_fops = { + .owner = THIS_MODULE, + .open = display_crc_ctl_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, + .write = display_crc_ctl_write +}; -- 1.8.5.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file 2014-01-14 14:14 ` [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file Ben Widawsky @ 2014-01-14 23:28 ` Daniel Vetter 2014-01-15 1:22 ` Damien Lespiau 1 sibling, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2014-01-14 23:28 UTC (permalink / raw) To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky On Tue, Jan 14, 2014 at 06:14:07AM -0800, Ben Widawsky wrote: > At almost 800 lines of code, with almost a function per platform, this > code was cluttering up the existing debugfs file, which has > historically had fairly small functions. > > Patch should have no functional changes. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/Makefile | 2 +- > drivers/gpu/drm/i915/i915_debugfs.c | 764 ---------------------------------- > drivers/gpu/drm/i915/i915_drv.h | 12 + > drivers/gpu/drm/i915/i915_pipecrc.c | 790 ++++++++++++++++++++++++++++++++++++ Yay, I like. But since I've asked for a respin on the first patch already I think more consistency here wont hurt either ... Can we call this intel_pipe_crc.c? Display stuff gets an intel_ prefix usually. The i915_ prefixes you could either drop (for static functions) or rename to intel_. -Daniel > 4 files changed, 803 insertions(+), 765 deletions(-) > create mode 100644 drivers/gpu/drm/i915/i915_pipecrc.c > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index da682cb..432d231 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -54,7 +54,7 @@ i915-$(CONFIG_ACPI) += intel_acpi.o > > i915-$(CONFIG_DRM_I915_FBDEV) += intel_fbdev.o > > -i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o > +i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o i915_pipecrc.o > > obj-$(CONFIG_DRM_I915) += i915.o > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index f2ef30c..c98f91b 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -27,8 +27,6 @@ > */ > > #include <linux/seq_file.h> > -#include <linux/circ_buf.h> > -#include <linux/ctype.h> > #include <linux/debugfs.h> > #include <linux/slab.h> > #include <linux/export.h> > @@ -1956,754 +1954,6 @@ static int i915_power_domain_info(struct seq_file *m, void *unused) > return 0; > } > > -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) > -{ > - struct pipe_crc_info *info = inode->i_private; > - struct drm_i915_private *dev_priv = info->dev->dev_private; > - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; > - > - if (info->pipe >= INTEL_INFO(info->dev)->num_pipes) > - return -ENODEV; > - > - spin_lock_irq(&pipe_crc->lock); > - > - if (pipe_crc->opened) { > - spin_unlock_irq(&pipe_crc->lock); > - return -EBUSY; /* already open */ > - } > - > - pipe_crc->opened = true; > - filep->private_data = inode->i_private; > - > - spin_unlock_irq(&pipe_crc->lock); > - > - return 0; > -} > - > -static int i915_pipe_crc_release(struct inode *inode, struct file *filep) > -{ > - struct pipe_crc_info *info = inode->i_private; > - struct drm_i915_private *dev_priv = info->dev->dev_private; > - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; > - > - spin_lock_irq(&pipe_crc->lock); > - pipe_crc->opened = false; > - spin_unlock_irq(&pipe_crc->lock); > - > - 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) > -{ > - assert_spin_locked(&pipe_crc->lock); > - return CIRC_CNT(pipe_crc->head, pipe_crc->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 */ > - spin_lock_irq(&pipe_crc->lock); > - while (pipe_crc_data_count(pipe_crc) == 0) { > - int ret; > - > - if (filep->f_flags & O_NONBLOCK) { > - spin_unlock_irq(&pipe_crc->lock); > - return -EAGAIN; > - } > - > - ret = wait_event_interruptible_lock_irq(pipe_crc->wq, > - pipe_crc_data_count(pipe_crc), pipe_crc->lock); > - if (ret) { > - spin_unlock_irq(&pipe_crc->lock); > - return ret; > - } > - } > - > - /* We now have one or more entries to read */ > - head = pipe_crc->head; > - tail = pipe_crc->tail; > - n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR), > - count / PIPE_CRC_LINE_LEN); > - spin_unlock_irq(&pipe_crc->lock); > - > - bytes_read = 0; > - n = 0; > - do { > - struct intel_pipe_crc_entry *entry = &pipe_crc->entries[tail]; > - int ret; > - > - 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); > - n++; > - } while (--n_entries); > - > - spin_lock_irq(&pipe_crc->lock); > - pipe_crc->tail = tail; > - spin_unlock_irq(&pipe_crc->lock); > - > - 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 (!ent) > - return -ENOMEM; > - > - return drm_debugfs_add_fake_info_node(minor, ent, info); > -} > - > -static const char * const pipe_crc_sources[] = { > - "none", > - "plane1", > - "plane2", > - "pf", > - "pipe", > - "TV", > - "DP-B", > - "DP-C", > - "DP-D", > - "auto", > -}; > - > -static const char *pipe_crc_source_name(enum intel_pipe_crc_source source) > -{ > - BUILD_BUG_ON(ARRAY_SIZE(pipe_crc_sources) != INTEL_PIPE_CRC_SOURCE_MAX); > - return pipe_crc_sources[source]; > -} > - > -static int display_crc_ctl_show(struct seq_file *m, void *data) > -{ > - struct drm_device *dev = m->private; > - struct drm_i915_private *dev_priv = dev->dev_private; > - int i; > - > - for (i = 0; i < I915_MAX_PIPES; i++) > - seq_printf(m, "%c %s\n", pipe_name(i), > - pipe_crc_source_name(dev_priv->pipe_crc[i].source)); > - > - return 0; > -} > - > -static int display_crc_ctl_open(struct inode *inode, struct file *file) > -{ > - struct drm_device *dev = inode->i_private; > - > - return single_open(file, display_crc_ctl_show, dev); > -} > - > -static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, > - uint32_t *val) > -{ > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > - *source = INTEL_PIPE_CRC_SOURCE_PIPE; > - > - switch (*source) { > - case INTEL_PIPE_CRC_SOURCE_PIPE: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_INCLUDE_BORDER_I8XX; > - break; > - case INTEL_PIPE_CRC_SOURCE_NONE: > - *val = 0; > - break; > - default: > - return -EINVAL; > - } > - > - return 0; > -} > - > -static int i9xx_pipe_crc_auto_source(struct drm_device *dev, enum pipe pipe, > - enum intel_pipe_crc_source *source) > -{ > - struct intel_encoder *encoder; > - struct intel_crtc *crtc; > - struct intel_digital_port *dig_port; > - int ret = 0; > - > - *source = INTEL_PIPE_CRC_SOURCE_PIPE; > - > - mutex_lock(&dev->mode_config.mutex); > - list_for_each_entry(encoder, &dev->mode_config.encoder_list, > - base.head) { > - if (!encoder->base.crtc) > - continue; > - > - crtc = to_intel_crtc(encoder->base.crtc); > - > - if (crtc->pipe != pipe) > - continue; > - > - switch (encoder->type) { > - case INTEL_OUTPUT_TVOUT: > - *source = INTEL_PIPE_CRC_SOURCE_TV; > - break; > - case INTEL_OUTPUT_DISPLAYPORT: > - case INTEL_OUTPUT_EDP: > - dig_port = enc_to_dig_port(&encoder->base); > - switch (dig_port->port) { > - case PORT_B: > - *source = INTEL_PIPE_CRC_SOURCE_DP_B; > - break; > - case PORT_C: > - *source = INTEL_PIPE_CRC_SOURCE_DP_C; > - break; > - case PORT_D: > - *source = INTEL_PIPE_CRC_SOURCE_DP_D; > - break; > - default: > - WARN(1, "nonexisting DP port %c\n", > - port_name(dig_port->port)); > - break; > - } > - break; > - } > - } > - mutex_unlock(&dev->mode_config.mutex); > - > - return ret; > -} > - > -static int vlv_pipe_crc_ctl_reg(struct drm_device *dev, > - enum pipe pipe, > - enum intel_pipe_crc_source *source, > - uint32_t *val) > -{ > - struct drm_i915_private *dev_priv = dev->dev_private; > - bool need_stable_symbols = false; > - > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { > - int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); > - if (ret) > - return ret; > - } > - > - switch (*source) { > - case INTEL_PIPE_CRC_SOURCE_PIPE: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_VLV; > - break; > - case INTEL_PIPE_CRC_SOURCE_DP_B: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_VLV; > - need_stable_symbols = true; > - break; > - case INTEL_PIPE_CRC_SOURCE_DP_C: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_VLV; > - need_stable_symbols = true; > - break; > - case INTEL_PIPE_CRC_SOURCE_NONE: > - *val = 0; > - break; > - default: > - return -EINVAL; > - } > - > - /* > - * When the pipe CRC tap point is after the transcoders we need > - * to tweak symbol-level features to produce a deterministic series of > - * symbols for a given frame. We need to reset those features only once > - * a frame (instead of every nth symbol): > - * - DC-balance: used to ensure a better clock recovery from the data > - * link (SDVO) > - * - DisplayPort scrambling: used for EMI reduction > - */ > - if (need_stable_symbols) { > - uint32_t tmp = I915_READ(PORT_DFT2_G4X); > - > - WARN_ON(!IS_G4X(dev)); > - > - tmp |= DC_BALANCE_RESET_VLV; > - if (pipe == PIPE_A) > - tmp |= PIPE_A_SCRAMBLE_RESET; > - else > - tmp |= PIPE_B_SCRAMBLE_RESET; > - > - I915_WRITE(PORT_DFT2_G4X, tmp); > - } > - > - return 0; > -} > - > -static int i9xx_pipe_crc_ctl_reg(struct drm_device *dev, > - enum pipe pipe, > - enum intel_pipe_crc_source *source, > - uint32_t *val) > -{ > - struct drm_i915_private *dev_priv = dev->dev_private; > - bool need_stable_symbols = false; > - > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { > - int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); > - if (ret) > - return ret; > - } > - > - switch (*source) { > - case INTEL_PIPE_CRC_SOURCE_PIPE: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_I9XX; > - break; > - case INTEL_PIPE_CRC_SOURCE_TV: > - if (!SUPPORTS_TV(dev)) > - return -EINVAL; > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_TV_PRE; > - break; > - case INTEL_PIPE_CRC_SOURCE_DP_B: > - if (!IS_G4X(dev)) > - return -EINVAL; > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_G4X; > - need_stable_symbols = true; > - break; > - case INTEL_PIPE_CRC_SOURCE_DP_C: > - if (!IS_G4X(dev)) > - return -EINVAL; > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_G4X; > - need_stable_symbols = true; > - break; > - case INTEL_PIPE_CRC_SOURCE_DP_D: > - if (!IS_G4X(dev)) > - return -EINVAL; > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_D_G4X; > - need_stable_symbols = true; > - break; > - case INTEL_PIPE_CRC_SOURCE_NONE: > - *val = 0; > - break; > - default: > - return -EINVAL; > - } > - > - /* > - * When the pipe CRC tap point is after the transcoders we need > - * to tweak symbol-level features to produce a deterministic series of > - * symbols for a given frame. We need to reset those features only once > - * a frame (instead of every nth symbol): > - * - DC-balance: used to ensure a better clock recovery from the data > - * link (SDVO) > - * - DisplayPort scrambling: used for EMI reduction > - */ > - if (need_stable_symbols) { > - uint32_t tmp = I915_READ(PORT_DFT2_G4X); > - > - WARN_ON(!IS_G4X(dev)); > - > - I915_WRITE(PORT_DFT_I9XX, > - I915_READ(PORT_DFT_I9XX) | DC_BALANCE_RESET); > - > - if (pipe == PIPE_A) > - tmp |= PIPE_A_SCRAMBLE_RESET; > - else > - tmp |= PIPE_B_SCRAMBLE_RESET; > - > - I915_WRITE(PORT_DFT2_G4X, tmp); > - } > - > - return 0; > -} > - > -static void vlv_undo_pipe_scramble_reset(struct drm_device *dev, > - enum pipe pipe) > -{ > - struct drm_i915_private *dev_priv = dev->dev_private; > - uint32_t tmp = I915_READ(PORT_DFT2_G4X); > - > - if (pipe == PIPE_A) > - tmp &= ~PIPE_A_SCRAMBLE_RESET; > - else > - tmp &= ~PIPE_B_SCRAMBLE_RESET; > - if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) > - tmp &= ~DC_BALANCE_RESET_VLV; > - I915_WRITE(PORT_DFT2_G4X, tmp); > - > -} > - > -static void g4x_undo_pipe_scramble_reset(struct drm_device *dev, > - enum pipe pipe) > -{ > - struct drm_i915_private *dev_priv = dev->dev_private; > - uint32_t tmp = I915_READ(PORT_DFT2_G4X); > - > - if (pipe == PIPE_A) > - tmp &= ~PIPE_A_SCRAMBLE_RESET; > - else > - tmp &= ~PIPE_B_SCRAMBLE_RESET; > - I915_WRITE(PORT_DFT2_G4X, tmp); > - > - if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) { > - I915_WRITE(PORT_DFT_I9XX, > - I915_READ(PORT_DFT_I9XX) & ~DC_BALANCE_RESET); > - } > -} > - > -static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, > - uint32_t *val) > -{ > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > - *source = INTEL_PIPE_CRC_SOURCE_PIPE; > - > - switch (*source) { > - case INTEL_PIPE_CRC_SOURCE_PLANE1: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_ILK; > - break; > - case INTEL_PIPE_CRC_SOURCE_PLANE2: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_ILK; > - break; > - case INTEL_PIPE_CRC_SOURCE_PIPE: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_ILK; > - break; > - case INTEL_PIPE_CRC_SOURCE_NONE: > - *val = 0; > - break; > - default: > - return -EINVAL; > - } > - > - return 0; > -} > - > -static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, > - uint32_t *val) > -{ > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > - *source = INTEL_PIPE_CRC_SOURCE_PF; > - > - switch (*source) { > - case INTEL_PIPE_CRC_SOURCE_PLANE1: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_IVB; > - break; > - case INTEL_PIPE_CRC_SOURCE_PLANE2: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB; > - break; > - case INTEL_PIPE_CRC_SOURCE_PF: > - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB; > - break; > - case INTEL_PIPE_CRC_SOURCE_NONE: > - *val = 0; > - break; > - default: > - return -EINVAL; > - } > - > - return 0; > -} > - > -static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, > - enum intel_pipe_crc_source source) > -{ > - struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; > - u32 val = 0; /* shut up gcc */ > - int ret; > - > - if (pipe_crc->source == source) > - return 0; > - > - /* forbid changing the source without going back to 'none' */ > - if (pipe_crc->source && source) > - return -EINVAL; > - > - if (IS_GEN2(dev)) > - ret = i8xx_pipe_crc_ctl_reg(&source, &val); > - else if (INTEL_INFO(dev)->gen < 5) > - ret = i9xx_pipe_crc_ctl_reg(dev, pipe, &source, &val); > - else if (IS_VALLEYVIEW(dev)) > - ret = vlv_pipe_crc_ctl_reg(dev,pipe, &source, &val); > - else if (IS_GEN5(dev) || IS_GEN6(dev)) > - ret = ilk_pipe_crc_ctl_reg(&source, &val); > - else > - ret = ivb_pipe_crc_ctl_reg(&source, &val); > - > - if (ret != 0) > - return ret; > - > - /* none -> real source transition */ > - if (source) { > - DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n", > - pipe_name(pipe), pipe_crc_source_name(source)); > - > - pipe_crc->entries = kzalloc(sizeof(*pipe_crc->entries) * > - INTEL_PIPE_CRC_ENTRIES_NR, > - GFP_KERNEL); > - if (!pipe_crc->entries) > - return -ENOMEM; > - > - spin_lock_irq(&pipe_crc->lock); > - pipe_crc->head = 0; > - pipe_crc->tail = 0; > - spin_unlock_irq(&pipe_crc->lock); > - } > - > - pipe_crc->source = source; > - > - I915_WRITE(PIPE_CRC_CTL(pipe), val); > - POSTING_READ(PIPE_CRC_CTL(pipe)); > - > - /* real source -> none transition */ > - if (source == INTEL_PIPE_CRC_SOURCE_NONE) { > - struct intel_pipe_crc_entry *entries; > - > - DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n", > - pipe_name(pipe)); > - > - intel_wait_for_vblank(dev, pipe); > - > - spin_lock_irq(&pipe_crc->lock); > - entries = pipe_crc->entries; > - pipe_crc->entries = NULL; > - spin_unlock_irq(&pipe_crc->lock); > - > - kfree(entries); > - > - if (IS_G4X(dev)) > - g4x_undo_pipe_scramble_reset(dev, pipe); > - else if (IS_VALLEYVIEW(dev)) > - vlv_undo_pipe_scramble_reset(dev, pipe); > - } > - > - return 0; > -} > - > -/* > - * Parse pipe CRC command strings: > - * command: wsp* object wsp+ name wsp+ source wsp* > - * object: 'pipe' > - * name: (A | B | C) > - * source: (none | plane1 | plane2 | pf) > - * wsp: (#0x20 | #0x9 | #0xA)+ > - * > - * eg.: > - * "pipe A plane1" -> Start CRC computations on plane1 of pipe A > - * "pipe A none" -> Stop CRC > - */ > -static int display_crc_ctl_tokenize(char *buf, char *words[], int max_words) > -{ > - int n_words = 0; > - > - while (*buf) { > - char *end; > - > - /* skip leading white space */ > - buf = skip_spaces(buf); > - if (!*buf) > - break; /* end of buffer */ > - > - /* find end of word */ > - for (end = buf; *end && !isspace(*end); end++) > - ; > - > - if (n_words == max_words) { > - DRM_DEBUG_DRIVER("too many words, allowed <= %d\n", > - max_words); > - return -EINVAL; /* ran out of words[] before bytes */ > - } > - > - if (*end) > - *end++ = '\0'; > - words[n_words++] = buf; > - buf = end; > - } > - > - return n_words; > -} > - > -enum intel_pipe_crc_object { > - PIPE_CRC_OBJECT_PIPE, > -}; > - > -static const char * const pipe_crc_objects[] = { > - "pipe", > -}; > - > -static int > -display_crc_ctl_parse_object(const char *buf, enum intel_pipe_crc_object *o) > -{ > - int i; > - > - for (i = 0; i < ARRAY_SIZE(pipe_crc_objects); i++) > - if (!strcmp(buf, pipe_crc_objects[i])) { > - *o = i; > - return 0; > - } > - > - return -EINVAL; > -} > - > -static int display_crc_ctl_parse_pipe(const char *buf, enum pipe *pipe) > -{ > - const char name = buf[0]; > - > - if (name < 'A' || name >= pipe_name(I915_MAX_PIPES)) > - return -EINVAL; > - > - *pipe = name - 'A'; > - > - return 0; > -} > - > -static int > -display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s) > -{ > - int i; > - > - for (i = 0; i < ARRAY_SIZE(pipe_crc_sources); i++) > - if (!strcmp(buf, pipe_crc_sources[i])) { > - *s = i; > - return 0; > - } > - > - return -EINVAL; > -} > - > -static int display_crc_ctl_parse(struct drm_device *dev, char *buf, size_t len) > -{ > -#define N_WORDS 3 > - int n_words; > - char *words[N_WORDS]; > - enum pipe pipe; > - enum intel_pipe_crc_object object; > - enum intel_pipe_crc_source source; > - > - n_words = display_crc_ctl_tokenize(buf, words, N_WORDS); > - if (n_words != N_WORDS) { > - DRM_DEBUG_DRIVER("tokenize failed, a command is %d words\n", > - N_WORDS); > - return -EINVAL; > - } > - > - if (display_crc_ctl_parse_object(words[0], &object) < 0) { > - DRM_DEBUG_DRIVER("unknown object %s\n", words[0]); > - return -EINVAL; > - } > - > - if (display_crc_ctl_parse_pipe(words[1], &pipe) < 0) { > - DRM_DEBUG_DRIVER("unknown pipe %s\n", words[1]); > - return -EINVAL; > - } > - > - if (display_crc_ctl_parse_source(words[2], &source) < 0) { > - DRM_DEBUG_DRIVER("unknown source %s\n", words[2]); > - return -EINVAL; > - } > - > - return pipe_crc_set_source(dev, pipe, source); > -} > - > -static ssize_t display_crc_ctl_write(struct file *file, const char __user *ubuf, > - size_t len, loff_t *offp) > -{ > - struct seq_file *m = file->private_data; > - struct drm_device *dev = m->private; > - char *tmpbuf; > - int ret; > - > - if (len == 0) > - return 0; > - > - if (len > PAGE_SIZE - 1) { > - DRM_DEBUG_DRIVER("expected <%lu bytes into pipe crc control\n", > - PAGE_SIZE); > - return -E2BIG; > - } > - > - tmpbuf = kmalloc(len + 1, GFP_KERNEL); > - if (!tmpbuf) > - return -ENOMEM; > - > - if (copy_from_user(tmpbuf, ubuf, len)) { > - ret = -EFAULT; > - goto out; > - } > - tmpbuf[len] = '\0'; > - > - ret = display_crc_ctl_parse(dev, tmpbuf, len); > - > -out: > - kfree(tmpbuf); > - if (ret < 0) > - return ret; > - > - *offp += len; > - return len; > -} > - > -static const struct file_operations i915_display_crc_ctl_fops = { > - .owner = THIS_MODULE, > - .open = display_crc_ctl_open, > - .read = seq_read, > - .llseek = seq_lseek, > - .release = single_release, > - .write = display_crc_ctl_write > -}; > - > static int > i915_wedged_get(void *data, u64 *val) > { > @@ -3219,20 +2469,6 @@ static const 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; > - enum pipe pipe; > - > - for_each_pipe(pipe) { > - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; > - > - pipe_crc->opened = false; > - spin_lock_init(&pipe_crc->lock); > - init_waitqueue_head(&pipe_crc->wq); > - } > -} > - > int i915_debugfs_init(struct drm_minor *minor) > { > int ret, i; > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index cc8afff..7ded723 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1356,6 +1356,18 @@ struct intel_pipe_crc { > wait_queue_head_t wq; > }; > > +struct pipe_crc_info { > + const char *name; > + struct drm_device *dev; > + enum pipe pipe; > +}; > + > +extern struct pipe_crc_info i915_pipe_crc_data[I915_MAX_PIPES]; > +extern const struct file_operations i915_display_crc_ctl_fops; > + > +int i915_pipe_crc_create(struct dentry *root, struct drm_minor *minor, > + enum pipe pipe); > + > typedef struct drm_i915_private { > struct drm_device *dev; > struct kmem_cache *slab; > diff --git a/drivers/gpu/drm/i915/i915_pipecrc.c b/drivers/gpu/drm/i915/i915_pipecrc.c > new file mode 100644 > index 0000000..7b0bec6 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_pipecrc.c > @@ -0,0 +1,790 @@ > +/* > + * Copyright © 2014 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + */ > + > +#include <linux/debugfs.h> > +#include <linux/fs.h> > +#include <linux/seq_file.h> > +#include <linux/circ_buf.h> > +#include <linux/ctype.h> > +#if 0 > +#include <drm/drm_crtc.h> > +#include <drm/drmP.h> > +#endif > +#include "i915_drv.h" > +#include "intel_drv.h" > + > +static int i915_pipe_crc_open(struct inode *inode, struct file *filep) > +{ > + struct pipe_crc_info *info = inode->i_private; > + struct drm_i915_private *dev_priv = info->dev->dev_private; > + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; > + > + if (info->pipe >= INTEL_INFO(info->dev)->num_pipes) > + return -ENODEV; > + > + spin_lock_irq(&pipe_crc->lock); > + > + if (pipe_crc->opened) { > + spin_unlock_irq(&pipe_crc->lock); > + return -EBUSY; /* already open */ > + } > + > + pipe_crc->opened = true; > + filep->private_data = inode->i_private; > + > + spin_unlock_irq(&pipe_crc->lock); > + > + return 0; > +} > + > +static int i915_pipe_crc_release(struct inode *inode, struct file *filep) > +{ > + struct pipe_crc_info *info = inode->i_private; > + struct drm_i915_private *dev_priv = info->dev->dev_private; > + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; > + > + spin_lock_irq(&pipe_crc->lock); > + pipe_crc->opened = false; > + spin_unlock_irq(&pipe_crc->lock); > + > + 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) > +{ > + assert_spin_locked(&pipe_crc->lock); > + return CIRC_CNT(pipe_crc->head, pipe_crc->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 */ > + spin_lock_irq(&pipe_crc->lock); > + while (pipe_crc_data_count(pipe_crc) == 0) { > + int ret; > + > + if (filep->f_flags & O_NONBLOCK) { > + spin_unlock_irq(&pipe_crc->lock); > + return -EAGAIN; > + } > + > + ret = wait_event_interruptible_lock_irq(pipe_crc->wq, > + pipe_crc_data_count(pipe_crc), pipe_crc->lock); > + if (ret) { > + spin_unlock_irq(&pipe_crc->lock); > + return ret; > + } > + } > + > + /* We now have one or more entries to read */ > + head = pipe_crc->head; > + tail = pipe_crc->tail; > + n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR), > + count / PIPE_CRC_LINE_LEN); > + spin_unlock_irq(&pipe_crc->lock); > + > + bytes_read = 0; > + n = 0; > + do { > + struct intel_pipe_crc_entry *entry = &pipe_crc->entries[tail]; > + int ret; > + > + 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); > + n++; > + } while (--n_entries); > + > + spin_lock_irq(&pipe_crc->lock); > + pipe_crc->tail = tail; > + spin_unlock_irq(&pipe_crc->lock); > + > + return bytes_read; > +} > + > +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, > +}; > + > +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, > + }, > +}; > + > +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 (!ent) > + return -ENOMEM; > + > + return drm_debugfs_add_fake_info_node(minor, ent, info); > +} > + > +static const char * const pipe_crc_sources[] = { > + "none", > + "plane1", > + "plane2", > + "pf", > + "pipe", > + "TV", > + "DP-B", > + "DP-C", > + "DP-D", > + "auto", > +}; > + > +static const char *pipe_crc_source_name(enum intel_pipe_crc_source source) > +{ > + BUILD_BUG_ON(ARRAY_SIZE(pipe_crc_sources) != INTEL_PIPE_CRC_SOURCE_MAX); > + return pipe_crc_sources[source]; > +} > + > +static int display_crc_ctl_show(struct seq_file *m, void *data) > +{ > + struct drm_device *dev = m->private; > + struct drm_i915_private *dev_priv = dev->dev_private; > + int i; > + > + for (i = 0; i < I915_MAX_PIPES; i++) > + seq_printf(m, "%c %s\n", pipe_name(i), > + pipe_crc_source_name(dev_priv->pipe_crc[i].source)); > + > + return 0; > +} > + > +static int display_crc_ctl_open(struct inode *inode, struct file *file) > +{ > + struct drm_device *dev = inode->i_private; > + > + return single_open(file, display_crc_ctl_show, dev); > +} > + > +static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, > + uint32_t *val) > +{ > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > + *source = INTEL_PIPE_CRC_SOURCE_PIPE; > + > + switch (*source) { > + case INTEL_PIPE_CRC_SOURCE_PIPE: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_INCLUDE_BORDER_I8XX; > + break; > + case INTEL_PIPE_CRC_SOURCE_NONE: > + *val = 0; > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int i9xx_pipe_crc_auto_source(struct drm_device *dev, enum pipe pipe, > + enum intel_pipe_crc_source *source) > +{ > + struct intel_encoder *encoder; > + struct intel_crtc *crtc; > + struct intel_digital_port *dig_port; > + int ret = 0; > + > + *source = INTEL_PIPE_CRC_SOURCE_PIPE; > + > + mutex_lock(&dev->mode_config.mutex); > + list_for_each_entry(encoder, &dev->mode_config.encoder_list, > + base.head) { > + if (!encoder->base.crtc) > + continue; > + > + crtc = to_intel_crtc(encoder->base.crtc); > + > + if (crtc->pipe != pipe) > + continue; > + > + switch (encoder->type) { > + case INTEL_OUTPUT_TVOUT: > + *source = INTEL_PIPE_CRC_SOURCE_TV; > + break; > + case INTEL_OUTPUT_DISPLAYPORT: > + case INTEL_OUTPUT_EDP: > + dig_port = enc_to_dig_port(&encoder->base); > + switch (dig_port->port) { > + case PORT_B: > + *source = INTEL_PIPE_CRC_SOURCE_DP_B; > + break; > + case PORT_C: > + *source = INTEL_PIPE_CRC_SOURCE_DP_C; > + break; > + case PORT_D: > + *source = INTEL_PIPE_CRC_SOURCE_DP_D; > + break; > + default: > + WARN(1, "nonexisting DP port %c\n", > + port_name(dig_port->port)); > + break; > + } > + break; > + } > + } > + mutex_unlock(&dev->mode_config.mutex); > + > + return ret; > +} > + > +static int vlv_pipe_crc_ctl_reg(struct drm_device *dev, > + enum pipe pipe, > + enum intel_pipe_crc_source *source, > + uint32_t *val) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + bool need_stable_symbols = false; > + > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { > + int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); > + if (ret) > + return ret; > + } > + > + switch (*source) { > + case INTEL_PIPE_CRC_SOURCE_PIPE: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_VLV; > + break; > + case INTEL_PIPE_CRC_SOURCE_DP_B: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_VLV; > + need_stable_symbols = true; > + break; > + case INTEL_PIPE_CRC_SOURCE_DP_C: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_VLV; > + need_stable_symbols = true; > + break; > + case INTEL_PIPE_CRC_SOURCE_NONE: > + *val = 0; > + break; > + default: > + return -EINVAL; > + } > + > + /* > + * When the pipe CRC tap point is after the transcoders we need > + * to tweak symbol-level features to produce a deterministic series of > + * symbols for a given frame. We need to reset those features only once > + * a frame (instead of every nth symbol): > + * - DC-balance: used to ensure a better clock recovery from the data > + * link (SDVO) > + * - DisplayPort scrambling: used for EMI reduction > + */ > + if (need_stable_symbols) { > + uint32_t tmp = I915_READ(PORT_DFT2_G4X); > + > + WARN_ON(!IS_G4X(dev)); > + > + tmp |= DC_BALANCE_RESET_VLV; > + if (pipe == PIPE_A) > + tmp |= PIPE_A_SCRAMBLE_RESET; > + else > + tmp |= PIPE_B_SCRAMBLE_RESET; > + > + I915_WRITE(PORT_DFT2_G4X, tmp); > + } > + > + return 0; > +} > + > +static int i9xx_pipe_crc_ctl_reg(struct drm_device *dev, > + enum pipe pipe, > + enum intel_pipe_crc_source *source, > + uint32_t *val) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + bool need_stable_symbols = false; > + > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { > + int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); > + if (ret) > + return ret; > + } > + > + switch (*source) { > + case INTEL_PIPE_CRC_SOURCE_PIPE: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_I9XX; > + break; > + case INTEL_PIPE_CRC_SOURCE_TV: > + if (!SUPPORTS_TV(dev)) > + return -EINVAL; > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_TV_PRE; > + break; > + case INTEL_PIPE_CRC_SOURCE_DP_B: > + if (!IS_G4X(dev)) > + return -EINVAL; > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_G4X; > + need_stable_symbols = true; > + break; > + case INTEL_PIPE_CRC_SOURCE_DP_C: > + if (!IS_G4X(dev)) > + return -EINVAL; > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_G4X; > + need_stable_symbols = true; > + break; > + case INTEL_PIPE_CRC_SOURCE_DP_D: > + if (!IS_G4X(dev)) > + return -EINVAL; > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_D_G4X; > + need_stable_symbols = true; > + break; > + case INTEL_PIPE_CRC_SOURCE_NONE: > + *val = 0; > + break; > + default: > + return -EINVAL; > + } > + > + /* > + * When the pipe CRC tap point is after the transcoders we need > + * to tweak symbol-level features to produce a deterministic series of > + * symbols for a given frame. We need to reset those features only once > + * a frame (instead of every nth symbol): > + * - DC-balance: used to ensure a better clock recovery from the data > + * link (SDVO) > + * - DisplayPort scrambling: used for EMI reduction > + */ > + if (need_stable_symbols) { > + uint32_t tmp = I915_READ(PORT_DFT2_G4X); > + > + WARN_ON(!IS_G4X(dev)); > + > + I915_WRITE(PORT_DFT_I9XX, > + I915_READ(PORT_DFT_I9XX) | DC_BALANCE_RESET); > + > + if (pipe == PIPE_A) > + tmp |= PIPE_A_SCRAMBLE_RESET; > + else > + tmp |= PIPE_B_SCRAMBLE_RESET; > + > + I915_WRITE(PORT_DFT2_G4X, tmp); > + } > + > + return 0; > +} > + > +static void vlv_undo_pipe_scramble_reset(struct drm_device *dev, > + enum pipe pipe) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + uint32_t tmp = I915_READ(PORT_DFT2_G4X); > + > + if (pipe == PIPE_A) > + tmp &= ~PIPE_A_SCRAMBLE_RESET; > + else > + tmp &= ~PIPE_B_SCRAMBLE_RESET; > + if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) > + tmp &= ~DC_BALANCE_RESET_VLV; > + I915_WRITE(PORT_DFT2_G4X, tmp); > + > +} > + > +static void g4x_undo_pipe_scramble_reset(struct drm_device *dev, > + enum pipe pipe) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + uint32_t tmp = I915_READ(PORT_DFT2_G4X); > + > + if (pipe == PIPE_A) > + tmp &= ~PIPE_A_SCRAMBLE_RESET; > + else > + tmp &= ~PIPE_B_SCRAMBLE_RESET; > + I915_WRITE(PORT_DFT2_G4X, tmp); > + > + if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) { > + I915_WRITE(PORT_DFT_I9XX, > + I915_READ(PORT_DFT_I9XX) & ~DC_BALANCE_RESET); > + } > +} > + > +static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, > + uint32_t *val) > +{ > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > + *source = INTEL_PIPE_CRC_SOURCE_PIPE; > + > + switch (*source) { > + case INTEL_PIPE_CRC_SOURCE_PLANE1: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_ILK; > + break; > + case INTEL_PIPE_CRC_SOURCE_PLANE2: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_ILK; > + break; > + case INTEL_PIPE_CRC_SOURCE_PIPE: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_ILK; > + break; > + case INTEL_PIPE_CRC_SOURCE_NONE: > + *val = 0; > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, > + uint32_t *val) > +{ > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > + *source = INTEL_PIPE_CRC_SOURCE_PF; > + > + switch (*source) { > + case INTEL_PIPE_CRC_SOURCE_PLANE1: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_IVB; > + break; > + case INTEL_PIPE_CRC_SOURCE_PLANE2: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB; > + break; > + case INTEL_PIPE_CRC_SOURCE_PF: > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB; > + break; > + case INTEL_PIPE_CRC_SOURCE_NONE: > + *val = 0; > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, > + enum intel_pipe_crc_source source) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; > + u32 val = 0; /* shut up gcc */ > + int ret; > + > + if (pipe_crc->source == source) > + return 0; > + > + /* forbid changing the source without going back to 'none' */ > + if (pipe_crc->source && source) > + return -EINVAL; > + > + if (IS_GEN2(dev)) > + ret = i8xx_pipe_crc_ctl_reg(&source, &val); > + else if (INTEL_INFO(dev)->gen < 5) > + ret = i9xx_pipe_crc_ctl_reg(dev, pipe, &source, &val); > + else if (IS_VALLEYVIEW(dev)) > + ret = vlv_pipe_crc_ctl_reg(dev,pipe, &source, &val); > + else if (IS_GEN5(dev) || IS_GEN6(dev)) > + ret = ilk_pipe_crc_ctl_reg(&source, &val); > + else > + ret = ivb_pipe_crc_ctl_reg(&source, &val); > + > + if (ret != 0) > + return ret; > + > + /* none -> real source transition */ > + if (source) { > + DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n", > + pipe_name(pipe), pipe_crc_source_name(source)); > + > + pipe_crc->entries = kzalloc(sizeof(*pipe_crc->entries) * > + INTEL_PIPE_CRC_ENTRIES_NR, > + GFP_KERNEL); > + if (!pipe_crc->entries) > + return -ENOMEM; > + > + spin_lock_irq(&pipe_crc->lock); > + pipe_crc->head = 0; > + pipe_crc->tail = 0; > + spin_unlock_irq(&pipe_crc->lock); > + } > + > + pipe_crc->source = source; > + > + I915_WRITE(PIPE_CRC_CTL(pipe), val); > + POSTING_READ(PIPE_CRC_CTL(pipe)); > + > + /* real source -> none transition */ > + if (source == INTEL_PIPE_CRC_SOURCE_NONE) { > + struct intel_pipe_crc_entry *entries; > + > + DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n", > + pipe_name(pipe)); > + > + intel_wait_for_vblank(dev, pipe); > + > + spin_lock_irq(&pipe_crc->lock); > + entries = pipe_crc->entries; > + pipe_crc->entries = NULL; > + spin_unlock_irq(&pipe_crc->lock); > + > + kfree(entries); > + > + if (IS_G4X(dev)) > + g4x_undo_pipe_scramble_reset(dev, pipe); > + else if (IS_VALLEYVIEW(dev)) > + vlv_undo_pipe_scramble_reset(dev, pipe); > + } > + > + return 0; > +} > + > +/* > + * Parse pipe CRC command strings: > + * command: wsp* object wsp+ name wsp+ source wsp* > + * object: 'pipe' > + * name: (A | B | C) > + * source: (none | plane1 | plane2 | pf) > + * wsp: (#0x20 | #0x9 | #0xA)+ > + * > + * eg.: > + * "pipe A plane1" -> Start CRC computations on plane1 of pipe A > + * "pipe A none" -> Stop CRC > + */ > +static int display_crc_ctl_tokenize(char *buf, char *words[], int max_words) > +{ > + int n_words = 0; > + > + while (*buf) { > + char *end; > + > + /* skip leading white space */ > + buf = skip_spaces(buf); > + if (!*buf) > + break; /* end of buffer */ > + > + /* find end of word */ > + for (end = buf; *end && !isspace(*end); end++) > + ; > + > + if (n_words == max_words) { > + DRM_DEBUG_DRIVER("too many words, allowed <= %d\n", > + max_words); > + return -EINVAL; /* ran out of words[] before bytes */ > + } > + > + if (*end) > + *end++ = '\0'; > + words[n_words++] = buf; > + buf = end; > + } > + > + return n_words; > +} > + > +enum intel_pipe_crc_object { > + PIPE_CRC_OBJECT_PIPE, > +}; > + > +static const char * const pipe_crc_objects[] = { > + "pipe", > +}; > + > +static int > +display_crc_ctl_parse_object(const char *buf, enum intel_pipe_crc_object *o) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(pipe_crc_objects); i++) > + if (!strcmp(buf, pipe_crc_objects[i])) { > + *o = i; > + return 0; > + } > + > + return -EINVAL; > +} > + > +static int display_crc_ctl_parse_pipe(const char *buf, enum pipe *pipe) > +{ > + const char name = buf[0]; > + > + if (name < 'A' || name >= pipe_name(I915_MAX_PIPES)) > + return -EINVAL; > + > + *pipe = name - 'A'; > + > + return 0; > +} > + > +static int > +display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(pipe_crc_sources); i++) > + if (!strcmp(buf, pipe_crc_sources[i])) { > + *s = i; > + return 0; > + } > + > + return -EINVAL; > +} > + > +static int display_crc_ctl_parse(struct drm_device *dev, char *buf, size_t len) > +{ > +#define N_WORDS 3 > + int n_words; > + char *words[N_WORDS]; > + enum pipe pipe; > + enum intel_pipe_crc_object object; > + enum intel_pipe_crc_source source; > + > + n_words = display_crc_ctl_tokenize(buf, words, N_WORDS); > + if (n_words != N_WORDS) { > + DRM_DEBUG_DRIVER("tokenize failed, a command is %d words\n", > + N_WORDS); > + return -EINVAL; > + } > + > + if (display_crc_ctl_parse_object(words[0], &object) < 0) { > + DRM_DEBUG_DRIVER("unknown object %s\n", words[0]); > + return -EINVAL; > + } > + > + if (display_crc_ctl_parse_pipe(words[1], &pipe) < 0) { > + DRM_DEBUG_DRIVER("unknown pipe %s\n", words[1]); > + return -EINVAL; > + } > + > + if (display_crc_ctl_parse_source(words[2], &source) < 0) { > + DRM_DEBUG_DRIVER("unknown source %s\n", words[2]); > + return -EINVAL; > + } > + > + return pipe_crc_set_source(dev, pipe, source); > +} > + > +static ssize_t display_crc_ctl_write(struct file *file, const char __user *ubuf, > + size_t len, loff_t *offp) > +{ > + struct seq_file *m = file->private_data; > + struct drm_device *dev = m->private; > + char *tmpbuf; > + int ret; > + > + if (len == 0) > + return 0; > + > + if (len > PAGE_SIZE - 1) { > + DRM_DEBUG_DRIVER("expected <%lu bytes into pipe crc control\n", > + PAGE_SIZE); > + return -E2BIG; > + } > + > + tmpbuf = kmalloc(len + 1, GFP_KERNEL); > + if (!tmpbuf) > + return -ENOMEM; > + > + if (copy_from_user(tmpbuf, ubuf, len)) { > + ret = -EFAULT; > + goto out; > + } > + tmpbuf[len] = '\0'; > + > + ret = display_crc_ctl_parse(dev, tmpbuf, len); > + > +out: > + kfree(tmpbuf); > + if (ret < 0) > + return ret; > + > + *offp += len; > + return len; > +} > + > +void intel_display_crc_init(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + enum pipe pipe; > + > + for_each_pipe(pipe) { > + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; > + > + pipe_crc->opened = false; > + spin_lock_init(&pipe_crc->lock); > + init_waitqueue_head(&pipe_crc->wq); > + } > +} > + > +const struct file_operations i915_display_crc_ctl_fops = { > + .owner = THIS_MODULE, > + .open = display_crc_ctl_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > + .write = display_crc_ctl_write > +}; > -- > 1.8.5.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file 2014-01-14 14:14 ` [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file Ben Widawsky 2014-01-14 23:28 ` Daniel Vetter @ 2014-01-15 1:22 ` Damien Lespiau 2014-01-15 8:46 ` Daniel Vetter 2014-01-15 20:04 ` Ben Widawsky 1 sibling, 2 replies; 19+ messages in thread From: Damien Lespiau @ 2014-01-15 1:22 UTC (permalink / raw) To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky On Tue, Jan 14, 2014 at 06:14:07AM -0800, Ben Widawsky wrote: > +#include <linux/debugfs.h> > +#include <linux/fs.h> > +#include <linux/seq_file.h> > +#include <linux/circ_buf.h> > +#include <linux/ctype.h> > +#if 0 > +#include <drm/drm_crtc.h> > +#include <drm/drmP.h> > +#endif Could remove that #if Last time I remember we talked about a intel_display_test.c where we could shove more stuff (Rodrigo's DP CRC maybe). Either that or Daniel last suggestion seems fine (definitely intel_ though, has to do with the display) -- Damien ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file 2014-01-15 1:22 ` Damien Lespiau @ 2014-01-15 8:46 ` Daniel Vetter 2014-01-15 20:04 ` Ben Widawsky 1 sibling, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2014-01-15 8:46 UTC (permalink / raw) To: Damien Lespiau; +Cc: Intel GFX, Ben Widawsky, Ben Widawsky On Wed, Jan 15, 2014 at 2:22 AM, Damien Lespiau <damien.lespiau@intel.com> wrote: > On Tue, Jan 14, 2014 at 06:14:07AM -0800, Ben Widawsky wrote: >> +#include <linux/debugfs.h> >> +#include <linux/fs.h> >> +#include <linux/seq_file.h> >> +#include <linux/circ_buf.h> >> +#include <linux/ctype.h> >> +#if 0 >> +#include <drm/drm_crtc.h> >> +#include <drm/drmP.h> >> +#endif > > Could remove that #if > > Last time I remember we talked about a intel_display_test.c where we > could shove more stuff (Rodrigo's DP CRC maybe). Either that or Daniel > last suggestion seems fine (definitely intel_ though, has to do with the > display) Concurred on intel_display_test.c, totally forgotten about that old discussion. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file 2014-01-15 1:22 ` Damien Lespiau 2014-01-15 8:46 ` Daniel Vetter @ 2014-01-15 20:04 ` Ben Widawsky 1 sibling, 0 replies; 19+ messages in thread From: Ben Widawsky @ 2014-01-15 20:04 UTC (permalink / raw) To: Damien Lespiau; +Cc: Intel GFX, Ben Widawsky On Wed, Jan 15, 2014 at 01:22:00AM +0000, Damien Lespiau wrote: > On Tue, Jan 14, 2014 at 06:14:07AM -0800, Ben Widawsky wrote: > > +#include <linux/debugfs.h> > > +#include <linux/fs.h> > > +#include <linux/seq_file.h> > > +#include <linux/circ_buf.h> > > +#include <linux/ctype.h> > > +#if 0 > > +#include <drm/drm_crtc.h> > > +#include <drm/drmP.h> > > +#endif > > Could remove that #if > Thanks, I left that in by accident. > Last time I remember we talked about a intel_display_test.c where we > could shove more stuff (Rodrigo's DP CRC maybe). Either that or Daniel > last suggestion seems fine (definitely intel_ though, has to do with the > display) > > -- > Damien -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm: share drm_add_fake_info_node 2014-01-14 14:14 [PATCH 1/2] drm: share drm_add_fake_info_node Ben Widawsky 2014-01-14 14:14 ` [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file Ben Widawsky @ 2014-01-14 23:25 ` Daniel Vetter 2014-01-14 23:40 ` Russell King - ARM Linux 2014-01-21 20:33 ` [PATCH 1/6] drm: Create a debugfs file creation helper Ben Widawsky 2 siblings, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2014-01-14 23:25 UTC (permalink / raw) To: Ben Widawsky; +Cc: linux-arm-kernel, intel-gfx, Ben Widawsky, DRI Development On Tue, Jan 14, 2014 at 06:14:06AM -0800, Ben Widawsky wrote: > Both i915 and Armada had the exact same implementation. For an upcoming > patch, I'd like to call this function from two different source files in > i915, and having it available externally helps there too. > > While moving, add 'debugfs_' to the name in order to match the other drm > debugfs helper functions. > > Cc: linux-arm-kernel@lists.infradead.org > Cc: intel-gfx@lists.freedesktop.org > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> drm_debugfs_create_files in drm_debugfs.c has the almost same code again. Now the problem here is that the interface is a bit botched up, since all the users in i915 and armada actaully faile to clean up teh debugfs dentry if drm_add_fake_info_node. So I think the right approach is to extrace a new drm_debugfs_create_node helper which is used by i915, armada and drm_debugfs_create_files. Also I think it's better to split this up into a drm core patch and then i915/armada driver patches, for easier merging. -Daniel > --- > drivers/gpu/drm/armada/armada_debugfs.c | 24 +----------------------- > drivers/gpu/drm/drm_debugfs.c | 24 ++++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_debugfs.c | 32 +++----------------------------- > include/drm/drmP.h | 9 +++++++++ > 4 files changed, 37 insertions(+), 52 deletions(-) > > diff --git a/drivers/gpu/drm/armada/armada_debugfs.c b/drivers/gpu/drm/armada/armada_debugfs.c > index 471e456..02f2978 100644 > --- a/drivers/gpu/drm/armada/armada_debugfs.c > +++ b/drivers/gpu/drm/armada/armada_debugfs.c > @@ -107,28 +107,6 @@ static struct drm_info_list armada_debugfs_list[] = { > }; > #define ARMADA_DEBUGFS_ENTRIES ARRAY_SIZE(armada_debugfs_list) > > -static int drm_add_fake_info_node(struct drm_minor *minor, struct dentry *ent, > - const void *key) > -{ > - struct drm_info_node *node; > - > - node = kmalloc(sizeof(struct drm_info_node), GFP_KERNEL); > - if (node == NULL) { > - debugfs_remove(ent); > - return -ENOMEM; > - } > - > - node->minor = minor; > - node->dent = ent; > - node->info_ent = (void *) key; > - > - mutex_lock(&minor->debugfs_lock); > - list_add(&node->list, &minor->debugfs_list); > - mutex_unlock(&minor->debugfs_lock); > - > - return 0; > -} > - > static int armada_debugfs_create(struct dentry *root, struct drm_minor *minor, > const char *name, umode_t mode, const struct file_operations *fops) > { > @@ -136,7 +114,7 @@ static int armada_debugfs_create(struct dentry *root, struct drm_minor *minor, > > de = debugfs_create_file(name, mode, root, minor->dev, fops); > > - return drm_add_fake_info_node(minor, de, fops); > + return drm_debugfs_add_fake_info_node(minor, de, fops); > } > > int armada_drm_debugfs_init(struct drm_minor *minor) > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index b4b51d4..1edaac0 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -237,5 +237,29 @@ int drm_debugfs_cleanup(struct drm_minor *minor) > return 0; > } > > +int drm_debugfs_add_fake_info_node(struct drm_minor *minor, > + struct dentry *ent, > + const void *key) > +{ > + struct drm_info_node *node; > + > + node = kmalloc(sizeof(*node), GFP_KERNEL); > + if (node == NULL) { > + debugfs_remove(ent); > + return -ENOMEM; > + } > + > + node->minor = minor; > + node->dent = ent; > + node->info_ent = (void *) key; > + > + mutex_lock(&minor->debugfs_lock); > + list_add(&node->list, &minor->debugfs_list); > + mutex_unlock(&minor->debugfs_lock); > + > + return 0; > +} > +EXPORT_SYMBOL(drm_debugfs_add_fake_info_node); > + > #endif /* CONFIG_DEBUG_FS */ > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 430eb3e..f2ef30c 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -51,32 +51,6 @@ static const char *yesno(int v) > return v ? "yes" : "no"; > } > > -/* As the drm_debugfs_init() routines are called before dev->dev_private is > - * allocated we need to hook into the minor for release. */ > -static int > -drm_add_fake_info_node(struct drm_minor *minor, > - struct dentry *ent, > - const void *key) > -{ > - struct drm_info_node *node; > - > - node = kmalloc(sizeof(*node), GFP_KERNEL); > - if (node == NULL) { > - debugfs_remove(ent); > - return -ENOMEM; > - } > - > - node->minor = minor; > - node->dent = ent; > - node->info_ent = (void *) key; > - > - mutex_lock(&minor->debugfs_lock); > - list_add(&node->list, &minor->debugfs_list); > - mutex_unlock(&minor->debugfs_lock); > - > - return 0; > -} > - > static int i915_capabilities(struct seq_file *m, void *data) > { > struct drm_info_node *node = (struct drm_info_node *) m->private; > @@ -2148,7 +2122,7 @@ static int i915_pipe_crc_create(struct dentry *root, struct drm_minor *minor, > if (!ent) > return -ENOMEM; > > - return drm_add_fake_info_node(minor, ent, info); > + return drm_debugfs_add_fake_info_node(minor, ent, info); > } > > static const char * const pipe_crc_sources[] = { > @@ -3164,7 +3138,7 @@ static int i915_forcewake_create(struct dentry *root, struct drm_minor *minor) > if (!ent) > return -ENOMEM; > > - return drm_add_fake_info_node(minor, ent, &i915_forcewake_fops); > + return drm_debugfs_add_fake_info_node(minor, ent, &i915_forcewake_fops); > } > > static int i915_debugfs_create(struct dentry *root, > @@ -3182,7 +3156,7 @@ static int i915_debugfs_create(struct dentry *root, > if (!ent) > return -ENOMEM; > > - return drm_add_fake_info_node(minor, ent, fops); > + return drm_debugfs_add_fake_info_node(minor, ent, fops); > } > > static const struct drm_info_list i915_debugfs_list[] = { > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 2fe9b5d..5788fb1 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1471,6 +1471,9 @@ extern int drm_debugfs_create_files(const struct drm_info_list *files, > extern int drm_debugfs_remove_files(const struct drm_info_list *files, > int count, struct drm_minor *minor); > extern int drm_debugfs_cleanup(struct drm_minor *minor); > +extern int drm_debugfs_add_fake_info_node(struct drm_minor *minor, > + struct dentry *ent, > + const void *key); > #else > static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id, > struct dentry *root) > @@ -1495,6 +1498,12 @@ static inline int drm_debugfs_cleanup(struct drm_minor *minor) > { > return 0; > } > +static int drm_debugfs_add_fake_info_node(struct drm_minor *minor, > + struct dentry *ent, > + const void *key) > +{ > + return 0; > +} > #endif > > /* Info file support */ > -- > 1.8.5.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm: share drm_add_fake_info_node 2014-01-14 23:25 ` [Intel-gfx] [PATCH 1/2] drm: share drm_add_fake_info_node Daniel Vetter @ 2014-01-14 23:40 ` Russell King - ARM Linux 2014-01-15 8:39 ` [Intel-gfx] " Daniel Vetter 0 siblings, 1 reply; 19+ messages in thread From: Russell King - ARM Linux @ 2014-01-14 23:40 UTC (permalink / raw) To: Daniel Vetter Cc: DRI Development, intel-gfx, Ben Widawsky, linux-arm-kernel, Ben Widawsky On Wed, Jan 15, 2014 at 12:25:10AM +0100, Daniel Vetter wrote: > On Tue, Jan 14, 2014 at 06:14:06AM -0800, Ben Widawsky wrote: > > Both i915 and Armada had the exact same implementation. For an upcoming > > patch, I'd like to call this function from two different source files in > > i915, and having it available externally helps there too. > > > > While moving, add 'debugfs_' to the name in order to match the other drm > > debugfs helper functions. > > > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: intel-gfx@lists.freedesktop.org > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > drm_debugfs_create_files in drm_debugfs.c has the almost same code again. > Now the problem here is that the interface is a bit botched up, since all > the users in i915 and armada actaully faile to clean up teh debugfs dentry > if drm_add_fake_info_node. That's not correct - armada does clean up these, I think you need to take a closer look at the code. We do this by setting node->info_ent to the file operations structure, which is a unique key to the file being registered. Upon failure to create the fake node, we appropriately call drm_debugfs_remove_files() with the first argument being a pointer to the file operations. This causes drm_debugfs_remove_files() to match the fake entry, call debugfs_remove() on the dentry, and remove the node from the list, and free the node structure we allocated. Upon driver teardown, we also call drm_debugfs_remove_files() but with each fops which should be registered, thus cleaning up each fake node which was created. So, Armada does clean up these entries properly. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm: share drm_add_fake_info_node 2014-01-14 23:40 ` Russell King - ARM Linux @ 2014-01-15 8:39 ` Daniel Vetter 2014-01-15 8:45 ` Daniel Vetter 0 siblings, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2014-01-15 8:39 UTC (permalink / raw) To: Russell King - ARM Linux Cc: DRI Development, intel-gfx, Ben Widawsky, linux-arm-kernel@lists.infradead.org, Ben Widawsky On Wed, Jan 15, 2014 at 12:40 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Wed, Jan 15, 2014 at 12:25:10AM +0100, Daniel Vetter wrote: >> On Tue, Jan 14, 2014 at 06:14:06AM -0800, Ben Widawsky wrote: >> > Both i915 and Armada had the exact same implementation. For an upcoming >> > patch, I'd like to call this function from two different source files in >> > i915, and having it available externally helps there too. >> > >> > While moving, add 'debugfs_' to the name in order to match the other drm >> > debugfs helper functions. >> > >> > Cc: linux-arm-kernel@lists.infradead.org >> > Cc: intel-gfx@lists.freedesktop.org >> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> >> >> drm_debugfs_create_files in drm_debugfs.c has the almost same code again. >> Now the problem here is that the interface is a bit botched up, since all >> the users in i915 and armada actaully faile to clean up teh debugfs dentry >> if drm_add_fake_info_node. > > That's not correct - armada does clean up these, I think you need to > take a closer look at the code. > > We do this by setting node->info_ent to the file operations structure, > which is a unique key to the file being registered. > > Upon failure to create the fake node, we appropriately call > drm_debugfs_remove_files() with the first argument being a pointer to > the file operations. This causes drm_debugfs_remove_files() to match > the fake entry, call debugfs_remove() on the dentry, and remove the > node from the list, and free the node structure we allocated. > > Upon driver teardown, we also call drm_debugfs_remove_files() but with > each fops which should be registered, thus cleaning up each fake node > which was created. > > So, Armada does clean up these entries properly. Indeed I've missed that and it's just i915 that botches this. I still think the helper would be saner if it cleans up all its leftovers in the failure case. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm: share drm_add_fake_info_node 2014-01-15 8:39 ` [Intel-gfx] " Daniel Vetter @ 2014-01-15 8:45 ` Daniel Vetter 2014-01-15 20:08 ` Ben Widawsky 0 siblings, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2014-01-15 8:45 UTC (permalink / raw) To: Russell King - ARM Linux Cc: DRI Development, intel-gfx, Ben Widawsky, linux-arm-kernel@lists.infradead.org, Ben Widawsky On Wed, Jan 15, 2014 at 9:39 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Wed, Jan 15, 2014 at 12:40 AM, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: >> On Wed, Jan 15, 2014 at 12:25:10AM +0100, Daniel Vetter wrote: >>> On Tue, Jan 14, 2014 at 06:14:06AM -0800, Ben Widawsky wrote: >>> > Both i915 and Armada had the exact same implementation. For an upcoming >>> > patch, I'd like to call this function from two different source files in >>> > i915, and having it available externally helps there too. >>> > >>> > While moving, add 'debugfs_' to the name in order to match the other drm >>> > debugfs helper functions. >>> > >>> > Cc: linux-arm-kernel@lists.infradead.org >>> > Cc: intel-gfx@lists.freedesktop.org >>> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> >>> >>> drm_debugfs_create_files in drm_debugfs.c has the almost same code again. >>> Now the problem here is that the interface is a bit botched up, since all >>> the users in i915 and armada actaully faile to clean up teh debugfs dentry >>> if drm_add_fake_info_node. >> >> That's not correct - armada does clean up these, I think you need to >> take a closer look at the code. >> >> We do this by setting node->info_ent to the file operations structure, >> which is a unique key to the file being registered. >> >> Upon failure to create the fake node, we appropriately call >> drm_debugfs_remove_files() with the first argument being a pointer to >> the file operations. This causes drm_debugfs_remove_files() to match >> the fake entry, call debugfs_remove() on the dentry, and remove the >> node from the list, and free the node structure we allocated. >> >> Upon driver teardown, we also call drm_debugfs_remove_files() but with >> each fops which should be registered, thus cleaning up each fake node >> which was created. >> >> So, Armada does clean up these entries properly. > > Indeed I've missed that and it's just i915 that botches this. I still > think the helper would be saner if it cleans up all its leftovers in > the failure case. Ok, now I've actually page all the stuff back in - if drm_add_fake_info_node fails we don't set up a drm_info_node structure and link it anywhere. Which means drm_debugfs_remove_files won't ever find it and hence can't possibly call debugfs_remove. Which means the debugfs dentry is leaked. So I think the semantics of that new debugfs helper should get fixed to also allocate and clean up the debugfs node. I agree that i915 is even worse since it doesn't bother to clean up any debugfs files at all in the failure case. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm: share drm_add_fake_info_node 2014-01-15 8:45 ` Daniel Vetter @ 2014-01-15 20:08 ` Ben Widawsky 2014-01-15 23:42 ` [Intel-gfx] " Daniel Vetter 0 siblings, 1 reply; 19+ messages in thread From: Ben Widawsky @ 2014-01-15 20:08 UTC (permalink / raw) To: Daniel Vetter Cc: intel-gfx, Russell King - ARM Linux, DRI Development, linux-arm-kernel@lists.infradead.org, Ben Widawsky On Wed, Jan 15, 2014 at 09:45:28AM +0100, Daniel Vetter wrote: > On Wed, Jan 15, 2014 at 9:39 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Wed, Jan 15, 2014 at 12:40 AM, Russell King - ARM Linux > > <linux@arm.linux.org.uk> wrote: > >> On Wed, Jan 15, 2014 at 12:25:10AM +0100, Daniel Vetter wrote: > >>> On Tue, Jan 14, 2014 at 06:14:06AM -0800, Ben Widawsky wrote: > >>> > Both i915 and Armada had the exact same implementation. For an upcoming > >>> > patch, I'd like to call this function from two different source files in > >>> > i915, and having it available externally helps there too. > >>> > > >>> > While moving, add 'debugfs_' to the name in order to match the other drm > >>> > debugfs helper functions. > >>> > > >>> > Cc: linux-arm-kernel@lists.infradead.org > >>> > Cc: intel-gfx@lists.freedesktop.org > >>> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > >>> > >>> drm_debugfs_create_files in drm_debugfs.c has the almost same code again. > >>> Now the problem here is that the interface is a bit botched up, since all > >>> the users in i915 and armada actaully faile to clean up teh debugfs dentry > >>> if drm_add_fake_info_node. > >> > >> That's not correct - armada does clean up these, I think you need to > >> take a closer look at the code. > >> > >> We do this by setting node->info_ent to the file operations structure, > >> which is a unique key to the file being registered. > >> > >> Upon failure to create the fake node, we appropriately call > >> drm_debugfs_remove_files() with the first argument being a pointer to > >> the file operations. This causes drm_debugfs_remove_files() to match > >> the fake entry, call debugfs_remove() on the dentry, and remove the > >> node from the list, and free the node structure we allocated. > >> > >> Upon driver teardown, we also call drm_debugfs_remove_files() but with > >> each fops which should be registered, thus cleaning up each fake node > >> which was created. > >> > >> So, Armada does clean up these entries properly. > > > > Indeed I've missed that and it's just i915 that botches this. I still > > think the helper would be saner if it cleans up all its leftovers in > > the failure case. > > Ok, now I've actually page all the stuff back in - if > drm_add_fake_info_node fails we don't set up a drm_info_node structure > and link it anywhere. Which means drm_debugfs_remove_files won't ever > find it and hence can't possibly call debugfs_remove. Which means the > debugfs dentry is leaked. So I think the semantics of that new debugfs > helper should get fixed to also allocate and clean up the debugfs > node. > > I agree that i915 is even worse since it doesn't bother to clean up > any debugfs files at all in the failure case. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch Perhaps I don't understand what you want here. The only failure path in the fake entry creation does already call debugfs_remove. if (node == NULL) { debugfs_remove(ent); return -ENOMEM; } So long as the function succeeds, the node will be findable and removable. -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm: share drm_add_fake_info_node 2014-01-15 20:08 ` Ben Widawsky @ 2014-01-15 23:42 ` Daniel Vetter 2014-01-21 19:05 ` Ben Widawsky 0 siblings, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2014-01-15 23:42 UTC (permalink / raw) To: Ben Widawsky Cc: Russell King - ARM Linux, intel-gfx, DRI Development, Daniel Vetter, Ben Widawsky, linux-arm-kernel@lists.infradead.org On Wed, Jan 15, 2014 at 12:08:19PM -0800, Ben Widawsky wrote: > On Wed, Jan 15, 2014 at 09:45:28AM +0100, Daniel Vetter wrote: > > On Wed, Jan 15, 2014 at 9:39 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Wed, Jan 15, 2014 at 12:40 AM, Russell King - ARM Linux > > > <linux@arm.linux.org.uk> wrote: > > >> On Wed, Jan 15, 2014 at 12:25:10AM +0100, Daniel Vetter wrote: > > >>> On Tue, Jan 14, 2014 at 06:14:06AM -0800, Ben Widawsky wrote: > > >>> > Both i915 and Armada had the exact same implementation. For an upcoming > > >>> > patch, I'd like to call this function from two different source files in > > >>> > i915, and having it available externally helps there too. > > >>> > > > >>> > While moving, add 'debugfs_' to the name in order to match the other drm > > >>> > debugfs helper functions. > > >>> > > > >>> > Cc: linux-arm-kernel@lists.infradead.org > > >>> > Cc: intel-gfx@lists.freedesktop.org > > >>> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > >>> > > >>> drm_debugfs_create_files in drm_debugfs.c has the almost same code again. > > >>> Now the problem here is that the interface is a bit botched up, since all > > >>> the users in i915 and armada actaully faile to clean up teh debugfs dentry > > >>> if drm_add_fake_info_node. > > >> > > >> That's not correct - armada does clean up these, I think you need to > > >> take a closer look at the code. > > >> > > >> We do this by setting node->info_ent to the file operations structure, > > >> which is a unique key to the file being registered. > > >> > > >> Upon failure to create the fake node, we appropriately call > > >> drm_debugfs_remove_files() with the first argument being a pointer to > > >> the file operations. This causes drm_debugfs_remove_files() to match > > >> the fake entry, call debugfs_remove() on the dentry, and remove the > > >> node from the list, and free the node structure we allocated. > > >> > > >> Upon driver teardown, we also call drm_debugfs_remove_files() but with > > >> each fops which should be registered, thus cleaning up each fake node > > >> which was created. > > >> > > >> So, Armada does clean up these entries properly. > > > > > > Indeed I've missed that and it's just i915 that botches this. I still > > > think the helper would be saner if it cleans up all its leftovers in > > > the failure case. > > > > Ok, now I've actually page all the stuff back in - if > > drm_add_fake_info_node fails we don't set up a drm_info_node structure > > and link it anywhere. Which means drm_debugfs_remove_files won't ever > > find it and hence can't possibly call debugfs_remove. Which means the > > debugfs dentry is leaked. So I think the semantics of that new debugfs > > helper should get fixed to also allocate and clean up the debugfs > > node. > > > > I agree that i915 is even worse since it doesn't bother to clean up > > any debugfs files at all in the failure case. > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > Perhaps I don't understand what you want here. The only failure path in > the fake entry creation does already call debugfs_remove. > > if (node == NULL) { > debugfs_remove(ent); > return -ENOMEM; > } > > So long as the function succeeds, the node will be findable and removable. Oh dear, I didn't see that. Still stand by my opinion though that we should shovel the debugfs_create_file into the helper - callers allocating something and then the helper freeing it (but only if it fails) is rather funky semantics. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] drm: share drm_add_fake_info_node 2014-01-15 23:42 ` [Intel-gfx] " Daniel Vetter @ 2014-01-21 19:05 ` Ben Widawsky 0 siblings, 0 replies; 19+ messages in thread From: Ben Widawsky @ 2014-01-21 19:05 UTC (permalink / raw) To: Daniel Vetter Cc: DRI Development, Ben Widawsky, Russell King - ARM Linux, intel-gfx, linux-arm-kernel@lists.infradead.org On Thu, Jan 16, 2014 at 12:42:22AM +0100, Daniel Vetter wrote: > On Wed, Jan 15, 2014 at 12:08:19PM -0800, Ben Widawsky wrote: > > On Wed, Jan 15, 2014 at 09:45:28AM +0100, Daniel Vetter wrote: > > > On Wed, Jan 15, 2014 at 9:39 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Wed, Jan 15, 2014 at 12:40 AM, Russell King - ARM Linux > > > > <linux@arm.linux.org.uk> wrote: > > > >> On Wed, Jan 15, 2014 at 12:25:10AM +0100, Daniel Vetter wrote: > > > >>> On Tue, Jan 14, 2014 at 06:14:06AM -0800, Ben Widawsky wrote: > > > >>> > Both i915 and Armada had the exact same implementation. For an upcoming > > > >>> > patch, I'd like to call this function from two different source files in > > > >>> > i915, and having it available externally helps there too. > > > >>> > > > > >>> > While moving, add 'debugfs_' to the name in order to match the other drm > > > >>> > debugfs helper functions. > > > >>> > > > > >>> > Cc: linux-arm-kernel@lists.infradead.org > > > >>> > Cc: intel-gfx@lists.freedesktop.org > > > >>> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > > >>> > > > >>> drm_debugfs_create_files in drm_debugfs.c has the almost same code again. > > > >>> Now the problem here is that the interface is a bit botched up, since all > > > >>> the users in i915 and armada actaully faile to clean up teh debugfs dentry > > > >>> if drm_add_fake_info_node. > > > >> > > > >> That's not correct - armada does clean up these, I think you need to > > > >> take a closer look at the code. > > > >> > > > >> We do this by setting node->info_ent to the file operations structure, > > > >> which is a unique key to the file being registered. > > > >> > > > >> Upon failure to create the fake node, we appropriately call > > > >> drm_debugfs_remove_files() with the first argument being a pointer to > > > >> the file operations. This causes drm_debugfs_remove_files() to match > > > >> the fake entry, call debugfs_remove() on the dentry, and remove the > > > >> node from the list, and free the node structure we allocated. > > > >> > > > >> Upon driver teardown, we also call drm_debugfs_remove_files() but with > > > >> each fops which should be registered, thus cleaning up each fake node > > > >> which was created. > > > >> > > > >> So, Armada does clean up these entries properly. > > > > > > > > Indeed I've missed that and it's just i915 that botches this. I still > > > > think the helper would be saner if it cleans up all its leftovers in > > > > the failure case. > > > > > > Ok, now I've actually page all the stuff back in - if > > > drm_add_fake_info_node fails we don't set up a drm_info_node structure > > > and link it anywhere. Which means drm_debugfs_remove_files won't ever > > > find it and hence can't possibly call debugfs_remove. Which means the > > > debugfs dentry is leaked. So I think the semantics of that new debugfs > > > helper should get fixed to also allocate and clean up the debugfs > > > node. > > > > > > I agree that i915 is even worse since it doesn't bother to clean up > > > any debugfs files at all in the failure case. > > > -Daniel > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > > Perhaps I don't understand what you want here. The only failure path in > > the fake entry creation does already call debugfs_remove. > > > > if (node == NULL) { > > debugfs_remove(ent); > > return -ENOMEM; > > } > > > > So long as the function succeeds, the node will be findable and removable. > > Oh dear, I didn't see that. Still stand by my opinion though that we > should shovel the debugfs_create_file into the helper - callers allocating > something and then the helper freeing it (but only if it fails) is rather > funky semantics. > -Daniel This actually falls apart for the one usage I originally cared about. For CRC, we store our own info structure instead of fops as the key in the drm debug list. Therefore, it doesn't align with the usage in ours or other drivers. One option is to make the helper function take both a fops as well as a key (an ugly interface if you ask me). I've already written the patches as you wanted, so I can submit them - it's just unfortunate that the duplication I was hoping to get rid of will need to remain. -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/6] drm: Create a debugfs file creation helper 2014-01-14 14:14 [PATCH 1/2] drm: share drm_add_fake_info_node Ben Widawsky 2014-01-14 14:14 ` [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file Ben Widawsky 2014-01-14 23:25 ` [Intel-gfx] [PATCH 1/2] drm: share drm_add_fake_info_node Daniel Vetter @ 2014-01-21 20:33 ` Ben Widawsky 2014-01-21 20:33 ` [PATCH 4/6] drm/i915: Use new drm debugfs file helper Ben Widawsky ` (2 more replies) 2 siblings, 3 replies; 19+ messages in thread From: Ben Widawsky @ 2014-01-21 20:33 UTC (permalink / raw) To: DRI Development; +Cc: Intel GFX, Ben Widawsky, linux-arm-kernel, Ben Widawsky DRM layer already provides a helper function to create a set of files following a specific idiom primarily characterized by the access flags of the file, and the file operations. This is great for writing concise code to handle these very common cases. This new helper function is provided to drivers that wish to change either the access flags, or the fops for the file - in particular the latter. This usage has become cropping up over many of the drivers. Providing the helper is therefore here for the usual reasons. Upcoming patches will update the callers. Currently the key is the same as fops. This is what most callers want, the exception is when you want to use 1 fops for multiple file nodes. I have found one case for this (in i915), and don't feel it's a good idea to have the interface reflect this one, currently fringe usage. In the future if more callers want to have a separate fops and key, the interface should be extended. v2: The original patch simply changes the way in which we wedge special files into the normal drm node list. Daniel requested a fuller helper function, which this patch addresses. His original claim that v1 was buggy is unfounded. Requested-by: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/drm_debugfs.c | 49 +++++++++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 14 +++++++++++++ 2 files changed, 63 insertions(+) diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index b4b51d4..ea470b7 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -237,5 +237,54 @@ int drm_debugfs_cleanup(struct drm_minor *minor) return 0; } +/** + * Helper function for DRM drivers to create a debugfs file. + * + * \param root parent directory entry for the new file + * \param minor minor device minor number + * \param name the new file's name + * \param fops file operations for the new file + * \param mode permissions for access + * \return zero on success, negative errno otherwise + * + * Instantiate a debugfs file with aforementioned data. The file will be added + * to the drm debugfs file list in the standard way, thus making cleanup + * possible through the normal drm_debugfs_remove_files() call. + * + * The primary motivation for this interface over the existing one is this + * interface provides explicit mode, and fops flags at the cost of some extra + * bookkeeping to be done by the driver. + */ +int drm_debugfs_create_file(struct dentry *root, + struct drm_minor *minor, + const char *name, + const struct file_operations *fops, + umode_t mode) +{ + struct drm_info_node *node; + struct dentry *ent; + + ent = debugfs_create_file(name, mode, root, minor->dev, fops); + if (!ent) + return PTR_ERR(ent); + + node = kmalloc(sizeof(*node), GFP_KERNEL); + if (node == NULL) { + debugfs_remove(ent); + return -ENOMEM; + } + + node->minor = minor; + node->dent = ent; + node->info_ent = (void *)fops; + + mutex_lock(&minor->debugfs_lock); + list_add(&node->list, &minor->debugfs_list); + mutex_unlock(&minor->debugfs_lock); + + return 0; +} +EXPORT_SYMBOL(drm_debugfs_create_file); + #endif /* CONFIG_DEBUG_FS */ diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 63eab2b..b8e2baf 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1458,6 +1458,11 @@ extern struct drm_local_map *drm_getsarea(struct drm_device *dev); #if defined(CONFIG_DEBUG_FS) extern int drm_debugfs_init(struct drm_minor *minor, int minor_id, struct dentry *root); +extern int drm_debugfs_create_file(struct dentry *root, + struct drm_minor *minor, + const char *name, + const struct file_operations *fops, + umode_t mode); extern int drm_debugfs_create_files(const struct drm_info_list *files, int count, struct dentry *root, struct drm_minor *minor); @@ -1478,6 +1483,15 @@ static inline int drm_debugfs_create_files(const struct drm_info_list *files, return 0; } +static inline int drm_debugfs_create_file(struct dentry *root, + struct drm_minor *minor, + const char *name, + const struct file_operations *fops, + umode_t mode) +{ + return 0; +} + static inline int drm_debugfs_remove_files(const struct drm_info_list *files, int count, struct drm_minor *minor) { -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/6] drm/i915: Use new drm debugfs file helper 2014-01-21 20:33 ` [PATCH 1/6] drm: Create a debugfs file creation helper Ben Widawsky @ 2014-01-21 20:33 ` Ben Widawsky 2014-01-21 20:33 ` [PATCH 5/6] drm/i915: Move forcewake debugfs setup also Ben Widawsky 2014-01-21 20:33 ` [PATCH 6/6] drm/i915: Move pipecrc debug functions to new file Ben Widawsky 2 siblings, 0 replies; 19+ messages in thread From: Ben Widawsky @ 2014-01-21 20:33 UTC (permalink / raw) To: Intel GFX; +Cc: Ben Widawsky, DRI Development, Ben Widawsky The debugfs helper duplicates the functionality used by Armada, so let's just use that. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_debugfs.c | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 473dda2..72b8388 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3221,24 +3221,6 @@ static int i915_forcewake_create(struct dentry *root, struct drm_minor *minor) return drm_add_fake_info_node(minor, ent, &i915_forcewake_fops); } -static int i915_debugfs_create(struct dentry *root, - struct drm_minor *minor, - const char *name, - const struct file_operations *fops) -{ - struct drm_device *dev = minor->dev; - struct dentry *ent; - - ent = debugfs_create_file(name, - S_IRUGO | S_IWUSR, - root, dev, - fops); - if (!ent) - return -ENOMEM; - - return drm_add_fake_info_node(minor, ent, fops); -} - static const struct drm_info_list i915_debugfs_list[] = { {"i915_capabilities", i915_capabilities, 0}, {"i915_gem_objects", i915_gem_object_info, 0}, @@ -3328,9 +3310,10 @@ int i915_debugfs_init(struct drm_minor *minor) } for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) { - ret = i915_debugfs_create(minor->debugfs_root, minor, - i915_debugfs_files[i].name, - i915_debugfs_files[i].fops); + ret = drm_debugfs_create_file(minor->debugfs_root, minor, + i915_debugfs_files[i].name, + i915_debugfs_files[i].fops, + S_IRUGO | S_IWUSR); if (ret) return ret; } -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/6] drm/i915: Move forcewake debugfs setup also 2014-01-21 20:33 ` [PATCH 1/6] drm: Create a debugfs file creation helper Ben Widawsky 2014-01-21 20:33 ` [PATCH 4/6] drm/i915: Use new drm debugfs file helper Ben Widawsky @ 2014-01-21 20:33 ` Ben Widawsky 2014-01-21 20:33 ` [PATCH 6/6] drm/i915: Move pipecrc debug functions to new file Ben Widawsky 2 siblings, 0 replies; 19+ messages in thread From: Ben Widawsky @ 2014-01-21 20:33 UTC (permalink / raw) To: Intel GFX; +Cc: Ben Widawsky, DRI Development, Ben Widawsky The forcewake setup is eerily similar, except it needs special mode flags. Inserting that info into our structure is trivial, and with that forcewake easily converts to using the new interface as well. Notice that CRC is lacking from this patch (CRC being very similar to forcewake in code). CRC is targeted for a new file, so there is no reason to move it. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_debugfs.c | 48 +++++++++++-------------------------- 1 file changed, 14 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 72b8388..d092631 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3206,21 +3206,6 @@ static const struct file_operations i915_forcewake_fops = { .release = i915_forcewake_release, }; -static int i915_forcewake_create(struct dentry *root, struct drm_minor *minor) -{ - struct drm_device *dev = minor->dev; - struct dentry *ent; - - ent = debugfs_create_file("i915_forcewake_user", - S_IRUSR, - root, dev, - &i915_forcewake_fops); - if (!ent) - return -ENOMEM; - - return drm_add_fake_info_node(minor, ent, &i915_forcewake_fops); -} - static const struct drm_info_list i915_debugfs_list[] = { {"i915_capabilities", i915_capabilities, 0}, {"i915_gem_objects", i915_gem_object_info, 0}, @@ -3267,18 +3252,20 @@ static const struct drm_info_list i915_debugfs_list[] = { static const struct i915_debugfs_files { const char *name; const struct file_operations *fops; + umode_t mode; } i915_debugfs_files[] = { - {"i915_wedged", &i915_wedged_fops}, - {"i915_max_freq", &i915_max_freq_fops}, - {"i915_min_freq", &i915_min_freq_fops}, - {"i915_cache_sharing", &i915_cache_sharing_fops}, - {"i915_ring_stop", &i915_ring_stop_fops}, - {"i915_ring_missed_irq", &i915_ring_missed_irq_fops}, - {"i915_ring_test_irq", &i915_ring_test_irq_fops}, - {"i915_gem_drop_caches", &i915_drop_caches_fops}, - {"i915_error_state", &i915_error_state_fops}, - {"i915_next_seqno", &i915_next_seqno_fops}, - {"i915_display_crc_ctl", &i915_display_crc_ctl_fops}, + {"i915_wedged", &i915_wedged_fops, S_IRUGO | S_IWUSR}, + {"i915_max_freq", &i915_max_freq_fops, S_IRUGO | S_IWUSR}, + {"i915_min_freq", &i915_min_freq_fops, S_IRUGO | S_IWUSR}, + {"i915_cache_sharing", &i915_cache_sharing_fops, S_IRUGO | S_IWUSR}, + {"i915_ring_stop", &i915_ring_stop_fops, S_IRUGO | S_IWUSR}, + {"i915_ring_missed_irq", &i915_ring_missed_irq_fops, S_IRUGO | S_IWUSR}, + {"i915_ring_test_irq", &i915_ring_test_irq_fops, S_IRUGO | S_IWUSR}, + {"i915_gem_drop_caches", &i915_drop_caches_fops, S_IRUGO | S_IWUSR}, + {"i915_error_state", &i915_error_state_fops, S_IRUGO | S_IWUSR}, + {"i915_next_seqno", &i915_next_seqno_fops, S_IRUGO | S_IWUSR}, + {"i915_display_crc_ctl", &i915_display_crc_ctl_fops, S_IRUGO | S_IWUSR}, + {"i915_forcewake_user", &i915_forcewake_fops, S_IRUGO}, }; void intel_display_crc_init(struct drm_device *dev) @@ -3299,10 +3286,6 @@ int i915_debugfs_init(struct drm_minor *minor) { int ret, i; - ret = i915_forcewake_create(minor->debugfs_root, 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) @@ -3313,7 +3296,7 @@ int i915_debugfs_init(struct drm_minor *minor) ret = drm_debugfs_create_file(minor->debugfs_root, minor, i915_debugfs_files[i].name, i915_debugfs_files[i].fops, - S_IRUGO | S_IWUSR); + i915_debugfs_files[i].mode); if (ret) return ret; } @@ -3330,9 +3313,6 @@ void i915_debugfs_cleanup(struct drm_minor *minor) 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 < ARRAY_SIZE(i915_pipe_crc_data); i++) { struct drm_info_list *info_list = (struct drm_info_list *)&i915_pipe_crc_data[i]; -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 6/6] drm/i915: Move pipecrc debug functions to new file 2014-01-21 20:33 ` [PATCH 1/6] drm: Create a debugfs file creation helper Ben Widawsky 2014-01-21 20:33 ` [PATCH 4/6] drm/i915: Use new drm debugfs file helper Ben Widawsky 2014-01-21 20:33 ` [PATCH 5/6] drm/i915: Move forcewake debugfs setup also Ben Widawsky @ 2014-01-21 20:33 ` Ben Widawsky 2014-01-21 21:44 ` Damien Lespiau 2 siblings, 1 reply; 19+ messages in thread From: Ben Widawsky @ 2014-01-21 20:33 UTC (permalink / raw) To: Intel GFX; +Cc: Ben Widawsky, Ben Widawsky At almost 800 lines of code, with almost a function per platform, this code was cluttering up the existing debugfs file, which has historically had fairly small functions. Patch should have no functional changes. Unfortunately, the patch Daniel requested of me for the DRM helper doesn't fit well with how we enable the CRC. Unlike most users of the interface, the CRC debugfs uses a separate fops and key. Instead of catering the interface to the one user, keep the CRC code the same as before, but embed the function that we don't want others to use (unless they have a good reason). v2: Remove spurious #if 0 (Damien) Rename to intel_display_test.c (Damien) Inline the drm_info_fake_node_add, since the DRM helper function should solve most users problem (Ben) Cc: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/Makefile | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 790 ----------------------------- drivers/gpu/drm/i915/i915_drv.h | 12 + drivers/gpu/drm/i915/intel_display_test.c | 801 ++++++++++++++++++++++++++++++ 4 files changed, 814 insertions(+), 791 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_display_test.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index da682cb..248ec41 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -54,7 +54,7 @@ i915-$(CONFIG_ACPI) += intel_acpi.o i915-$(CONFIG_DRM_I915_FBDEV) += intel_fbdev.o -i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o +i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_display_test.o obj-$(CONFIG_DRM_I915) += i915.o diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d092631..4c3382a 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -27,8 +27,6 @@ */ #include <linux/seq_file.h> -#include <linux/circ_buf.h> -#include <linux/ctype.h> #include <linux/debugfs.h> #include <linux/slab.h> #include <linux/export.h> @@ -51,32 +49,6 @@ static const char *yesno(int v) return v ? "yes" : "no"; } -/* As the drm_debugfs_init() routines are called before dev->dev_private is - * allocated we need to hook into the minor for release. */ -static int -drm_add_fake_info_node(struct drm_minor *minor, - struct dentry *ent, - const void *key) -{ - struct drm_info_node *node; - - node = kmalloc(sizeof(*node), GFP_KERNEL); - if (node == NULL) { - debugfs_remove(ent); - return -ENOMEM; - } - - node->minor = minor; - node->dent = ent; - node->info_ent = (void *) key; - - mutex_lock(&minor->debugfs_lock); - list_add(&node->list, &minor->debugfs_list); - mutex_unlock(&minor->debugfs_lock); - - return 0; -} - static int i915_capabilities(struct seq_file *m, void *data) { struct drm_info_node *node = (struct drm_info_node *) m->private; @@ -2036,754 +2008,6 @@ static int i915_power_domain_info(struct seq_file *m, void *unused) return 0; } -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) -{ - struct pipe_crc_info *info = inode->i_private; - struct drm_i915_private *dev_priv = info->dev->dev_private; - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; - - if (info->pipe >= INTEL_INFO(info->dev)->num_pipes) - return -ENODEV; - - spin_lock_irq(&pipe_crc->lock); - - if (pipe_crc->opened) { - spin_unlock_irq(&pipe_crc->lock); - return -EBUSY; /* already open */ - } - - pipe_crc->opened = true; - filep->private_data = inode->i_private; - - spin_unlock_irq(&pipe_crc->lock); - - return 0; -} - -static int i915_pipe_crc_release(struct inode *inode, struct file *filep) -{ - struct pipe_crc_info *info = inode->i_private; - struct drm_i915_private *dev_priv = info->dev->dev_private; - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; - - spin_lock_irq(&pipe_crc->lock); - pipe_crc->opened = false; - spin_unlock_irq(&pipe_crc->lock); - - 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) -{ - assert_spin_locked(&pipe_crc->lock); - return CIRC_CNT(pipe_crc->head, pipe_crc->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 */ - spin_lock_irq(&pipe_crc->lock); - while (pipe_crc_data_count(pipe_crc) == 0) { - int ret; - - if (filep->f_flags & O_NONBLOCK) { - spin_unlock_irq(&pipe_crc->lock); - return -EAGAIN; - } - - ret = wait_event_interruptible_lock_irq(pipe_crc->wq, - pipe_crc_data_count(pipe_crc), pipe_crc->lock); - if (ret) { - spin_unlock_irq(&pipe_crc->lock); - return ret; - } - } - - /* We now have one or more entries to read */ - head = pipe_crc->head; - tail = pipe_crc->tail; - n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR), - count / PIPE_CRC_LINE_LEN); - spin_unlock_irq(&pipe_crc->lock); - - bytes_read = 0; - n = 0; - do { - struct intel_pipe_crc_entry *entry = &pipe_crc->entries[tail]; - int ret; - - 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); - n++; - } while (--n_entries); - - spin_lock_irq(&pipe_crc->lock); - pipe_crc->tail = tail; - spin_unlock_irq(&pipe_crc->lock); - - 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 (!ent) - return -ENOMEM; - - return drm_add_fake_info_node(minor, ent, info); -} - -static const char * const pipe_crc_sources[] = { - "none", - "plane1", - "plane2", - "pf", - "pipe", - "TV", - "DP-B", - "DP-C", - "DP-D", - "auto", -}; - -static const char *pipe_crc_source_name(enum intel_pipe_crc_source source) -{ - BUILD_BUG_ON(ARRAY_SIZE(pipe_crc_sources) != INTEL_PIPE_CRC_SOURCE_MAX); - return pipe_crc_sources[source]; -} - -static int display_crc_ctl_show(struct seq_file *m, void *data) -{ - struct drm_device *dev = m->private; - struct drm_i915_private *dev_priv = dev->dev_private; - int i; - - for (i = 0; i < I915_MAX_PIPES; i++) - seq_printf(m, "%c %s\n", pipe_name(i), - pipe_crc_source_name(dev_priv->pipe_crc[i].source)); - - return 0; -} - -static int display_crc_ctl_open(struct inode *inode, struct file *file) -{ - struct drm_device *dev = inode->i_private; - - return single_open(file, display_crc_ctl_show, dev); -} - -static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, - uint32_t *val) -{ - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) - *source = INTEL_PIPE_CRC_SOURCE_PIPE; - - switch (*source) { - case INTEL_PIPE_CRC_SOURCE_PIPE: - *val = PIPE_CRC_ENABLE | PIPE_CRC_INCLUDE_BORDER_I8XX; - break; - case INTEL_PIPE_CRC_SOURCE_NONE: - *val = 0; - break; - default: - return -EINVAL; - } - - return 0; -} - -static int i9xx_pipe_crc_auto_source(struct drm_device *dev, enum pipe pipe, - enum intel_pipe_crc_source *source) -{ - struct intel_encoder *encoder; - struct intel_crtc *crtc; - struct intel_digital_port *dig_port; - int ret = 0; - - *source = INTEL_PIPE_CRC_SOURCE_PIPE; - - mutex_lock(&dev->mode_config.mutex); - list_for_each_entry(encoder, &dev->mode_config.encoder_list, - base.head) { - if (!encoder->base.crtc) - continue; - - crtc = to_intel_crtc(encoder->base.crtc); - - if (crtc->pipe != pipe) - continue; - - switch (encoder->type) { - case INTEL_OUTPUT_TVOUT: - *source = INTEL_PIPE_CRC_SOURCE_TV; - break; - case INTEL_OUTPUT_DISPLAYPORT: - case INTEL_OUTPUT_EDP: - dig_port = enc_to_dig_port(&encoder->base); - switch (dig_port->port) { - case PORT_B: - *source = INTEL_PIPE_CRC_SOURCE_DP_B; - break; - case PORT_C: - *source = INTEL_PIPE_CRC_SOURCE_DP_C; - break; - case PORT_D: - *source = INTEL_PIPE_CRC_SOURCE_DP_D; - break; - default: - WARN(1, "nonexisting DP port %c\n", - port_name(dig_port->port)); - break; - } - break; - } - } - mutex_unlock(&dev->mode_config.mutex); - - return ret; -} - -static int vlv_pipe_crc_ctl_reg(struct drm_device *dev, - enum pipe pipe, - enum intel_pipe_crc_source *source, - uint32_t *val) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - bool need_stable_symbols = false; - - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { - int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); - if (ret) - return ret; - } - - switch (*source) { - case INTEL_PIPE_CRC_SOURCE_PIPE: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_VLV; - break; - case INTEL_PIPE_CRC_SOURCE_DP_B: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_VLV; - need_stable_symbols = true; - break; - case INTEL_PIPE_CRC_SOURCE_DP_C: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_VLV; - need_stable_symbols = true; - break; - case INTEL_PIPE_CRC_SOURCE_NONE: - *val = 0; - break; - default: - return -EINVAL; - } - - /* - * When the pipe CRC tap point is after the transcoders we need - * to tweak symbol-level features to produce a deterministic series of - * symbols for a given frame. We need to reset those features only once - * a frame (instead of every nth symbol): - * - DC-balance: used to ensure a better clock recovery from the data - * link (SDVO) - * - DisplayPort scrambling: used for EMI reduction - */ - if (need_stable_symbols) { - uint32_t tmp = I915_READ(PORT_DFT2_G4X); - - WARN_ON(!IS_G4X(dev)); - - tmp |= DC_BALANCE_RESET_VLV; - if (pipe == PIPE_A) - tmp |= PIPE_A_SCRAMBLE_RESET; - else - tmp |= PIPE_B_SCRAMBLE_RESET; - - I915_WRITE(PORT_DFT2_G4X, tmp); - } - - return 0; -} - -static int i9xx_pipe_crc_ctl_reg(struct drm_device *dev, - enum pipe pipe, - enum intel_pipe_crc_source *source, - uint32_t *val) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - bool need_stable_symbols = false; - - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { - int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); - if (ret) - return ret; - } - - switch (*source) { - case INTEL_PIPE_CRC_SOURCE_PIPE: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_I9XX; - break; - case INTEL_PIPE_CRC_SOURCE_TV: - if (!SUPPORTS_TV(dev)) - return -EINVAL; - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_TV_PRE; - break; - case INTEL_PIPE_CRC_SOURCE_DP_B: - if (!IS_G4X(dev)) - return -EINVAL; - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_G4X; - need_stable_symbols = true; - break; - case INTEL_PIPE_CRC_SOURCE_DP_C: - if (!IS_G4X(dev)) - return -EINVAL; - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_G4X; - need_stable_symbols = true; - break; - case INTEL_PIPE_CRC_SOURCE_DP_D: - if (!IS_G4X(dev)) - return -EINVAL; - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_D_G4X; - need_stable_symbols = true; - break; - case INTEL_PIPE_CRC_SOURCE_NONE: - *val = 0; - break; - default: - return -EINVAL; - } - - /* - * When the pipe CRC tap point is after the transcoders we need - * to tweak symbol-level features to produce a deterministic series of - * symbols for a given frame. We need to reset those features only once - * a frame (instead of every nth symbol): - * - DC-balance: used to ensure a better clock recovery from the data - * link (SDVO) - * - DisplayPort scrambling: used for EMI reduction - */ - if (need_stable_symbols) { - uint32_t tmp = I915_READ(PORT_DFT2_G4X); - - WARN_ON(!IS_G4X(dev)); - - I915_WRITE(PORT_DFT_I9XX, - I915_READ(PORT_DFT_I9XX) | DC_BALANCE_RESET); - - if (pipe == PIPE_A) - tmp |= PIPE_A_SCRAMBLE_RESET; - else - tmp |= PIPE_B_SCRAMBLE_RESET; - - I915_WRITE(PORT_DFT2_G4X, tmp); - } - - return 0; -} - -static void vlv_undo_pipe_scramble_reset(struct drm_device *dev, - enum pipe pipe) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - uint32_t tmp = I915_READ(PORT_DFT2_G4X); - - if (pipe == PIPE_A) - tmp &= ~PIPE_A_SCRAMBLE_RESET; - else - tmp &= ~PIPE_B_SCRAMBLE_RESET; - if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) - tmp &= ~DC_BALANCE_RESET_VLV; - I915_WRITE(PORT_DFT2_G4X, tmp); - -} - -static void g4x_undo_pipe_scramble_reset(struct drm_device *dev, - enum pipe pipe) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - uint32_t tmp = I915_READ(PORT_DFT2_G4X); - - if (pipe == PIPE_A) - tmp &= ~PIPE_A_SCRAMBLE_RESET; - else - tmp &= ~PIPE_B_SCRAMBLE_RESET; - I915_WRITE(PORT_DFT2_G4X, tmp); - - if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) { - I915_WRITE(PORT_DFT_I9XX, - I915_READ(PORT_DFT_I9XX) & ~DC_BALANCE_RESET); - } -} - -static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, - uint32_t *val) -{ - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) - *source = INTEL_PIPE_CRC_SOURCE_PIPE; - - switch (*source) { - case INTEL_PIPE_CRC_SOURCE_PLANE1: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_ILK; - break; - case INTEL_PIPE_CRC_SOURCE_PLANE2: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_ILK; - break; - case INTEL_PIPE_CRC_SOURCE_PIPE: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_ILK; - break; - case INTEL_PIPE_CRC_SOURCE_NONE: - *val = 0; - break; - default: - return -EINVAL; - } - - return 0; -} - -static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, - uint32_t *val) -{ - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) - *source = INTEL_PIPE_CRC_SOURCE_PF; - - switch (*source) { - case INTEL_PIPE_CRC_SOURCE_PLANE1: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_IVB; - break; - case INTEL_PIPE_CRC_SOURCE_PLANE2: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB; - break; - case INTEL_PIPE_CRC_SOURCE_PF: - *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB; - break; - case INTEL_PIPE_CRC_SOURCE_NONE: - *val = 0; - break; - default: - return -EINVAL; - } - - return 0; -} - -static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, - enum intel_pipe_crc_source source) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; - u32 val = 0; /* shut up gcc */ - int ret; - - if (pipe_crc->source == source) - return 0; - - /* forbid changing the source without going back to 'none' */ - if (pipe_crc->source && source) - return -EINVAL; - - if (IS_GEN2(dev)) - ret = i8xx_pipe_crc_ctl_reg(&source, &val); - else if (INTEL_INFO(dev)->gen < 5) - ret = i9xx_pipe_crc_ctl_reg(dev, pipe, &source, &val); - else if (IS_VALLEYVIEW(dev)) - ret = vlv_pipe_crc_ctl_reg(dev,pipe, &source, &val); - else if (IS_GEN5(dev) || IS_GEN6(dev)) - ret = ilk_pipe_crc_ctl_reg(&source, &val); - else - ret = ivb_pipe_crc_ctl_reg(&source, &val); - - if (ret != 0) - return ret; - - /* none -> real source transition */ - if (source) { - DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n", - pipe_name(pipe), pipe_crc_source_name(source)); - - pipe_crc->entries = kzalloc(sizeof(*pipe_crc->entries) * - INTEL_PIPE_CRC_ENTRIES_NR, - GFP_KERNEL); - if (!pipe_crc->entries) - return -ENOMEM; - - spin_lock_irq(&pipe_crc->lock); - pipe_crc->head = 0; - pipe_crc->tail = 0; - spin_unlock_irq(&pipe_crc->lock); - } - - pipe_crc->source = source; - - I915_WRITE(PIPE_CRC_CTL(pipe), val); - POSTING_READ(PIPE_CRC_CTL(pipe)); - - /* real source -> none transition */ - if (source == INTEL_PIPE_CRC_SOURCE_NONE) { - struct intel_pipe_crc_entry *entries; - - DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n", - pipe_name(pipe)); - - intel_wait_for_vblank(dev, pipe); - - spin_lock_irq(&pipe_crc->lock); - entries = pipe_crc->entries; - pipe_crc->entries = NULL; - spin_unlock_irq(&pipe_crc->lock); - - kfree(entries); - - if (IS_G4X(dev)) - g4x_undo_pipe_scramble_reset(dev, pipe); - else if (IS_VALLEYVIEW(dev)) - vlv_undo_pipe_scramble_reset(dev, pipe); - } - - return 0; -} - -/* - * Parse pipe CRC command strings: - * command: wsp* object wsp+ name wsp+ source wsp* - * object: 'pipe' - * name: (A | B | C) - * source: (none | plane1 | plane2 | pf) - * wsp: (#0x20 | #0x9 | #0xA)+ - * - * eg.: - * "pipe A plane1" -> Start CRC computations on plane1 of pipe A - * "pipe A none" -> Stop CRC - */ -static int display_crc_ctl_tokenize(char *buf, char *words[], int max_words) -{ - int n_words = 0; - - while (*buf) { - char *end; - - /* skip leading white space */ - buf = skip_spaces(buf); - if (!*buf) - break; /* end of buffer */ - - /* find end of word */ - for (end = buf; *end && !isspace(*end); end++) - ; - - if (n_words == max_words) { - DRM_DEBUG_DRIVER("too many words, allowed <= %d\n", - max_words); - return -EINVAL; /* ran out of words[] before bytes */ - } - - if (*end) - *end++ = '\0'; - words[n_words++] = buf; - buf = end; - } - - return n_words; -} - -enum intel_pipe_crc_object { - PIPE_CRC_OBJECT_PIPE, -}; - -static const char * const pipe_crc_objects[] = { - "pipe", -}; - -static int -display_crc_ctl_parse_object(const char *buf, enum intel_pipe_crc_object *o) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(pipe_crc_objects); i++) - if (!strcmp(buf, pipe_crc_objects[i])) { - *o = i; - return 0; - } - - return -EINVAL; -} - -static int display_crc_ctl_parse_pipe(const char *buf, enum pipe *pipe) -{ - const char name = buf[0]; - - if (name < 'A' || name >= pipe_name(I915_MAX_PIPES)) - return -EINVAL; - - *pipe = name - 'A'; - - return 0; -} - -static int -display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(pipe_crc_sources); i++) - if (!strcmp(buf, pipe_crc_sources[i])) { - *s = i; - return 0; - } - - return -EINVAL; -} - -static int display_crc_ctl_parse(struct drm_device *dev, char *buf, size_t len) -{ -#define N_WORDS 3 - int n_words; - char *words[N_WORDS]; - enum pipe pipe; - enum intel_pipe_crc_object object; - enum intel_pipe_crc_source source; - - n_words = display_crc_ctl_tokenize(buf, words, N_WORDS); - if (n_words != N_WORDS) { - DRM_DEBUG_DRIVER("tokenize failed, a command is %d words\n", - N_WORDS); - return -EINVAL; - } - - if (display_crc_ctl_parse_object(words[0], &object) < 0) { - DRM_DEBUG_DRIVER("unknown object %s\n", words[0]); - return -EINVAL; - } - - if (display_crc_ctl_parse_pipe(words[1], &pipe) < 0) { - DRM_DEBUG_DRIVER("unknown pipe %s\n", words[1]); - return -EINVAL; - } - - if (display_crc_ctl_parse_source(words[2], &source) < 0) { - DRM_DEBUG_DRIVER("unknown source %s\n", words[2]); - return -EINVAL; - } - - return pipe_crc_set_source(dev, pipe, source); -} - -static ssize_t display_crc_ctl_write(struct file *file, const char __user *ubuf, - size_t len, loff_t *offp) -{ - struct seq_file *m = file->private_data; - struct drm_device *dev = m->private; - char *tmpbuf; - int ret; - - if (len == 0) - return 0; - - if (len > PAGE_SIZE - 1) { - DRM_DEBUG_DRIVER("expected <%lu bytes into pipe crc control\n", - PAGE_SIZE); - return -E2BIG; - } - - tmpbuf = kmalloc(len + 1, GFP_KERNEL); - if (!tmpbuf) - return -ENOMEM; - - if (copy_from_user(tmpbuf, ubuf, len)) { - ret = -EFAULT; - goto out; - } - tmpbuf[len] = '\0'; - - ret = display_crc_ctl_parse(dev, tmpbuf, len); - -out: - kfree(tmpbuf); - if (ret < 0) - return ret; - - *offp += len; - return len; -} - -static const struct file_operations i915_display_crc_ctl_fops = { - .owner = THIS_MODULE, - .open = display_crc_ctl_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, - .write = display_crc_ctl_write -}; - static int i915_wedged_get(void *data, u64 *val) { @@ -3268,20 +2492,6 @@ static const struct i915_debugfs_files { {"i915_forcewake_user", &i915_forcewake_fops, S_IRUGO}, }; -void intel_display_crc_init(struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - enum pipe pipe; - - for_each_pipe(pipe) { - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; - - pipe_crc->opened = false; - spin_lock_init(&pipe_crc->lock); - init_waitqueue_head(&pipe_crc->wq); - } -} - int i915_debugfs_init(struct drm_minor *minor) { int ret, i; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f888fea..3bdc34c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1356,6 +1356,18 @@ struct intel_pipe_crc { wait_queue_head_t wq; }; +struct pipe_crc_info { + const char *name; + struct drm_device *dev; + enum pipe pipe; +}; + +extern struct pipe_crc_info i915_pipe_crc_data[I915_MAX_PIPES]; +extern const struct file_operations i915_display_crc_ctl_fops; + +int i915_pipe_crc_create(struct dentry *root, struct drm_minor *minor, + enum pipe pipe); + typedef struct drm_i915_private { struct drm_device *dev; struct kmem_cache *slab; diff --git a/drivers/gpu/drm/i915/intel_display_test.c b/drivers/gpu/drm/i915/intel_display_test.c new file mode 100644 index 0000000..12c4189 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_display_test.c @@ -0,0 +1,801 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include <linux/debugfs.h> +#include <linux/fs.h> +#include <linux/seq_file.h> +#include <linux/circ_buf.h> +#include <linux/ctype.h> +#include "i915_drv.h" +#include "intel_drv.h" + +static int i915_pipe_crc_open(struct inode *inode, struct file *filep) +{ + struct pipe_crc_info *info = inode->i_private; + struct drm_i915_private *dev_priv = info->dev->dev_private; + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; + + if (info->pipe >= INTEL_INFO(info->dev)->num_pipes) + return -ENODEV; + + spin_lock_irq(&pipe_crc->lock); + + if (pipe_crc->opened) { + spin_unlock_irq(&pipe_crc->lock); + return -EBUSY; /* already open */ + } + + pipe_crc->opened = true; + filep->private_data = inode->i_private; + + spin_unlock_irq(&pipe_crc->lock); + + return 0; +} + +static int i915_pipe_crc_release(struct inode *inode, struct file *filep) +{ + struct pipe_crc_info *info = inode->i_private; + struct drm_i915_private *dev_priv = info->dev->dev_private; + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; + + spin_lock_irq(&pipe_crc->lock); + pipe_crc->opened = false; + spin_unlock_irq(&pipe_crc->lock); + + 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) +{ + assert_spin_locked(&pipe_crc->lock); + return CIRC_CNT(pipe_crc->head, pipe_crc->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 */ + spin_lock_irq(&pipe_crc->lock); + while (pipe_crc_data_count(pipe_crc) == 0) { + int ret; + + if (filep->f_flags & O_NONBLOCK) { + spin_unlock_irq(&pipe_crc->lock); + return -EAGAIN; + } + + ret = wait_event_interruptible_lock_irq(pipe_crc->wq, + pipe_crc_data_count(pipe_crc), pipe_crc->lock); + if (ret) { + spin_unlock_irq(&pipe_crc->lock); + return ret; + } + } + + /* We now have one or more entries to read */ + head = pipe_crc->head; + tail = pipe_crc->tail; + n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR), + count / PIPE_CRC_LINE_LEN); + spin_unlock_irq(&pipe_crc->lock); + + bytes_read = 0; + n = 0; + do { + struct intel_pipe_crc_entry *entry = &pipe_crc->entries[tail]; + int ret; + + 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); + n++; + } while (--n_entries); + + spin_lock_irq(&pipe_crc->lock); + pipe_crc->tail = tail; + spin_unlock_irq(&pipe_crc->lock); + + return bytes_read; +} + +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, +}; + +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, + }, +}; + +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]; + struct drm_info_node *node; + + info->dev = dev; + ent = debugfs_create_file(info->name, S_IRUGO, root, info, + &i915_pipe_crc_fops); + if (!ent) + return -ENOMEM; + + node = kmalloc(sizeof(*node), GFP_KERNEL); + if (node == NULL) { + debugfs_remove(ent); + return -ENOMEM; + } + + node->minor = minor; + node->dent = ent; + node->info_ent = (void *)info; + + mutex_lock(&minor->debugfs_lock); + list_add(&node->list, &minor->debugfs_list); + mutex_unlock(&minor->debugfs_lock); + + return 0; +} + +static const char * const pipe_crc_sources[] = { + "none", + "plane1", + "plane2", + "pf", + "pipe", + "TV", + "DP-B", + "DP-C", + "DP-D", + "auto", +}; + +static const char *pipe_crc_source_name(enum intel_pipe_crc_source source) +{ + BUILD_BUG_ON(ARRAY_SIZE(pipe_crc_sources) != INTEL_PIPE_CRC_SOURCE_MAX); + return pipe_crc_sources[source]; +} + +static int display_crc_ctl_show(struct seq_file *m, void *data) +{ + struct drm_device *dev = m->private; + struct drm_i915_private *dev_priv = dev->dev_private; + int i; + + for (i = 0; i < I915_MAX_PIPES; i++) + seq_printf(m, "%c %s\n", pipe_name(i), + pipe_crc_source_name(dev_priv->pipe_crc[i].source)); + + return 0; +} + +static int display_crc_ctl_open(struct inode *inode, struct file *file) +{ + struct drm_device *dev = inode->i_private; + + return single_open(file, display_crc_ctl_show, dev); +} + +static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, + uint32_t *val) +{ + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) + *source = INTEL_PIPE_CRC_SOURCE_PIPE; + + switch (*source) { + case INTEL_PIPE_CRC_SOURCE_PIPE: + *val = PIPE_CRC_ENABLE | PIPE_CRC_INCLUDE_BORDER_I8XX; + break; + case INTEL_PIPE_CRC_SOURCE_NONE: + *val = 0; + break; + default: + return -EINVAL; + } + + return 0; +} + +static int i9xx_pipe_crc_auto_source(struct drm_device *dev, enum pipe pipe, + enum intel_pipe_crc_source *source) +{ + struct intel_encoder *encoder; + struct intel_crtc *crtc; + struct intel_digital_port *dig_port; + int ret = 0; + + *source = INTEL_PIPE_CRC_SOURCE_PIPE; + + mutex_lock(&dev->mode_config.mutex); + list_for_each_entry(encoder, &dev->mode_config.encoder_list, + base.head) { + if (!encoder->base.crtc) + continue; + + crtc = to_intel_crtc(encoder->base.crtc); + + if (crtc->pipe != pipe) + continue; + + switch (encoder->type) { + case INTEL_OUTPUT_TVOUT: + *source = INTEL_PIPE_CRC_SOURCE_TV; + break; + case INTEL_OUTPUT_DISPLAYPORT: + case INTEL_OUTPUT_EDP: + dig_port = enc_to_dig_port(&encoder->base); + switch (dig_port->port) { + case PORT_B: + *source = INTEL_PIPE_CRC_SOURCE_DP_B; + break; + case PORT_C: + *source = INTEL_PIPE_CRC_SOURCE_DP_C; + break; + case PORT_D: + *source = INTEL_PIPE_CRC_SOURCE_DP_D; + break; + default: + WARN(1, "nonexisting DP port %c\n", + port_name(dig_port->port)); + break; + } + break; + } + } + mutex_unlock(&dev->mode_config.mutex); + + return ret; +} + +static int vlv_pipe_crc_ctl_reg(struct drm_device *dev, + enum pipe pipe, + enum intel_pipe_crc_source *source, + uint32_t *val) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + bool need_stable_symbols = false; + + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { + int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); + if (ret) + return ret; + } + + switch (*source) { + case INTEL_PIPE_CRC_SOURCE_PIPE: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_VLV; + break; + case INTEL_PIPE_CRC_SOURCE_DP_B: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_VLV; + need_stable_symbols = true; + break; + case INTEL_PIPE_CRC_SOURCE_DP_C: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_VLV; + need_stable_symbols = true; + break; + case INTEL_PIPE_CRC_SOURCE_NONE: + *val = 0; + break; + default: + return -EINVAL; + } + + /* + * When the pipe CRC tap point is after the transcoders we need + * to tweak symbol-level features to produce a deterministic series of + * symbols for a given frame. We need to reset those features only once + * a frame (instead of every nth symbol): + * - DC-balance: used to ensure a better clock recovery from the data + * link (SDVO) + * - DisplayPort scrambling: used for EMI reduction + */ + if (need_stable_symbols) { + uint32_t tmp = I915_READ(PORT_DFT2_G4X); + + WARN_ON(!IS_G4X(dev)); + + tmp |= DC_BALANCE_RESET_VLV; + if (pipe == PIPE_A) + tmp |= PIPE_A_SCRAMBLE_RESET; + else + tmp |= PIPE_B_SCRAMBLE_RESET; + + I915_WRITE(PORT_DFT2_G4X, tmp); + } + + return 0; +} + +static int i9xx_pipe_crc_ctl_reg(struct drm_device *dev, + enum pipe pipe, + enum intel_pipe_crc_source *source, + uint32_t *val) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + bool need_stable_symbols = false; + + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { + int ret = i9xx_pipe_crc_auto_source(dev, pipe, source); + if (ret) + return ret; + } + + switch (*source) { + case INTEL_PIPE_CRC_SOURCE_PIPE: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_I9XX; + break; + case INTEL_PIPE_CRC_SOURCE_TV: + if (!SUPPORTS_TV(dev)) + return -EINVAL; + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_TV_PRE; + break; + case INTEL_PIPE_CRC_SOURCE_DP_B: + if (!IS_G4X(dev)) + return -EINVAL; + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_B_G4X; + need_stable_symbols = true; + break; + case INTEL_PIPE_CRC_SOURCE_DP_C: + if (!IS_G4X(dev)) + return -EINVAL; + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_C_G4X; + need_stable_symbols = true; + break; + case INTEL_PIPE_CRC_SOURCE_DP_D: + if (!IS_G4X(dev)) + return -EINVAL; + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DP_D_G4X; + need_stable_symbols = true; + break; + case INTEL_PIPE_CRC_SOURCE_NONE: + *val = 0; + break; + default: + return -EINVAL; + } + + /* + * When the pipe CRC tap point is after the transcoders we need + * to tweak symbol-level features to produce a deterministic series of + * symbols for a given frame. We need to reset those features only once + * a frame (instead of every nth symbol): + * - DC-balance: used to ensure a better clock recovery from the data + * link (SDVO) + * - DisplayPort scrambling: used for EMI reduction + */ + if (need_stable_symbols) { + uint32_t tmp = I915_READ(PORT_DFT2_G4X); + + WARN_ON(!IS_G4X(dev)); + + I915_WRITE(PORT_DFT_I9XX, + I915_READ(PORT_DFT_I9XX) | DC_BALANCE_RESET); + + if (pipe == PIPE_A) + tmp |= PIPE_A_SCRAMBLE_RESET; + else + tmp |= PIPE_B_SCRAMBLE_RESET; + + I915_WRITE(PORT_DFT2_G4X, tmp); + } + + return 0; +} + +static void vlv_undo_pipe_scramble_reset(struct drm_device *dev, + enum pipe pipe) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + uint32_t tmp = I915_READ(PORT_DFT2_G4X); + + if (pipe == PIPE_A) + tmp &= ~PIPE_A_SCRAMBLE_RESET; + else + tmp &= ~PIPE_B_SCRAMBLE_RESET; + if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) + tmp &= ~DC_BALANCE_RESET_VLV; + I915_WRITE(PORT_DFT2_G4X, tmp); + +} + +static void g4x_undo_pipe_scramble_reset(struct drm_device *dev, + enum pipe pipe) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + uint32_t tmp = I915_READ(PORT_DFT2_G4X); + + if (pipe == PIPE_A) + tmp &= ~PIPE_A_SCRAMBLE_RESET; + else + tmp &= ~PIPE_B_SCRAMBLE_RESET; + I915_WRITE(PORT_DFT2_G4X, tmp); + + if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) { + I915_WRITE(PORT_DFT_I9XX, + I915_READ(PORT_DFT_I9XX) & ~DC_BALANCE_RESET); + } +} + +static int ilk_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, + uint32_t *val) +{ + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) + *source = INTEL_PIPE_CRC_SOURCE_PIPE; + + switch (*source) { + case INTEL_PIPE_CRC_SOURCE_PLANE1: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_ILK; + break; + case INTEL_PIPE_CRC_SOURCE_PLANE2: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_ILK; + break; + case INTEL_PIPE_CRC_SOURCE_PIPE: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PIPE_ILK; + break; + case INTEL_PIPE_CRC_SOURCE_NONE: + *val = 0; + break; + default: + return -EINVAL; + } + + return 0; +} + +static int ivb_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, + uint32_t *val) +{ + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) + *source = INTEL_PIPE_CRC_SOURCE_PF; + + switch (*source) { + case INTEL_PIPE_CRC_SOURCE_PLANE1: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PRIMARY_IVB; + break; + case INTEL_PIPE_CRC_SOURCE_PLANE2: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB; + break; + case INTEL_PIPE_CRC_SOURCE_PF: + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB; + break; + case INTEL_PIPE_CRC_SOURCE_NONE: + *val = 0; + break; + default: + return -EINVAL; + } + + return 0; +} + +static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, + enum intel_pipe_crc_source source) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; + u32 val = 0; /* shut up gcc */ + int ret; + + if (pipe_crc->source == source) + return 0; + + /* forbid changing the source without going back to 'none' */ + if (pipe_crc->source && source) + return -EINVAL; + + if (IS_GEN2(dev)) + ret = i8xx_pipe_crc_ctl_reg(&source, &val); + else if (INTEL_INFO(dev)->gen < 5) + ret = i9xx_pipe_crc_ctl_reg(dev, pipe, &source, &val); + else if (IS_VALLEYVIEW(dev)) + ret = vlv_pipe_crc_ctl_reg(dev,pipe, &source, &val); + else if (IS_GEN5(dev) || IS_GEN6(dev)) + ret = ilk_pipe_crc_ctl_reg(&source, &val); + else + ret = ivb_pipe_crc_ctl_reg(&source, &val); + + if (ret != 0) + return ret; + + /* none -> real source transition */ + if (source) { + DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n", + pipe_name(pipe), pipe_crc_source_name(source)); + + pipe_crc->entries = kzalloc(sizeof(*pipe_crc->entries) * + INTEL_PIPE_CRC_ENTRIES_NR, + GFP_KERNEL); + if (!pipe_crc->entries) + return -ENOMEM; + + spin_lock_irq(&pipe_crc->lock); + pipe_crc->head = 0; + pipe_crc->tail = 0; + spin_unlock_irq(&pipe_crc->lock); + } + + pipe_crc->source = source; + + I915_WRITE(PIPE_CRC_CTL(pipe), val); + POSTING_READ(PIPE_CRC_CTL(pipe)); + + /* real source -> none transition */ + if (source == INTEL_PIPE_CRC_SOURCE_NONE) { + struct intel_pipe_crc_entry *entries; + + DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n", + pipe_name(pipe)); + + intel_wait_for_vblank(dev, pipe); + + spin_lock_irq(&pipe_crc->lock); + entries = pipe_crc->entries; + pipe_crc->entries = NULL; + spin_unlock_irq(&pipe_crc->lock); + + kfree(entries); + + if (IS_G4X(dev)) + g4x_undo_pipe_scramble_reset(dev, pipe); + else if (IS_VALLEYVIEW(dev)) + vlv_undo_pipe_scramble_reset(dev, pipe); + } + + return 0; +} + +/* + * Parse pipe CRC command strings: + * command: wsp* object wsp+ name wsp+ source wsp* + * object: 'pipe' + * name: (A | B | C) + * source: (none | plane1 | plane2 | pf) + * wsp: (#0x20 | #0x9 | #0xA)+ + * + * eg.: + * "pipe A plane1" -> Start CRC computations on plane1 of pipe A + * "pipe A none" -> Stop CRC + */ +static int display_crc_ctl_tokenize(char *buf, char *words[], int max_words) +{ + int n_words = 0; + + while (*buf) { + char *end; + + /* skip leading white space */ + buf = skip_spaces(buf); + if (!*buf) + break; /* end of buffer */ + + /* find end of word */ + for (end = buf; *end && !isspace(*end); end++) + ; + + if (n_words == max_words) { + DRM_DEBUG_DRIVER("too many words, allowed <= %d\n", + max_words); + return -EINVAL; /* ran out of words[] before bytes */ + } + + if (*end) + *end++ = '\0'; + words[n_words++] = buf; + buf = end; + } + + return n_words; +} + +enum intel_pipe_crc_object { + PIPE_CRC_OBJECT_PIPE, +}; + +static const char * const pipe_crc_objects[] = { + "pipe", +}; + +static int +display_crc_ctl_parse_object(const char *buf, enum intel_pipe_crc_object *o) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(pipe_crc_objects); i++) + if (!strcmp(buf, pipe_crc_objects[i])) { + *o = i; + return 0; + } + + return -EINVAL; +} + +static int display_crc_ctl_parse_pipe(const char *buf, enum pipe *pipe) +{ + const char name = buf[0]; + + if (name < 'A' || name >= pipe_name(I915_MAX_PIPES)) + return -EINVAL; + + *pipe = name - 'A'; + + return 0; +} + +static int +display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(pipe_crc_sources); i++) + if (!strcmp(buf, pipe_crc_sources[i])) { + *s = i; + return 0; + } + + return -EINVAL; +} + +static int display_crc_ctl_parse(struct drm_device *dev, char *buf, size_t len) +{ +#define N_WORDS 3 + int n_words; + char *words[N_WORDS]; + enum pipe pipe; + enum intel_pipe_crc_object object; + enum intel_pipe_crc_source source; + + n_words = display_crc_ctl_tokenize(buf, words, N_WORDS); + if (n_words != N_WORDS) { + DRM_DEBUG_DRIVER("tokenize failed, a command is %d words\n", + N_WORDS); + return -EINVAL; + } + + if (display_crc_ctl_parse_object(words[0], &object) < 0) { + DRM_DEBUG_DRIVER("unknown object %s\n", words[0]); + return -EINVAL; + } + + if (display_crc_ctl_parse_pipe(words[1], &pipe) < 0) { + DRM_DEBUG_DRIVER("unknown pipe %s\n", words[1]); + return -EINVAL; + } + + if (display_crc_ctl_parse_source(words[2], &source) < 0) { + DRM_DEBUG_DRIVER("unknown source %s\n", words[2]); + return -EINVAL; + } + + return pipe_crc_set_source(dev, pipe, source); +} + +static ssize_t display_crc_ctl_write(struct file *file, const char __user *ubuf, + size_t len, loff_t *offp) +{ + struct seq_file *m = file->private_data; + struct drm_device *dev = m->private; + char *tmpbuf; + int ret; + + if (len == 0) + return 0; + + if (len > PAGE_SIZE - 1) { + DRM_DEBUG_DRIVER("expected <%lu bytes into pipe crc control\n", + PAGE_SIZE); + return -E2BIG; + } + + tmpbuf = kmalloc(len + 1, GFP_KERNEL); + if (!tmpbuf) + return -ENOMEM; + + if (copy_from_user(tmpbuf, ubuf, len)) { + ret = -EFAULT; + goto out; + } + tmpbuf[len] = '\0'; + + ret = display_crc_ctl_parse(dev, tmpbuf, len); + +out: + kfree(tmpbuf); + if (ret < 0) + return ret; + + *offp += len; + return len; +} + +void intel_display_crc_init(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + enum pipe pipe; + + for_each_pipe(pipe) { + struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe]; + + pipe_crc->opened = false; + spin_lock_init(&pipe_crc->lock); + init_waitqueue_head(&pipe_crc->wq); + } +} + +const struct file_operations i915_display_crc_ctl_fops = { + .owner = THIS_MODULE, + .open = display_crc_ctl_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, + .write = display_crc_ctl_write +}; -- 1.8.5.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 6/6] drm/i915: Move pipecrc debug functions to new file 2014-01-21 20:33 ` [PATCH 6/6] drm/i915: Move pipecrc debug functions to new file Ben Widawsky @ 2014-01-21 21:44 ` Damien Lespiau 2014-01-21 21:50 ` Damien Lespiau 0 siblings, 1 reply; 19+ messages in thread From: Damien Lespiau @ 2014-01-21 21:44 UTC (permalink / raw) To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky On Tue, Jan 21, 2014 at 12:33:22PM -0800, Ben Widawsky wrote: > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1356,6 +1356,18 @@ struct intel_pipe_crc { > wait_queue_head_t wq; > }; > > +struct pipe_crc_info { > + const char *name; > + struct drm_device *dev; > + enum pipe pipe; > +}; > + > +extern struct pipe_crc_info i915_pipe_crc_data[I915_MAX_PIPES]; > +extern const struct file_operations i915_display_crc_ctl_fops; > + > +int i915_pipe_crc_create(struct dentry *root, struct drm_minor *minor, > + enum pipe pipe); > + > typedef struct drm_i915_private { > struct drm_device *dev; > struct kmem_cache *slab; Oops, I think I should have looked at that hunk before, hidden in that big diff it wasn't obvious. We try to put display related things into intel_drv.h, it was in i915_drv.h because it was in debugfs.c. Also intel_drv.h and i915_drv.h are neatly separated by file, so it'd be nice to have a separate entry for intel_display_test.c. Bikeshedding territory, I know, sorry. With that changed or not you have my: Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Who knows Daniel may not insist on this particular OCD. -- Damien ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 6/6] drm/i915: Move pipecrc debug functions to new file 2014-01-21 21:44 ` Damien Lespiau @ 2014-01-21 21:50 ` Damien Lespiau 0 siblings, 0 replies; 19+ messages in thread From: Damien Lespiau @ 2014-01-21 21:50 UTC (permalink / raw) To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky On Tue, Jan 21, 2014 at 09:44:51PM +0000, Damien Lespiau wrote: > We try to put display related things into intel_drv.h, it was in > i915_drv.h because it was in debugfs.c. Also intel_drv.h and i915_drv.h > are neatly separated by file, so it'd be nice to have a separate entry > for intel_display_test.c. Also, there's another chunk in i915_drv.h you may want to move to intel_drv.h if you decide the OCD is justifed: #ifdef CONFIG_DEBUG_FS void intel_display_crc_init(struct drm_device *dev); #else static inline void intel_display_crc_init(struct drm_device *dev) {} #endif -- Damien ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2014-01-21 21:50 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-14 14:14 [PATCH 1/2] drm: share drm_add_fake_info_node Ben Widawsky 2014-01-14 14:14 ` [PATCH 2/2] drm/i915: Move pipecrc debug functions to new file Ben Widawsky 2014-01-14 23:28 ` Daniel Vetter 2014-01-15 1:22 ` Damien Lespiau 2014-01-15 8:46 ` Daniel Vetter 2014-01-15 20:04 ` Ben Widawsky 2014-01-14 23:25 ` [Intel-gfx] [PATCH 1/2] drm: share drm_add_fake_info_node Daniel Vetter 2014-01-14 23:40 ` Russell King - ARM Linux 2014-01-15 8:39 ` [Intel-gfx] " Daniel Vetter 2014-01-15 8:45 ` Daniel Vetter 2014-01-15 20:08 ` Ben Widawsky 2014-01-15 23:42 ` [Intel-gfx] " Daniel Vetter 2014-01-21 19:05 ` Ben Widawsky 2014-01-21 20:33 ` [PATCH 1/6] drm: Create a debugfs file creation helper Ben Widawsky 2014-01-21 20:33 ` [PATCH 4/6] drm/i915: Use new drm debugfs file helper Ben Widawsky 2014-01-21 20:33 ` [PATCH 5/6] drm/i915: Move forcewake debugfs setup also Ben Widawsky 2014-01-21 20:33 ` [PATCH 6/6] drm/i915: Move pipecrc debug functions to new file Ben Widawsky 2014-01-21 21:44 ` Damien Lespiau 2014-01-21 21:50 ` Damien Lespiau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox