* [PATCH] drm/i915: Remove support for legacy debugfs crc interface
@ 2017-11-02 13:56 Maarten Lankhorst
2017-11-02 14:27 ` Ville Syrjälä
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Maarten Lankhorst @ 2017-11-02 13:56 UTC (permalink / raw)
To: intel-gfx; +Cc: Tomi Sarvela, Jani Nikula
This interface is deprecated, and has been replaced by the upstream
drm crc interface.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 7 +-
drivers/gpu/drm/i915/i915_drv.h | 10 -
drivers/gpu/drm/i915/i915_irq.c | 79 ++----
drivers/gpu/drm/i915/intel_drv.h | 2 -
drivers/gpu/drm/i915/intel_pipe_crc.c | 446 ----------------------------------
5 files changed, 23 insertions(+), 521 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7efe57c0703e..a362370e5a68 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4991,7 +4991,6 @@ static const struct i915_debugfs_files {
{"i915_gpu_info", &i915_gpu_info_fops},
#endif
{"i915_next_seqno", &i915_next_seqno_fops},
- {"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
{"i915_pri_wm_latency", &i915_pri_wm_latency_fops},
{"i915_spr_wm_latency", &i915_spr_wm_latency_fops},
{"i915_cur_wm_latency", &i915_cur_wm_latency_fops},
@@ -5008,7 +5007,7 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
{
struct drm_minor *minor = dev_priv->drm.primary;
struct dentry *ent;
- int ret, i;
+ int i;
ent = debugfs_create_file("i915_forcewake_user", S_IRUSR,
minor->debugfs_root, to_i915(minor->dev),
@@ -5016,10 +5015,6 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
if (!ent)
return -ENOMEM;
- ret = intel_pipe_crc_create(minor);
- if (ret)
- return ret;
-
for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) {
ent = debugfs_create_file(i915_debugfs_files[i].name,
S_IRUGO | S_IWUSR,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d37fd11908d0..f4290c9739e1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1948,19 +1948,9 @@ enum intel_pipe_crc_source {
INTEL_PIPE_CRC_SOURCE_MAX,
};
-struct intel_pipe_crc_entry {
- uint32_t frame;
- uint32_t crc[5];
-};
-
#define INTEL_PIPE_CRC_ENTRIES_NR 128
struct intel_pipe_crc {
spinlock_t lock;
- bool opened; /* exclusive access to the result file */
- struct intel_pipe_crc_entry *entries;
- enum intel_pipe_crc_source source;
- int head, tail;
- wait_queue_head_t wq;
int skipped;
};
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ff00e462697a..be119cb567a4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1613,69 +1613,34 @@ static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
uint32_t crc4)
{
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
- struct intel_pipe_crc_entry *entry;
struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
- struct drm_driver *driver = dev_priv->drm.driver;
uint32_t crcs[5];
- int head, tail;
spin_lock(&pipe_crc->lock);
- if (pipe_crc->source) {
- if (!pipe_crc->entries) {
- spin_unlock(&pipe_crc->lock);
- DRM_DEBUG_KMS("spurious interrupt\n");
- return;
- }
-
- head = pipe_crc->head;
- tail = pipe_crc->tail;
-
- if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
- spin_unlock(&pipe_crc->lock);
- DRM_ERROR("CRC buffer overflowing\n");
- return;
- }
-
- entry = &pipe_crc->entries[head];
-
- entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
- entry->crc[0] = crc0;
- entry->crc[1] = crc1;
- entry->crc[2] = crc2;
- entry->crc[3] = crc3;
- entry->crc[4] = crc4;
-
- head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
- pipe_crc->head = head;
-
- spin_unlock(&pipe_crc->lock);
-
- wake_up_interruptible(&pipe_crc->wq);
- } else {
- /*
- * For some not yet identified reason, the first CRC is
- * bonkers. So let's just wait for the next vblank and read
- * out the buggy result.
- *
- * On GEN8+ sometimes the second CRC is bonkers as well, so
- * don't trust that one either.
- */
- if (pipe_crc->skipped == 0 ||
- (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
- pipe_crc->skipped++;
- spin_unlock(&pipe_crc->lock);
- return;
- }
+ /*
+ * For some not yet identified reason, the first CRC is
+ * bonkers. So let's just wait for the next vblank and read
+ * out the buggy result.
+ *
+ * On GEN8+ sometimes the second CRC is bonkers as well, so
+ * don't trust that one either.
+ */
+ if (pipe_crc->skipped == 0 ||
+ (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
+ pipe_crc->skipped++;
spin_unlock(&pipe_crc->lock);
- crcs[0] = crc0;
- crcs[1] = crc1;
- crcs[2] = crc2;
- crcs[3] = crc3;
- crcs[4] = crc4;
- drm_crtc_add_crc_entry(&crtc->base, true,
- drm_crtc_accurate_vblank_count(&crtc->base),
- crcs);
+ return;
}
+ spin_unlock(&pipe_crc->lock);
+
+ crcs[0] = crc0;
+ crcs[1] = crc1;
+ crcs[2] = crc2;
+ crcs[3] = crc3;
+ crcs[4] = crc4;
+ drm_crtc_add_crc_entry(&crtc->base, true,
+ drm_crtc_accurate_vblank_count(&crtc->base),
+ crcs);
}
#else
static inline void
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2f8b9af225ef..6b030b6fb700 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2013,12 +2013,10 @@ void lspcon_resume(struct intel_lspcon *lspcon);
void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
/* intel_pipe_crc.c */
-int intel_pipe_crc_create(struct drm_minor *minor);
#ifdef CONFIG_DEBUG_FS
int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
size_t *values_cnt);
#else
#define intel_crtc_set_crc_source NULL
#endif
-extern const struct file_operations i915_display_crc_ctl_fops;
#endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 899839f2f7c6..4ca4ba5a145b 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -30,160 +30,6 @@
#include <linux/debugfs.h>
#include "intel_drv.h"
-struct pipe_crc_info {
- const char *name;
- struct drm_i915_private *dev_priv;
- 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_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
- if (info->pipe >= INTEL_INFO(dev_priv)->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_priv;
- 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)
-{
- lockdep_assert_held(&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_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
- char buf[PIPE_CRC_BUFFER_LEN];
- int n_entries;
- 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 */
- n_entries = count / PIPE_CRC_LINE_LEN;
-
- bytes_read = 0;
- while (n_entries > 0) {
- struct intel_pipe_crc_entry *entry =
- &pipe_crc->entries[pipe_crc->tail];
-
- if (CIRC_CNT(pipe_crc->head, pipe_crc->tail,
- INTEL_PIPE_CRC_ENTRIES_NR) < 1)
- break;
-
- BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
- pipe_crc->tail = (pipe_crc->tail + 1) &
- (INTEL_PIPE_CRC_ENTRIES_NR - 1);
-
- 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]);
-
- spin_unlock_irq(&pipe_crc->lock);
-
- if (copy_to_user(user_buf, buf, PIPE_CRC_LINE_LEN))
- return -EFAULT;
-
- user_buf += PIPE_CRC_LINE_LEN;
- n_entries--;
-
- spin_lock_irq(&pipe_crc->lock);
- }
-
- 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 const char * const pipe_crc_sources[] = {
"none",
"plane1",
@@ -197,29 +43,6 @@ static const char * const pipe_crc_sources[] = {
"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_i915_private *dev_priv = m->private;
- enum pipe pipe;
-
- for_each_pipe(dev_priv, pipe)
- seq_printf(m, "%c %s\n", pipe_name(pipe),
- pipe_crc_source_name(dev_priv->pipe_crc[pipe].source));
-
- return 0;
-}
-
-static int display_crc_ctl_open(struct inode *inode, struct file *file)
-{
- return single_open(file, display_crc_ctl_show, inode->i_private);
-}
-
static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
uint32_t *val)
{
@@ -616,178 +439,6 @@ static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
}
-static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
- enum pipe pipe,
- enum intel_pipe_crc_source source)
-{
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
- enum intel_display_power_domain power_domain;
- 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;
-
- power_domain = POWER_DOMAIN_PIPE(pipe);
- if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) {
- DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
- return -EIO;
- }
-
- ret = get_new_crc_ctl_reg(dev_priv, pipe, &source, &val);
- if (ret != 0)
- goto out;
-
- /* none -> real source transition */
- if (source) {
- struct intel_pipe_crc_entry *entries;
-
- DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n",
- pipe_name(pipe), pipe_crc_source_name(source));
-
- entries = kcalloc(INTEL_PIPE_CRC_ENTRIES_NR,
- sizeof(pipe_crc->entries[0]),
- GFP_KERNEL);
- if (!entries) {
- ret = -ENOMEM;
- goto out;
- }
-
- spin_lock_irq(&pipe_crc->lock);
- kfree(pipe_crc->entries);
- pipe_crc->entries = entries;
- 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) {
- struct intel_pipe_crc_entry *entries;
- struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
- pipe);
-
- DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
- pipe_name(pipe));
-
- drm_modeset_lock(&crtc->base.mutex, NULL);
- if (crtc->base.state->active)
- intel_wait_for_vblank(dev_priv, pipe);
- drm_modeset_unlock(&crtc->base.mutex);
-
- spin_lock_irq(&pipe_crc->lock);
- entries = pipe_crc->entries;
- pipe_crc->entries = NULL;
- pipe_crc->head = 0;
- pipe_crc->tail = 0;
- spin_unlock_irq(&pipe_crc->lock);
-
- kfree(entries);
-
- if (IS_G4X(dev_priv))
- g4x_undo_pipe_scramble_reset(dev_priv, pipe);
- else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- vlv_undo_pipe_scramble_reset(dev_priv, pipe);
- else if ((IS_HASWELL(dev_priv) ||
- IS_BROADWELL(dev_priv)) && pipe == PIPE_A)
- hsw_pipe_A_crc_wa(dev_priv, false);
- }
-
- ret = 0;
-
-out:
- intel_display_power_put(dev_priv, power_domain);
-
- return ret;
-}
-
-/*
- * 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(struct drm_i915_private *dev_priv,
- const char *buf, enum pipe *pipe)
-{
- const char name = buf[0];
-
- if (name < 'A' || name >= pipe_name(INTEL_INFO(dev_priv)->num_pipes))
- return -EINVAL;
-
- *pipe = name - 'A';
-
- return 0;
-}
-
static int
display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
{
@@ -807,81 +458,6 @@ display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
return -EINVAL;
}
-static int display_crc_ctl_parse(struct drm_i915_private *dev_priv,
- 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(dev_priv, 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_priv, 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_i915_private *dev_priv = 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 = memdup_user_nul(ubuf, len);
- if (IS_ERR(tmpbuf))
- return PTR_ERR(tmpbuf);
-
- ret = display_crc_ctl_parse(dev_priv, tmpbuf, len);
-
- kfree(tmpbuf);
- if (ret < 0)
- return ret;
-
- *offp += len;
- return len;
-}
-
-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
-};
-
void intel_display_crc_init(struct drm_i915_private *dev_priv)
{
enum pipe pipe;
@@ -889,32 +465,10 @@ void intel_display_crc_init(struct drm_i915_private *dev_priv)
for_each_pipe(dev_priv, 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 intel_pipe_crc_create(struct drm_minor *minor)
-{
- struct drm_i915_private *dev_priv = to_i915(minor->dev);
- struct dentry *ent;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(i915_pipe_crc_data); i++) {
- struct pipe_crc_info *info = &i915_pipe_crc_data[i];
-
- info->dev_priv = dev_priv;
- ent = debugfs_create_file(info->name, S_IRUGO,
- minor->debugfs_root, info,
- &i915_pipe_crc_fops);
- if (!ent)
- return -ENOMEM;
- }
-
- return 0;
-}
-
int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
size_t *values_cnt)
{
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Remove support for legacy debugfs crc interface
2017-11-02 13:56 [PATCH] drm/i915: Remove support for legacy debugfs crc interface Maarten Lankhorst
@ 2017-11-02 14:27 ` Ville Syrjälä
2017-11-02 15:19 ` Jani Nikula
2017-11-02 14:46 ` ✗ Fi.CI.BAT: warning for " Patchwork
2017-11-21 11:51 ` Patchwork
2 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2017-11-02 14:27 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Tomi Sarvela, Jani Nikula, intel-gfx
On Thu, Nov 02, 2017 at 02:56:51PM +0100, Maarten Lankhorst wrote:
> This interface is deprecated, and has been replaced by the upstream
> drm crc interface.
Before we nuke this I would like to see an option in the new interface
to not filter out the "bad" CRCs. When analyzing how the hardware
behaves seeing every CRC can be valuable. And I'm not at all convinced
we should be dropping as many CRCs as we are currently.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 7 +-
> drivers/gpu/drm/i915/i915_drv.h | 10 -
> drivers/gpu/drm/i915/i915_irq.c | 79 ++----
> drivers/gpu/drm/i915/intel_drv.h | 2 -
> drivers/gpu/drm/i915/intel_pipe_crc.c | 446 ----------------------------------
> 5 files changed, 23 insertions(+), 521 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7efe57c0703e..a362370e5a68 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4991,7 +4991,6 @@ static const struct i915_debugfs_files {
> {"i915_gpu_info", &i915_gpu_info_fops},
> #endif
> {"i915_next_seqno", &i915_next_seqno_fops},
> - {"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
> {"i915_pri_wm_latency", &i915_pri_wm_latency_fops},
> {"i915_spr_wm_latency", &i915_spr_wm_latency_fops},
> {"i915_cur_wm_latency", &i915_cur_wm_latency_fops},
> @@ -5008,7 +5007,7 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
> {
> struct drm_minor *minor = dev_priv->drm.primary;
> struct dentry *ent;
> - int ret, i;
> + int i;
>
> ent = debugfs_create_file("i915_forcewake_user", S_IRUSR,
> minor->debugfs_root, to_i915(minor->dev),
> @@ -5016,10 +5015,6 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
> if (!ent)
> return -ENOMEM;
>
> - ret = intel_pipe_crc_create(minor);
> - if (ret)
> - return ret;
> -
> for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) {
> ent = debugfs_create_file(i915_debugfs_files[i].name,
> S_IRUGO | S_IWUSR,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d37fd11908d0..f4290c9739e1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1948,19 +1948,9 @@ enum intel_pipe_crc_source {
> INTEL_PIPE_CRC_SOURCE_MAX,
> };
>
> -struct intel_pipe_crc_entry {
> - uint32_t frame;
> - uint32_t crc[5];
> -};
> -
> #define INTEL_PIPE_CRC_ENTRIES_NR 128
> struct intel_pipe_crc {
> spinlock_t lock;
> - bool opened; /* exclusive access to the result file */
> - struct intel_pipe_crc_entry *entries;
> - enum intel_pipe_crc_source source;
> - int head, tail;
> - wait_queue_head_t wq;
> int skipped;
> };
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ff00e462697a..be119cb567a4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1613,69 +1613,34 @@ static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
> uint32_t crc4)
> {
> struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
> - struct intel_pipe_crc_entry *entry;
> struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> - struct drm_driver *driver = dev_priv->drm.driver;
> uint32_t crcs[5];
> - int head, tail;
>
> spin_lock(&pipe_crc->lock);
> - if (pipe_crc->source) {
> - if (!pipe_crc->entries) {
> - spin_unlock(&pipe_crc->lock);
> - DRM_DEBUG_KMS("spurious interrupt\n");
> - return;
> - }
> -
> - head = pipe_crc->head;
> - tail = pipe_crc->tail;
> -
> - if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
> - spin_unlock(&pipe_crc->lock);
> - DRM_ERROR("CRC buffer overflowing\n");
> - return;
> - }
> -
> - entry = &pipe_crc->entries[head];
> -
> - entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
> - entry->crc[0] = crc0;
> - entry->crc[1] = crc1;
> - entry->crc[2] = crc2;
> - entry->crc[3] = crc3;
> - entry->crc[4] = crc4;
> -
> - head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
> - pipe_crc->head = head;
> -
> - spin_unlock(&pipe_crc->lock);
> -
> - wake_up_interruptible(&pipe_crc->wq);
> - } else {
> - /*
> - * For some not yet identified reason, the first CRC is
> - * bonkers. So let's just wait for the next vblank and read
> - * out the buggy result.
> - *
> - * On GEN8+ sometimes the second CRC is bonkers as well, so
> - * don't trust that one either.
> - */
> - if (pipe_crc->skipped == 0 ||
> - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
> - pipe_crc->skipped++;
> - spin_unlock(&pipe_crc->lock);
> - return;
> - }
> + /*
> + * For some not yet identified reason, the first CRC is
> + * bonkers. So let's just wait for the next vblank and read
> + * out the buggy result.
> + *
> + * On GEN8+ sometimes the second CRC is bonkers as well, so
> + * don't trust that one either.
> + */
> + if (pipe_crc->skipped == 0 ||
> + (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
> + pipe_crc->skipped++;
> spin_unlock(&pipe_crc->lock);
> - crcs[0] = crc0;
> - crcs[1] = crc1;
> - crcs[2] = crc2;
> - crcs[3] = crc3;
> - crcs[4] = crc4;
> - drm_crtc_add_crc_entry(&crtc->base, true,
> - drm_crtc_accurate_vblank_count(&crtc->base),
> - crcs);
> + return;
> }
> + spin_unlock(&pipe_crc->lock);
> +
> + crcs[0] = crc0;
> + crcs[1] = crc1;
> + crcs[2] = crc2;
> + crcs[3] = crc3;
> + crcs[4] = crc4;
> + drm_crtc_add_crc_entry(&crtc->base, true,
> + drm_crtc_accurate_vblank_count(&crtc->base),
> + crcs);
> }
> #else
> static inline void
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2f8b9af225ef..6b030b6fb700 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2013,12 +2013,10 @@ void lspcon_resume(struct intel_lspcon *lspcon);
> void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
>
> /* intel_pipe_crc.c */
> -int intel_pipe_crc_create(struct drm_minor *minor);
> #ifdef CONFIG_DEBUG_FS
> int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
> size_t *values_cnt);
> #else
> #define intel_crtc_set_crc_source NULL
> #endif
> -extern const struct file_operations i915_display_crc_ctl_fops;
> #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
> index 899839f2f7c6..4ca4ba5a145b 100644
> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> @@ -30,160 +30,6 @@
> #include <linux/debugfs.h>
> #include "intel_drv.h"
>
> -struct pipe_crc_info {
> - const char *name;
> - struct drm_i915_private *dev_priv;
> - 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_priv;
> - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
> -
> - if (info->pipe >= INTEL_INFO(dev_priv)->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_priv;
> - 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)
> -{
> - lockdep_assert_held(&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_i915_private *dev_priv = info->dev_priv;
> - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
> - char buf[PIPE_CRC_BUFFER_LEN];
> - int n_entries;
> - 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 */
> - n_entries = count / PIPE_CRC_LINE_LEN;
> -
> - bytes_read = 0;
> - while (n_entries > 0) {
> - struct intel_pipe_crc_entry *entry =
> - &pipe_crc->entries[pipe_crc->tail];
> -
> - if (CIRC_CNT(pipe_crc->head, pipe_crc->tail,
> - INTEL_PIPE_CRC_ENTRIES_NR) < 1)
> - break;
> -
> - BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
> - pipe_crc->tail = (pipe_crc->tail + 1) &
> - (INTEL_PIPE_CRC_ENTRIES_NR - 1);
> -
> - 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]);
> -
> - spin_unlock_irq(&pipe_crc->lock);
> -
> - if (copy_to_user(user_buf, buf, PIPE_CRC_LINE_LEN))
> - return -EFAULT;
> -
> - user_buf += PIPE_CRC_LINE_LEN;
> - n_entries--;
> -
> - spin_lock_irq(&pipe_crc->lock);
> - }
> -
> - 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 const char * const pipe_crc_sources[] = {
> "none",
> "plane1",
> @@ -197,29 +43,6 @@ static const char * const pipe_crc_sources[] = {
> "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_i915_private *dev_priv = m->private;
> - enum pipe pipe;
> -
> - for_each_pipe(dev_priv, pipe)
> - seq_printf(m, "%c %s\n", pipe_name(pipe),
> - pipe_crc_source_name(dev_priv->pipe_crc[pipe].source));
> -
> - return 0;
> -}
> -
> -static int display_crc_ctl_open(struct inode *inode, struct file *file)
> -{
> - return single_open(file, display_crc_ctl_show, inode->i_private);
> -}
> -
> static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
> uint32_t *val)
> {
> @@ -616,178 +439,6 @@ static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
> return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
> }
>
> -static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
> - enum pipe pipe,
> - enum intel_pipe_crc_source source)
> -{
> - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
> - enum intel_display_power_domain power_domain;
> - 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;
> -
> - power_domain = POWER_DOMAIN_PIPE(pipe);
> - if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) {
> - DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
> - return -EIO;
> - }
> -
> - ret = get_new_crc_ctl_reg(dev_priv, pipe, &source, &val);
> - if (ret != 0)
> - goto out;
> -
> - /* none -> real source transition */
> - if (source) {
> - struct intel_pipe_crc_entry *entries;
> -
> - DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n",
> - pipe_name(pipe), pipe_crc_source_name(source));
> -
> - entries = kcalloc(INTEL_PIPE_CRC_ENTRIES_NR,
> - sizeof(pipe_crc->entries[0]),
> - GFP_KERNEL);
> - if (!entries) {
> - ret = -ENOMEM;
> - goto out;
> - }
> -
> - spin_lock_irq(&pipe_crc->lock);
> - kfree(pipe_crc->entries);
> - pipe_crc->entries = entries;
> - 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) {
> - struct intel_pipe_crc_entry *entries;
> - struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
> - pipe);
> -
> - DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
> - pipe_name(pipe));
> -
> - drm_modeset_lock(&crtc->base.mutex, NULL);
> - if (crtc->base.state->active)
> - intel_wait_for_vblank(dev_priv, pipe);
> - drm_modeset_unlock(&crtc->base.mutex);
> -
> - spin_lock_irq(&pipe_crc->lock);
> - entries = pipe_crc->entries;
> - pipe_crc->entries = NULL;
> - pipe_crc->head = 0;
> - pipe_crc->tail = 0;
> - spin_unlock_irq(&pipe_crc->lock);
> -
> - kfree(entries);
> -
> - if (IS_G4X(dev_priv))
> - g4x_undo_pipe_scramble_reset(dev_priv, pipe);
> - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> - vlv_undo_pipe_scramble_reset(dev_priv, pipe);
> - else if ((IS_HASWELL(dev_priv) ||
> - IS_BROADWELL(dev_priv)) && pipe == PIPE_A)
> - hsw_pipe_A_crc_wa(dev_priv, false);
> - }
> -
> - ret = 0;
> -
> -out:
> - intel_display_power_put(dev_priv, power_domain);
> -
> - return ret;
> -}
> -
> -/*
> - * 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(struct drm_i915_private *dev_priv,
> - const char *buf, enum pipe *pipe)
> -{
> - const char name = buf[0];
> -
> - if (name < 'A' || name >= pipe_name(INTEL_INFO(dev_priv)->num_pipes))
> - return -EINVAL;
> -
> - *pipe = name - 'A';
> -
> - return 0;
> -}
> -
> static int
> display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
> {
> @@ -807,81 +458,6 @@ display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
> return -EINVAL;
> }
>
> -static int display_crc_ctl_parse(struct drm_i915_private *dev_priv,
> - 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(dev_priv, 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_priv, 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_i915_private *dev_priv = 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 = memdup_user_nul(ubuf, len);
> - if (IS_ERR(tmpbuf))
> - return PTR_ERR(tmpbuf);
> -
> - ret = display_crc_ctl_parse(dev_priv, tmpbuf, len);
> -
> - kfree(tmpbuf);
> - if (ret < 0)
> - return ret;
> -
> - *offp += len;
> - return len;
> -}
> -
> -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
> -};
> -
> void intel_display_crc_init(struct drm_i915_private *dev_priv)
> {
> enum pipe pipe;
> @@ -889,32 +465,10 @@ void intel_display_crc_init(struct drm_i915_private *dev_priv)
> for_each_pipe(dev_priv, 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 intel_pipe_crc_create(struct drm_minor *minor)
> -{
> - struct drm_i915_private *dev_priv = to_i915(minor->dev);
> - struct dentry *ent;
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(i915_pipe_crc_data); i++) {
> - struct pipe_crc_info *info = &i915_pipe_crc_data[i];
> -
> - info->dev_priv = dev_priv;
> - ent = debugfs_create_file(info->name, S_IRUGO,
> - minor->debugfs_root, info,
> - &i915_pipe_crc_fops);
> - if (!ent)
> - return -ENOMEM;
> - }
> -
> - return 0;
> -}
> -
> int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
> size_t *values_cnt)
> {
> --
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915: Remove support for legacy debugfs crc interface
2017-11-02 13:56 [PATCH] drm/i915: Remove support for legacy debugfs crc interface Maarten Lankhorst
2017-11-02 14:27 ` Ville Syrjälä
@ 2017-11-02 14:46 ` Patchwork
2017-11-21 11:51 ` Patchwork
2 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-11-02 14:46 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Remove support for legacy debugfs crc interface
URL : https://patchwork.freedesktop.org/series/33053/
State : warning
== Summary ==
Series 33053v1 drm/i915: Remove support for legacy debugfs crc interface
https://patchwork.freedesktop.org/api/1.0/series/33053/revisions/1/mbox/
Test chamelium:
Subgroup dp-crc-fast:
fail -> PASS (fi-kbl-7500u) fdo#102514
Test kms_cursor_legacy:
Subgroup basic-flip-before-cursor-varying-size:
skip -> PASS (fi-hsw-4770r)
Test kms_pipe_crc_basic:
Subgroup bad-nb-words-1:
pass -> SKIP (fi-gdg-551)
pass -> SKIP (fi-blb-e6850)
pass -> SKIP (fi-pnv-d510)
pass -> SKIP (fi-bwr-2160)
pass -> SKIP (fi-elk-e7500)
pass -> SKIP (fi-ilk-650)
pass -> SKIP (fi-snb-2520m)
pass -> SKIP (fi-snb-2600)
pass -> SKIP (fi-ivb-3520m)
pass -> SKIP (fi-ivb-3770)
pass -> SKIP (fi-byt-j1900)
pass -> SKIP (fi-byt-n2820)
pass -> SKIP (fi-hsw-4770)
pass -> SKIP (fi-hsw-4770r)
pass -> SKIP (fi-bdw-5557u)
pass -> SKIP (fi-bdw-gvtdvm)
pass -> SKIP (fi-bsw-n3050)
pass -> SKIP (fi-skl-6260u)
pass -> SKIP (fi-skl-6600u)
pass -> SKIP (fi-skl-6700hq)
pass -> SKIP (fi-skl-6700k)
pass -> SKIP (fi-skl-6770hq)
pass -> SKIP (fi-skl-gvtdvm)
pass -> SKIP (fi-bxt-dsi)
pass -> SKIP (fi-bxt-j4205)
pass -> SKIP (fi-kbl-7500u)
pass -> SKIP (fi-kbl-7560u)
pass -> SKIP (fi-kbl-7567u) fdo#103165 +1
pass -> SKIP (fi-kbl-r)
pass -> SKIP (fi-glk-1)
pass -> SKIP (fi-glk-dsi)
pass -> SKIP (fi-cnl-y)
Subgroup bad-nb-words-3:
pass -> SKIP (fi-gdg-551)
pass -> SKIP (fi-blb-e6850)
pass -> SKIP (fi-pnv-d510)
pass -> SKIP (fi-bwr-2160)
pass -> SKIP (fi-elk-e7500)
pass -> SKIP (fi-ilk-650)
pass -> SKIP (fi-snb-2520m)
pass -> SKIP (fi-snb-2600)
pass -> SKIP (fi-ivb-3520m)
pass -> SKIP (fi-ivb-3770)
pass -> SKIP (fi-byt-j1900)
pass -> SKIP (fi-byt-n2820)
pass -> SKIP (fi-hsw-4770)
pass -> SKIP (fi-hsw-4770r)
pass -> SKIP (fi-bdw-5557u)
pass -> SKIP (fi-bdw-gvtdvm)
pass -> SKIP (fi-bsw-n3050)
pass -> SKIP (fi-skl-6260u)
pass -> SKIP (fi-skl-6600u)
pass -> SKIP (fi-skl-6700hq)
pass -> SKIP (fi-skl-6700k)
pass -> SKIP (fi-skl-6770hq)
pass -> SKIP (fi-skl-gvtdvm)
pass -> SKIP (fi-bxt-dsi)
pass -> SKIP (fi-bxt-j4205)
pass -> SKIP (fi-kbl-7500u)
pass -> SKIP (fi-kbl-7560u)
pass -> SKIP (fi-kbl-7567u)
pass -> SKIP (fi-kbl-r)
pass -> SKIP (fi-glk-1)
pass -> SKIP (fi-glk-dsi)
pass -> SKIP (fi-cnl-y)
Subgroup bad-pipe:
pass -> SKIP (fi-gdg-551)
pass -> SKIP (fi-blb-e6850)
pass -> SKIP (fi-pnv-d510)
pass -> SKIP (fi-bwr-2160)
pass -> SKIP (fi-elk-e7500)
pass -> SKIP (fi-ilk-650)
pass -> SKIP (fi-snb-2520m)
pass -> SKIP (fi-snb-2600)
pass -> SKIP (fi-ivb-3520m)
pass -> SKIP (fi-ivb-3770)
pass -> SKIP (fi-byt-j1900)
pass -> SKIP (fi-byt-n2820)
pass -> SKIP (fi-hsw-4770)
pass -> SKIP (fi-hsw-4770r)
pass -> SKIP (fi-bdw-5557u)
pass -> SKIP (fi-bdw-gvtdvm)
pass -> SKIP (fi-bsw-n3050)
pass -> SKIP (fi-skl-6260u)
pass -> SKIP (fi-skl-6600u)
pass -> SKIP (fi-skl-6700hq)
pass -> SKIP (fi-skl-6700k)
pass -> SKIP (fi-skl-6770hq)
pass -> SKIP (fi-skl-gvtdvm)
WARNING: Long output truncated
fca2506bc5d492609e3f1b6e59d667e376a1eb3f drm-tip: 2017y-11m-02d-13h-10m-58s UTC integration manifest
46cddd082892 drm/i915: Remove support for legacy debugfs crc interface
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6929/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Remove support for legacy debugfs crc interface
2017-11-02 14:27 ` Ville Syrjälä
@ 2017-11-02 15:19 ` Jani Nikula
2017-11-02 16:11 ` Ville Syrjälä
0 siblings, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2017-11-02 15:19 UTC (permalink / raw)
To: Ville Syrjälä, Maarten Lankhorst; +Cc: Tomi Sarvela, intel-gfx
On Thu, 02 Nov 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Nov 02, 2017 at 02:56:51PM +0100, Maarten Lankhorst wrote:
>> This interface is deprecated, and has been replaced by the upstream
>> drm crc interface.
>
> Before we nuke this I would like to see an option in the new interface
> to not filter out the "bad" CRCs. When analyzing how the hardware
> behaves seeing every CRC can be valuable. And I'm not at all convinced
> we should be dropping as many CRCs as we are currently.
I'm not against it, but do you have a concrete proposal on how that
option would look like?
BR,
Jani.
>
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 7 +-
>> drivers/gpu/drm/i915/i915_drv.h | 10 -
>> drivers/gpu/drm/i915/i915_irq.c | 79 ++----
>> drivers/gpu/drm/i915/intel_drv.h | 2 -
>> drivers/gpu/drm/i915/intel_pipe_crc.c | 446 ----------------------------------
>> 5 files changed, 23 insertions(+), 521 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 7efe57c0703e..a362370e5a68 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -4991,7 +4991,6 @@ static const struct i915_debugfs_files {
>> {"i915_gpu_info", &i915_gpu_info_fops},
>> #endif
>> {"i915_next_seqno", &i915_next_seqno_fops},
>> - {"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
>> {"i915_pri_wm_latency", &i915_pri_wm_latency_fops},
>> {"i915_spr_wm_latency", &i915_spr_wm_latency_fops},
>> {"i915_cur_wm_latency", &i915_cur_wm_latency_fops},
>> @@ -5008,7 +5007,7 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
>> {
>> struct drm_minor *minor = dev_priv->drm.primary;
>> struct dentry *ent;
>> - int ret, i;
>> + int i;
>>
>> ent = debugfs_create_file("i915_forcewake_user", S_IRUSR,
>> minor->debugfs_root, to_i915(minor->dev),
>> @@ -5016,10 +5015,6 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
>> if (!ent)
>> return -ENOMEM;
>>
>> - ret = intel_pipe_crc_create(minor);
>> - if (ret)
>> - return ret;
>> -
>> for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) {
>> ent = debugfs_create_file(i915_debugfs_files[i].name,
>> S_IRUGO | S_IWUSR,
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index d37fd11908d0..f4290c9739e1 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1948,19 +1948,9 @@ enum intel_pipe_crc_source {
>> INTEL_PIPE_CRC_SOURCE_MAX,
>> };
>>
>> -struct intel_pipe_crc_entry {
>> - uint32_t frame;
>> - uint32_t crc[5];
>> -};
>> -
>> #define INTEL_PIPE_CRC_ENTRIES_NR 128
>> struct intel_pipe_crc {
>> spinlock_t lock;
>> - bool opened; /* exclusive access to the result file */
>> - struct intel_pipe_crc_entry *entries;
>> - enum intel_pipe_crc_source source;
>> - int head, tail;
>> - wait_queue_head_t wq;
>> int skipped;
>> };
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index ff00e462697a..be119cb567a4 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1613,69 +1613,34 @@ static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
>> uint32_t crc4)
>> {
>> struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
>> - struct intel_pipe_crc_entry *entry;
>> struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
>> - struct drm_driver *driver = dev_priv->drm.driver;
>> uint32_t crcs[5];
>> - int head, tail;
>>
>> spin_lock(&pipe_crc->lock);
>> - if (pipe_crc->source) {
>> - if (!pipe_crc->entries) {
>> - spin_unlock(&pipe_crc->lock);
>> - DRM_DEBUG_KMS("spurious interrupt\n");
>> - return;
>> - }
>> -
>> - head = pipe_crc->head;
>> - tail = pipe_crc->tail;
>> -
>> - if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
>> - spin_unlock(&pipe_crc->lock);
>> - DRM_ERROR("CRC buffer overflowing\n");
>> - return;
>> - }
>> -
>> - entry = &pipe_crc->entries[head];
>> -
>> - entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
>> - entry->crc[0] = crc0;
>> - entry->crc[1] = crc1;
>> - entry->crc[2] = crc2;
>> - entry->crc[3] = crc3;
>> - entry->crc[4] = crc4;
>> -
>> - head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
>> - pipe_crc->head = head;
>> -
>> - spin_unlock(&pipe_crc->lock);
>> -
>> - wake_up_interruptible(&pipe_crc->wq);
>> - } else {
>> - /*
>> - * For some not yet identified reason, the first CRC is
>> - * bonkers. So let's just wait for the next vblank and read
>> - * out the buggy result.
>> - *
>> - * On GEN8+ sometimes the second CRC is bonkers as well, so
>> - * don't trust that one either.
>> - */
>> - if (pipe_crc->skipped == 0 ||
>> - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
>> - pipe_crc->skipped++;
>> - spin_unlock(&pipe_crc->lock);
>> - return;
>> - }
>> + /*
>> + * For some not yet identified reason, the first CRC is
>> + * bonkers. So let's just wait for the next vblank and read
>> + * out the buggy result.
>> + *
>> + * On GEN8+ sometimes the second CRC is bonkers as well, so
>> + * don't trust that one either.
>> + */
>> + if (pipe_crc->skipped == 0 ||
>> + (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
>> + pipe_crc->skipped++;
>> spin_unlock(&pipe_crc->lock);
>> - crcs[0] = crc0;
>> - crcs[1] = crc1;
>> - crcs[2] = crc2;
>> - crcs[3] = crc3;
>> - crcs[4] = crc4;
>> - drm_crtc_add_crc_entry(&crtc->base, true,
>> - drm_crtc_accurate_vblank_count(&crtc->base),
>> - crcs);
>> + return;
>> }
>> + spin_unlock(&pipe_crc->lock);
>> +
>> + crcs[0] = crc0;
>> + crcs[1] = crc1;
>> + crcs[2] = crc2;
>> + crcs[3] = crc3;
>> + crcs[4] = crc4;
>> + drm_crtc_add_crc_entry(&crtc->base, true,
>> + drm_crtc_accurate_vblank_count(&crtc->base),
>> + crcs);
>> }
>> #else
>> static inline void
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 2f8b9af225ef..6b030b6fb700 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -2013,12 +2013,10 @@ void lspcon_resume(struct intel_lspcon *lspcon);
>> void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
>>
>> /* intel_pipe_crc.c */
>> -int intel_pipe_crc_create(struct drm_minor *minor);
>> #ifdef CONFIG_DEBUG_FS
>> int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
>> size_t *values_cnt);
>> #else
>> #define intel_crtc_set_crc_source NULL
>> #endif
>> -extern const struct file_operations i915_display_crc_ctl_fops;
>> #endif /* __INTEL_DRV_H__ */
>> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
>> index 899839f2f7c6..4ca4ba5a145b 100644
>> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
>> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
>> @@ -30,160 +30,6 @@
>> #include <linux/debugfs.h>
>> #include "intel_drv.h"
>>
>> -struct pipe_crc_info {
>> - const char *name;
>> - struct drm_i915_private *dev_priv;
>> - 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_priv;
>> - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
>> -
>> - if (info->pipe >= INTEL_INFO(dev_priv)->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_priv;
>> - 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)
>> -{
>> - lockdep_assert_held(&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_i915_private *dev_priv = info->dev_priv;
>> - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
>> - char buf[PIPE_CRC_BUFFER_LEN];
>> - int n_entries;
>> - 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 */
>> - n_entries = count / PIPE_CRC_LINE_LEN;
>> -
>> - bytes_read = 0;
>> - while (n_entries > 0) {
>> - struct intel_pipe_crc_entry *entry =
>> - &pipe_crc->entries[pipe_crc->tail];
>> -
>> - if (CIRC_CNT(pipe_crc->head, pipe_crc->tail,
>> - INTEL_PIPE_CRC_ENTRIES_NR) < 1)
>> - break;
>> -
>> - BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
>> - pipe_crc->tail = (pipe_crc->tail + 1) &
>> - (INTEL_PIPE_CRC_ENTRIES_NR - 1);
>> -
>> - 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]);
>> -
>> - spin_unlock_irq(&pipe_crc->lock);
>> -
>> - if (copy_to_user(user_buf, buf, PIPE_CRC_LINE_LEN))
>> - return -EFAULT;
>> -
>> - user_buf += PIPE_CRC_LINE_LEN;
>> - n_entries--;
>> -
>> - spin_lock_irq(&pipe_crc->lock);
>> - }
>> -
>> - 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 const char * const pipe_crc_sources[] = {
>> "none",
>> "plane1",
>> @@ -197,29 +43,6 @@ static const char * const pipe_crc_sources[] = {
>> "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_i915_private *dev_priv = m->private;
>> - enum pipe pipe;
>> -
>> - for_each_pipe(dev_priv, pipe)
>> - seq_printf(m, "%c %s\n", pipe_name(pipe),
>> - pipe_crc_source_name(dev_priv->pipe_crc[pipe].source));
>> -
>> - return 0;
>> -}
>> -
>> -static int display_crc_ctl_open(struct inode *inode, struct file *file)
>> -{
>> - return single_open(file, display_crc_ctl_show, inode->i_private);
>> -}
>> -
>> static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
>> uint32_t *val)
>> {
>> @@ -616,178 +439,6 @@ static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
>> return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
>> }
>>
>> -static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
>> - enum pipe pipe,
>> - enum intel_pipe_crc_source source)
>> -{
>> - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
>> - enum intel_display_power_domain power_domain;
>> - 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;
>> -
>> - power_domain = POWER_DOMAIN_PIPE(pipe);
>> - if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>> - DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
>> - return -EIO;
>> - }
>> -
>> - ret = get_new_crc_ctl_reg(dev_priv, pipe, &source, &val);
>> - if (ret != 0)
>> - goto out;
>> -
>> - /* none -> real source transition */
>> - if (source) {
>> - struct intel_pipe_crc_entry *entries;
>> -
>> - DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n",
>> - pipe_name(pipe), pipe_crc_source_name(source));
>> -
>> - entries = kcalloc(INTEL_PIPE_CRC_ENTRIES_NR,
>> - sizeof(pipe_crc->entries[0]),
>> - GFP_KERNEL);
>> - if (!entries) {
>> - ret = -ENOMEM;
>> - goto out;
>> - }
>> -
>> - spin_lock_irq(&pipe_crc->lock);
>> - kfree(pipe_crc->entries);
>> - pipe_crc->entries = entries;
>> - 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) {
>> - struct intel_pipe_crc_entry *entries;
>> - struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
>> - pipe);
>> -
>> - DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
>> - pipe_name(pipe));
>> -
>> - drm_modeset_lock(&crtc->base.mutex, NULL);
>> - if (crtc->base.state->active)
>> - intel_wait_for_vblank(dev_priv, pipe);
>> - drm_modeset_unlock(&crtc->base.mutex);
>> -
>> - spin_lock_irq(&pipe_crc->lock);
>> - entries = pipe_crc->entries;
>> - pipe_crc->entries = NULL;
>> - pipe_crc->head = 0;
>> - pipe_crc->tail = 0;
>> - spin_unlock_irq(&pipe_crc->lock);
>> -
>> - kfree(entries);
>> -
>> - if (IS_G4X(dev_priv))
>> - g4x_undo_pipe_scramble_reset(dev_priv, pipe);
>> - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>> - vlv_undo_pipe_scramble_reset(dev_priv, pipe);
>> - else if ((IS_HASWELL(dev_priv) ||
>> - IS_BROADWELL(dev_priv)) && pipe == PIPE_A)
>> - hsw_pipe_A_crc_wa(dev_priv, false);
>> - }
>> -
>> - ret = 0;
>> -
>> -out:
>> - intel_display_power_put(dev_priv, power_domain);
>> -
>> - return ret;
>> -}
>> -
>> -/*
>> - * 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(struct drm_i915_private *dev_priv,
>> - const char *buf, enum pipe *pipe)
>> -{
>> - const char name = buf[0];
>> -
>> - if (name < 'A' || name >= pipe_name(INTEL_INFO(dev_priv)->num_pipes))
>> - return -EINVAL;
>> -
>> - *pipe = name - 'A';
>> -
>> - return 0;
>> -}
>> -
>> static int
>> display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
>> {
>> @@ -807,81 +458,6 @@ display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
>> return -EINVAL;
>> }
>>
>> -static int display_crc_ctl_parse(struct drm_i915_private *dev_priv,
>> - 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(dev_priv, 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_priv, 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_i915_private *dev_priv = 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 = memdup_user_nul(ubuf, len);
>> - if (IS_ERR(tmpbuf))
>> - return PTR_ERR(tmpbuf);
>> -
>> - ret = display_crc_ctl_parse(dev_priv, tmpbuf, len);
>> -
>> - kfree(tmpbuf);
>> - if (ret < 0)
>> - return ret;
>> -
>> - *offp += len;
>> - return len;
>> -}
>> -
>> -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
>> -};
>> -
>> void intel_display_crc_init(struct drm_i915_private *dev_priv)
>> {
>> enum pipe pipe;
>> @@ -889,32 +465,10 @@ void intel_display_crc_init(struct drm_i915_private *dev_priv)
>> for_each_pipe(dev_priv, 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 intel_pipe_crc_create(struct drm_minor *minor)
>> -{
>> - struct drm_i915_private *dev_priv = to_i915(minor->dev);
>> - struct dentry *ent;
>> - int i;
>> -
>> - for (i = 0; i < ARRAY_SIZE(i915_pipe_crc_data); i++) {
>> - struct pipe_crc_info *info = &i915_pipe_crc_data[i];
>> -
>> - info->dev_priv = dev_priv;
>> - ent = debugfs_create_file(info->name, S_IRUGO,
>> - minor->debugfs_root, info,
>> - &i915_pipe_crc_fops);
>> - if (!ent)
>> - return -ENOMEM;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
>> size_t *values_cnt)
>> {
>> --
>> 2.14.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Remove support for legacy debugfs crc interface
2017-11-02 15:19 ` Jani Nikula
@ 2017-11-02 16:11 ` Ville Syrjälä
2017-11-08 10:40 ` Maarten Lankhorst
0 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2017-11-02 16:11 UTC (permalink / raw)
To: Jani Nikula; +Cc: Tomi Sarvela, intel-gfx
On Thu, Nov 02, 2017 at 05:19:07PM +0200, Jani Nikula wrote:
> On Thu, 02 Nov 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Nov 02, 2017 at 02:56:51PM +0100, Maarten Lankhorst wrote:
> >> This interface is deprecated, and has been replaced by the upstream
> >> drm crc interface.
> >
> > Before we nuke this I would like to see an option in the new interface
> > to not filter out the "bad" CRCs. When analyzing how the hardware
> > behaves seeing every CRC can be valuable. And I'm not at all convinced
> > we should be dropping as many CRCs as we are currently.
>
> I'm not against it, but do you have a concrete proposal on how that
> option would look like?
Some kind of of filter_bad_crcs file with a bool value perhaps?
>
> BR,
> Jani.
>
> >
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> >> Cc: Petri Latvala <petri.latvala@intel.com>
> >> Cc: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_debugfs.c | 7 +-
> >> drivers/gpu/drm/i915/i915_drv.h | 10 -
> >> drivers/gpu/drm/i915/i915_irq.c | 79 ++----
> >> drivers/gpu/drm/i915/intel_drv.h | 2 -
> >> drivers/gpu/drm/i915/intel_pipe_crc.c | 446 ----------------------------------
> >> 5 files changed, 23 insertions(+), 521 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >> index 7efe57c0703e..a362370e5a68 100644
> >> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >> @@ -4991,7 +4991,6 @@ static const struct i915_debugfs_files {
> >> {"i915_gpu_info", &i915_gpu_info_fops},
> >> #endif
> >> {"i915_next_seqno", &i915_next_seqno_fops},
> >> - {"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
> >> {"i915_pri_wm_latency", &i915_pri_wm_latency_fops},
> >> {"i915_spr_wm_latency", &i915_spr_wm_latency_fops},
> >> {"i915_cur_wm_latency", &i915_cur_wm_latency_fops},
> >> @@ -5008,7 +5007,7 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
> >> {
> >> struct drm_minor *minor = dev_priv->drm.primary;
> >> struct dentry *ent;
> >> - int ret, i;
> >> + int i;
> >>
> >> ent = debugfs_create_file("i915_forcewake_user", S_IRUSR,
> >> minor->debugfs_root, to_i915(minor->dev),
> >> @@ -5016,10 +5015,6 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
> >> if (!ent)
> >> return -ENOMEM;
> >>
> >> - ret = intel_pipe_crc_create(minor);
> >> - if (ret)
> >> - return ret;
> >> -
> >> for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) {
> >> ent = debugfs_create_file(i915_debugfs_files[i].name,
> >> S_IRUGO | S_IWUSR,
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index d37fd11908d0..f4290c9739e1 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -1948,19 +1948,9 @@ enum intel_pipe_crc_source {
> >> INTEL_PIPE_CRC_SOURCE_MAX,
> >> };
> >>
> >> -struct intel_pipe_crc_entry {
> >> - uint32_t frame;
> >> - uint32_t crc[5];
> >> -};
> >> -
> >> #define INTEL_PIPE_CRC_ENTRIES_NR 128
> >> struct intel_pipe_crc {
> >> spinlock_t lock;
> >> - bool opened; /* exclusive access to the result file */
> >> - struct intel_pipe_crc_entry *entries;
> >> - enum intel_pipe_crc_source source;
> >> - int head, tail;
> >> - wait_queue_head_t wq;
> >> int skipped;
> >> };
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> index ff00e462697a..be119cb567a4 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -1613,69 +1613,34 @@ static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
> >> uint32_t crc4)
> >> {
> >> struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
> >> - struct intel_pipe_crc_entry *entry;
> >> struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> >> - struct drm_driver *driver = dev_priv->drm.driver;
> >> uint32_t crcs[5];
> >> - int head, tail;
> >>
> >> spin_lock(&pipe_crc->lock);
> >> - if (pipe_crc->source) {
> >> - if (!pipe_crc->entries) {
> >> - spin_unlock(&pipe_crc->lock);
> >> - DRM_DEBUG_KMS("spurious interrupt\n");
> >> - return;
> >> - }
> >> -
> >> - head = pipe_crc->head;
> >> - tail = pipe_crc->tail;
> >> -
> >> - if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
> >> - spin_unlock(&pipe_crc->lock);
> >> - DRM_ERROR("CRC buffer overflowing\n");
> >> - return;
> >> - }
> >> -
> >> - entry = &pipe_crc->entries[head];
> >> -
> >> - entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
> >> - entry->crc[0] = crc0;
> >> - entry->crc[1] = crc1;
> >> - entry->crc[2] = crc2;
> >> - entry->crc[3] = crc3;
> >> - entry->crc[4] = crc4;
> >> -
> >> - head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
> >> - pipe_crc->head = head;
> >> -
> >> - spin_unlock(&pipe_crc->lock);
> >> -
> >> - wake_up_interruptible(&pipe_crc->wq);
> >> - } else {
> >> - /*
> >> - * For some not yet identified reason, the first CRC is
> >> - * bonkers. So let's just wait for the next vblank and read
> >> - * out the buggy result.
> >> - *
> >> - * On GEN8+ sometimes the second CRC is bonkers as well, so
> >> - * don't trust that one either.
> >> - */
> >> - if (pipe_crc->skipped == 0 ||
> >> - (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
> >> - pipe_crc->skipped++;
> >> - spin_unlock(&pipe_crc->lock);
> >> - return;
> >> - }
> >> + /*
> >> + * For some not yet identified reason, the first CRC is
> >> + * bonkers. So let's just wait for the next vblank and read
> >> + * out the buggy result.
> >> + *
> >> + * On GEN8+ sometimes the second CRC is bonkers as well, so
> >> + * don't trust that one either.
> >> + */
> >> + if (pipe_crc->skipped == 0 ||
> >> + (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
> >> + pipe_crc->skipped++;
> >> spin_unlock(&pipe_crc->lock);
> >> - crcs[0] = crc0;
> >> - crcs[1] = crc1;
> >> - crcs[2] = crc2;
> >> - crcs[3] = crc3;
> >> - crcs[4] = crc4;
> >> - drm_crtc_add_crc_entry(&crtc->base, true,
> >> - drm_crtc_accurate_vblank_count(&crtc->base),
> >> - crcs);
> >> + return;
> >> }
> >> + spin_unlock(&pipe_crc->lock);
> >> +
> >> + crcs[0] = crc0;
> >> + crcs[1] = crc1;
> >> + crcs[2] = crc2;
> >> + crcs[3] = crc3;
> >> + crcs[4] = crc4;
> >> + drm_crtc_add_crc_entry(&crtc->base, true,
> >> + drm_crtc_accurate_vblank_count(&crtc->base),
> >> + crcs);
> >> }
> >> #else
> >> static inline void
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> index 2f8b9af225ef..6b030b6fb700 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -2013,12 +2013,10 @@ void lspcon_resume(struct intel_lspcon *lspcon);
> >> void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
> >>
> >> /* intel_pipe_crc.c */
> >> -int intel_pipe_crc_create(struct drm_minor *minor);
> >> #ifdef CONFIG_DEBUG_FS
> >> int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
> >> size_t *values_cnt);
> >> #else
> >> #define intel_crtc_set_crc_source NULL
> >> #endif
> >> -extern const struct file_operations i915_display_crc_ctl_fops;
> >> #endif /* __INTEL_DRV_H__ */
> >> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
> >> index 899839f2f7c6..4ca4ba5a145b 100644
> >> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> >> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> >> @@ -30,160 +30,6 @@
> >> #include <linux/debugfs.h>
> >> #include "intel_drv.h"
> >>
> >> -struct pipe_crc_info {
> >> - const char *name;
> >> - struct drm_i915_private *dev_priv;
> >> - 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_priv;
> >> - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
> >> -
> >> - if (info->pipe >= INTEL_INFO(dev_priv)->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_priv;
> >> - 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)
> >> -{
> >> - lockdep_assert_held(&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_i915_private *dev_priv = info->dev_priv;
> >> - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
> >> - char buf[PIPE_CRC_BUFFER_LEN];
> >> - int n_entries;
> >> - 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 */
> >> - n_entries = count / PIPE_CRC_LINE_LEN;
> >> -
> >> - bytes_read = 0;
> >> - while (n_entries > 0) {
> >> - struct intel_pipe_crc_entry *entry =
> >> - &pipe_crc->entries[pipe_crc->tail];
> >> -
> >> - if (CIRC_CNT(pipe_crc->head, pipe_crc->tail,
> >> - INTEL_PIPE_CRC_ENTRIES_NR) < 1)
> >> - break;
> >> -
> >> - BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
> >> - pipe_crc->tail = (pipe_crc->tail + 1) &
> >> - (INTEL_PIPE_CRC_ENTRIES_NR - 1);
> >> -
> >> - 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]);
> >> -
> >> - spin_unlock_irq(&pipe_crc->lock);
> >> -
> >> - if (copy_to_user(user_buf, buf, PIPE_CRC_LINE_LEN))
> >> - return -EFAULT;
> >> -
> >> - user_buf += PIPE_CRC_LINE_LEN;
> >> - n_entries--;
> >> -
> >> - spin_lock_irq(&pipe_crc->lock);
> >> - }
> >> -
> >> - 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 const char * const pipe_crc_sources[] = {
> >> "none",
> >> "plane1",
> >> @@ -197,29 +43,6 @@ static const char * const pipe_crc_sources[] = {
> >> "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_i915_private *dev_priv = m->private;
> >> - enum pipe pipe;
> >> -
> >> - for_each_pipe(dev_priv, pipe)
> >> - seq_printf(m, "%c %s\n", pipe_name(pipe),
> >> - pipe_crc_source_name(dev_priv->pipe_crc[pipe].source));
> >> -
> >> - return 0;
> >> -}
> >> -
> >> -static int display_crc_ctl_open(struct inode *inode, struct file *file)
> >> -{
> >> - return single_open(file, display_crc_ctl_show, inode->i_private);
> >> -}
> >> -
> >> static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
> >> uint32_t *val)
> >> {
> >> @@ -616,178 +439,6 @@ static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
> >> return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
> >> }
> >>
> >> -static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
> >> - enum pipe pipe,
> >> - enum intel_pipe_crc_source source)
> >> -{
> >> - struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
> >> - enum intel_display_power_domain power_domain;
> >> - 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;
> >> -
> >> - power_domain = POWER_DOMAIN_PIPE(pipe);
> >> - if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) {
> >> - DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
> >> - return -EIO;
> >> - }
> >> -
> >> - ret = get_new_crc_ctl_reg(dev_priv, pipe, &source, &val);
> >> - if (ret != 0)
> >> - goto out;
> >> -
> >> - /* none -> real source transition */
> >> - if (source) {
> >> - struct intel_pipe_crc_entry *entries;
> >> -
> >> - DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n",
> >> - pipe_name(pipe), pipe_crc_source_name(source));
> >> -
> >> - entries = kcalloc(INTEL_PIPE_CRC_ENTRIES_NR,
> >> - sizeof(pipe_crc->entries[0]),
> >> - GFP_KERNEL);
> >> - if (!entries) {
> >> - ret = -ENOMEM;
> >> - goto out;
> >> - }
> >> -
> >> - spin_lock_irq(&pipe_crc->lock);
> >> - kfree(pipe_crc->entries);
> >> - pipe_crc->entries = entries;
> >> - 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) {
> >> - struct intel_pipe_crc_entry *entries;
> >> - struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
> >> - pipe);
> >> -
> >> - DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
> >> - pipe_name(pipe));
> >> -
> >> - drm_modeset_lock(&crtc->base.mutex, NULL);
> >> - if (crtc->base.state->active)
> >> - intel_wait_for_vblank(dev_priv, pipe);
> >> - drm_modeset_unlock(&crtc->base.mutex);
> >> -
> >> - spin_lock_irq(&pipe_crc->lock);
> >> - entries = pipe_crc->entries;
> >> - pipe_crc->entries = NULL;
> >> - pipe_crc->head = 0;
> >> - pipe_crc->tail = 0;
> >> - spin_unlock_irq(&pipe_crc->lock);
> >> -
> >> - kfree(entries);
> >> -
> >> - if (IS_G4X(dev_priv))
> >> - g4x_undo_pipe_scramble_reset(dev_priv, pipe);
> >> - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> >> - vlv_undo_pipe_scramble_reset(dev_priv, pipe);
> >> - else if ((IS_HASWELL(dev_priv) ||
> >> - IS_BROADWELL(dev_priv)) && pipe == PIPE_A)
> >> - hsw_pipe_A_crc_wa(dev_priv, false);
> >> - }
> >> -
> >> - ret = 0;
> >> -
> >> -out:
> >> - intel_display_power_put(dev_priv, power_domain);
> >> -
> >> - return ret;
> >> -}
> >> -
> >> -/*
> >> - * 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(struct drm_i915_private *dev_priv,
> >> - const char *buf, enum pipe *pipe)
> >> -{
> >> - const char name = buf[0];
> >> -
> >> - if (name < 'A' || name >= pipe_name(INTEL_INFO(dev_priv)->num_pipes))
> >> - return -EINVAL;
> >> -
> >> - *pipe = name - 'A';
> >> -
> >> - return 0;
> >> -}
> >> -
> >> static int
> >> display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
> >> {
> >> @@ -807,81 +458,6 @@ display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
> >> return -EINVAL;
> >> }
> >>
> >> -static int display_crc_ctl_parse(struct drm_i915_private *dev_priv,
> >> - 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(dev_priv, 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_priv, 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_i915_private *dev_priv = 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 = memdup_user_nul(ubuf, len);
> >> - if (IS_ERR(tmpbuf))
> >> - return PTR_ERR(tmpbuf);
> >> -
> >> - ret = display_crc_ctl_parse(dev_priv, tmpbuf, len);
> >> -
> >> - kfree(tmpbuf);
> >> - if (ret < 0)
> >> - return ret;
> >> -
> >> - *offp += len;
> >> - return len;
> >> -}
> >> -
> >> -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
> >> -};
> >> -
> >> void intel_display_crc_init(struct drm_i915_private *dev_priv)
> >> {
> >> enum pipe pipe;
> >> @@ -889,32 +465,10 @@ void intel_display_crc_init(struct drm_i915_private *dev_priv)
> >> for_each_pipe(dev_priv, 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 intel_pipe_crc_create(struct drm_minor *minor)
> >> -{
> >> - struct drm_i915_private *dev_priv = to_i915(minor->dev);
> >> - struct dentry *ent;
> >> - int i;
> >> -
> >> - for (i = 0; i < ARRAY_SIZE(i915_pipe_crc_data); i++) {
> >> - struct pipe_crc_info *info = &i915_pipe_crc_data[i];
> >> -
> >> - info->dev_priv = dev_priv;
> >> - ent = debugfs_create_file(info->name, S_IRUGO,
> >> - minor->debugfs_root, info,
> >> - &i915_pipe_crc_fops);
> >> - if (!ent)
> >> - return -ENOMEM;
> >> - }
> >> -
> >> - return 0;
> >> -}
> >> -
> >> int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
> >> size_t *values_cnt)
> >> {
> >> --
> >> 2.14.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Remove support for legacy debugfs crc interface
2017-11-02 16:11 ` Ville Syrjälä
@ 2017-11-08 10:40 ` Maarten Lankhorst
2017-11-08 11:25 ` Ville Syrjälä
0 siblings, 1 reply; 11+ messages in thread
From: Maarten Lankhorst @ 2017-11-08 10:40 UTC (permalink / raw)
To: Ville Syrjälä, Jani Nikula; +Cc: Tomi Sarvela, intel-gfx
Op 02-11-17 om 17:11 schreef Ville Syrjälä:
> On Thu, Nov 02, 2017 at 05:19:07PM +0200, Jani Nikula wrote:
>> On Thu, 02 Nov 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>> On Thu, Nov 02, 2017 at 02:56:51PM +0100, Maarten Lankhorst wrote:
>>>> This interface is deprecated, and has been replaced by the upstream
>>>> drm crc interface.
>>> Before we nuke this I would like to see an option in the new interface
>>> to not filter out the "bad" CRCs. When analyzing how the hardware
>>> behaves seeing every CRC can be valuable. And I'm not at all convinced
>>> we should be dropping as many CRCs as we are currently.
>> I'm not against it, but do you have a concrete proposal on how that
>> option would look like?
> Some kind of of filter_bad_crcs file with a bool value perhaps?
You can set sources, might as well add a nofilter option.. But I don't see what it has to do
with this patch? This problem existed since before the api was introduced.. Only difference
is kernel eats possibly corrupt CRCs now instead of IGT.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Remove support for legacy debugfs crc interface
2017-11-08 10:40 ` Maarten Lankhorst
@ 2017-11-08 11:25 ` Ville Syrjälä
2017-11-21 10:39 ` Maarten Lankhorst
0 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2017-11-08 11:25 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Jani Nikula, Tomi Sarvela, intel-gfx
On Wed, Nov 08, 2017 at 11:40:19AM +0100, Maarten Lankhorst wrote:
> Op 02-11-17 om 17:11 schreef Ville Syrjälä:
> > On Thu, Nov 02, 2017 at 05:19:07PM +0200, Jani Nikula wrote:
> >> On Thu, 02 Nov 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >>> On Thu, Nov 02, 2017 at 02:56:51PM +0100, Maarten Lankhorst wrote:
> >>>> This interface is deprecated, and has been replaced by the upstream
> >>>> drm crc interface.
> >>> Before we nuke this I would like to see an option in the new interface
> >>> to not filter out the "bad" CRCs. When analyzing how the hardware
> >>> behaves seeing every CRC can be valuable. And I'm not at all convinced
> >>> we should be dropping as many CRCs as we are currently.
> >> I'm not against it, but do you have a concrete proposal on how that
> >> option would look like?
> > Some kind of of filter_bad_crcs file with a bool value perhaps?
>
> You can set sources, might as well add a nofilter option.. But I don't see what it has to do
> with this patch? This problem existed since before the api was introduced.. Only difference
> is kernel eats possibly corrupt CRCs now instead of IGT.
I don't use igt for this.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Remove support for legacy debugfs crc interface
2017-11-08 11:25 ` Ville Syrjälä
@ 2017-11-21 10:39 ` Maarten Lankhorst
2017-11-21 14:07 ` Ville Syrjälä
0 siblings, 1 reply; 11+ messages in thread
From: Maarten Lankhorst @ 2017-11-21 10:39 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Jani Nikula, Tomi Sarvela, intel-gfx
Op 08-11-17 om 12:25 schreef Ville Syrjälä:
> On Wed, Nov 08, 2017 at 11:40:19AM +0100, Maarten Lankhorst wrote:
>> Op 02-11-17 om 17:11 schreef Ville Syrjälä:
>>> On Thu, Nov 02, 2017 at 05:19:07PM +0200, Jani Nikula wrote:
>>>> On Thu, 02 Nov 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>>>> On Thu, Nov 02, 2017 at 02:56:51PM +0100, Maarten Lankhorst wrote:
>>>>>> This interface is deprecated, and has been replaced by the upstream
>>>>>> drm crc interface.
>>>>> Before we nuke this I would like to see an option in the new interface
>>>>> to not filter out the "bad" CRCs. When analyzing how the hardware
>>>>> behaves seeing every CRC can be valuable. And I'm not at all convinced
>>>>> we should be dropping as many CRCs as we are currently.
>>>> I'm not against it, but do you have a concrete proposal on how that
>>>> option would look like?
>>> Some kind of of filter_bad_crcs file with a bool value perhaps?
>> You can set sources, might as well add a nofilter option.. But I don't see what it has to do
>> with this patch? This problem existed since before the api was introduced.. Only difference
>> is kernel eats possibly corrupt CRCs now instead of IGT.
> I don't use igt for this.
>
If it's not in IGT then I'm not sure we should hold upthis patch for it tbh. You can
always change skipped = 0 to skipped = 2 for the new debugfs interface to find bugs
with garbage CRC values, or add a flag to pipe source parsing for not skipping
garbage CRC's.
Either way I think that it shouldn't hold up this patch.
Cheers,
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915: Remove support for legacy debugfs crc interface
2017-11-02 13:56 [PATCH] drm/i915: Remove support for legacy debugfs crc interface Maarten Lankhorst
2017-11-02 14:27 ` Ville Syrjälä
2017-11-02 14:46 ` ✗ Fi.CI.BAT: warning for " Patchwork
@ 2017-11-21 11:51 ` Patchwork
2 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-11-21 11:51 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Remove support for legacy debugfs crc interface
URL : https://patchwork.freedesktop.org/series/33053/
State : warning
== Summary ==
Series 33053v1 drm/i915: Remove support for legacy debugfs crc interface
https://patchwork.freedesktop.org/api/1.0/series/33053/revisions/1/mbox/
Test gem_exec_reloc:
Subgroup basic-write-cpu-active:
fail -> PASS (fi-gdg-551) fdo#102582
Test kms_cursor_legacy:
Subgroup basic-flip-before-cursor-varying-size:
skip -> PASS (fi-hsw-4770r)
Test kms_pipe_crc_basic:
Subgroup bad-nb-words-1:
pass -> SKIP (fi-gdg-551)
pass -> SKIP (fi-blb-e6850)
pass -> SKIP (fi-pnv-d510)
pass -> SKIP (fi-bwr-2160)
pass -> SKIP (fi-elk-e7500)
pass -> SKIP (fi-ilk-650)
pass -> SKIP (fi-snb-2520m)
pass -> SKIP (fi-snb-2600)
pass -> SKIP (fi-ivb-3520m)
pass -> SKIP (fi-ivb-3770)
pass -> SKIP (fi-byt-j1900)
pass -> SKIP (fi-byt-n2820)
pass -> SKIP (fi-hsw-4770)
pass -> SKIP (fi-hsw-4770r)
pass -> SKIP (fi-bdw-5557u)
pass -> SKIP (fi-bdw-gvtdvm)
pass -> SKIP (fi-bsw-n3050)
pass -> SKIP (fi-skl-6260u)
pass -> SKIP (fi-skl-6600u)
pass -> SKIP (fi-skl-6700hq)
pass -> SKIP (fi-skl-6700k)
pass -> SKIP (fi-skl-6770hq)
pass -> SKIP (fi-skl-gvtdvm)
pass -> SKIP (fi-bxt-dsi)
pass -> SKIP (fi-bxt-j4205)
pass -> SKIP (fi-kbl-7500u)
pass -> SKIP (fi-kbl-7560u) fdo#103165 +5
pass -> SKIP (fi-glk-1)
pass -> SKIP (fi-cfl-s2)
Subgroup bad-nb-words-3:
pass -> SKIP (fi-gdg-551)
pass -> SKIP (fi-blb-e6850)
pass -> SKIP (fi-pnv-d510)
pass -> SKIP (fi-bwr-2160)
pass -> SKIP (fi-elk-e7500)
pass -> SKIP (fi-ilk-650)
pass -> SKIP (fi-snb-2520m)
pass -> SKIP (fi-snb-2600)
pass -> SKIP (fi-ivb-3520m)
pass -> SKIP (fi-ivb-3770)
pass -> SKIP (fi-byt-j1900)
pass -> SKIP (fi-byt-n2820)
pass -> SKIP (fi-hsw-4770)
pass -> SKIP (fi-hsw-4770r)
pass -> SKIP (fi-bdw-5557u)
pass -> SKIP (fi-bdw-gvtdvm)
pass -> SKIP (fi-bsw-n3050)
pass -> SKIP (fi-skl-6260u)
pass -> SKIP (fi-skl-6600u)
pass -> SKIP (fi-skl-6700hq)
pass -> SKIP (fi-skl-6700k)
pass -> SKIP (fi-skl-6770hq)
pass -> SKIP (fi-skl-gvtdvm)
pass -> SKIP (fi-bxt-dsi)
pass -> SKIP (fi-bxt-j4205)
pass -> SKIP (fi-kbl-7500u)
pass -> SKIP (fi-kbl-7560u)
pass -> SKIP (fi-kbl-7567u)
pass -> SKIP (fi-kbl-r)
pass -> SKIP (fi-glk-1)
pass -> SKIP (fi-cfl-s2)
Subgroup bad-pipe:
pass -> SKIP (fi-gdg-551)
pass -> SKIP (fi-blb-e6850)
pass -> SKIP (fi-pnv-d510)
pass -> SKIP (fi-bwr-2160)
pass -> SKIP (fi-elk-e7500)
pass -> SKIP (fi-ilk-650)
pass -> SKIP (fi-snb-2520m)
pass -> SKIP (fi-snb-2600)
pass -> SKIP (fi-ivb-3520m)
pass -> SKIP (fi-ivb-3770)
pass -> SKIP (fi-byt-j1900)
pass -> SKIP (fi-byt-n2820)
pass -> SKIP (fi-hsw-4770)
pass -> SKIP (fi-hsw-4770r)
pass -> SKIP (fi-bdw-5557u)
pass -> SKIP (fi-bdw-gvtdvm)
pass -> SKIP (fi-bsw-n3050)
pass -> SKIP (fi-skl-6260u)
pass -> SKIP (fi-skl-6600u)
pass -> SKIP (fi-skl-6700hq)
pass -> SKIP (fi-skl-6700k)
pass -> SKIP (fi-skl-6770hq)
pass -> SKIP (fi-skl-gvtdvm)
pass -> SKIP (fi-bxt-dsi)
pass -> SKIP (fi-bxt-j4205)
pass -> SKIP (fi-kbl-7500u)
pass -> SKIP (fi-glk-1)
WARNING: Long output truncated
ee3fc3c956f817479cfe2bac3cc2a72112dbdec1 drm-tip: 2017y-11m-21d-09h-02m-11s UTC integration manifest
b8e2044e50f1 drm/i915: Remove support for legacy debugfs crc interface
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7216/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Remove support for legacy debugfs crc interface
2017-11-21 10:39 ` Maarten Lankhorst
@ 2017-11-21 14:07 ` Ville Syrjälä
0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2017-11-21 14:07 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Jani Nikula, Tomi Sarvela, intel-gfx
On Tue, Nov 21, 2017 at 11:39:01AM +0100, Maarten Lankhorst wrote:
> Op 08-11-17 om 12:25 schreef Ville Syrjälä:
> > On Wed, Nov 08, 2017 at 11:40:19AM +0100, Maarten Lankhorst wrote:
> >> Op 02-11-17 om 17:11 schreef Ville Syrjälä:
> >>> On Thu, Nov 02, 2017 at 05:19:07PM +0200, Jani Nikula wrote:
> >>>> On Thu, 02 Nov 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >>>>> On Thu, Nov 02, 2017 at 02:56:51PM +0100, Maarten Lankhorst wrote:
> >>>>>> This interface is deprecated, and has been replaced by the upstream
> >>>>>> drm crc interface.
> >>>>> Before we nuke this I would like to see an option in the new interface
> >>>>> to not filter out the "bad" CRCs. When analyzing how the hardware
> >>>>> behaves seeing every CRC can be valuable. And I'm not at all convinced
> >>>>> we should be dropping as many CRCs as we are currently.
> >>>> I'm not against it, but do you have a concrete proposal on how that
> >>>> option would look like?
> >>> Some kind of of filter_bad_crcs file with a bool value perhaps?
> >> You can set sources, might as well add a nofilter option.. But I don't see what it has to do
> >> with this patch? This problem existed since before the api was introduced.. Only difference
> >> is kernel eats possibly corrupt CRCs now instead of IGT.
> > I don't use igt for this.
> >
> If it's not in IGT then I'm not sure we should hold upthis patch for it tbh. You can
> always change skipped = 0 to skipped = 2 for the new debugfs interface to find bugs
> with garbage CRC values, or add a flag to pipe source parsing for not skipping
> garbage CRC's.
Why do you want to intentionally generate more pointless work for me?
If you don't want to fix the new interface to have feature parity
with the old one then just don't remove the old one. It's not
hurting anyone AFAICS.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/i915: Remove support for legacy debugfs crc interface
@ 2018-03-08 18:34 Maarten Lankhorst
0 siblings, 0 replies; 11+ messages in thread
From: Maarten Lankhorst @ 2018-03-08 18:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Tomi Sarvela, Jani Nikula
This interface is deprecated, and has been replaced by the upstream
drm crc interface.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 7 +-
drivers/gpu/drm/i915/i915_drv.h | 11 +-
drivers/gpu/drm/i915/i915_irq.c | 79 ++----
drivers/gpu/drm/i915/intel_drv.h | 2 -
drivers/gpu/drm/i915/intel_pipe_crc.c | 446 ----------------------------------
5 files changed, 24 insertions(+), 521 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 89f7ff2c652e..8f0570def4c8 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4771,7 +4771,6 @@ static const struct i915_debugfs_files {
{"i915_gpu_info", &i915_gpu_info_fops},
#endif
{"i915_next_seqno", &i915_next_seqno_fops},
- {"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
{"i915_pri_wm_latency", &i915_pri_wm_latency_fops},
{"i915_spr_wm_latency", &i915_spr_wm_latency_fops},
{"i915_cur_wm_latency", &i915_cur_wm_latency_fops},
@@ -4789,7 +4788,7 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
{
struct drm_minor *minor = dev_priv->drm.primary;
struct dentry *ent;
- int ret, i;
+ int i;
ent = debugfs_create_file("i915_forcewake_user", S_IRUSR,
minor->debugfs_root, to_i915(minor->dev),
@@ -4797,10 +4796,6 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
if (!ent)
return -ENOMEM;
- ret = intel_pipe_crc_create(minor);
- if (ret)
- return ret;
-
for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) {
ent = debugfs_create_file(i915_debugfs_files[i].name,
S_IRUGO | S_IWUSR,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6e740f6fe33f..0a51eae8ce27 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1525,20 +1525,11 @@ enum intel_pipe_crc_source {
INTEL_PIPE_CRC_SOURCE_MAX,
};
-struct intel_pipe_crc_entry {
- uint32_t frame;
- uint32_t crc[5];
-};
-
#define INTEL_PIPE_CRC_ENTRIES_NR 128
struct intel_pipe_crc {
spinlock_t lock;
- bool opened; /* exclusive access to the result file */
- struct intel_pipe_crc_entry *entries;
- enum intel_pipe_crc_source source;
- int head, tail;
- wait_queue_head_t wq;
int skipped;
+ enum intel_pipe_crc_source source;
};
struct i915_frontbuffer_tracking {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index babf81cf668b..26c1014eb1f7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1620,69 +1620,34 @@ static void display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
uint32_t crc4)
{
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
- struct intel_pipe_crc_entry *entry;
struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
- struct drm_driver *driver = dev_priv->drm.driver;
uint32_t crcs[5];
- int head, tail;
spin_lock(&pipe_crc->lock);
- if (pipe_crc->source && !crtc->base.crc.opened) {
- if (!pipe_crc->entries) {
- spin_unlock(&pipe_crc->lock);
- DRM_DEBUG_KMS("spurious interrupt\n");
- return;
- }
-
- head = pipe_crc->head;
- tail = pipe_crc->tail;
-
- if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
- spin_unlock(&pipe_crc->lock);
- DRM_ERROR("CRC buffer overflowing\n");
- return;
- }
-
- entry = &pipe_crc->entries[head];
-
- entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
- entry->crc[0] = crc0;
- entry->crc[1] = crc1;
- entry->crc[2] = crc2;
- entry->crc[3] = crc3;
- entry->crc[4] = crc4;
-
- head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
- pipe_crc->head = head;
-
- spin_unlock(&pipe_crc->lock);
-
- wake_up_interruptible(&pipe_crc->wq);
- } else {
- /*
- * For some not yet identified reason, the first CRC is
- * bonkers. So let's just wait for the next vblank and read
- * out the buggy result.
- *
- * On GEN8+ sometimes the second CRC is bonkers as well, so
- * don't trust that one either.
- */
- if (pipe_crc->skipped <= 0 ||
- (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
- pipe_crc->skipped++;
- spin_unlock(&pipe_crc->lock);
- return;
- }
+ /*
+ * For some not yet identified reason, the first CRC is
+ * bonkers. So let's just wait for the next vblank and read
+ * out the buggy result.
+ *
+ * On GEN8+ sometimes the second CRC is bonkers as well, so
+ * don't trust that one either.
+ */
+ if (pipe_crc->skipped <= 0 ||
+ (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped == 1)) {
+ pipe_crc->skipped++;
spin_unlock(&pipe_crc->lock);
- crcs[0] = crc0;
- crcs[1] = crc1;
- crcs[2] = crc2;
- crcs[3] = crc3;
- crcs[4] = crc4;
- drm_crtc_add_crc_entry(&crtc->base, true,
- drm_crtc_accurate_vblank_count(&crtc->base),
- crcs);
+ return;
}
+ spin_unlock(&pipe_crc->lock);
+
+ crcs[0] = crc0;
+ crcs[1] = crc1;
+ crcs[2] = crc2;
+ crcs[3] = crc3;
+ crcs[4] = crc4;
+ drm_crtc_add_crc_entry(&crtc->base, true,
+ drm_crtc_accurate_vblank_count(&crtc->base),
+ crcs);
}
#else
static inline void
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fce5d3072d97..85dc09d3b19c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2134,7 +2134,6 @@ void lspcon_resume(struct intel_lspcon *lspcon);
void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
/* intel_pipe_crc.c */
-int intel_pipe_crc_create(struct drm_minor *minor);
#ifdef CONFIG_DEBUG_FS
int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
size_t *values_cnt);
@@ -2150,5 +2149,4 @@ static inline void intel_crtc_enable_pipe_crc(struct intel_crtc *crtc)
{
}
#endif
-extern const struct file_operations i915_display_crc_ctl_fops;
#endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 4f367c16e9e5..0e38c382e231 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -30,160 +30,6 @@
#include <linux/debugfs.h>
#include "intel_drv.h"
-struct pipe_crc_info {
- const char *name;
- struct drm_i915_private *dev_priv;
- 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_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
- if (info->pipe >= INTEL_INFO(dev_priv)->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_priv;
- 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)
-{
- lockdep_assert_held(&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_i915_private *dev_priv = info->dev_priv;
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
- char buf[PIPE_CRC_BUFFER_LEN];
- int n_entries;
- 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 */
- n_entries = count / PIPE_CRC_LINE_LEN;
-
- bytes_read = 0;
- while (n_entries > 0) {
- struct intel_pipe_crc_entry *entry =
- &pipe_crc->entries[pipe_crc->tail];
-
- if (CIRC_CNT(pipe_crc->head, pipe_crc->tail,
- INTEL_PIPE_CRC_ENTRIES_NR) < 1)
- break;
-
- BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
- pipe_crc->tail = (pipe_crc->tail + 1) &
- (INTEL_PIPE_CRC_ENTRIES_NR - 1);
-
- 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]);
-
- spin_unlock_irq(&pipe_crc->lock);
-
- if (copy_to_user(user_buf, buf, PIPE_CRC_LINE_LEN))
- return -EFAULT;
-
- user_buf += PIPE_CRC_LINE_LEN;
- n_entries--;
-
- spin_lock_irq(&pipe_crc->lock);
- }
-
- 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 const char * const pipe_crc_sources[] = {
"none",
"plane1",
@@ -197,29 +43,6 @@ static const char * const pipe_crc_sources[] = {
"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_i915_private *dev_priv = m->private;
- enum pipe pipe;
-
- for_each_pipe(dev_priv, pipe)
- seq_printf(m, "%c %s\n", pipe_name(pipe),
- pipe_crc_source_name(dev_priv->pipe_crc[pipe].source));
-
- return 0;
-}
-
-static int display_crc_ctl_open(struct inode *inode, struct file *file)
-{
- return single_open(file, display_crc_ctl_show, inode->i_private);
-}
-
static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source,
uint32_t *val)
{
@@ -616,178 +439,6 @@ static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val, set_wa);
}
-static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
- enum pipe pipe,
- enum intel_pipe_crc_source source)
-{
- struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
- enum intel_display_power_domain power_domain;
- 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;
-
- power_domain = POWER_DOMAIN_PIPE(pipe);
- if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) {
- DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
- return -EIO;
- }
-
- ret = get_new_crc_ctl_reg(dev_priv, pipe, &source, &val, true);
- if (ret != 0)
- goto out;
-
- /* none -> real source transition */
- if (source) {
- struct intel_pipe_crc_entry *entries;
-
- DRM_DEBUG_DRIVER("collecting CRCs for pipe %c, %s\n",
- pipe_name(pipe), pipe_crc_source_name(source));
-
- entries = kcalloc(INTEL_PIPE_CRC_ENTRIES_NR,
- sizeof(pipe_crc->entries[0]),
- GFP_KERNEL);
- if (!entries) {
- ret = -ENOMEM;
- goto out;
- }
-
- spin_lock_irq(&pipe_crc->lock);
- kfree(pipe_crc->entries);
- pipe_crc->entries = entries;
- 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) {
- struct intel_pipe_crc_entry *entries;
- struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv,
- pipe);
-
- DRM_DEBUG_DRIVER("stopping CRCs for pipe %c\n",
- pipe_name(pipe));
-
- drm_modeset_lock(&crtc->base.mutex, NULL);
- if (crtc->base.state->active)
- intel_wait_for_vblank(dev_priv, pipe);
- drm_modeset_unlock(&crtc->base.mutex);
-
- spin_lock_irq(&pipe_crc->lock);
- entries = pipe_crc->entries;
- pipe_crc->entries = NULL;
- pipe_crc->head = 0;
- pipe_crc->tail = 0;
- spin_unlock_irq(&pipe_crc->lock);
-
- kfree(entries);
-
- if (IS_G4X(dev_priv))
- g4x_undo_pipe_scramble_reset(dev_priv, pipe);
- else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
- vlv_undo_pipe_scramble_reset(dev_priv, pipe);
- else if ((IS_HASWELL(dev_priv) ||
- IS_BROADWELL(dev_priv)) && pipe == PIPE_A)
- hsw_pipe_A_crc_wa(dev_priv, false);
- }
-
- ret = 0;
-
-out:
- intel_display_power_put(dev_priv, power_domain);
-
- return ret;
-}
-
-/*
- * 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(struct drm_i915_private *dev_priv,
- const char *buf, enum pipe *pipe)
-{
- const char name = buf[0];
-
- if (name < 'A' || name >= pipe_name(INTEL_INFO(dev_priv)->num_pipes))
- return -EINVAL;
-
- *pipe = name - 'A';
-
- return 0;
-}
-
static int
display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
{
@@ -807,81 +458,6 @@ display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
return -EINVAL;
}
-static int display_crc_ctl_parse(struct drm_i915_private *dev_priv,
- 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(dev_priv, 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_priv, 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_i915_private *dev_priv = 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 = memdup_user_nul(ubuf, len);
- if (IS_ERR(tmpbuf))
- return PTR_ERR(tmpbuf);
-
- ret = display_crc_ctl_parse(dev_priv, tmpbuf, len);
-
- kfree(tmpbuf);
- if (ret < 0)
- return ret;
-
- *offp += len;
- return len;
-}
-
-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
-};
-
void intel_display_crc_init(struct drm_i915_private *dev_priv)
{
enum pipe pipe;
@@ -889,30 +465,8 @@ void intel_display_crc_init(struct drm_i915_private *dev_priv)
for_each_pipe(dev_priv, 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 intel_pipe_crc_create(struct drm_minor *minor)
-{
- struct drm_i915_private *dev_priv = to_i915(minor->dev);
- struct dentry *ent;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(i915_pipe_crc_data); i++) {
- struct pipe_crc_info *info = &i915_pipe_crc_data[i];
-
- info->dev_priv = dev_priv;
- ent = debugfs_create_file(info->name, S_IRUGO,
- minor->debugfs_root, info,
- &i915_pipe_crc_fops);
- if (!ent)
- return -ENOMEM;
}
-
- return 0;
}
int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
--
2.16.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-03-08 18:34 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-02 13:56 [PATCH] drm/i915: Remove support for legacy debugfs crc interface Maarten Lankhorst
2017-11-02 14:27 ` Ville Syrjälä
2017-11-02 15:19 ` Jani Nikula
2017-11-02 16:11 ` Ville Syrjälä
2017-11-08 10:40 ` Maarten Lankhorst
2017-11-08 11:25 ` Ville Syrjälä
2017-11-21 10:39 ` Maarten Lankhorst
2017-11-21 14:07 ` Ville Syrjälä
2017-11-02 14:46 ` ✗ Fi.CI.BAT: warning for " Patchwork
2017-11-21 11:51 ` Patchwork
2018-03-08 18:34 [PATCH] " Maarten Lankhorst
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.