Hi,
thanks foe the review.
On 7/10/2018 5:07 PM, Laurent Pinchart wrote:
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 ?
Agree, it duplicates the code but "index" is needed by set_crc_source call
anyway will create a wrapper to avoid duplication of code.
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.
for that I think rcar_du_crtc_create function can build a dynamic list
during initializing crtc functions, unless plane can be dynamically
allocated to any CRTC. this is the place where I need input from maintainer.
-Mahesh
+}
+
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,
};
/* ------------------------------------------------------------------------
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
