Hi Mahesh, Thank you for the patch. On Monday, 2 July 2018 14:07:24 EEST Mahesh Kumar wrote: > This patch implements "verify_crc_source" callback function for > rcar drm driver. > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@xxxxxxxxx> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 40 +++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 15dc9caa128b..24eeaa7e14d7 > 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -756,6 +756,45 @@ static void rcar_du_crtc_disable_vblank(struct drm_crtc > *crtc) rcrtc->vblank_enable = false; > } > > +static int rcar_du_crtc_verify_crc_source(struct drm_crtc *crtc, > + const char *source_name, > + size_t *values_cnt) > +{ > + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); > + unsigned int index = 0; > + unsigned int i; > + int ret; > + > + /* > + * Parse the source name. Supported values are "plane%u" to compute the > + * CRC on an input plane (%u is the plane ID), and "auto" to compute the > + * CRC on the composer (VSP) output. > + */ > + if (!source_name || !strcmp(source_name, "auto")) { > + goto out; > + } else if (strstarts(source_name, "plane")) { > + ret = kstrtouint(source_name + strlen("plane"), 10, &index); > + if (ret < 0) > + return ret; > + > + for (i = 0; i < rcrtc->vsp->num_planes; ++i) { > + if (index == rcrtc->vsp->planes[i].plane.base.id) { > + index = i; > + break; > + } > + } > + > + if (i >= rcrtc->vsp->num_planes) > + return -EINVAL; > + } else { > + return -EINVAL; > + } > + > +out: > + *values_cnt = 1; > + return 0; This duplicates lots of code from the rcar_du_crtc_set_crc_source() function. Could you please extract it to a shared function ? Could you please also implement support for the .get_crc_sources() operation ? I think doing so might show limitations in the current API, namely the fact that the list will need to be dynamically created for this driver. > +} > + > static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc, > const char *source_name, > size_t *values_cnt) > @@ -861,6 +900,7 @@ static const struct drm_crtc_funcs crtc_funcs_gen3 = { > .enable_vblank = rcar_du_crtc_enable_vblank, > .disable_vblank = rcar_du_crtc_disable_vblank, > .set_crc_source = rcar_du_crtc_set_crc_source, > + .verify_crc_source = rcar_du_crtc_verify_crc_source, > }; > > /* ------------------------------------------------------------------------ -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel