Re: [PATCH 05/10] drm/rcar-du/crc: Implement verify_crc_source callback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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,
  };

  /* ------------------------------------------------------------------------

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux