Hi Mahesh,
Thank you for the patch.
On Monday, 2 July 2018 14:07:20 EEST Mahesh Kumar wrote:
> This patch adds a new callback function "verify_crc_source" which will
> be used during setting the crc source in control node and while opening
> data node for crc reading. This will help in avoiding setting of wrong
> string for source.
Why do you need to verify the source in the open() operation ? Isn't it enough
to verify it in the write() handler ? The answer to this question might lie in
patch 08/10, in which case I'd add the .verify_crc_source() call in open() in
that patch, not here.
> Signed-off-by: Mahesh Kumar <mahesh1.ku...@intel.com>
> Cc: dri-de...@lists.freedesktop.org
> Reviewed-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> ---
> drivers/gpu/drm/drm_debugfs_crc.c | 15 +++++++++++++++
> include/drm/drm_crtc.h | 15 +++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> b/drivers/gpu/drm/drm_debugfs_crc.c index 9f8312137cad..c6a725b79ac6 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -87,6 +87,8 @@ static ssize_t crc_control_write(struct file *file, const
> char __user *ubuf, struct drm_crtc *crtc = m->private;
> struct drm_crtc_crc *crc = &crtc->crc;
> char *source;
> + size_t values_cnt;
> + int ret = 0;
>
> if (len == 0)
> return 0;
> @@ -104,6 +106,12 @@ static ssize_t crc_control_write(struct file *file,
> const char __user *ubuf, if (source[len] == '\n')
> source[len] = '\0';
>
> + if (crtc->funcs->verify_crc_source) {
> + ret = crtc->funcs->verify_crc_source(crtc, source, &values_cnt);
> + if (ret)
> + return ret;
> + }
> +
> spin_lock_irq(&crc->lock);
>
> if (crc->opened) {
> @@ -167,6 +175,13 @@ static int crtc_crc_open(struct inode *inode, struct
> file *filep) return ret;
> }
>
> + if (crtc->funcs->verify_crc_source) {
> + ret = crtc->funcs->verify_crc_source(crtc, crc->source,
> + &values_cnt);
> + if (ret)
> + return ret;
> + }
> +
> spin_lock_irq(&crc->lock);
> if (!crc->opened)
> crc->opened = true;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 23eddbccab10..1a6dcbf91744 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -661,6 +661,21 @@ struct drm_crtc_funcs {
> */
> int (*set_crc_source)(struct drm_crtc *crtc, const char *source,
> size_t *values_cnt);
> + /**
> + * @verify_crc_source:
> + *
> + * verifies the source of CRC checksums of frames before setting the
> + * source for CRC and during crc open.
> + *
> + * 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 (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
> + size_t *values_cnt);
>
> /**
> * @atomic_print_state:
--
Regards,
Laurent Pinchart
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
