| Message ID | 20171102135651.9329-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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.
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
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.
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) {
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(-)