On Tue, Jun 21, 2016 at 01:06:41PM +0200, Tomeu Vizoso wrote:
> Adds a per-device debugfile "drm_crc_control" that allows selecting a
> source for frame checksums in each CRTC that supports them.
> 
> The checksums for each subsequent frame can be read from the per-CRTC
> file "drm_crtc_N_crc".
> 
> The code is taken from the i915 driver and other drivers can now provide
> frame CRCs by implementing the set_crc_source callback in
> drm_crtc_funcs.
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso at collabora.com>
> ---
> 
>  drivers/gpu/drm/drm_crtc.c     |  28 ++-
>  drivers/gpu/drm/drm_debugfs.c  | 506 
> ++++++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/drm_internal.h |  10 +
>  include/drm/drmP.h             |   5 +
>  include/drm/drm_crtc.h         |  72 ++++++
>  5 files changed, 611 insertions(+), 10 deletions(-)

I think we should finalize the internal and external api first, but this
needs a bit better documentation. For that I think it'd be good to extract
this to a new file like drm_debugfs_crc.[hc] and pull that into a new
section (maybe under the testing and validation section, next to the igt
howto) in drm-uapi.rst.

More doc bikeshedding below.

> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e7c862bd2f19..4dae42b122d9 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -657,8 +657,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev, 
> struct drm_crtc *crtc,
>                                      drm_num_crtcs(dev));
>       }
>       if (!crtc->name) {
> -             drm_mode_object_unregister(dev, &crtc->base);
> -             return -ENOMEM;
> +             ret = -ENOMEM;
> +             goto err_unregister;
>       }
>  
>       crtc->base.properties = &crtc->properties;
> @@ -673,12 +673,30 @@ int drm_crtc_init_with_planes(struct drm_device *dev, 
> struct drm_crtc *crtc,
>       if (cursor)
>               cursor->possible_crtcs = 1 << drm_crtc_index(crtc);
>  
> +#ifdef CONFIG_DEBUG_FS
> +     spin_lock_init(&crtc->crc.lock);
> +     init_waitqueue_head(&crtc->crc.wq);
> +     crtc->crc.debugfs_entries = kmalloc_array(DRM_MINOR_CNT,
> +                                       sizeof(*crtc->crc.debugfs_entries),
> +                                       GFP_KERNEL);
> +
> +     ret = drm_debugfs_crtc_add(crtc);
> +     if (ret)
> +             goto err_free_name;
> +#endif
> +
>       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);
>       }
>  
>       return 0;
> +
> +err_free_name:
> +     kfree(crtc->name);
> +err_unregister:
> +     drm_mode_object_unregister(dev, &crtc->base);
> +     return ret;
>  }
>  EXPORT_SYMBOL(drm_crtc_init_with_planes);
>  
> @@ -699,6 +717,12 @@ void drm_crtc_cleanup(struct drm_crtc *crtc)
>        * the indices on the drm_crtc after us in the crtc_list.
>        */
>  
> +#ifdef CONFIG_DEBUG_FS
> +     drm_debugfs_crtc_remove(crtc);
> +     kfree(crtc->crc.debugfs_entries);
> +     kfree(crtc->crc.source);
> +#endif
> +
>       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 fa10cef2ba37..cdc8836bc22a 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -30,6 +30,8 @@
>   * OTHER DEALINGS IN THE SOFTWARE.
>   */
>  
> +#include <linux/circ_buf.h>
> +#include <linux/ctype.h>
>  #include <linux/debugfs.h>
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
> @@ -127,6 +129,259 @@ fail:
>  }
>  EXPORT_SYMBOL(drm_debugfs_create_files);
>  
> +static int
> +drm_add_fake_info_node(struct drm_minor *minor,
> +                    struct dentry *ent,
> +                    const void *key)
> +{
> +     struct drm_info_node *node;
> +
> +     node = kmalloc(sizeof(*node), GFP_KERNEL);
> +     if (node == NULL) {
> +             debugfs_remove(ent);
> +             return -ENOMEM;
> +     }
> +
> +     node->minor = minor;
> +     node->dent = ent;
> +     node->info_ent = (void *) key;
> +
> +     mutex_lock(&minor->debugfs_lock);
> +     list_add(&node->list, &minor->debugfs_list);
> +     mutex_unlock(&minor->debugfs_lock);
> +
> +     return 0;
> +}
> +
> +static int crc_control_show(struct seq_file *m, void *data)
> +{
> +     struct drm_device *dev = m->private;
> +     struct drm_crtc *crtc;
> +
> +     drm_for_each_crtc(crtc, dev)
> +             seq_printf(m, "crtc %d %s\n", crtc->index,
> +                        crtc->crc.source ? crtc->crc.source : "none");
> +
> +     return 0;
> +}
> +
> +static int crc_control_open(struct inode *inode, struct file *file)
> +{
> +     struct drm_device *dev = inode->i_private;
> +
> +     return single_open(file, crc_control_show, dev);
> +}
> +
> +static int crc_control_update_crtc(struct drm_crtc *crtc, const char *source)
> +{
> +     struct drm_crtc_crc *crc = &crtc->crc;
> +     struct drm_crtc_crc_entry *entries;
> +     int ret;
> +
> +     if (!strcmp(source, "none"))
> +             source = NULL;
> +
> +     if (!crc->source && !source)
> +             return 0;
> +
> +     if (crc->source && source && !strcmp(crc->source, source))
> +             return 0;
> +
> +     /* Forbid changing the source without going back to "none". */
> +     if (crc->source && source)
> +             return -EINVAL;
> +
> +     if (!crtc->funcs->set_crc_source)
> +             return -ENOTSUPP;
> +
> +     if (source) {
> +             entries = kcalloc(DRM_CRTC_CRC_ENTRIES_NR,
> +                               sizeof(crc->entries[0]),
> +                               GFP_KERNEL);
> +             if (!entries)
> +                     return -ENOMEM;
> +
> +             spin_lock_irq(&crc->lock);
> +             kfree(crc->entries);
> +             crc->entries = entries;
> +             crc->head = 0;
> +             crc->tail = 0;
> +             spin_unlock_irq(&crc->lock);
> +     }
> +
> +     ret = crtc->funcs->set_crc_source(crtc, source);
> +     if (ret)
> +             return ret;
> +
> +     kfree(crc->source);
> +     crc->source = source ? kstrdup(source, GFP_KERNEL) : NULL;
> +
> +     if (!source) {
> +             spin_lock_irq(&crc->lock);
> +             entries = crc->entries;
> +             crc->entries = NULL;
> +             crc->head = 0;
> +             crc->tail = 0;
> +             spin_unlock_irq(&crc->lock);
> +
> +             kfree(entries);
> +     }
> +
> +     return 0;
> +}
> +
> +static struct drm_crtc *crtc_from_index(struct drm_device *dev, int index)
> +{
> +     struct drm_crtc *crtc;
> +     int i = 0;
> +
> +     drm_for_each_crtc(crtc, dev)
> +             if (i++ == index)
> +                     return crtc;
> +
> +     return NULL;
> +}
> +
> +/*
> + * Parse CRC command strings:
> + *   command: wsp* object wsp+ (crtc | pipe) wsp+ source wsp*
> + *   object: ('crtc' | 'pipe')
> + *   crtc: (0 | 1 | 2 | ...)
> + *   pipe: (A | B | C)
> + *   source: (none | plane1 | plane2 | ...)
> + *   wsp: (#0x20 | #0x9 | #0xA)+
> + *
> + * eg.:
> + *  "crtc 0 plane1"  ->  Start CRC computations on plane1 of first CRTC
> + *  "crtc 0 none"    ->  Stop CRC
> + */
> +static int crc_control_tokenize(char *buf, char *words[], int max_words)
> +{
> +     int n_words = 0;
> +
> +     while (*buf) {
> +             char *end;
> +
> +             /* skip leading white space */
> +             buf = skip_spaces(buf);
> +             if (!*buf)
> +                     break;  /* end of buffer */
> +
> +             /* find end of word */
> +             for (end = buf; *end && !isspace(*end); end++)
> +                     ;
> +
> +             if (n_words == max_words) {
> +                     DRM_DEBUG_KMS("too many words, allowed <= %d\n",
> +                                   max_words);
> +                     return -EINVAL; /* ran out of words[] before bytes */
> +             }
> +
> +             if (*end)
> +                     *end++ = '\0';
> +             words[n_words++] = buf;
> +             buf = end;
> +     }
> +
> +     return n_words;
> +}
> +
> +static int crc_control_parse_crtc(const char *buf, unsigned int *crtc_index)
> +{
> +     const char letter = buf[0];
> +
> +     if (!kstrtouint(buf, 10, crtc_index))
> +             return 0;
> +
> +     /* Backwards compatibility for Intel-style pipe letters */
> +     if (letter < 'A' || letter > 'Z')
> +             return -EINVAL;
> +
> +     *crtc_index = letter - 'A';
> +
> +     return 0;
> +}
> +
> +static int crc_control_parse(struct drm_device *dev, char *buf, size_t len)
> +{
> +#define N_WORDS 3
> +     int n_words;
> +     char *words[N_WORDS];
> +     unsigned int crtc_index;
> +     struct drm_crtc *crtc;
> +
> +     n_words = crc_control_tokenize(buf, words, N_WORDS);
> +     if (n_words != N_WORDS) {
> +             DRM_DEBUG_KMS("tokenize failed, a command is %d words\n",
> +                           N_WORDS);
> +             return -EINVAL;
> +     }
> +
> +     if (strcmp(words[0], "crtc") && strcmp(words[0], "pipe")) {
> +             DRM_DEBUG_KMS("Invalid command %s\n", words[0]);
> +             return -EINVAL;
> +     }
> +
> +     if (crc_control_parse_crtc(words[1], &crtc_index) < 0) {
> +             DRM_DEBUG_KMS("Invalid CRTC index: %s\n", words[1]);
> +             return -EINVAL;
> +     }
> +
> +     crtc = crtc_from_index(dev, crtc_index);
> +     if (!crtc) {
> +             DRM_DEBUG_KMS("Unknown CRTC index: %d\n", crtc_index);
> +             return -EINVAL;
> +     }
> +
> +     return crc_control_update_crtc(crtc, words[2]);
> +}
> +
> +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_device *dev = m->private;
> +     char *tmpbuf;
> +     int ret;
> +
> +     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;
> +     }
> +
> +     tmpbuf = kmalloc(len + 1, GFP_KERNEL);
> +     if (!tmpbuf)
> +             return -ENOMEM;
> +
> +     if (copy_from_user(tmpbuf, ubuf, len)) {
> +             ret = -EFAULT;
> +             goto out;
> +     }
> +     tmpbuf[len] = '\0';
> +
> +     ret = crc_control_parse(dev, tmpbuf, len);
> +out:
> +     kfree(tmpbuf);
> +     if (ret < 0)
> +             return ret;
> +
> +     *offp += len;
> +     return len;
> +}
> +
> +const struct file_operations drm_crc_control_fops = {
> +     .owner = THIS_MODULE,
> +     .open = crc_control_open,
> +     .read = seq_read,
> +     .llseek = seq_lseek,
> +     .release = single_release,
> +     .write = crc_control_write
> +};
> +
>  /**
>   * Initialize the DRI debugfs filesystem for a device
>   *
> @@ -142,8 +397,9 @@ int drm_debugfs_init(struct drm_minor *minor, int 
> minor_id,
>                    struct dentry *root)
>  {
>       struct drm_device *dev = minor->dev;
> +     struct dentry *ent;
>       char name[64];
> -     int ret;
> +     int ret = 0;
>  
>       INIT_LIST_HEAD(&minor->debugfs_list);
>       mutex_init(&minor->debugfs_lock);
> @@ -157,10 +413,23 @@ int drm_debugfs_init(struct drm_minor *minor, int 
> minor_id,
>       ret = drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
>                                      minor->debugfs_root, minor);
>       if (ret) {
> -             debugfs_remove(minor->debugfs_root);
> -             minor->debugfs_root = NULL;
>               DRM_ERROR("Failed to create core drm debugfs files\n");
> -             return ret;
> +             goto error_remove_dir;
> +     }
> +
> +     ent = debugfs_create_file("drm_crc_control", S_IRUGO | S_IWUSR,
> +                               minor->debugfs_root, dev,
> +                               &drm_crc_control_fops);
> +     if (!ent) {
> +             DRM_ERROR("Failed to create CRC control debugfs files\n");
> +             ret = -ENOMEM;
> +             goto error_remove_files;
> +     }
> +
> +     ret = drm_add_fake_info_node(minor, ent, &drm_crc_control_fops);
> +     if (ret) {
> +             DRM_ERROR("Failed to create CRC control debugfs files\n");
> +             goto error_remove_crc;
>       }
>  
>       if (dev->driver->debugfs_init) {
> @@ -171,7 +440,18 @@ int drm_debugfs_init(struct drm_minor *minor, int 
> minor_id,
>                       return ret;
>               }
>       }
> +
>       return 0;
> +
> +error_remove_crc:
> +     debugfs_remove(ent);
> +error_remove_files:
> +     drm_debugfs_remove_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES, minor);
> +error_remove_dir:
> +     debugfs_remove(minor->debugfs_root);
> +     minor->debugfs_root = NULL;
> +
> +     return ret;
>  }
>  
>  
> @@ -407,13 +687,223 @@ error:
>  
>  void drm_debugfs_connector_remove(struct drm_connector *connector)
>  {
> -     if (!connector->debugfs_entry)
> -             return;
> -
>       debugfs_remove_recursive(connector->debugfs_entry);
>  
>       connector->debugfs_entry = NULL;
>  }
>  
> -#endif /* CONFIG_DEBUG_FS */
> +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;
> +
> +     spin_lock_irq(&crc->lock);
> +
> +     if (crc->opened) {
> +             spin_unlock_irq(&crc->lock);
> +             return -EBUSY;
> +     }
> +
> +     crc->opened = true;
> +
> +     spin_unlock_irq(&crc->lock);
> +
> +     return 0;
> +}
> +
> +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;
> +
> +     spin_lock_irq(&crc->lock);
> +     crc->opened = false;
> +     spin_unlock_irq(&crc->lock);
> +
> +     return 0;
> +}
> +
> +/* (6 fields, 8 chars each, space separated (5) + '\n') */
> +#define CRC_LINE_LEN (6 * 8 + 5 + 1)
> +/* account for \'0' */
> +#define CRC_BUFFER_LEN       (CRC_LINE_LEN + 1)
> +
> +static int crtc_crc_data_count(struct drm_crtc_crc *crc)
> +{
> +     assert_spin_locked(&crc->lock);
> +     return CIRC_CNT(crc->head, crc->tail,
> +                     DRM_CRTC_CRC_ENTRIES_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;
> +     char buf[CRC_BUFFER_LEN];
> +     int n_entries;
> +     ssize_t bytes_read;
> +
> +     /*
> +      * Don't allow user space to provide buffers not big enough to hold
> +      * a line of data.
> +      */
> +     if (count < CRC_LINE_LEN)
> +             return -EINVAL;
> +
> +     if (!crc->source)
> +             return 0;
> +
> +     /* Nothing to read? */
> +     spin_lock_irq(&crc->lock);
> +     while (crtc_crc_data_count(crc) == 0) {
> +             int ret;
> +
> +             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 now have one or more entries to read */
> +     n_entries = count / CRC_LINE_LEN;
> +
> +     bytes_read = 0;
> +     while (n_entries > 0) {
> +             struct drm_crtc_crc_entry *entry =
> +                     &crc->entries[crc->tail];
> +             int ret;
> +
> +             if (CIRC_CNT(crc->head, crc->tail, DRM_CRTC_CRC_ENTRIES_NR) < 1)
> +                     break;
> +
> +             BUILD_BUG_ON_NOT_POWER_OF_2(DRM_CRTC_CRC_ENTRIES_NR);
> +             crc->tail = (crc->tail + 1) & (DRM_CRTC_CRC_ENTRIES_NR - 1);
> +
> +             bytes_read += snprintf(buf, CRC_BUFFER_LEN,
> +                                    "%8u %8x %8x %8x %8x %8x\n",
> +                                    entry->frame, entry->crc[0],
> +                                    entry->crc[1], entry->crc[2],
> +                                    entry->crc[3], entry->crc[4]);
> +
> +             spin_unlock_irq(&crc->lock);
> +
> +             ret = copy_to_user(user_buf, buf, CRC_LINE_LEN);
> +             if (ret == CRC_LINE_LEN)
> +                     return -EFAULT;
>  
> +             user_buf += CRC_LINE_LEN;
> +             n_entries--;
> +
> +             spin_lock_irq(&crc->lock);
> +     }
> +
> +     spin_unlock_irq(&crc->lock);
> +
> +     return bytes_read;
> +}
> +
> +const struct file_operations drm_crtc_crc_fops = {
> +     .owner = THIS_MODULE,
> +     .open = crtc_crc_open,
> +     .read = crtc_crc_read,
> +     .release = crtc_crc_release,
> +};
> +
> +static int drm_debugfs_crtc_add_for_minor(struct drm_crtc *crtc,
> +                                       struct drm_minor *minor)
> +{
> +     struct dentry *ent;
> +     char *name;
> +
> +     if (!minor->debugfs_root)
> +             return -1;
> +
> +     name = kasprintf(GFP_KERNEL, "drm_crtc_%d_crc", crtc->index);

I'd check for set_crc_source here and just not add the file if it's not
set. Seems a bit silly to expose an interface when it's not there.
-Daniel

> +     if (!name)
> +             return -ENOMEM;
> +
> +     ent = debugfs_create_file(name, S_IRUGO, minor->debugfs_root, crtc,
> +                               &drm_crtc_crc_fops);
> +     kfree(name);
> +     if (!ent)
> +             return PTR_ERR(ent);
> +
> +     crtc->crc.debugfs_entries[minor->type] = ent;
> +
> +     return 0;
> +}
> +
> +int drm_debugfs_crtc_add(struct drm_crtc *crtc)
> +{
> +     int ret;
> +
> +     ret = drm_debugfs_crtc_add_for_minor(crtc, crtc->dev->control);
> +     if (ret)
> +             return ret;
> +
> +     ret = drm_debugfs_crtc_add_for_minor(crtc, crtc->dev->primary);
> +     if (ret)
> +             return ret;
> +
> +     ret = drm_debugfs_crtc_add_for_minor(crtc, crtc->dev->render);
> +     if (ret)
> +             return ret;
> +
> +     return 0;
> +}
> +
> +void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
> +{
> +     int i;
> +
> +     for (i = 0; i < DRM_MINOR_CNT; i++) {
> +             debugfs_remove_recursive(crtc->crc.debugfs_entries[i]);
> +             crtc->crc.debugfs_entries[i] = NULL;
> +     }
> +}
> +
> +void drm_crtc_add_crc_entry(struct drm_crtc *crtc, uint32_t frame,
> +                         uint32_t crc0, uint32_t crc1, uint32_t crc2,
> +                         uint32_t crc3, uint32_t crc4)
> +{
> +     struct drm_crtc_crc *crc = &crtc->crc;
> +     struct drm_crtc_crc_entry *entry;
> +     int head, tail;
> +
> +     spin_lock(&crc->lock);
> +
> +     head = crc->head;
> +     tail = crc->tail;
> +
> +     if (CIRC_SPACE(head, tail, DRM_CRTC_CRC_ENTRIES_NR) < 1) {
> +             spin_unlock(&crc->lock);
> +             DRM_ERROR("Overflow of CRC buffer, userspace reads too 
> slow.\n");
> +             return;
> +     }
> +
> +     entry = &crc->entries[head];
> +
> +     entry->frame = frame;
> +     entry->crc[0] = crc0;
> +     entry->crc[1] = crc1;
> +     entry->crc[2] = crc2;
> +     entry->crc[3] = crc3;
> +     entry->crc[4] = crc4;
> +
> +     head = (head + 1) & (DRM_CRTC_CRC_ENTRIES_NR - 1);
> +     crc->head = head;
> +
> +     spin_unlock(&crc->lock);
> +
> +     wake_up_interruptible(&crc->wq);
> +}
> +
> +#endif /* CONFIG_DEBUG_FS */
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 38401d406532..e5b124d937f5 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -100,6 +100,8 @@ 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);
>  #else
>  static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>                                  struct dentry *root)
> @@ -119,4 +121,12 @@ 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)
> +{
> +}
>  #endif
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 084fd141e8bf..ec2f91c8b7cd 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1142,6 +1142,11 @@ static __inline__ bool drm_can_sleep(void)
>       return true;
>  }
>  
> +#if defined(CONFIG_DEBUG_FS)
> +extern const struct file_operations drm_crc_control_fops;
> +extern const struct file_operations drm_crtc_crc_fops;
> +#endif
> +
>  /* helper for handling conditionals in various for_each macros */
>  #define for_each_if(condition) if (!(condition)) {} else
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c2734979f164..141335a3c647 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -376,6 +376,22 @@ struct drm_crtc_state {
>       struct drm_atomic_state *state;
>  };
>  
> +struct drm_crtc_crc_entry {
> +     uint32_t frame;
> +     uint32_t crc[5];
> +};
> +
> +#define DRM_CRTC_CRC_ENTRIES_NR      128
> +struct drm_crtc_crc {
> +     spinlock_t lock;
> +     const char *source;
> +     bool opened;            /* exclusive access to the result file */
> +     struct drm_crtc_crc_entry *entries;
> +     int head, tail;
> +     wait_queue_head_t wq;
> +     struct dentry **debugfs_entries;
> +};

I think the above two should be moved into drm_debugfs_crc.h and also
documented with kerneldoc.

> +
>  /**
>   * struct drm_crtc_funcs - control CRTCs for a given device
>   *
> @@ -704,6 +720,29 @@ struct drm_crtc_funcs {
>                                  const struct drm_crtc_state *state,
>                                  struct drm_property *property,
>                                  uint64_t *val);
> +
> +     /**
> +      * @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 should 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);
>  };
>  
>  /**
> @@ -817,6 +856,15 @@ struct drm_crtc {
>        * context.
>        */
>       struct drm_modeset_acquire_ctx *acquire_ctx;
> +
> +#ifdef CONFIG_DEBUG_FS
> +     /**
> +      * @drm_crtc_crc:
> +      *
> +      * Configuration settings of CRC capture.
> +      */
> +     struct drm_crtc_crc crc;
> +#endif
>  };
>  
>  /**
> @@ -2496,6 +2544,30 @@ static inline uint32_t drm_crtc_mask(struct drm_crtc 
> *crtc)
>       return 1 << drm_crtc_index(crtc);
>  }
>  
> +/**
> + * drm_crtc_add_crc_entry - Add entry with CRC information for a frame
> + * @crtc: CRTC to which the frame belongs
> + * @frame: number of the frame characterized by the CRC data
> + * @crc0: piece of data about frame
> + * @crc1: piece of data about frame
> + * @crc2: piece of data about frame
> + * @crc3: piece of data about frame
> + * @crc4: piece of data about frame
> + *
> + * 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.
> + */

Generally we put the kerneldoc for functions into the source file. Only
static inlines and structures are documented in the headers.
> +#if defined(CONFIG_DEBUG_FS)
> +void drm_crtc_add_crc_entry(struct drm_crtc *crtc, uint32_t frame,
> +                         uint32_t crc0, uint32_t crc1, uint32_t crc2,
> +                         uint32_t crc3, uint32_t crc4);
> +#else
> +static inline void drm_crtc_add_crc_entry(struct drm_crtc *crtc, uint32_t 
> frame,
> +                                       uint32_t crc0, uint32_t crc1,
> +                                       uint32_t crc2, uint32_t crc3,
> +                                       uint32_t crc4) {}
> +#endif
> +
>  extern void drm_connector_ida_init(void);
>  extern void drm_connector_ida_destroy(void);
>  extern int drm_connector_init(struct drm_device *dev,
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Reply via email to