[01/10] drm: crc: Introduce verify_crc_source callback
diff mbox

Message ID 20180712083635.7472-2-mahesh1.kumar@intel.com
State New
Headers show

Commit Message

Kumar, Mahesh July 12, 2018, 8:36 a.m. UTC
This patch adds a new callback function "verify_crc_source" which will
be used during setting the crc source in control node. This will help
in avoiding setting of wrong string for source.

Changes since V1:
 - do not yet verify_crc_source during open.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
 include/drm/drm_crtc.h            | 15 +++++++++++++++
 2 files changed, 23 insertions(+)

Comments

Laurent Pinchart July 12, 2018, 11:06 a.m. UTC | #1
Hi Mahesh,

Thank you for the patch.

On Thursday, 12 July 2018 11:36:26 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. This will help
> in avoiding setting of wrong string for source.
> 
> Changes since V1:
>  - do not yet verify_crc_source during open.
> 
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
>  include/drm/drm_crtc.h            | 15 +++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> b/drivers/gpu/drm/drm_debugfs_crc.c index 99961192bf03..1956acb61cc8 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;

No need to initialize ret to 0 here.

> 
>  	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) {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 17f4f93340b8..3ce5d2fe133d 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -684,6 +684,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.

Please also document whether the source parameter can be NULL.

It would also be nice to document values_cnt, but as that's not documented for 
the .set_crc_source() operation at the moment, I won't block the patch for 
this.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	 *
> +	 * 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:

Patch
diff mbox

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
index 99961192bf03..1956acb61cc8 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) {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 17f4f93340b8..3ce5d2fe133d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -684,6 +684,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: