| Message ID | 20180712083635.7472-2-mahesh1.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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:
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: