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: