| Message ID | 1475767268-14379-3-git-send-email-tomeu.vizoso@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Adding Benjamin Gaignard to CC in case he wants to comment on the usage of the registration functions, as suggested by Daniel Vetter. Regards, Tomeu On 6 October 2016 at 17:21, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: > Adds files and directories to debugfs for controlling and reading frame > CRCs, per CRTC: > > dri/0/crtc-0/crc > dri/0/crtc-0/crc/control > dri/0/crtc-0/crc/data > > Drivers can implement the set_crc_source callback() in drm_crtc_funcs to > start and stop generating frame CRCs and can add entries to the output > by calling drm_crtc_add_crc_entry. > > v2: > - Lots of good fixes suggested by Thierry. > - Added documentation. > - Changed the debugfs layout. > - Moved to allocate the entries circular queue once when frame > generation gets enabled for the first time. > v3: > - Use the control file just to select the source, and start and stop > capture when the data file is opened and closed, respectively. > - Make variable the number of CRC values per entry, per source. > - Allocate entries queue each time we start capturing as now there > isn't a fixed number of CRC values per entry. > - Store the frame counter in the data file as a 8-digit hex number. > - For sources that cannot provide useful frame numbers, place > XXXXXXXX in the frame field. > > v4: > - Build only if CONFIG_DEBUG_FS is enabled. > - Use memdup_user_nul. > - Consolidate calculation of the size of an entry in a helper. > - Add 0x prefix to hex numbers in the data file. > - Remove unnecessary snprintf and strlen usage in read callback. > > v5: > - Made the crcs array in drm_crtc_crc_entry fixed-size > - Lots of other smaller improvements suggested by Emil Velikov > > v7: > - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h > > v8: > - Call debugfs_remove_recursive when we fail to create the minor > device > > v9: > - Register the debugfs directory for a crtc from > drm_crtc_register_all() > > v10: > - Don't let debugfs failures interrupt CRTC registration (Emil > Velikov) > > v11: > - Remove extra brace that broke compilation. Sorry! > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > --- > > Documentation/gpu/drm-uapi.rst | 6 + > drivers/gpu/drm/Makefile | 3 +- > drivers/gpu/drm/drm_crtc.c | 34 +++- > drivers/gpu/drm/drm_debugfs.c | 34 +++- > drivers/gpu/drm/drm_debugfs_crc.c | 351 ++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_internal.h | 16 ++ > include/drm/drm_crtc.h | 41 +++++ > include/drm/drm_debugfs_crc.h | 73 ++++++++ > 8 files changed, 555 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c > create mode 100644 include/drm/drm_debugfs_crc.h > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 1ba301cebe16..de3ac9f90f8f 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -216,3 +216,9 @@ interfaces. Especially since all hardware-acceleration interfaces to > userspace are driver specific for efficiency and other reasons these > interfaces can be rather substantial. Hence every driver has its own > chapter. > + > +Testing and validation > +====================== > + > +.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c > + :doc: CRC ABI > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 25c720454017..74579d2e796e 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -9,7 +9,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ > drm_scatter.o drm_pci.o \ > drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \ > drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \ > - drm_info.o drm_debugfs.o drm_encoder_slave.o \ > + drm_info.o drm_encoder_slave.o \ > drm_trace_points.o drm_global.o drm_prime.o \ > drm_rect.o drm_vma_manager.o drm_flip_work.o \ > drm_modeset_lock.o drm_atomic.o drm_bridge.o \ > @@ -23,6 +23,7 @@ drm-$(CONFIG_PCI) += ati_pcigart.o > drm-$(CONFIG_DRM_PANEL) += drm_panel.o > drm-$(CONFIG_OF) += drm_of.o > drm-$(CONFIG_AGP) += drm_agpsupport.o > +drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o > > drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ > drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \ > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 2d7bedf28647..60403bf7a4ff 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -40,7 +40,7 @@ > #include <drm/drm_modeset_lock.h> > #include <drm/drm_atomic.h> > #include <drm/drm_auth.h> > -#include <drm/drm_framebuffer.h> > +#include <drm/drm_debugfs_crc.h> > > #include "drm_crtc_internal.h" > #include "drm_internal.h" > @@ -122,6 +122,10 @@ static int drm_crtc_register_all(struct drm_device *dev) > int ret = 0; > > drm_for_each_crtc(crtc, dev) { > + if (drm_debugfs_crtc_add(crtc)) > + DRM_ERROR("Failed to initialize debugfs entry for CRTC '%s'.\n", > + crtc->name); > + > if (crtc->funcs->late_register) > ret = crtc->funcs->late_register(crtc); > if (ret) > @@ -138,9 +142,29 @@ static void drm_crtc_unregister_all(struct drm_device *dev) > drm_for_each_crtc(crtc, dev) { > if (crtc->funcs->early_unregister) > crtc->funcs->early_unregister(crtc); > + drm_debugfs_crtc_remove(crtc); > } > } > > +static int drm_crtc_crc_init(struct drm_crtc *crtc) > +{ > +#ifdef CONFIG_DEBUG_FS > + spin_lock_init(&crtc->crc.lock); > + init_waitqueue_head(&crtc->crc.wq); > + crtc->crc.source = kstrdup("auto", GFP_KERNEL); > + if (!crtc->crc.source) > + return -ENOMEM; > +#endif > + return 0; > +} > + > +static void drm_crtc_crc_fini(struct drm_crtc *crtc) > +{ > +#ifdef CONFIG_DEBUG_FS > + kfree(crtc->crc.source); > +#endif > +} > + > /** > * drm_crtc_init_with_planes - Initialise a new CRTC object with > * specified primary and cursor planes. > @@ -210,6 +234,12 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, > if (cursor) > cursor->possible_crtcs = 1 << drm_crtc_index(crtc); > > + ret = drm_crtc_crc_init(crtc); > + if (ret) { > + drm_mode_object_unregister(dev, &crtc->base); > + return ret; > + } > + > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { > drm_object_attach_property(&crtc->base, config->prop_active, 0); > drm_object_attach_property(&crtc->base, config->prop_mode_id, 0); > @@ -236,6 +266,8 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) > * the indices on the drm_crtc after us in the crtc_list. > */ > > + drm_crtc_crc_fini(crtc); > + > kfree(crtc->gamma_store); > crtc->gamma_store = NULL; > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index 1205790ed960..800055c39cdb 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -415,5 +415,37 @@ void drm_debugfs_connector_remove(struct drm_connector *connector) > connector->debugfs_entry = NULL; > } > > -#endif /* CONFIG_DEBUG_FS */ > +int drm_debugfs_crtc_add(struct drm_crtc *crtc) > +{ > + struct drm_minor *minor = crtc->dev->primary; > + struct dentry *root; > + char *name; > + > + name = kasprintf(GFP_KERNEL, "crtc-%d", crtc->index); > + if (!name) > + return -ENOMEM; > + > + root = debugfs_create_dir(name, minor->debugfs_root); > + kfree(name); > + if (!root) > + return -ENOMEM; > + > + crtc->debugfs_entry = root; > + > + if (drm_debugfs_crtc_crc_add(crtc)) > + goto error; > > + return 0; > + > +error: > + drm_debugfs_crtc_remove(crtc); > + return -ENOMEM; > +} > + > +void drm_debugfs_crtc_remove(struct drm_crtc *crtc) > +{ > + debugfs_remove_recursive(crtc->debugfs_entry); > + crtc->debugfs_entry = NULL; > +} > + > +#endif /* CONFIG_DEBUG_FS */ > diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c > new file mode 100644 > index 000000000000..4129405d17c0 > --- /dev/null > +++ b/drivers/gpu/drm/drm_debugfs_crc.c > @@ -0,0 +1,351 @@ > +/* > + * Copyright © 2008 Intel Corporation > + * Copyright © 2016 Collabora Ltd > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + * Based on code from the i915 driver. > + * Original author: Damien Lespiau <damien.lespiau@intel.com> > + * > + */ > + > +#include <linux/circ_buf.h> > +#include <linux/ctype.h> > +#include <linux/debugfs.h> > +#include <drm/drmP.h> > + > +/** > + * DOC: CRC ABI > + * > + * DRM device drivers can provide to userspace CRC information of each frame as > + * it reached a given hardware component (a "source"). > + * > + * Userspace can control generation of CRCs in a given CRTC by writing to the > + * file dri/0/crtc-N/crc/control in debugfs, with N being the index of the CRTC. > + * Accepted values are source names (which are driver-specific) and the "auto" > + * keyword, which will let the driver select a default source of frame CRCs > + * for this CRTC. > + * > + * Once frame CRC generation is enabled, userspace can capture them by reading > + * the dri/0/crtc-N/crc/data file. Each line in that file contains the frame > + * number in the first field and then a number of unsigned integer fields > + * containing the CRC data. Fields are separated by a single space and the number > + * of CRC fields is source-specific. > + * > + * Note that though in some cases the CRC is computed in a specified way and on > + * the frame contents as supplied by userspace (eDP 1.3), in general the CRC > + * computation is performed in an unspecified way and on frame contents that have > + * been already processed in also an unspecified way and thus userspace cannot > + * rely on being able to generate matching CRC values for the frame contents that > + * it submits. In this general case, the maximum userspace can do is to compare > + * the reported CRCs of frames that should have the same contents. > + */ > + > +static int crc_control_show(struct seq_file *m, void *data) > +{ > + struct drm_crtc *crtc = m->private; > + > + seq_printf(m, "%s\n", crtc->crc.source); > + > + return 0; > +} > + > +static int crc_control_open(struct inode *inode, struct file *file) > +{ > + struct drm_crtc *crtc = inode->i_private; > + > + return single_open(file, crc_control_show, crtc); > +} > + > +static ssize_t crc_control_write(struct file *file, const char __user *ubuf, > + size_t len, loff_t *offp) > +{ > + struct seq_file *m = file->private_data; > + struct drm_crtc *crtc = m->private; > + struct drm_crtc_crc *crc = &crtc->crc; > + char *source; > + > + if (len == 0) > + return 0; > + > + if (len > PAGE_SIZE - 1) { > + DRM_DEBUG_KMS("Expected < %lu bytes into crtc crc control\n", > + PAGE_SIZE); > + return -E2BIG; > + } > + > + source = memdup_user_nul(ubuf, len); > + if (IS_ERR(source)) > + return PTR_ERR(source); > + > + if (source[len] == '\n') > + source[len] = '\0'; > + > + spin_lock_irq(&crc->lock); > + > + if (crc->opened) { > + spin_unlock_irq(&crc->lock); > + kfree(source); > + return -EBUSY; > + } > + > + kfree(crc->source); > + crc->source = source; > + > + spin_unlock_irq(&crc->lock); > + > + *offp += len; > + return len; > +} > + > +const struct file_operations drm_crtc_crc_control_fops = { > + .owner = THIS_MODULE, > + .open = crc_control_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > + .write = crc_control_write > +}; > + > +static int crtc_crc_open(struct inode *inode, struct file *filep) > +{ > + struct drm_crtc *crtc = inode->i_private; > + struct drm_crtc_crc *crc = &crtc->crc; > + struct drm_crtc_crc_entry *entries = NULL; > + size_t values_cnt; > + int ret; > + > + if (crc->opened) > + return -EBUSY; > + > + ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt); > + if (ret) > + return ret; > + > + if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) { > + ret = -EINVAL; > + goto err_disable; > + } > + > + if (WARN_ON(values_cnt == 0)) { > + ret = -EINVAL; > + goto err_disable; > + } > + > + entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL); > + if (!entries) { > + ret = -ENOMEM; > + goto err_disable; > + } > + > + spin_lock_irq(&crc->lock); > + crc->entries = entries; > + crc->values_cnt = values_cnt; > + crc->opened = true; > + spin_unlock_irq(&crc->lock); > + > + return 0; > + > +err_disable: > + crtc->funcs->set_crc_source(crtc, NULL, &values_cnt); > + return ret; > +} > + > +static int crtc_crc_release(struct inode *inode, struct file *filep) > +{ > + struct drm_crtc *crtc = filep->f_inode->i_private; > + struct drm_crtc_crc *crc = &crtc->crc; > + size_t values_cnt; > + > + spin_lock_irq(&crc->lock); > + kfree(crc->entries); > + crc->entries = NULL; > + crc->head = 0; > + crc->tail = 0; > + crc->values_cnt = 0; > + crc->opened = false; > + spin_unlock_irq(&crc->lock); > + > + crtc->funcs->set_crc_source(crtc, NULL, &values_cnt); > + > + return 0; > +} > + > +static int crtc_crc_data_count(struct drm_crtc_crc *crc) > +{ > + assert_spin_locked(&crc->lock); > + return CIRC_CNT(crc->head, crc->tail, DRM_CRC_ENTRIES_NR); > +} > + > +/* > + * 1 frame field of 10 chars plus a number of CRC fields of 10 chars each, space > + * separated, with a newline at the end and null-terminated. > + */ > +#define LINE_LEN(values_cnt) (10 + 11 * values_cnt + 1 + 1) > +#define MAX_LINE_LEN (LINE_LEN(DRM_MAX_CRC_NR)) > + > +static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf, > + size_t count, loff_t *pos) > +{ > + struct drm_crtc *crtc = filep->f_inode->i_private; > + struct drm_crtc_crc *crc = &crtc->crc; > + struct drm_crtc_crc_entry *entry; > + char buf[MAX_LINE_LEN]; > + int ret, i; > + > + spin_lock_irq(&crc->lock); > + > + if (!crc->source) { > + spin_unlock_irq(&crc->lock); > + return 0; > + } > + > + /* Nothing to read? */ > + while (crtc_crc_data_count(crc) == 0) { > + if (filep->f_flags & O_NONBLOCK) { > + spin_unlock_irq(&crc->lock); > + return -EAGAIN; > + } > + > + ret = wait_event_interruptible_lock_irq(crc->wq, > + crtc_crc_data_count(crc), > + crc->lock); > + if (ret) { > + spin_unlock_irq(&crc->lock); > + return ret; > + } > + } > + > + /* We know we have an entry to be read */ > + entry = &crc->entries[crc->tail]; > + > + if (count < LINE_LEN(crc->values_cnt)) { > + spin_unlock_irq(&crc->lock); > + return -EINVAL; > + } > + > + BUILD_BUG_ON_NOT_POWER_OF_2(DRM_CRC_ENTRIES_NR); > + crc->tail = (crc->tail + 1) & (DRM_CRC_ENTRIES_NR - 1); > + > + spin_unlock_irq(&crc->lock); > + > + if (entry->has_frame_counter) > + sprintf(buf, "0x%08x", entry->frame); > + else > + sprintf(buf, "XXXXXXXXXX"); > + > + for (i = 0; i < crc->values_cnt; i++) > + sprintf(buf + 10 + i * 11, " 0x%08x", entry->crcs[i]); > + sprintf(buf + 10 + crc->values_cnt * 11, "\n"); > + > + if (copy_to_user(user_buf, buf, LINE_LEN(crc->values_cnt))) > + return -EFAULT; > + > + return LINE_LEN(crc->values_cnt); > +} > + > +const struct file_operations drm_crtc_crc_data_fops = { > + .owner = THIS_MODULE, > + .open = crtc_crc_open, > + .read = crtc_crc_read, > + .release = crtc_crc_release, > +}; > + > +/** > + * drm_debugfs_crtc_crc_add - Add files to debugfs for capture of frame CRCs > + * @crtc: CRTC to whom the frames will belong > + * > + * Adds files to debugfs directory that allows userspace to control the > + * generation of frame CRCs and to read them. > + * > + * Returns: > + * Zero on success, error code on failure. > + */ > +int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) > +{ > + struct dentry *crc_ent, *ent; > + > + if (!crtc->funcs->set_crc_source) > + return 0; > + > + crc_ent = debugfs_create_dir("crc", crtc->debugfs_entry); > + if (!crc_ent) > + return -ENOMEM; > + > + ent = debugfs_create_file("control", S_IRUGO, crc_ent, crtc, > + &drm_crtc_crc_control_fops); > + if (!ent) > + goto error; > + > + ent = debugfs_create_file("data", S_IRUGO, crc_ent, crtc, > + &drm_crtc_crc_data_fops); > + if (!ent) > + goto error; > + > + return 0; > + > +error: > + debugfs_remove_recursive(crc_ent); > + > + return -ENOMEM; > +} > + > +/** > + * drm_crtc_add_crc_entry - Add entry with CRC information for a frame > + * @crtc: CRTC to which the frame belongs > + * @has_frame: whether this entry has a frame number to go with > + * @frame: number of the frame these CRCs are about > + * @crcs: array of CRC values, with length matching #drm_crtc_crc.values_cnt > + * > + * For each frame, the driver polls the source of CRCs for new data and calls > + * this function to add them to the buffer from where userspace reads. > + */ > +int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, > + uint32_t frame, uint32_t *crcs) > +{ > + struct drm_crtc_crc *crc = &crtc->crc; > + struct drm_crtc_crc_entry *entry; > + int head, tail; > + > + assert_spin_locked(&crc->lock); > + > + /* Caller may not have noticed yet that userspace has stopped reading */ > + if (!crc->opened) > + return -EINVAL; > + > + head = crc->head; > + tail = crc->tail; > + > + if (CIRC_SPACE(head, tail, DRM_CRC_ENTRIES_NR) < 1) { > + DRM_ERROR("Overflow of CRC buffer, userspace reads too slow.\n"); > + return -ENOBUFS; > + } > + > + entry = &crc->entries[head]; > + entry->frame = frame; > + entry->has_frame_counter = has_frame; > + memcpy(&entry->crcs, crcs, sizeof(*crcs) * crc->values_cnt); > + > + head = (head + 1) & (DRM_CRC_ENTRIES_NR - 1); > + crc->head = head; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(drm_crtc_add_crc_entry); > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h > index e66af289a016..abd209863ef4 100644 > --- a/drivers/gpu/drm/drm_internal.h > +++ b/drivers/gpu/drm/drm_internal.h > @@ -100,6 +100,9 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id, > int drm_debugfs_cleanup(struct drm_minor *minor); > int drm_debugfs_connector_add(struct drm_connector *connector); > void drm_debugfs_connector_remove(struct drm_connector *connector); > +int drm_debugfs_crtc_add(struct drm_crtc *crtc); > +void drm_debugfs_crtc_remove(struct drm_crtc *crtc); > +int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc); > #else > static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id, > struct dentry *root) > @@ -119,4 +122,17 @@ static inline int drm_debugfs_connector_add(struct drm_connector *connector) > static inline void drm_debugfs_connector_remove(struct drm_connector *connector) > { > } > + > +static inline int drm_debugfs_crtc_add(struct drm_crtc *crtc) > +{ > + return 0; > +} > +static inline void drm_debugfs_crtc_remove(struct drm_crtc *crtc) > +{ > +} > + > +static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) > +{ > + return 0; > +} > #endif > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 61932f55f788..0908b2cf6c2c 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -47,6 +47,7 @@ > #include <drm/drm_plane.h> > #include <drm/drm_blend.h> > #include <drm/drm_color_mgmt.h> > +#include <drm/drm_debugfs_crc.h> > > struct drm_device; > struct drm_mode_set; > @@ -564,6 +565,30 @@ struct drm_crtc_funcs { > * before data structures are torndown. > */ > void (*early_unregister)(struct drm_crtc *crtc); > + > + /** > + * @set_crc_source: > + * > + * Changes the source of CRC checksums of frames at the request of > + * userspace, typically for testing purposes. The sources available are > + * specific of each driver and a %NULL value indicates that CRC > + * generation is to be switched off. > + * > + * When CRC generation is enabled, the driver should call > + * drm_crtc_add_crc_entry() at each frame, providing any information > + * that characterizes the frame contents in the crcN arguments, as > + * provided from the configured source. Drivers must accept a "auto" > + * source name that will select a default source for this CRTC. > + * > + * This callback is optional if the driver does not support any CRC > + * generation functionality. > + * > + * RETURNS: > + * > + * 0 on success or a negative error code on failure. > + */ > + int (*set_crc_source)(struct drm_crtc *crtc, const char *source, > + size_t *values_cnt); > }; > > /** > @@ -680,6 +705,22 @@ struct drm_crtc { > * context. > */ > struct drm_modeset_acquire_ctx *acquire_ctx; > + > +#ifdef CONFIG_DEBUG_FS > + /** > + * @debugfs_entry: > + * > + * Debugfs directory for this CRTC. > + */ > + struct dentry *debugfs_entry; > + > + /** > + * @crc: > + * > + * Configuration settings of CRC capture. > + */ > + struct drm_crtc_crc crc; > +#endif > }; > > /** > diff --git a/include/drm/drm_debugfs_crc.h b/include/drm/drm_debugfs_crc.h > new file mode 100644 > index 000000000000..7d63b1d4adb9 > --- /dev/null > +++ b/include/drm/drm_debugfs_crc.h > @@ -0,0 +1,73 @@ > +/* > + * Copyright © 2016 Collabora Ltd. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > + * OTHER DEALINGS IN THE SOFTWARE. > + */ > +#ifndef __DRM_DEBUGFS_CRC_H__ > +#define __DRM_DEBUGFS_CRC_H__ > + > +#define DRM_MAX_CRC_NR 10 > + > +/** > + * struct drm_crtc_crc_entry - entry describing a frame's content > + * @has_frame_counter: whether the source was able to provide a frame number > + * @frame: number of the frame this CRC is about, if @has_frame_counter is true > + * @crc: array of values that characterize the frame > + */ > +struct drm_crtc_crc_entry { > + bool has_frame_counter; > + uint32_t frame; > + uint32_t crcs[DRM_MAX_CRC_NR]; > +}; > + > +#define DRM_CRC_ENTRIES_NR 128 > + > +/** > + * struct drm_crtc_crc - data supporting CRC capture on a given CRTC > + * @lock: protects the fields in this struct > + * @source: name of the currently configured source of CRCs > + * @opened: whether userspace has opened the data file for reading > + * @entries: array of entries, with size of %DRM_CRC_ENTRIES_NR > + * @head: head of circular queue > + * @tail: tail of circular queue > + * @values_cnt: number of CRC values per entry, up to %DRM_MAX_CRC_NR > + * @wq: workqueue used to synchronize reading and writing > + */ > +struct drm_crtc_crc { > + spinlock_t lock; > + const char *source; > + bool opened; > + struct drm_crtc_crc_entry *entries; > + int head, tail; > + size_t values_cnt; > + wait_queue_head_t wq; > +}; > + > +#if defined(CONFIG_DEBUG_FS) > +int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, > + uint32_t frame, uint32_t *crcs); > +#else > +static inline int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, > + uint32_t frame, uint32_t *crcs) > +{ > + return -EINVAL; > +} > +#endif /* defined(CONFIG_DEBUG_FS) */ > + > +#endif /* __DRM_DEBUGFS_CRC_H__ */ > -- > 2.7.4 >
On 6 October 2016 at 16:21, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: > Adds files and directories to debugfs for controlling and reading frame > CRCs, per CRTC: > > dri/0/crtc-0/crc > dri/0/crtc-0/crc/control > dri/0/crtc-0/crc/data > > Drivers can implement the set_crc_source callback() in drm_crtc_funcs to > start and stop generating frame CRCs and can add entries to the output > by calling drm_crtc_add_crc_entry. > > v2: > - Lots of good fixes suggested by Thierry. > - Added documentation. > - Changed the debugfs layout. > - Moved to allocate the entries circular queue once when frame > generation gets enabled for the first time. > v3: > - Use the control file just to select the source, and start and stop > capture when the data file is opened and closed, respectively. > - Make variable the number of CRC values per entry, per source. > - Allocate entries queue each time we start capturing as now there > isn't a fixed number of CRC values per entry. > - Store the frame counter in the data file as a 8-digit hex number. > - For sources that cannot provide useful frame numbers, place > XXXXXXXX in the frame field. > > v4: > - Build only if CONFIG_DEBUG_FS is enabled. > - Use memdup_user_nul. > - Consolidate calculation of the size of an entry in a helper. > - Add 0x prefix to hex numbers in the data file. > - Remove unnecessary snprintf and strlen usage in read callback. > > v5: > - Made the crcs array in drm_crtc_crc_entry fixed-size > - Lots of other smaller improvements suggested by Emil Velikov > > v7: > - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h > > v8: > - Call debugfs_remove_recursive when we fail to create the minor > device > > v9: > - Register the debugfs directory for a crtc from > drm_crtc_register_all() > > v10: > - Don't let debugfs failures interrupt CRTC registration (Emil > Velikov) > > v11: > - Remove extra brace that broke compilation. Sorry! > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> Reviewed-by: Emil Velikov <emil.velikov@collabora.com> Emil
Hello, I have been able to test it on my setup and, after implementing set_crc_source function, I have crc/data and crc/control entries for each of my crtc. "cat /sys/kernel/debug/dri/0/crtc-0/crc/data" is blocking but I'm really sure of my driver implementation... Anyway you could add my ack on this patch Regards, Benjamin 2016-10-10 15:12 GMT+02:00 Emil Velikov <emil.l.velikov@gmail.com>: > On 6 October 2016 at 16:21, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote: >> Adds files and directories to debugfs for controlling and reading frame >> CRCs, per CRTC: >> >> dri/0/crtc-0/crc >> dri/0/crtc-0/crc/control >> dri/0/crtc-0/crc/data >> >> Drivers can implement the set_crc_source callback() in drm_crtc_funcs to >> start and stop generating frame CRCs and can add entries to the output >> by calling drm_crtc_add_crc_entry. >> >> v2: >> - Lots of good fixes suggested by Thierry. >> - Added documentation. >> - Changed the debugfs layout. >> - Moved to allocate the entries circular queue once when frame >> generation gets enabled for the first time. >> v3: >> - Use the control file just to select the source, and start and stop >> capture when the data file is opened and closed, respectively. >> - Make variable the number of CRC values per entry, per source. >> - Allocate entries queue each time we start capturing as now there >> isn't a fixed number of CRC values per entry. >> - Store the frame counter in the data file as a 8-digit hex number. >> - For sources that cannot provide useful frame numbers, place >> XXXXXXXX in the frame field. >> >> v4: >> - Build only if CONFIG_DEBUG_FS is enabled. >> - Use memdup_user_nul. >> - Consolidate calculation of the size of an entry in a helper. >> - Add 0x prefix to hex numbers in the data file. >> - Remove unnecessary snprintf and strlen usage in read callback. >> >> v5: >> - Made the crcs array in drm_crtc_crc_entry fixed-size >> - Lots of other smaller improvements suggested by Emil Velikov >> >> v7: >> - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h >> >> v8: >> - Call debugfs_remove_recursive when we fail to create the minor >> device >> >> v9: >> - Register the debugfs directory for a crtc from >> drm_crtc_register_all() >> >> v10: >> - Don't let debugfs failures interrupt CRTC registration (Emil >> Velikov) >> >> v11: >> - Remove extra brace that broke compilation. Sorry! >> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > Reviewed-by: Emil Velikov <emil.velikov@collabora.com> > > Emil > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Oct 06, 2016 at 05:21:06PM +0200, Tomeu Vizoso wrote: > Adds files and directories to debugfs for controlling and reading frame > CRCs, per CRTC: > > dri/0/crtc-0/crc > dri/0/crtc-0/crc/control > dri/0/crtc-0/crc/data > > Drivers can implement the set_crc_source callback() in drm_crtc_funcs to > start and stop generating frame CRCs and can add entries to the output > by calling drm_crtc_add_crc_entry. > > v2: > - Lots of good fixes suggested by Thierry. > - Added documentation. > - Changed the debugfs layout. > - Moved to allocate the entries circular queue once when frame > generation gets enabled for the first time. > v3: > - Use the control file just to select the source, and start and stop > capture when the data file is opened and closed, respectively. > - Make variable the number of CRC values per entry, per source. > - Allocate entries queue each time we start capturing as now there > isn't a fixed number of CRC values per entry. > - Store the frame counter in the data file as a 8-digit hex number. > - For sources that cannot provide useful frame numbers, place > XXXXXXXX in the frame field. > > v4: > - Build only if CONFIG_DEBUG_FS is enabled. > - Use memdup_user_nul. > - Consolidate calculation of the size of an entry in a helper. > - Add 0x prefix to hex numbers in the data file. > - Remove unnecessary snprintf and strlen usage in read callback. > > v5: > - Made the crcs array in drm_crtc_crc_entry fixed-size > - Lots of other smaller improvements suggested by Emil Velikov > > v7: > - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h > > v8: > - Call debugfs_remove_recursive when we fail to create the minor > device > > v9: > - Register the debugfs directory for a crtc from > drm_crtc_register_all() > > v10: > - Don't let debugfs failures interrupt CRTC registration (Emil > Velikov) > > v11: > - Remove extra brace that broke compilation. Sorry! > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > --- > > Documentation/gpu/drm-uapi.rst | 6 + > drivers/gpu/drm/Makefile | 3 +- > drivers/gpu/drm/drm_crtc.c | 34 +++- > drivers/gpu/drm/drm_debugfs.c | 34 +++- > drivers/gpu/drm/drm_debugfs_crc.c | 351 ++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_internal.h | 16 ++ > include/drm/drm_crtc.h | 41 +++++ > include/drm/drm_debugfs_crc.h | 73 ++++++++ > 8 files changed, 555 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c > create mode 100644 include/drm/drm_debugfs_crc.h > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 1ba301cebe16..de3ac9f90f8f 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -216,3 +216,9 @@ interfaces. Especially since all hardware-acceleration interfaces to > userspace are driver specific for efficiency and other reasons these > interfaces can be rather substantial. Hence every driver has its own > chapter. > + > +Testing and validation > +====================== > + > +.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c > + :doc: CRC ABI Just realized that you don't pull in the abi docs itself drom drm_debugfs_crc.[hc]. Can you pls address that in a follow-up patch? Merged this one here meanwhile to drm-misc. I couldn't apply the i915 patches to drm-misc, stuff is diverging too much :( I think it's better to get the i915 patches in once I've done a backmerge from drm-misc to drm-intel. Thanks, Daniel > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 25c720454017..74579d2e796e 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -9,7 +9,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ > drm_scatter.o drm_pci.o \ > drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \ > drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \ > - drm_info.o drm_debugfs.o drm_encoder_slave.o \ > + drm_info.o drm_encoder_slave.o \ > drm_trace_points.o drm_global.o drm_prime.o \ > drm_rect.o drm_vma_manager.o drm_flip_work.o \ > drm_modeset_lock.o drm_atomic.o drm_bridge.o \ > @@ -23,6 +23,7 @@ drm-$(CONFIG_PCI) += ati_pcigart.o > drm-$(CONFIG_DRM_PANEL) += drm_panel.o > drm-$(CONFIG_OF) += drm_of.o > drm-$(CONFIG_AGP) += drm_agpsupport.o > +drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o > > drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ > drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \ > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 2d7bedf28647..60403bf7a4ff 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -40,7 +40,7 @@ > #include <drm/drm_modeset_lock.h> > #include <drm/drm_atomic.h> > #include <drm/drm_auth.h> > -#include <drm/drm_framebuffer.h> > +#include <drm/drm_debugfs_crc.h> > > #include "drm_crtc_internal.h" > #include "drm_internal.h" > @@ -122,6 +122,10 @@ static int drm_crtc_register_all(struct drm_device *dev) > int ret = 0; > > drm_for_each_crtc(crtc, dev) { > + if (drm_debugfs_crtc_add(crtc)) > + DRM_ERROR("Failed to initialize debugfs entry for CRTC '%s'.\n", > + crtc->name); > + > if (crtc->funcs->late_register) > ret = crtc->funcs->late_register(crtc); > if (ret) > @@ -138,9 +142,29 @@ static void drm_crtc_unregister_all(struct drm_device *dev) > drm_for_each_crtc(crtc, dev) { > if (crtc->funcs->early_unregister) > crtc->funcs->early_unregister(crtc); > + drm_debugfs_crtc_remove(crtc); > } > } > > +static int drm_crtc_crc_init(struct drm_crtc *crtc) > +{ > +#ifdef CONFIG_DEBUG_FS > + spin_lock_init(&crtc->crc.lock); > + init_waitqueue_head(&crtc->crc.wq); > + crtc->crc.source = kstrdup("auto", GFP_KERNEL); > + if (!crtc->crc.source) > + return -ENOMEM; > +#endif > + return 0; > +} > + > +static void drm_crtc_crc_fini(struct drm_crtc *crtc) > +{ > +#ifdef CONFIG_DEBUG_FS > + kfree(crtc->crc.source); > +#endif > +} > + > /** > * drm_crtc_init_with_planes - Initialise a new CRTC object with > * specified primary and cursor planes. > @@ -210,6 +234,12 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, > if (cursor) > cursor->possible_crtcs = 1 << drm_crtc_index(crtc); > > + ret = drm_crtc_crc_init(crtc); > + if (ret) { > + drm_mode_object_unregister(dev, &crtc->base); > + return ret; > + } > + > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { > drm_object_attach_property(&crtc->base, config->prop_active, 0); > drm_object_attach_property(&crtc->base, config->prop_mode_id, 0); > @@ -236,6 +266,8 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) > * the indices on the drm_crtc after us in the crtc_list. > */ > > + drm_crtc_crc_fini(crtc); > + > kfree(crtc->gamma_store); > crtc->gamma_store = NULL; > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index 1205790ed960..800055c39cdb 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -415,5 +415,37 @@ void drm_debugfs_connector_remove(struct drm_connector *connector) > connector->debugfs_entry = NULL; > } > > -#endif /* CONFIG_DEBUG_FS */ > +int drm_debugfs_crtc_add(struct drm_crtc *crtc) > +{ > + struct drm_minor *minor = crtc->dev->primary; > + struct dentry *root; > + char *name; > + > + name = kasprintf(GFP_KERNEL, "crtc-%d", crtc->index); > + if (!name) > + return -ENOMEM; > + > + root = debugfs_create_dir(name, minor->debugfs_root); > + kfree(name); > + if (!root) > + return -ENOMEM; > + > + crtc->debugfs_entry = root; > + > + if (drm_debugfs_crtc_crc_add(crtc)) > + goto error; > > + return 0; > + > +error: > + drm_debugfs_crtc_remove(crtc); > + return -ENOMEM; > +} > + > +void drm_debugfs_crtc_remove(struct drm_crtc *crtc) > +{ > + debugfs_remove_recursive(crtc->debugfs_entry); > + crtc->debugfs_entry = NULL; > +} > + > +#endif /* CONFIG_DEBUG_FS */ > diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c > new file mode 100644 > index 000000000000..4129405d17c0 > --- /dev/null > +++ b/drivers/gpu/drm/drm_debugfs_crc.c > @@ -0,0 +1,351 @@ > +/* > + * Copyright © 2008 Intel Corporation > + * Copyright © 2016 Collabora Ltd > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + * Based on code from the i915 driver. > + * Original author: Damien Lespiau <damien.lespiau@intel.com> > + * > + */ > + > +#include <linux/circ_buf.h> > +#include <linux/ctype.h> > +#include <linux/debugfs.h> > +#include <drm/drmP.h> > + > +/** > + * DOC: CRC ABI > + * > + * DRM device drivers can provide to userspace CRC information of each frame as > + * it reached a given hardware component (a "source"). > + * > + * Userspace can control generation of CRCs in a given CRTC by writing to the > + * file dri/0/crtc-N/crc/control in debugfs, with N being the index of the CRTC. > + * Accepted values are source names (which are driver-specific) and the "auto" > + * keyword, which will let the driver select a default source of frame CRCs > + * for this CRTC. > + * > + * Once frame CRC generation is enabled, userspace can capture them by reading > + * the dri/0/crtc-N/crc/data file. Each line in that file contains the frame > + * number in the first field and then a number of unsigned integer fields > + * containing the CRC data. Fields are separated by a single space and the number > + * of CRC fields is source-specific. > + * > + * Note that though in some cases the CRC is computed in a specified way and on > + * the frame contents as supplied by userspace (eDP 1.3), in general the CRC > + * computation is performed in an unspecified way and on frame contents that have > + * been already processed in also an unspecified way and thus userspace cannot > + * rely on being able to generate matching CRC values for the frame contents that > + * it submits. In this general case, the maximum userspace can do is to compare > + * the reported CRCs of frames that should have the same contents. > + */ > + > +static int crc_control_show(struct seq_file *m, void *data) > +{ > + struct drm_crtc *crtc = m->private; > + > + seq_printf(m, "%s\n", crtc->crc.source); > + > + return 0; > +} > + > +static int crc_control_open(struct inode *inode, struct file *file) > +{ > + struct drm_crtc *crtc = inode->i_private; > + > + return single_open(file, crc_control_show, crtc); > +} > + > +static ssize_t crc_control_write(struct file *file, const char __user *ubuf, > + size_t len, loff_t *offp) > +{ > + struct seq_file *m = file->private_data; > + struct drm_crtc *crtc = m->private; > + struct drm_crtc_crc *crc = &crtc->crc; > + char *source; > + > + if (len == 0) > + return 0; > + > + if (len > PAGE_SIZE - 1) { > + DRM_DEBUG_KMS("Expected < %lu bytes into crtc crc control\n", > + PAGE_SIZE); > + return -E2BIG; > + } > + > + source = memdup_user_nul(ubuf, len); > + if (IS_ERR(source)) > + return PTR_ERR(source); > + > + if (source[len] == '\n') > + source[len] = '\0'; > + > + spin_lock_irq(&crc->lock); > + > + if (crc->opened) { > + spin_unlock_irq(&crc->lock); > + kfree(source); > + return -EBUSY; > + } > + > + kfree(crc->source); > + crc->source = source; > + > + spin_unlock_irq(&crc->lock); > + > + *offp += len; > + return len; > +} > + > +const struct file_operations drm_crtc_crc_control_fops = { > + .owner = THIS_MODULE, > + .open = crc_control_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > + .write = crc_control_write > +}; > + > +static int crtc_crc_open(struct inode *inode, struct file *filep) > +{ > + struct drm_crtc *crtc = inode->i_private; > + struct drm_crtc_crc *crc = &crtc->crc; > + struct drm_crtc_crc_entry *entries = NULL; > + size_t values_cnt; > + int ret; > + > + if (crc->opened) > + return -EBUSY; > + > + ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt); > + if (ret) > + return ret; > + > + if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) { > + ret = -EINVAL; > + goto err_disable; > + } > + > + if (WARN_ON(values_cnt == 0)) { > + ret = -EINVAL; > + goto err_disable; > + } > + > + entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL); > + if (!entries) { > + ret = -ENOMEM; > + goto err_disable; > + } > + > + spin_lock_irq(&crc->lock); > + crc->entries = entries; > + crc->values_cnt = values_cnt; > + crc->opened = true; > + spin_unlock_irq(&crc->lock); > + > + return 0; > + > +err_disable: > + crtc->funcs->set_crc_source(crtc, NULL, &values_cnt); > + return ret; > +} > + > +static int crtc_crc_release(struct inode *inode, struct file *filep) > +{ > + struct drm_crtc *crtc = filep->f_inode->i_private; > + struct drm_crtc_crc *crc = &crtc->crc; > + size_t values_cnt; > + > + spin_lock_irq(&crc->lock); > + kfree(crc->entries); > + crc->entries = NULL; > + crc->head = 0; > + crc->tail = 0; > + crc->values_cnt = 0; > + crc->opened = false; > + spin_unlock_irq(&crc->lock); > + > + crtc->funcs->set_crc_source(crtc, NULL, &values_cnt); > + > + return 0; > +} > + > +static int crtc_crc_data_count(struct drm_crtc_crc *crc) > +{ > + assert_spin_locked(&crc->lock); > + return CIRC_CNT(crc->head, crc->tail, DRM_CRC_ENTRIES_NR); > +} > + > +/* > + * 1 frame field of 10 chars plus a number of CRC fields of 10 chars each, space > + * separated, with a newline at the end and null-terminated. > + */ > +#define LINE_LEN(values_cnt) (10 + 11 * values_cnt + 1 + 1) > +#define MAX_LINE_LEN (LINE_LEN(DRM_MAX_CRC_NR)) > + > +static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf, > + size_t count, loff_t *pos) > +{ > + struct drm_crtc *crtc = filep->f_inode->i_private; > + struct drm_crtc_crc *crc = &crtc->crc; > + struct drm_crtc_crc_entry *entry; > + char buf[MAX_LINE_LEN]; > + int ret, i; > + > + spin_lock_irq(&crc->lock); > + > + if (!crc->source) { > + spin_unlock_irq(&crc->lock); > + return 0; > + } > + > + /* Nothing to read? */ > + while (crtc_crc_data_count(crc) == 0) { > + if (filep->f_flags & O_NONBLOCK) { > + spin_unlock_irq(&crc->lock); > + return -EAGAIN; > + } > + > + ret = wait_event_interruptible_lock_irq(crc->wq, > + crtc_crc_data_count(crc), > + crc->lock); > + if (ret) { > + spin_unlock_irq(&crc->lock); > + return ret; > + } > + } > + > + /* We know we have an entry to be read */ > + entry = &crc->entries[crc->tail]; > + > + if (count < LINE_LEN(crc->values_cnt)) { > + spin_unlock_irq(&crc->lock); > + return -EINVAL; > + } > + > + BUILD_BUG_ON_NOT_POWER_OF_2(DRM_CRC_ENTRIES_NR); > + crc->tail = (crc->tail + 1) & (DRM_CRC_ENTRIES_NR - 1); > + > + spin_unlock_irq(&crc->lock); > + > + if (entry->has_frame_counter) > + sprintf(buf, "0x%08x", entry->frame); > + else > + sprintf(buf, "XXXXXXXXXX"); > + > + for (i = 0; i < crc->values_cnt; i++) > + sprintf(buf + 10 + i * 11, " 0x%08x", entry->crcs[i]); > + sprintf(buf + 10 + crc->values_cnt * 11, "\n"); > + > + if (copy_to_user(user_buf, buf, LINE_LEN(crc->values_cnt))) > + return -EFAULT; > + > + return LINE_LEN(crc->values_cnt); > +} > + > +const struct file_operations drm_crtc_crc_data_fops = { > + .owner = THIS_MODULE, > + .open = crtc_crc_open, > + .read = crtc_crc_read, > + .release = crtc_crc_release, > +}; > + > +/** > + * drm_debugfs_crtc_crc_add - Add files to debugfs for capture of frame CRCs > + * @crtc: CRTC to whom the frames will belong > + * > + * Adds files to debugfs directory that allows userspace to control the > + * generation of frame CRCs and to read them. > + * > + * Returns: > + * Zero on success, error code on failure. > + */ > +int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) > +{ > + struct dentry *crc_ent, *ent; > + > + if (!crtc->funcs->set_crc_source) > + return 0; > + > + crc_ent = debugfs_create_dir("crc", crtc->debugfs_entry); > + if (!crc_ent) > + return -ENOMEM; > + > + ent = debugfs_create_file("control", S_IRUGO, crc_ent, crtc, > + &drm_crtc_crc_control_fops); > + if (!ent) > + goto error; > + > + ent = debugfs_create_file("data", S_IRUGO, crc_ent, crtc, > + &drm_crtc_crc_data_fops); > + if (!ent) > + goto error; > + > + return 0; > + > +error: > + debugfs_remove_recursive(crc_ent); > + > + return -ENOMEM; > +} > + > +/** > + * drm_crtc_add_crc_entry - Add entry with CRC information for a frame > + * @crtc: CRTC to which the frame belongs > + * @has_frame: whether this entry has a frame number to go with > + * @frame: number of the frame these CRCs are about > + * @crcs: array of CRC values, with length matching #drm_crtc_crc.values_cnt > + * > + * For each frame, the driver polls the source of CRCs for new data and calls > + * this function to add them to the buffer from where userspace reads. > + */ > +int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, > + uint32_t frame, uint32_t *crcs) > +{ > + struct drm_crtc_crc *crc = &crtc->crc; > + struct drm_crtc_crc_entry *entry; > + int head, tail; > + > + assert_spin_locked(&crc->lock); > + > + /* Caller may not have noticed yet that userspace has stopped reading */ > + if (!crc->opened) > + return -EINVAL; > + > + head = crc->head; > + tail = crc->tail; > + > + if (CIRC_SPACE(head, tail, DRM_CRC_ENTRIES_NR) < 1) { > + DRM_ERROR("Overflow of CRC buffer, userspace reads too slow.\n"); > + return -ENOBUFS; > + } > + > + entry = &crc->entries[head]; > + entry->frame = frame; > + entry->has_frame_counter = has_frame; > + memcpy(&entry->crcs, crcs, sizeof(*crcs) * crc->values_cnt); > + > + head = (head + 1) & (DRM_CRC_ENTRIES_NR - 1); > + crc->head = head; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(drm_crtc_add_crc_entry); > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h > index e66af289a016..abd209863ef4 100644 > --- a/drivers/gpu/drm/drm_internal.h > +++ b/drivers/gpu/drm/drm_internal.h > @@ -100,6 +100,9 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id, > int drm_debugfs_cleanup(struct drm_minor *minor); > int drm_debugfs_connector_add(struct drm_connector *connector); > void drm_debugfs_connector_remove(struct drm_connector *connector); > +int drm_debugfs_crtc_add(struct drm_crtc *crtc); > +void drm_debugfs_crtc_remove(struct drm_crtc *crtc); > +int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc); > #else > static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id, > struct dentry *root) > @@ -119,4 +122,17 @@ static inline int drm_debugfs_connector_add(struct drm_connector *connector) > static inline void drm_debugfs_connector_remove(struct drm_connector *connector) > { > } > + > +static inline int drm_debugfs_crtc_add(struct drm_crtc *crtc) > +{ > + return 0; > +} > +static inline void drm_debugfs_crtc_remove(struct drm_crtc *crtc) > +{ > +} > + > +static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) > +{ > + return 0; > +} > #endif > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 61932f55f788..0908b2cf6c2c 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -47,6 +47,7 @@ > #include <drm/drm_plane.h> > #include <drm/drm_blend.h> > #include <drm/drm_color_mgmt.h> > +#include <drm/drm_debugfs_crc.h> > > struct drm_device; > struct drm_mode_set; > @@ -564,6 +565,30 @@ struct drm_crtc_funcs { > * before data structures are torndown. > */ > void (*early_unregister)(struct drm_crtc *crtc); > + > + /** > + * @set_crc_source: > + * > + * Changes the source of CRC checksums of frames at the request of > + * userspace, typically for testing purposes. The sources available are > + * specific of each driver and a %NULL value indicates that CRC > + * generation is to be switched off. > + * > + * When CRC generation is enabled, the driver should call > + * drm_crtc_add_crc_entry() at each frame, providing any information > + * that characterizes the frame contents in the crcN arguments, as > + * provided from the configured source. Drivers must accept a "auto" > + * source name that will select a default source for this CRTC. > + * > + * This callback is optional if the driver does not support any CRC > + * generation functionality. > + * > + * RETURNS: > + * > + * 0 on success or a negative error code on failure. > + */ > + int (*set_crc_source)(struct drm_crtc *crtc, const char *source, > + size_t *values_cnt); > }; > > /** > @@ -680,6 +705,22 @@ struct drm_crtc { > * context. > */ > struct drm_modeset_acquire_ctx *acquire_ctx; > + > +#ifdef CONFIG_DEBUG_FS > + /** > + * @debugfs_entry: > + * > + * Debugfs directory for this CRTC. > + */ > + struct dentry *debugfs_entry; > + > + /** > + * @crc: > + * > + * Configuration settings of CRC capture. > + */ > + struct drm_crtc_crc crc; > +#endif > }; > > /** > diff --git a/include/drm/drm_debugfs_crc.h b/include/drm/drm_debugfs_crc.h > new file mode 100644 > index 000000000000..7d63b1d4adb9 > --- /dev/null > +++ b/include/drm/drm_debugfs_crc.h > @@ -0,0 +1,73 @@ > +/* > + * Copyright © 2016 Collabora Ltd. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > + * OTHER DEALINGS IN THE SOFTWARE. > + */ > +#ifndef __DRM_DEBUGFS_CRC_H__ > +#define __DRM_DEBUGFS_CRC_H__ > + > +#define DRM_MAX_CRC_NR 10 > + > +/** > + * struct drm_crtc_crc_entry - entry describing a frame's content > + * @has_frame_counter: whether the source was able to provide a frame number > + * @frame: number of the frame this CRC is about, if @has_frame_counter is true > + * @crc: array of values that characterize the frame > + */ > +struct drm_crtc_crc_entry { > + bool has_frame_counter; > + uint32_t frame; > + uint32_t crcs[DRM_MAX_CRC_NR]; > +}; > + > +#define DRM_CRC_ENTRIES_NR 128 > + > +/** > + * struct drm_crtc_crc - data supporting CRC capture on a given CRTC > + * @lock: protects the fields in this struct > + * @source: name of the currently configured source of CRCs > + * @opened: whether userspace has opened the data file for reading > + * @entries: array of entries, with size of %DRM_CRC_ENTRIES_NR > + * @head: head of circular queue > + * @tail: tail of circular queue > + * @values_cnt: number of CRC values per entry, up to %DRM_MAX_CRC_NR > + * @wq: workqueue used to synchronize reading and writing > + */ > +struct drm_crtc_crc { > + spinlock_t lock; > + const char *source; > + bool opened; > + struct drm_crtc_crc_entry *entries; > + int head, tail; > + size_t values_cnt; > + wait_queue_head_t wq; > +}; > + > +#if defined(CONFIG_DEBUG_FS) > +int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, > + uint32_t frame, uint32_t *crcs); > +#else > +static inline int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, > + uint32_t frame, uint32_t *crcs) > +{ > + return -EINVAL; > +} > +#endif /* defined(CONFIG_DEBUG_FS) */ > + > +#endif /* __DRM_DEBUGFS_CRC_H__ */ > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 1ba301cebe16..de3ac9f90f8f 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -216,3 +216,9 @@ interfaces. Especially since all hardware-acceleration interfaces to userspace are driver specific for efficiency and other reasons these interfaces can be rather substantial. Hence every driver has its own chapter. + +Testing and validation +====================== + +.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c + :doc: CRC ABI diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 25c720454017..74579d2e796e 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -9,7 +9,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ drm_scatter.o drm_pci.o \ drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \ drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \ - drm_info.o drm_debugfs.o drm_encoder_slave.o \ + drm_info.o drm_encoder_slave.o \ drm_trace_points.o drm_global.o drm_prime.o \ drm_rect.o drm_vma_manager.o drm_flip_work.o \ drm_modeset_lock.o drm_atomic.o drm_bridge.o \ @@ -23,6 +23,7 @@ drm-$(CONFIG_PCI) += ati_pcigart.o drm-$(CONFIG_DRM_PANEL) += drm_panel.o drm-$(CONFIG_OF) += drm_of.o drm-$(CONFIG_AGP) += drm_agpsupport.o +drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \ diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 2d7bedf28647..60403bf7a4ff 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -40,7 +40,7 @@ #include <drm/drm_modeset_lock.h> #include <drm/drm_atomic.h> #include <drm/drm_auth.h> -#include <drm/drm_framebuffer.h> +#include <drm/drm_debugfs_crc.h> #include "drm_crtc_internal.h" #include "drm_internal.h" @@ -122,6 +122,10 @@ static int drm_crtc_register_all(struct drm_device *dev) int ret = 0; drm_for_each_crtc(crtc, dev) { + if (drm_debugfs_crtc_add(crtc)) + DRM_ERROR("Failed to initialize debugfs entry for CRTC '%s'.\n", + crtc->name); + if (crtc->funcs->late_register) ret = crtc->funcs->late_register(crtc); if (ret) @@ -138,9 +142,29 @@ static void drm_crtc_unregister_all(struct drm_device *dev) drm_for_each_crtc(crtc, dev) { if (crtc->funcs->early_unregister) crtc->funcs->early_unregister(crtc); + drm_debugfs_crtc_remove(crtc); } } +static int drm_crtc_crc_init(struct drm_crtc *crtc) +{ +#ifdef CONFIG_DEBUG_FS + spin_lock_init(&crtc->crc.lock); + init_waitqueue_head(&crtc->crc.wq); + crtc->crc.source = kstrdup("auto", GFP_KERNEL); + if (!crtc->crc.source) + return -ENOMEM; +#endif + return 0; +} + +static void drm_crtc_crc_fini(struct drm_crtc *crtc) +{ +#ifdef CONFIG_DEBUG_FS + kfree(crtc->crc.source); +#endif +} + /** * drm_crtc_init_with_planes - Initialise a new CRTC object with * specified primary and cursor planes. @@ -210,6 +234,12 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, if (cursor) cursor->possible_crtcs = 1 << drm_crtc_index(crtc); + ret = drm_crtc_crc_init(crtc); + if (ret) { + drm_mode_object_unregister(dev, &crtc->base); + return ret; + } + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { drm_object_attach_property(&crtc->base, config->prop_active, 0); drm_object_attach_property(&crtc->base, config->prop_mode_id, 0); @@ -236,6 +266,8 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) * the indices on the drm_crtc after us in the crtc_list. */ + drm_crtc_crc_fini(crtc); + kfree(crtc->gamma_store); crtc->gamma_store = NULL; diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 1205790ed960..800055c39cdb 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -415,5 +415,37 @@ void drm_debugfs_connector_remove(struct drm_connector *connector) connector->debugfs_entry = NULL; } -#endif /* CONFIG_DEBUG_FS */ +int drm_debugfs_crtc_add(struct drm_crtc *crtc) +{ + struct drm_minor *minor = crtc->dev->primary; + struct dentry *root; + char *name; + + name = kasprintf(GFP_KERNEL, "crtc-%d", crtc->index); + if (!name) + return -ENOMEM; + + root = debugfs_create_dir(name, minor->debugfs_root); + kfree(name); + if (!root) + return -ENOMEM; + + crtc->debugfs_entry = root; + + if (drm_debugfs_crtc_crc_add(crtc)) + goto error; + return 0; + +error: + drm_debugfs_crtc_remove(crtc); + return -ENOMEM; +} + +void drm_debugfs_crtc_remove(struct drm_crtc *crtc) +{ + debugfs_remove_recursive(crtc->debugfs_entry); + crtc->debugfs_entry = NULL; +} + +#endif /* CONFIG_DEBUG_FS */ diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c new file mode 100644 index 000000000000..4129405d17c0 --- /dev/null +++ b/drivers/gpu/drm/drm_debugfs_crc.c @@ -0,0 +1,351 @@ +/* + * Copyright © 2008 Intel Corporation + * Copyright © 2016 Collabora Ltd + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Based on code from the i915 driver. + * Original author: Damien Lespiau <damien.lespiau@intel.com> + * + */ + +#include <linux/circ_buf.h> +#include <linux/ctype.h> +#include <linux/debugfs.h> +#include <drm/drmP.h> + +/** + * DOC: CRC ABI + * + * DRM device drivers can provide to userspace CRC information of each frame as + * it reached a given hardware component (a "source"). + * + * Userspace can control generation of CRCs in a given CRTC by writing to the + * file dri/0/crtc-N/crc/control in debugfs, with N being the index of the CRTC. + * Accepted values are source names (which are driver-specific) and the "auto" + * keyword, which will let the driver select a default source of frame CRCs + * for this CRTC. + * + * Once frame CRC generation is enabled, userspace can capture them by reading + * the dri/0/crtc-N/crc/data file. Each line in that file contains the frame + * number in the first field and then a number of unsigned integer fields + * containing the CRC data. Fields are separated by a single space and the number + * of CRC fields is source-specific. + * + * Note that though in some cases the CRC is computed in a specified way and on + * the frame contents as supplied by userspace (eDP 1.3), in general the CRC + * computation is performed in an unspecified way and on frame contents that have + * been already processed in also an unspecified way and thus userspace cannot + * rely on being able to generate matching CRC values for the frame contents that + * it submits. In this general case, the maximum userspace can do is to compare + * the reported CRCs of frames that should have the same contents. + */ + +static int crc_control_show(struct seq_file *m, void *data) +{ + struct drm_crtc *crtc = m->private; + + seq_printf(m, "%s\n", crtc->crc.source); + + return 0; +} + +static int crc_control_open(struct inode *inode, struct file *file) +{ + struct drm_crtc *crtc = inode->i_private; + + return single_open(file, crc_control_show, crtc); +} + +static ssize_t crc_control_write(struct file *file, const char __user *ubuf, + size_t len, loff_t *offp) +{ + struct seq_file *m = file->private_data; + struct drm_crtc *crtc = m->private; + struct drm_crtc_crc *crc = &crtc->crc; + char *source; + + if (len == 0) + return 0; + + if (len > PAGE_SIZE - 1) { + DRM_DEBUG_KMS("Expected < %lu bytes into crtc crc control\n", + PAGE_SIZE); + return -E2BIG; + } + + source = memdup_user_nul(ubuf, len); + if (IS_ERR(source)) + return PTR_ERR(source); + + if (source[len] == '\n') + source[len] = '\0'; + + spin_lock_irq(&crc->lock); + + if (crc->opened) { + spin_unlock_irq(&crc->lock); + kfree(source); + return -EBUSY; + } + + kfree(crc->source); + crc->source = source; + + spin_unlock_irq(&crc->lock); + + *offp += len; + return len; +} + +const struct file_operations drm_crtc_crc_control_fops = { + .owner = THIS_MODULE, + .open = crc_control_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, + .write = crc_control_write +}; + +static int crtc_crc_open(struct inode *inode, struct file *filep) +{ + struct drm_crtc *crtc = inode->i_private; + struct drm_crtc_crc *crc = &crtc->crc; + struct drm_crtc_crc_entry *entries = NULL; + size_t values_cnt; + int ret; + + if (crc->opened) + return -EBUSY; + + ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt); + if (ret) + return ret; + + if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) { + ret = -EINVAL; + goto err_disable; + } + + if (WARN_ON(values_cnt == 0)) { + ret = -EINVAL; + goto err_disable; + } + + entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL); + if (!entries) { + ret = -ENOMEM; + goto err_disable; + } + + spin_lock_irq(&crc->lock); + crc->entries = entries; + crc->values_cnt = values_cnt; + crc->opened = true; + spin_unlock_irq(&crc->lock); + + return 0; + +err_disable: + crtc->funcs->set_crc_source(crtc, NULL, &values_cnt); + return ret; +} + +static int crtc_crc_release(struct inode *inode, struct file *filep) +{ + struct drm_crtc *crtc = filep->f_inode->i_private; + struct drm_crtc_crc *crc = &crtc->crc; + size_t values_cnt; + + spin_lock_irq(&crc->lock); + kfree(crc->entries); + crc->entries = NULL; + crc->head = 0; + crc->tail = 0; + crc->values_cnt = 0; + crc->opened = false; + spin_unlock_irq(&crc->lock); + + crtc->funcs->set_crc_source(crtc, NULL, &values_cnt); + + return 0; +} + +static int crtc_crc_data_count(struct drm_crtc_crc *crc) +{ + assert_spin_locked(&crc->lock); + return CIRC_CNT(crc->head, crc->tail, DRM_CRC_ENTRIES_NR); +} + +/* + * 1 frame field of 10 chars plus a number of CRC fields of 10 chars each, space + * separated, with a newline at the end and null-terminated. + */ +#define LINE_LEN(values_cnt) (10 + 11 * values_cnt + 1 + 1) +#define MAX_LINE_LEN (LINE_LEN(DRM_MAX_CRC_NR)) + +static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf, + size_t count, loff_t *pos) +{ + struct drm_crtc *crtc = filep->f_inode->i_private; + struct drm_crtc_crc *crc = &crtc->crc; + struct drm_crtc_crc_entry *entry; + char buf[MAX_LINE_LEN]; + int ret, i; + + spin_lock_irq(&crc->lock); + + if (!crc->source) { + spin_unlock_irq(&crc->lock); + return 0; + } + + /* Nothing to read? */ + while (crtc_crc_data_count(crc) == 0) { + if (filep->f_flags & O_NONBLOCK) { + spin_unlock_irq(&crc->lock); + return -EAGAIN; + } + + ret = wait_event_interruptible_lock_irq(crc->wq, + crtc_crc_data_count(crc), + crc->lock); + if (ret) { + spin_unlock_irq(&crc->lock); + return ret; + } + } + + /* We know we have an entry to be read */ + entry = &crc->entries[crc->tail]; + + if (count < LINE_LEN(crc->values_cnt)) { + spin_unlock_irq(&crc->lock); + return -EINVAL; + } + + BUILD_BUG_ON_NOT_POWER_OF_2(DRM_CRC_ENTRIES_NR); + crc->tail = (crc->tail + 1) & (DRM_CRC_ENTRIES_NR - 1); + + spin_unlock_irq(&crc->lock); + + if (entry->has_frame_counter) + sprintf(buf, "0x%08x", entry->frame); + else + sprintf(buf, "XXXXXXXXXX"); + + for (i = 0; i < crc->values_cnt; i++) + sprintf(buf + 10 + i * 11, " 0x%08x", entry->crcs[i]); + sprintf(buf + 10 + crc->values_cnt * 11, "\n"); + + if (copy_to_user(user_buf, buf, LINE_LEN(crc->values_cnt))) + return -EFAULT; + + return LINE_LEN(crc->values_cnt); +} + +const struct file_operations drm_crtc_crc_data_fops = { + .owner = THIS_MODULE, + .open = crtc_crc_open, + .read = crtc_crc_read, + .release = crtc_crc_release, +}; + +/** + * drm_debugfs_crtc_crc_add - Add files to debugfs for capture of frame CRCs + * @crtc: CRTC to whom the frames will belong + * + * Adds files to debugfs directory that allows userspace to control the + * generation of frame CRCs and to read them. + * + * Returns: + * Zero on success, error code on failure. + */ +int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) +{ + struct dentry *crc_ent, *ent; + + if (!crtc->funcs->set_crc_source) + return 0; + + crc_ent = debugfs_create_dir("crc", crtc->debugfs_entry); + if (!crc_ent) + return -ENOMEM; + + ent = debugfs_create_file("control", S_IRUGO, crc_ent, crtc, + &drm_crtc_crc_control_fops); + if (!ent) + goto error; + + ent = debugfs_create_file("data", S_IRUGO, crc_ent, crtc, + &drm_crtc_crc_data_fops); + if (!ent) + goto error; + + return 0; + +error: + debugfs_remove_recursive(crc_ent); + + return -ENOMEM; +} + +/** + * drm_crtc_add_crc_entry - Add entry with CRC information for a frame + * @crtc: CRTC to which the frame belongs + * @has_frame: whether this entry has a frame number to go with + * @frame: number of the frame these CRCs are about + * @crcs: array of CRC values, with length matching #drm_crtc_crc.values_cnt + * + * For each frame, the driver polls the source of CRCs for new data and calls + * this function to add them to the buffer from where userspace reads. + */ +int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, + uint32_t frame, uint32_t *crcs) +{ + struct drm_crtc_crc *crc = &crtc->crc; + struct drm_crtc_crc_entry *entry; + int head, tail; + + assert_spin_locked(&crc->lock); + + /* Caller may not have noticed yet that userspace has stopped reading */ + if (!crc->opened) + return -EINVAL; + + head = crc->head; + tail = crc->tail; + + if (CIRC_SPACE(head, tail, DRM_CRC_ENTRIES_NR) < 1) { + DRM_ERROR("Overflow of CRC buffer, userspace reads too slow.\n"); + return -ENOBUFS; + } + + entry = &crc->entries[head]; + entry->frame = frame; + entry->has_frame_counter = has_frame; + memcpy(&entry->crcs, crcs, sizeof(*crcs) * crc->values_cnt); + + head = (head + 1) & (DRM_CRC_ENTRIES_NR - 1); + crc->head = head; + + return 0; +} +EXPORT_SYMBOL_GPL(drm_crtc_add_crc_entry); diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index e66af289a016..abd209863ef4 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -100,6 +100,9 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id, int drm_debugfs_cleanup(struct drm_minor *minor); int drm_debugfs_connector_add(struct drm_connector *connector); void drm_debugfs_connector_remove(struct drm_connector *connector); +int drm_debugfs_crtc_add(struct drm_crtc *crtc); +void drm_debugfs_crtc_remove(struct drm_crtc *crtc); +int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc); #else static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id, struct dentry *root) @@ -119,4 +122,17 @@ static inline int drm_debugfs_connector_add(struct drm_connector *connector) static inline void drm_debugfs_connector_remove(struct drm_connector *connector) { } + +static inline int drm_debugfs_crtc_add(struct drm_crtc *crtc) +{ + return 0; +} +static inline void drm_debugfs_crtc_remove(struct drm_crtc *crtc) +{ +} + +static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc) +{ + return 0; +} #endif diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 61932f55f788..0908b2cf6c2c 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -47,6 +47,7 @@ #include <drm/drm_plane.h> #include <drm/drm_blend.h> #include <drm/drm_color_mgmt.h> +#include <drm/drm_debugfs_crc.h> struct drm_device; struct drm_mode_set; @@ -564,6 +565,30 @@ struct drm_crtc_funcs { * before data structures are torndown. */ void (*early_unregister)(struct drm_crtc *crtc); + + /** + * @set_crc_source: + * + * Changes the source of CRC checksums of frames at the request of + * userspace, typically for testing purposes. The sources available are + * specific of each driver and a %NULL value indicates that CRC + * generation is to be switched off. + * + * When CRC generation is enabled, the driver should call + * drm_crtc_add_crc_entry() at each frame, providing any information + * that characterizes the frame contents in the crcN arguments, as + * provided from the configured source. Drivers must accept a "auto" + * source name that will select a default source for this CRTC. + * + * This callback is optional if the driver does not support any CRC + * generation functionality. + * + * RETURNS: + * + * 0 on success or a negative error code on failure. + */ + int (*set_crc_source)(struct drm_crtc *crtc, const char *source, + size_t *values_cnt); }; /** @@ -680,6 +705,22 @@ struct drm_crtc { * context. */ struct drm_modeset_acquire_ctx *acquire_ctx; + +#ifdef CONFIG_DEBUG_FS + /** + * @debugfs_entry: + * + * Debugfs directory for this CRTC. + */ + struct dentry *debugfs_entry; + + /** + * @crc: + * + * Configuration settings of CRC capture. + */ + struct drm_crtc_crc crc; +#endif }; /** diff --git a/include/drm/drm_debugfs_crc.h b/include/drm/drm_debugfs_crc.h new file mode 100644 index 000000000000..7d63b1d4adb9 --- /dev/null +++ b/include/drm/drm_debugfs_crc.h @@ -0,0 +1,73 @@ +/* + * Copyright © 2016 Collabora Ltd. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ +#ifndef __DRM_DEBUGFS_CRC_H__ +#define __DRM_DEBUGFS_CRC_H__ + +#define DRM_MAX_CRC_NR 10 + +/** + * struct drm_crtc_crc_entry - entry describing a frame's content + * @has_frame_counter: whether the source was able to provide a frame number + * @frame: number of the frame this CRC is about, if @has_frame_counter is true + * @crc: array of values that characterize the frame + */ +struct drm_crtc_crc_entry { + bool has_frame_counter; + uint32_t frame; + uint32_t crcs[DRM_MAX_CRC_NR]; +}; + +#define DRM_CRC_ENTRIES_NR 128 + +/** + * struct drm_crtc_crc - data supporting CRC capture on a given CRTC + * @lock: protects the fields in this struct + * @source: name of the currently configured source of CRCs + * @opened: whether userspace has opened the data file for reading + * @entries: array of entries, with size of %DRM_CRC_ENTRIES_NR + * @head: head of circular queue + * @tail: tail of circular queue + * @values_cnt: number of CRC values per entry, up to %DRM_MAX_CRC_NR + * @wq: workqueue used to synchronize reading and writing + */ +struct drm_crtc_crc { + spinlock_t lock; + const char *source; + bool opened; + struct drm_crtc_crc_entry *entries; + int head, tail; + size_t values_cnt; + wait_queue_head_t wq; +}; + +#if defined(CONFIG_DEBUG_FS) +int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, + uint32_t frame, uint32_t *crcs); +#else +static inline int drm_crtc_add_crc_entry(struct drm_crtc *crtc, bool has_frame, + uint32_t frame, uint32_t *crcs) +{ + return -EINVAL; +} +#endif /* defined(CONFIG_DEBUG_FS) */ + +#endif /* __DRM_DEBUGFS_CRC_H__ */
Adds files and directories to debugfs for controlling and reading frame CRCs, per CRTC: dri/0/crtc-0/crc dri/0/crtc-0/crc/control dri/0/crtc-0/crc/data Drivers can implement the set_crc_source callback() in drm_crtc_funcs to start and stop generating frame CRCs and can add entries to the output by calling drm_crtc_add_crc_entry. v2: - Lots of good fixes suggested by Thierry. - Added documentation. - Changed the debugfs layout. - Moved to allocate the entries circular queue once when frame generation gets enabled for the first time. v3: - Use the control file just to select the source, and start and stop capture when the data file is opened and closed, respectively. - Make variable the number of CRC values per entry, per source. - Allocate entries queue each time we start capturing as now there isn't a fixed number of CRC values per entry. - Store the frame counter in the data file as a 8-digit hex number. - For sources that cannot provide useful frame numbers, place XXXXXXXX in the frame field. v4: - Build only if CONFIG_DEBUG_FS is enabled. - Use memdup_user_nul. - Consolidate calculation of the size of an entry in a helper. - Add 0x prefix to hex numbers in the data file. - Remove unnecessary snprintf and strlen usage in read callback. v5: - Made the crcs array in drm_crtc_crc_entry fixed-size - Lots of other smaller improvements suggested by Emil Velikov v7: - Move definition of drm_debugfs_crtc_crc_add to drm_internal.h v8: - Call debugfs_remove_recursive when we fail to create the minor device v9: - Register the debugfs directory for a crtc from drm_crtc_register_all() v10: - Don't let debugfs failures interrupt CRTC registration (Emil Velikov) v11: - Remove extra brace that broke compilation. Sorry! Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> --- Documentation/gpu/drm-uapi.rst | 6 + drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/drm_crtc.c | 34 +++- drivers/gpu/drm/drm_debugfs.c | 34 +++- drivers/gpu/drm/drm_debugfs_crc.c | 351 ++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_internal.h | 16 ++ include/drm/drm_crtc.h | 41 +++++ include/drm/drm_debugfs_crc.h | 73 ++++++++ 8 files changed, 555 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c create mode 100644 include/drm/drm_debugfs_crc.h