[PATCH 00/10] Improve crc-core driver interface

[PATCH 00/10] Improve crc-core driver interface

[Mahesh Kumar]

This series improves crc-core <-> driver interface.
This series adds following functionality in the crc-core
 - Now control node will print all the available sources if
   implemented by driver along with current source.
 - Setting of sorce will fail if provided source is not supported
 - cleanup of crtc_crc_open function first allocate memory before
   enabling CRC generation
 - Don't block open() call instead wait in crc read call.

Following IGT will fail due to crc-core <-> driver interface change
igt@kms_pipe_crc_basic@bad-source <now setting bad-source itself will fail>
ig@kms_pipe_crc_basic@nonblocking-crc-pipe-X 
ig@kms_pipe_crc_basic@nonblocking-crc-pipe-X-frame-sequence
In nonblocking crc tests we'll get lesser crc's due to skipping crc

AMD/Rockchip/rcar code path is not validated and need inputs

Changes:
 - Add dri-devel in Cc
Changes rev2:
 - now get_crc_sources returns a constant pointer to an array of
   source list and crc-core does the verification

Cc: dri-devel@lists.freedesktop.org

Mahesh Kumar (10):
  drm: crc: Introduce verify_crc_source callback
  drm: crc: Introduce pre_crc_read function
  drm: crc: Introduce get_crc_sources callback
  drm/rockchip/crc: Implement verify_crc_source callback
  drm/amdgpu_dm/crc: Implement verify_crc_source callback
  drm/rcar-du/crc: Implement verify_crc_source callback
  drm/i915/crc: implement verify_crc_source callback
  drm/i915/crc: implement get_crc_sources callback
  drm/crc: Cleanup crtc_crc_open function
  Revert "drm: crc: Wait for a frame before returning from open()"

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |   1 +
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |   7 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c  |  20 +++-
 drivers/gpu/drm/drm_debugfs_crc.c                  |  78 ++++++++------
 drivers/gpu/drm/i915/intel_display.c               |   2 +
 drivers/gpu/drm/i915/intel_drv.h                   |   8 +-
 drivers/gpu/drm/i915/intel_pipe_crc.c              | 118 ++++++++++++++++++++-
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c             |  45 +++++++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c        |  26 ++++-
 include/drm/drm_crtc.h                             |  47 +++++++-
 10 files changed, 301 insertions(+), 51 deletions(-)

-- 
2.16.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 01/10] drm: crc: Introduce verify_crc_source callback

[Mahesh Kumar]

This patch adds a new callback function "verify_crc_source" which will
be used during setting the crc source in control node and while opening
data node for crc reading. This will help in avoiding setting of wrong
string for source.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_debugfs_crc.c | 15 +++++++++++++++
 include/drm/drm_crtc.h            | 15 +++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
index 9f8312137cad..c6a725b79ac6 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) {
@@ -167,6 +175,13 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
 			return ret;
 	}
 
+	if (crtc->funcs->verify_crc_source) {
+		ret = crtc->funcs->verify_crc_source(crtc, crc->source,
+						     &values_cnt);
+		if (ret)
+			return ret;
+	}
+
 	spin_lock_irq(&crc->lock);
 	if (!crc->opened)
 		crc->opened = true;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 23eddbccab10..1a6dcbf91744 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -661,6 +661,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:
-- 
2.16.2

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

[PATCH 02/10] drm: crc: Introduce pre_crc_read function

[Mahesh Kumar]

This patch implements a callback function "pre_crc_read" which will
be called before crc read. In this function driver can implement and
preparation work required for successfully reading CRC data.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
 include/drm/drm_crtc.h            | 14 ++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
index c6a725b79ac6..2b4a737c5aeb 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -278,6 +278,14 @@ static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
 		return 0;
 	}
 
+	if (crtc->funcs->pre_crc_read) {
+		ret = crtc->funcs->pre_crc_read(crtc);
+		if (ret) {
+			spin_unlock_irq(&crc->lock);
+			return ret;
+		}
+	}
+
 	/* Nothing to read? */
 	while (crtc_crc_data_count(crc) == 0) {
 		if (filep->f_flags & O_NONBLOCK) {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 1a6dcbf91744..bae432469616 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -676,6 +676,20 @@ struct drm_crtc_funcs {
 	 */
 	int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
 				 size_t *values_cnt);
+	/**
+	 * @pre_crc_read:
+	 *
+	 * Driver callback for performing any preparation work required by
+	 * driver before reading CRC
+	 *
+	 * This callback is optional if the driver does not support CRC
+	 * generation or no prework is required before reading the crc
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or a negative error code on failure.
+	 */
+	int (*pre_crc_read)(struct drm_crtc *crtc);
 
 	/**
 	 * @atomic_print_state:
-- 
2.16.2

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

[PATCH 03/10] drm: crc: Introduce get_crc_sources callback

[Mahesh Kumar]

This patch implements a callback function "get_crc_sources" which
will be called during read of control node. It is an optional
callback function and if driver implements this callback, driver
should print list of available CRC sources in seq_file privided
as an input to the callback.

Changes Since V1: (Daniel)
 - return const pointer to an array of crc sources list
 - do validation of sources in CRC-core

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_debugfs_crc.c | 20 +++++++++++++++++++-
 include/drm/drm_crtc.h            | 16 ++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
index 2b4a737c5aeb..a7b6d5de81de 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -67,9 +67,27 @@
 static int crc_control_show(struct seq_file *m, void *data)
 {
 	struct drm_crtc *crtc = m->private;
+	size_t count;
+
+	if (crtc->funcs->get_crc_sources) {
+		const char *const *sources = crtc->funcs->get_crc_sources(crtc,
+									&count);
+		size_t values_cnt;
+		int i;
+
+		if (count <= 0 || !sources)
+			goto out;
+
+		seq_puts(m, "[");
+		for (i = 0; i < count; i++)
+			if (!crtc->funcs->verify_crc_source(crtc, sources[i],
+							    &values_cnt))
+				seq_printf(m, "%s ", sources[i]);
+		seq_puts(m, "] ");
+	}
 
+out:
 	seq_printf(m, "%s\n", crtc->crc.source);
-
 	return 0;
 }
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index bae432469616..594326664abe 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -690,6 +690,22 @@ struct drm_crtc_funcs {
 	 * 0 on success or a negative error code on failure.
 	 */
 	int (*pre_crc_read)(struct drm_crtc *crtc);
+	/**
+	 * @get_crc_sources:
+	 *
+	 * Driver callback for getting a list of all the available sources for
+	 * CRC generation.
+	 *
+	 * This callback is optional if the driver does not support exporting of
+	 * possible CRC sources list. CRC-core does the verification of sources.
+	 *
+	 * RETURNS:
+	 *
+	 * a constant character pointer to the list of all the available CRC
+	 * sources
+	 */
+	const char *const *(*get_crc_sources)(struct drm_crtc *crtc,
+					      size_t *count);
 
 	/**
 	 * @atomic_print_state:
-- 
2.16.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/10] drm/rockchip/crc: Implement verify_crc_source callback

[Mahesh Kumar]

This patch implements "verify_crc_source" callback function for
rockchip drm driver.

Changes since V1:
 - simplify the verification (Jani N)

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index effecbed2d11..77e91b15ddb4 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1138,12 +1138,31 @@ static int vop_crtc_set_crc_source(struct drm_crtc *crtc,
 
 	return ret;
 }
+
+static int
+vop_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
+			   size_t *values_cnt)
+{
+	if (source_name && strcmp(source_name, "auto") != 0)
+		return -EINVAL;
+
+	*values_cnt = 3;
+	return 0;
+}
+
 #else
 static int vop_crtc_set_crc_source(struct drm_crtc *crtc,
 				   const char *source_name, size_t *values_cnt)
 {
 	return -ENODEV;
 }
+
+static int
+vop_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
+			   size_t *values_cnt)
+{
+	return -ENODEV;
+}
 #endif
 
 static const struct drm_crtc_funcs vop_crtc_funcs = {
@@ -1156,6 +1175,7 @@ static const struct drm_crtc_funcs vop_crtc_funcs = {
 	.enable_vblank = vop_crtc_enable_vblank,
 	.disable_vblank = vop_crtc_disable_vblank,
 	.set_crc_source = vop_crtc_set_crc_source,
+	.verify_crc_source = vop_crtc_verify_crc_source,
 };
 
 static void vop_fb_unref_worker(struct drm_flip_work *work, void *val)
-- 
2.16.2

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

[PATCH 05/10] drm/amdgpu_dm/crc: Implement verify_crc_source callback

[Mahesh Kumar]

This patch implements "verify_crc_source" callback function for
AMD drm driver.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c     |  1 +
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h     |  4 ++++
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 16 ++++++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 655950102827..b2f357cb8f7f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2563,6 +2563,7 @@ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
 	.atomic_duplicate_state = dm_crtc_duplicate_state,
 	.atomic_destroy_state = dm_crtc_destroy_state,
 	.set_crc_source = amdgpu_dm_crtc_set_crc_source,
+	.verify_crc_source = amdgpu_dm_crtc_verify_crc_source,
 	.enable_vblank = dm_enable_vblank,
 	.disable_vblank = dm_disable_vblank,
 };
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index a29dc35954c9..e43ed064dc46 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -260,9 +260,13 @@ amdgpu_dm_remove_sink_from_freesync_module(struct drm_connector *connector);
 #ifdef CONFIG_DEBUG_FS
 int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 				  size_t *values_cnt);
+int amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc,
+				     const char *src_name,
+				     size_t *values_cnt);
 void amdgpu_dm_crtc_handle_crc_irq(struct drm_crtc *crtc);
 #else
 #define amdgpu_dm_crtc_set_crc_source NULL
+#define amdgpu_dm_crtc_verify_crc_source NULL
 #define amdgpu_dm_crtc_handle_crc_irq(x)
 #endif
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
index 52f2c01349e3..dfcca594d52a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
@@ -46,6 +46,22 @@ static enum amdgpu_dm_pipe_crc_source dm_parse_crc_source(const char *source)
 	return AMDGPU_DM_PIPE_CRC_SOURCE_INVALID;
 }
 
+int
+amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
+				 size_t *values_cnt)
+{
+	enum amdgpu_dm_pipe_crc_source source = dm_parse_crc_source(src_name);
+
+	if (source < 0) {
+		DRM_DEBUG_DRIVER("Unknown CRC source %s for CRTC%d\n",
+				 src_name, crtc->index);
+		return -EINVAL;
+	}
+
+	*values_cnt = 3;
+	return 0;
+}
+
 int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 			   size_t *values_cnt)
 {
-- 
2.16.2

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

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

[Mahesh Kumar]

This patch implements "verify_crc_source" callback function for
rcar drm driver.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 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;
+}
+
 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,
 };
 
 /* -----------------------------------------------------------------------------
-- 
2.16.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/10] drm/i915/crc: implement verify_crc_source callback

[Mahesh Kumar]

This patch implements verify_crc_source callback function introduced
earlier in this series.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/i915/intel_display.c  |   1 +
 drivers/gpu/drm/i915/intel_drv.h      |   3 +
 drivers/gpu/drm/i915/intel_pipe_crc.c | 108 ++++++++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index eaa0663963a5..821754fb49f1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12893,6 +12893,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.atomic_duplicate_state = intel_crtc_duplicate_state,
 	.atomic_destroy_state = intel_crtc_destroy_state,
 	.set_crc_source = intel_crtc_set_crc_source,
+	.verify_crc_source = intel_crtc_verify_crc_source,
 };
 
 struct wait_rps_boost {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a6ff2600a3a0..764b53dfe7de 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2156,10 +2156,13 @@ int intel_pipe_crc_create(struct drm_minor *minor);
 #ifdef CONFIG_DEBUG_FS
 int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
 			      size_t *values_cnt);
+int intel_crtc_verify_crc_source(struct drm_crtc *crtc,
+				 const char *source_name, size_t *values_cnt);
 void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc);
 void intel_crtc_enable_pipe_crc(struct intel_crtc *crtc);
 #else
 #define intel_crtc_set_crc_source NULL
+#define intel_crtc_verify_crc_source NULL
 static inline void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc)
 {
 }
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 39a4e4edda07..a37521380f94 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -913,6 +913,114 @@ int intel_pipe_crc_create(struct drm_minor *minor)
 	return 0;
 }
 
+static int i8xx_crc_source_valid(struct drm_i915_private *dev_priv,
+				 const enum intel_pipe_crc_source source)
+{
+	switch (source) {
+	case INTEL_PIPE_CRC_SOURCE_PIPE:
+	case INTEL_PIPE_CRC_SOURCE_NONE:
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int i9xx_crc_source_valid(struct drm_i915_private *dev_priv,
+				 const enum intel_pipe_crc_source source)
+{
+	switch (source) {
+	case INTEL_PIPE_CRC_SOURCE_PIPE:
+	case INTEL_PIPE_CRC_SOURCE_TV:
+	case INTEL_PIPE_CRC_SOURCE_DP_B:
+	case INTEL_PIPE_CRC_SOURCE_DP_C:
+	case INTEL_PIPE_CRC_SOURCE_DP_D:
+	case INTEL_PIPE_CRC_SOURCE_NONE:
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int vlv_crc_source_valid(struct drm_i915_private *dev_priv,
+				const enum intel_pipe_crc_source source)
+{
+	switch (source) {
+	case INTEL_PIPE_CRC_SOURCE_PIPE:
+	case INTEL_PIPE_CRC_SOURCE_DP_B:
+	case INTEL_PIPE_CRC_SOURCE_DP_C:
+	case INTEL_PIPE_CRC_SOURCE_DP_D:
+	case INTEL_PIPE_CRC_SOURCE_NONE:
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ilk_crc_source_valid(struct drm_i915_private *dev_priv,
+				const enum intel_pipe_crc_source source)
+{
+	switch (source) {
+	case INTEL_PIPE_CRC_SOURCE_PIPE:
+	case INTEL_PIPE_CRC_SOURCE_PLANE1:
+	case INTEL_PIPE_CRC_SOURCE_PLANE2:
+	case INTEL_PIPE_CRC_SOURCE_NONE:
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ivb_crc_source_valid(struct drm_i915_private *dev_priv,
+				const enum intel_pipe_crc_source source)
+{
+	switch (source) {
+	case INTEL_PIPE_CRC_SOURCE_PIPE:
+	case INTEL_PIPE_CRC_SOURCE_PLANE1:
+	case INTEL_PIPE_CRC_SOURCE_PLANE2:
+	case INTEL_PIPE_CRC_SOURCE_PF:
+	case INTEL_PIPE_CRC_SOURCE_NONE:
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int
+intel_is_valid_crc_source(struct drm_i915_private *dev_priv,
+			  const enum intel_pipe_crc_source source)
+{
+	if (IS_GEN2(dev_priv))
+		return i8xx_crc_source_valid(dev_priv, source);
+	else if (INTEL_GEN(dev_priv) < 5)
+		return i9xx_crc_source_valid(dev_priv, source);
+	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		return vlv_crc_source_valid(dev_priv, source);
+	else if (IS_GEN5(dev_priv) || IS_GEN6(dev_priv))
+		return ilk_crc_source_valid(dev_priv, source);
+	else
+		return ivb_crc_source_valid(dev_priv, source);
+}
+
+int intel_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
+				 size_t *values_cnt)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	enum intel_pipe_crc_source source;
+
+	if (display_crc_ctl_parse_source(source_name, &source) < 0) {
+		DRM_DEBUG_DRIVER("unknown source %s\n", source_name);
+		return -EINVAL;
+	}
+
+	if (source == INTEL_PIPE_CRC_SOURCE_AUTO ||
+	    intel_is_valid_crc_source(dev_priv, source) == 0) {
+		*values_cnt = 5;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
 			      size_t *values_cnt)
 {
-- 
2.16.2

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

[PATCH 08/10] drm/i915/crc: implement get_crc_sources callback

[Mahesh Kumar]

This patch implements get_crc_sources callback, which returns list of
all the valid crc sources supported by driver in current platform.

Changes since V1:
 - Return array of crc sources

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/i915/intel_display.c  | 1 +
 drivers/gpu/drm/i915/intel_drv.h      | 3 +++
 drivers/gpu/drm/i915/intel_pipe_crc.c | 7 +++++++
 3 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 821754fb49f1..b7951b3587e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12894,6 +12894,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.atomic_destroy_state = intel_crtc_destroy_state,
 	.set_crc_source = intel_crtc_set_crc_source,
 	.verify_crc_source = intel_crtc_verify_crc_source,
+	.get_crc_sources = intel_crtc_get_crc_sources,
 };
 
 struct wait_rps_boost {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 764b53dfe7de..99e3b6477d39 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2158,11 +2158,14 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
 			      size_t *values_cnt);
 int intel_crtc_verify_crc_source(struct drm_crtc *crtc,
 				 const char *source_name, size_t *values_cnt);
+const char *const *intel_crtc_get_crc_sources(struct drm_crtc *crtc,
+					      size_t *count);
 void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc);
 void intel_crtc_enable_pipe_crc(struct intel_crtc *crtc);
 #else
 #define intel_crtc_set_crc_source NULL
 #define intel_crtc_verify_crc_source NULL
+#define intel_crtc_get_crc_sources NULL
 static inline void intel_crtc_disable_pipe_crc(struct intel_crtc *crtc)
 {
 }
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
index a37521380f94..1dffc346f1bc 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -1001,6 +1001,13 @@ intel_is_valid_crc_source(struct drm_i915_private *dev_priv,
 		return ivb_crc_source_valid(dev_priv, source);
 }
 
+const char *const *intel_crtc_get_crc_sources(struct drm_crtc *crtc,
+					      size_t *count)
+{
+	*count = ARRAY_SIZE(pipe_crc_sources);
+	return pipe_crc_sources;
+}
+
 int intel_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
 				 size_t *values_cnt)
 {
-- 
2.16.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/10] drm/crc: Cleanup crtc_crc_open function

[Mahesh Kumar]

This patch make changes to allocate crc-entries buffer before
enabling CRC generation.
It moves all the failure check early in the function before setting
the source or memory allocation.
Now set_crc_source takes only two variable inputs, values_cnt we
already gets as part of verify_crc_source.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  3 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c  |  4 +-
 drivers/gpu/drm/drm_debugfs_crc.c                  | 52 +++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h                   |  3 +-
 drivers/gpu/drm/i915/intel_pipe_crc.c              |  4 +-
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c             |  5 +--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c        |  6 +--
 include/drm/drm_crtc.h                             |  3 +-
 8 files changed, 30 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index e43ed064dc46..54056d180003 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -258,8 +258,7 @@ amdgpu_dm_remove_sink_from_freesync_module(struct drm_connector *connector);
 
 /* amdgpu_dm_crc.c */
 #ifdef CONFIG_DEBUG_FS
-int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name,
-				  size_t *values_cnt);
+int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name);
 int amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc,
 				     const char *src_name,
 				     size_t *values_cnt);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
index dfcca594d52a..e7ad528f5853 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
@@ -62,8 +62,7 @@ amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
 	return 0;
 }
 
-int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name,
-			   size_t *values_cnt)
+int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
 {
 	struct dm_crtc_state *crtc_state = to_dm_crtc_state(crtc->state);
 	struct dc_stream_state *stream_state = crtc_state->stream;
@@ -99,7 +98,6 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 			return -EINVAL;
 	}
 
-	*values_cnt = 3;
 	/* Reset crc_skipped on dm state */
 	crtc_state->crc_skip_count = 0;
 	return 0;
diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
index a7b6d5de81de..08dfe19b6286 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -124,11 +124,9 @@ 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;
-	}
+	ret = crtc->funcs->verify_crc_source(crtc, source, &values_cnt);
+	if (ret)
+		return ret;
 
 	spin_lock_irq(&crc->lock);
 
@@ -193,12 +191,15 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
 			return ret;
 	}
 
-	if (crtc->funcs->verify_crc_source) {
-		ret = crtc->funcs->verify_crc_source(crtc, crc->source,
-						     &values_cnt);
-		if (ret)
-			return ret;
-	}
+	ret = crtc->funcs->verify_crc_source(crtc, crc->source, &values_cnt);
+	if (ret)
+		return ret;
+
+	if (WARN_ON(values_cnt > DRM_MAX_CRC_NR))
+		return -EINVAL;
+
+	if (WARN_ON(values_cnt == 0))
+		return -EINVAL;
 
 	spin_lock_irq(&crc->lock);
 	if (!crc->opened)
@@ -210,30 +211,22 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
 	if (ret)
 		return ret;
 
-	ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt);
-	if (ret)
-		goto err;
-
-	if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) {
-		ret = -EINVAL;
-		goto err_disable;
-	}
-
-	if (WARN_ON(values_cnt == 0)) {
-		ret = -EINVAL;
-		goto err_disable;
-	}
-
 	entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL);
 	if (!entries) {
 		ret = -ENOMEM;
-		goto err_disable;
+		goto err;
 	}
 
 	spin_lock_irq(&crc->lock);
 	crc->entries = entries;
 	crc->values_cnt = values_cnt;
+	spin_unlock_irq(&crc->lock);
 
+	ret = crtc->funcs->set_crc_source(crtc, crc->source);
+	if (ret)
+		goto err;
+
+	spin_lock_irq(&crc->lock);
 	/*
 	 * Only return once we got a first frame, so userspace doesn't have to
 	 * guess when this particular piece of HW will be ready to start
@@ -250,7 +243,7 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
 	return 0;
 
 err_disable:
-	crtc->funcs->set_crc_source(crtc, NULL, &values_cnt);
+	crtc->funcs->set_crc_source(crtc, NULL);
 err:
 	spin_lock_irq(&crc->lock);
 	crtc_crc_cleanup(crc);
@@ -262,9 +255,8 @@ static int crtc_crc_release(struct inode *inode, struct file *filep)
 {
 	struct drm_crtc *crtc = filep->f_inode->i_private;
 	struct drm_crtc_crc *crc = &crtc->crc;
-	size_t values_cnt;
 
-	crtc->funcs->set_crc_source(crtc, NULL, &values_cnt);
+	crtc->funcs->set_crc_source(crtc, NULL);
 
 	spin_lock_irq(&crc->lock);
 	crtc_crc_cleanup(crc);
@@ -378,7 +370,7 @@ int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc)
 {
 	struct dentry *crc_ent, *ent;
 
-	if (!crtc->funcs->set_crc_source)
+	if (!crtc->funcs->set_crc_source || !crtc->funcs->verify_crc_source)
 		return 0;
 
 	crc_ent = debugfs_create_dir("crc", crtc->debugfs_entry);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 99e3b6477d39..7fe0341bcc27 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2154,8 +2154,7 @@ void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
 /* intel_pipe_crc.c */
 int intel_pipe_crc_create(struct drm_minor *minor);
 #ifdef CONFIG_DEBUG_FS
-int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
-			      size_t *values_cnt);
+int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name);
 int intel_crtc_verify_crc_source(struct drm_crtc *crtc,
 				 const char *source_name, size_t *values_cnt);
 const char *const *intel_crtc_get_crc_sources(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 1dffc346f1bc..27d560f7a817 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -1028,8 +1028,7 @@ int intel_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
 	return -EINVAL;
 }
 
-int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
-			      size_t *values_cnt)
+int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[crtc->index];
@@ -1068,7 +1067,6 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
 	}
 
 	pipe_crc->skipped = 0;
-	*values_cnt = 5;
 
 out:
 	intel_display_power_put(dev_priv, power_domain);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 24eeaa7e14d7..829c644addba 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -796,8 +796,7 @@ static int rcar_du_crtc_verify_crc_source(struct drm_crtc *crtc,
 }
 
 static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc,
-				       const char *source_name,
-				       size_t *values_cnt)
+				       const char *source_name)
 {
 	struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
 	struct drm_modeset_acquire_ctx ctx;
@@ -837,8 +836,6 @@ static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc,
 		return -EINVAL;
 	}
 
-	*values_cnt = 1;
-
 	/* Perform an atomic commit to set the CRC source. */
 	drm_modeset_acquire_init(&ctx, 0);
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 77e91b15ddb4..657372306623 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1117,7 +1117,7 @@ static struct drm_connector *vop_get_edp_connector(struct vop *vop)
 }
 
 static int vop_crtc_set_crc_source(struct drm_crtc *crtc,
-				   const char *source_name, size_t *values_cnt)
+				   const char *source_name)
 {
 	struct vop *vop = to_vop(crtc);
 	struct drm_connector *connector;
@@ -1127,8 +1127,6 @@ static int vop_crtc_set_crc_source(struct drm_crtc *crtc,
 	if (!connector)
 		return -EINVAL;
 
-	*values_cnt = 3;
-
 	if (source_name && strcmp(source_name, "auto") == 0)
 		ret = analogix_dp_start_crc(connector);
 	else if (!source_name)
@@ -1152,7 +1150,7 @@ vop_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
 
 #else
 static int vop_crtc_set_crc_source(struct drm_crtc *crtc,
-				   const char *source_name, size_t *values_cnt)
+				   const char *source_name)
 {
 	return -ENODEV;
 }
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 594326664abe..e8d2319f381f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -659,8 +659,7 @@ struct drm_crtc_funcs {
 	 *
 	 * 0 on success or a negative error code on failure.
 	 */
-	int (*set_crc_source)(struct drm_crtc *crtc, const char *source,
-			      size_t *values_cnt);
+	int (*set_crc_source)(struct drm_crtc *crtc, const char *source);
 	/**
 	 * @verify_crc_source:
 	 *
-- 
2.16.2

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

[PATCH 10/10] Revert "drm: crc: Wait for a frame before returning from open()"

[Mahesh Kumar]

This reverts commit e8fa5671183c80342d520ad81d14fa79a9d4a680.

Don't wait for first CRC during crtc_crc_open. It avoids one frame wait
during open. If application want to wait after read call, it can use
poll/read blocking read() call.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/gpu/drm/drm_debugfs_crc.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
index 08dfe19b6286..7aeed89f934a 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -226,24 +226,8 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
 	if (ret)
 		goto err;
 
-	spin_lock_irq(&crc->lock);
-	/*
-	 * Only return once we got a first frame, so userspace doesn't have to
-	 * guess when this particular piece of HW will be ready to start
-	 * generating CRCs.
-	 */
-	ret = wait_event_interruptible_lock_irq(crc->wq,
-						crtc_crc_data_count(crc),
-						crc->lock);
-	spin_unlock_irq(&crc->lock);
-
-	if (ret)
-		goto err_disable;
-
 	return 0;
 
-err_disable:
-	crtc->funcs->set_crc_source(crtc, NULL);
 err:
 	spin_lock_irq(&crc->lock);
 	crtc_crc_cleanup(crc);
-- 
2.16.2

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

Re: [PATCH 02/10] drm: crc: Introduce pre_crc_read function

[Maarten Lankhorst]

Op 27-06-18 om 16:44 schreef Mahesh Kumar:
> This patch implements a callback function "pre_crc_read" which will
> be called before crc read. In this function driver can implement and
> preparation work required for successfully reading CRC data.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
>  include/drm/drm_crtc.h            | 14 ++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
> index c6a725b79ac6..2b4a737c5aeb 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -278,6 +278,14 @@ static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
>  		return 0;
>  	}
>  
> +	if (crtc->funcs->pre_crc_read) {
> +		ret = crtc->funcs->pre_crc_read(crtc);
> +		if (ret) {
> +			spin_unlock_irq(&crc->lock);
> +			return ret;
> +		}
> +	}
> +
>  	/* Nothing to read? */
>  	while (crtc_crc_data_count(crc) == 0) {
>  		if (filep->f_flags & O_NONBLOCK) {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 1a6dcbf91744..bae432469616 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -676,6 +676,20 @@ struct drm_crtc_funcs {
>  	 */
>  	int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
>  				 size_t *values_cnt);
> +	/**
> +	 * @pre_crc_read:
> +	 *
> +	 * Driver callback for performing any preparation work required by
> +	 * driver before reading CRC
> +	 *
> +	 * This callback is optional if the driver does not support CRC
> +	 * generation or no prework is required before reading the crc
> +	 *
> +	 * RETURNS:
> +	 *
> +	 * 0 on success or a negative error code on failure.
> +	 */
> +	int (*pre_crc_read)(struct drm_crtc *crtc);
>  
>  	/**
>  	 * @atomic_print_state:

I think this patch might have to be dropped, or reordered after the revert in 10/10, because else in theory open could block. :)

Or maybe just drop until we have upstream kernel users?

~Maarten

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/10] drm: crc: Introduce pre_crc_read function

[Kumar, Mahesh]

Hi,


On 6/27/2018 8:48 PM, Maarten Lankhorst wrote:
> Op 27-06-18 om 16:44 schreef Mahesh Kumar:
>> This patch implements a callback function "pre_crc_read" which will
>> be called before crc read. In this function driver can implement and
>> preparation work required for successfully reading CRC data.
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>>   drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
>>   include/drm/drm_crtc.h            | 14 ++++++++++++++
>>   2 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
>> index c6a725b79ac6..2b4a737c5aeb 100644
>> --- a/drivers/gpu/drm/drm_debugfs_crc.c
>> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
>> @@ -278,6 +278,14 @@ static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
>>   		return 0;
>>   	}
>>   
>> +	if (crtc->funcs->pre_crc_read) {
>> +		ret = crtc->funcs->pre_crc_read(crtc);
>> +		if (ret) {
>> +			spin_unlock_irq(&crc->lock);
>> +			return ret;
>> +		}
>> +	}
>> +
>>   	/* Nothing to read? */
>>   	while (crtc_crc_data_count(crc) == 0) {
>>   		if (filep->f_flags & O_NONBLOCK) {
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 1a6dcbf91744..bae432469616 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -676,6 +676,20 @@ struct drm_crtc_funcs {
>>   	 */
>>   	int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
>>   				 size_t *values_cnt);
>> +	/**
>> +	 * @pre_crc_read:
>> +	 *
>> +	 * Driver callback for performing any preparation work required by
>> +	 * driver before reading CRC
>> +	 *
>> +	 * This callback is optional if the driver does not support CRC
>> +	 * generation or no prework is required before reading the crc
>> +	 *
>> +	 * RETURNS:
>> +	 *
>> +	 * 0 on success or a negative error code on failure.
>> +	 */
>> +	int (*pre_crc_read)(struct drm_crtc *crtc);
>>   
>>   	/**
>>   	 * @atomic_print_state:
> I think this patch might have to be dropped, or reordered after the revert in 10/10, because else in theory open could block. :)
>
> Or maybe just drop until we have upstream kernel users?
thanks,
Can reorder it after patch-10 and if others also vote to drop it will 
take decision accordingly, atleast it should not block other patches :)

-Mahesh
>
> ~Maarten
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/10] Revert "drm: crc: Wait for a frame before returning from open()"

[Maarten Lankhorst]

Op 27-06-18 om 16:44 schreef Mahesh Kumar:
> This reverts commit e8fa5671183c80342d520ad81d14fa79a9d4a680.
>
> Don't wait for first CRC during crtc_crc_open. It avoids one frame wait
> during open. If application want to wait after read call, it can use
> poll/read blocking read() call.
>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
>  drivers/gpu/drm/drm_debugfs_crc.c | 16 ----------------
>  1 file changed, 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
> index 08dfe19b6286..7aeed89f934a 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -226,24 +226,8 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
>  	if (ret)
>  		goto err;
>  
> -	spin_lock_irq(&crc->lock);
> -	/*
> -	 * Only return once we got a first frame, so userspace doesn't have to
> -	 * guess when this particular piece of HW will be ready to start
> -	 * generating CRCs.
> -	 */
> -	ret = wait_event_interruptible_lock_irq(crc->wq,
> -						crtc_crc_data_count(crc),
> -						crc->lock);
> -	spin_unlock_irq(&crc->lock);
> -
> -	if (ret)
> -		goto err_disable;
> -
>  	return 0;
>  
> -err_disable:
> -	crtc->funcs->set_crc_source(crtc, NULL);
>  err:
>  	spin_lock_irq(&crc->lock);
>  	crtc_crc_cleanup(crc);

For the whole series except 2/10:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

✗ Fi.CI.BAT: failure for Improve crc-core driver interface (rev3)

[Patchwork]

== Series Details ==

Series: Improve crc-core driver interface (rev3)
URL   : https://patchwork.freedesktop.org/series/45246/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4388 -> Patchwork_9447 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9447 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9447, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45246/revisions/3/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9447:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_pipe_crc_basic@bad-source:
      fi-skl-6770hq:      PASS -> FAIL +6
      fi-bwr-2160:        PASS -> FAIL +4
      fi-hsw-4770r:       PASS -> FAIL +6
      fi-glk-j4005:       PASS -> FAIL
      fi-cfl-8700k:       PASS -> FAIL +6
      fi-cnl-psr:         PASS -> FAIL
      {fi-kbl-x1275}:     PASS -> FAIL
      fi-kbl-guc:         PASS -> FAIL
      fi-glk-dsi:         PASS -> FAIL
      fi-bsw-n3050:       PASS -> FAIL

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-ivb-3520m:       PASS -> FAIL +6
      fi-cfl-s3:          PASS -> FAIL +6
      fi-skl-6700hq:      PASS -> FAIL +6
      fi-skl-guc:         PASS -> FAIL +6
      fi-blb-e6850:       PASS -> FAIL +4
      fi-byt-j1900:       PASS -> FAIL +4

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-ilk-650:         PASS -> FAIL +4
      fi-elk-e7500:       PASS -> FAIL +4
      fi-byt-n2820:       PASS -> FAIL +4
      fi-snb-2520m:       PASS -> FAIL +4

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-bdw-5557u:       PASS -> FAIL +6
      fi-pnv-d510:        PASS -> FAIL +4
      fi-skl-6600u:       PASS -> FAIL +6
      fi-bxt-dsi:         PASS -> FAIL +6
      fi-hsw-4770:        PASS -> FAIL +6
      fi-cfl-guc:         PASS -> FAIL +6
      fi-ivb-3770:        PASS -> FAIL +6

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      fi-hsw-peppy:       PASS -> FAIL +6
      fi-bdw-gvtdvm:      PASS -> FAIL +6
      fi-gdg-551:         PASS -> FAIL +4
      fi-kbl-7500u:       PASS -> FAIL +6
      fi-snb-2600:        PASS -> FAIL +4

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
      fi-kbl-7567u:       PASS -> FAIL +6
      fi-skl-6260u:       PASS -> FAIL +6
      fi-skl-6700k2:      PASS -> FAIL +6
      fi-skl-gvtdvm:      PASS -> FAIL +6
      fi-bxt-j4205:       PASS -> FAIL +6
      fi-kbl-7560u:       PASS -> FAIL +6
      fi-whl-u:           PASS -> FAIL +6
      fi-kbl-r:           PASS -> FAIL +6

    
== Known issues ==

  Here are the changes found in Patchwork_9447 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      fi-cnl-psr:         PASS -> FAIL (fdo#106211) +5

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
      fi-glk-j4005:       PASS -> FAIL (fdo#106211) +5
      fi-bsw-n3050:       PASS -> FAIL (fdo#106211) +1

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c-frame-sequence:
      fi-glk-dsi:         PASS -> FAIL (fdo#106211) +5

    
    ==== Possible fixes ====

    igt@gem_exec_gttfill@basic:
      fi-byt-n2820:       FAIL (fdo#106744) -> PASS

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       FAIL (fdo#103841) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#106211 https://bugs.freedesktop.org/show_bug.cgi?id=106211
  fdo#106744 https://bugs.freedesktop.org/show_bug.cgi?id=106744


== Participating hosts (45 -> 40) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4388 -> Patchwork_9447

  CI_DRM_4388: b3654d40e9004f17bb612e71aee129347ea2c4aa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4530: 0e98bf69f146eb72fe3a7c3b19a049b5786f0ca3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9447: 904f779ffebbf617c4dcfc9d88c376b859f287fe @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

904f779ffebb Revert "drm: crc: Wait for a frame before returning from open()"
96aafbd70ef6 drm/crc: Cleanup crtc_crc_open function
942dc4f19471 drm/i915/crc: implement get_crc_sources callback
2c0afe823995 drm/i915/crc: implement verify_crc_source callback
6a5efb0f1291 drm/rcar-du/crc: Implement verify_crc_source callback
c3ae842570e2 drm/amdgpu_dm/crc: Implement verify_crc_source callback
7a6a81775e48 drm/rockchip/crc: Implement verify_crc_source callback
ba79e3af7743 drm: crc: Introduce get_crc_sources callback
3be8e4e07270 drm: crc: Introduce pre_crc_read function
6d6d59e691a8 drm: crc: Introduce verify_crc_source callback

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9447/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/10] drm: crc: Introduce pre_crc_read function

[Mahesh Kumar]

This patch implements a callback function "pre_crc_read" which will
be called before crc read. In this function driver can implement and
preparation work required for successfully reading CRC data.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
 include/drm/drm_crtc.h            | 14 ++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
index c6a725b79ac6..2b4a737c5aeb 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -278,6 +278,14 @@ static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
 		return 0;
 	}
 
+	if (crtc->funcs->pre_crc_read) {
+		ret = crtc->funcs->pre_crc_read(crtc);
+		if (ret) {
+			spin_unlock_irq(&crc->lock);
+			return ret;
+		}
+	}
+
 	/* Nothing to read? */
 	while (crtc_crc_data_count(crc) == 0) {
 		if (filep->f_flags & O_NONBLOCK) {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 1a6dcbf91744..bae432469616 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -676,6 +676,20 @@ struct drm_crtc_funcs {
 	 */
 	int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
 				 size_t *values_cnt);
+	/**
+	 * @pre_crc_read:
+	 *
+	 * Driver callback for performing any preparation work required by
+	 * driver before reading CRC
+	 *
+	 * This callback is optional if the driver does not support CRC
+	 * generation or no prework is required before reading the crc
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or a negative error code on failure.
+	 */
+	int (*pre_crc_read)(struct drm_crtc *crtc);
 
 	/**
 	 * @atomic_print_state:
-- 
2.16.2

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

[PATCH 02/10] drm: crc: Introduce pre_crc_read function

[Mahesh Kumar]

This patch implements a callback function "pre_crc_read" which will
be called before crc read. In this function driver can implement and
preparation work required for successfully reading CRC data.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
 include/drm/drm_crtc.h            | 14 ++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
index c6a725b79ac6..2b4a737c5aeb 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -278,6 +278,14 @@ static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
 		return 0;
 	}
 
+	if (crtc->funcs->pre_crc_read) {
+		ret = crtc->funcs->pre_crc_read(crtc);
+		if (ret) {
+			spin_unlock_irq(&crc->lock);
+			return ret;
+		}
+	}
+
 	/* Nothing to read? */
 	while (crtc_crc_data_count(crc) == 0) {
 		if (filep->f_flags & O_NONBLOCK) {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 1a6dcbf91744..bae432469616 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -676,6 +676,20 @@ struct drm_crtc_funcs {
 	 */
 	int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
 				 size_t *values_cnt);
+	/**
+	 * @pre_crc_read:
+	 *
+	 * Driver callback for performing any preparation work required by
+	 * driver before reading CRC
+	 *
+	 * This callback is optional if the driver does not support CRC
+	 * generation or no prework is required before reading the crc
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or a negative error code on failure.
+	 */
+	int (*pre_crc_read)(struct drm_crtc *crtc);
 
 	/**
 	 * @atomic_print_state:
-- 
2.16.2

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