Re: [PATCH v11 2/4] drm: Add API for capturing frame CRCs

[Daniel Vetter]

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@xxxxxxxxxxxxx>
> ---
> 
>  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@xxxxxxxxx>
> + *
> + */
> +
> +#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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch