[PATCH v5 0/3] Improve crc-core driver interface

[PATCH v5 0/3] 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 may need inputs

Changes:
- Rebase on top of VKMS driver

Cc: dri-devel@lists.freedesktop.org

Mahesh Kumar (3):
  drm/vkms/crc: Implement verify_crc_source 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.h  |  3 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c  |  4 +-
 drivers/gpu/drm/drm_debugfs_crc.c                  | 71 ++++++++--------------
 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             |  4 +-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c        |  6 +-
 drivers/gpu/drm/vkms/vkms_crc.c                    | 41 ++++++++++---
 drivers/gpu/drm/vkms/vkms_crtc.c                   |  1 +
 drivers/gpu/drm/vkms/vkms_drv.h                    |  5 +-
 include/drm/drm_crtc.h                             |  3 +-
 11 files changed, 71 insertions(+), 74 deletions(-)

-- 
2.16.2

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

[PATCH v1 1/3] drm/vkms/crc: Implement verify_crc_source callback

[Mahesh Kumar]

This patch implements "verify_crc_source" callback function for
Virtual KMS drm driver.

Cc: dri-devel@lists.freedesktop.org
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/vkms/vkms_crc.c  | 36 ++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/vkms/vkms_crtc.c |  1 +
 drivers/gpu/drm/vkms/vkms_drv.h  |  2 ++
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
index 37d717f38e3c..ce5ee0461f80 100644
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_crc.c
@@ -70,6 +70,35 @@ void vkms_crc_work_handle(struct work_struct *work)
 	drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, &crc32);
 }
 
+static int vkms_crc_parse_source(const char *src_name, bool *enabled)
+{
+	int ret = 0;
+
+	if (!src_name) {
+		*enabled = false;
+	} else if (strcmp(src_name, "auto") == 0) {
+		*enabled = true;
+	} else {
+		*enabled = false;
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
+			   size_t *values_cnt)
+{
+	bool enabled;
+
+	if (vkms_crc_parse_source(src_name, &enabled) < 0) {
+		DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 			size_t *values_cnt)
 {
@@ -78,10 +107,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 	unsigned long flags;
 	int ret = 0;
 
-	if (src_name && strcmp(src_name, "auto") == 0)
-		enabled = true;
-	else if (src_name)
-		ret = -EINVAL;
+	ret = vkms_crc_parse_source(src_name, &enabled);
+	if (ret)
+		return ret;
 
 	*values_cnt = 1;
 
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index bfe6e0312cc4..9d0b1a325a78 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
 	.enable_vblank		= vkms_enable_vblank,
 	.disable_vblank		= vkms_disable_vblank,
 	.set_crc_source		= vkms_set_crc_source,
+	.verify_crc_source	= vkms_verify_crc_source,
 };
 
 static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index f156c930366a..090c5e4f5544 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
 /* CRC Support */
 int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 			size_t *values_cnt);
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
+			   size_t *values_cnt);
 void vkms_crc_work_handle(struct work_struct *work);
 
 #endif /* _VKMS_DRV_H_ */
-- 
2.16.2

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

[PATCH v5 2/3] 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.

Changes since V1:
 - refactor code to use single spin lock
Changes since V2:
 - rebase
Changes since V3:
 - rebase on top of VKMS driver

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (V2)
Acked-by: Leo Li <sunpeng.li@amd.com> (V2)
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> (V3)
---
 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                  | 61 ++++++++++------------
 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             |  4 +-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c        |  6 +--
 drivers/gpu/drm/vkms/vkms_crc.c                    |  5 +-
 drivers/gpu/drm/vkms/vkms_drv.h                    |  3 +-
 include/drm/drm_crtc.h                             |  3 +-
 10 files changed, 39 insertions(+), 57 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 d7e626331eca..3e0a2cfaa35c 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -127,11 +127,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);
 
@@ -197,40 +195,40 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
 			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;
+
+	entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL);
+	if (!entries)
+		return -ENOMEM;
+
 	spin_lock_irq(&crc->lock);
-	if (!crc->opened)
+	if (!crc->opened) {
 		crc->opened = true;
-	else
+		crc->entries = entries;
+		crc->values_cnt = values_cnt;
+	} else {
 		ret = -EBUSY;
+	}
 	spin_unlock_irq(&crc->lock);
 
-	if (ret)
+	if (ret) {
+		kfree(entries);
 		return ret;
+	}
 
-	ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt);
+	ret = crtc->funcs->set_crc_source(crtc, crc->source);
 	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;
-	}
-
 	spin_lock_irq(&crc->lock);
-	crc->entries = entries;
-	crc->values_cnt = values_cnt;
-
 	/*
 	 * 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
@@ -247,7 +245,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);
@@ -259,9 +257,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);
@@ -367,7 +364,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 7b984aefce98..2cb8d8007a50 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2178,8 +2178,7 @@ void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
 
 /* intel_pipe_crc.c */
 #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 83f9ade0cd81..f3c9010e332a 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -583,8 +583,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];
@@ -623,7 +622,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 57db868da4fe..8a9e5e6f16b4 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -887,8 +887,7 @@ const char *const *rcar_du_crtc_get_crc_sources(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;
@@ -903,7 +902,6 @@ static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc,
 		return ret;
 
 	index = ret;
-	*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 c9a5ea38e86b..38f8cae7ef51 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1111,7 +1111,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;
@@ -1121,8 +1121,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)
@@ -1146,7 +1144,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/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
index ce5ee0461f80..44775434945f 100644
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_crc.c
@@ -99,8 +99,7 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
 	return 0;
 }
 
-int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
-			size_t *values_cnt)
+int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name)
 {
 	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
 	bool enabled = false;
@@ -111,8 +110,6 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 	if (ret)
 		return ret;
 
-	*values_cnt = 1;
-
 	/* make sure nothing is scheduled on crtc workq */
 	flush_workqueue(out->crc_workq);
 
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 090c5e4f5544..2017a2ccc43d 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -123,8 +123,7 @@ int vkms_gem_vmap(struct drm_gem_object *obj);
 void vkms_gem_vunmap(struct drm_gem_object *obj);
 
 /* CRC Support */
-int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
-			size_t *values_cnt);
+int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name);
 int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
 			   size_t *values_cnt);
 void vkms_crc_work_handle(struct work_struct *work);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f2dd180a867a..b21437bc95bf 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -744,8 +744,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 v4 3/3] 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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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 3e0a2cfaa35c..00e743153e94 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -228,24 +228,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

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

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

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4663 -> Patchwork_9931 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9931 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9931, 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/11/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-ivb-3520m:       PASS -> FAIL +5
      {fi-icl-u}:         PASS -> FAIL +5
      fi-skl-6700hq:      PASS -> FAIL +5
      fi-skl-guc:         PASS -> FAIL +5
      fi-blb-e6850:       PASS -> FAIL +3
      fi-byt-j1900:       PASS -> FAIL +3

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

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-bdw-5557u:       PASS -> FAIL +5
      fi-pnv-d510:        PASS -> FAIL +3
      fi-skl-6600u:       PASS -> FAIL +5
      {fi-byt-clapper}:   PASS -> FAIL +3
      fi-bxt-dsi:         PASS -> FAIL +5
      fi-hsw-4770:        PASS -> FAIL +5
      fi-ivb-3770:        PASS -> FAIL +5
      fi-bwr-2160:        PASS -> FAIL +3

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      {fi-bdw-samus}:     PASS -> FAIL +5
      fi-hsw-peppy:       PASS -> FAIL +5
      fi-bdw-gvtdvm:      PASS -> FAIL +5
      fi-gdg-551:         NOTRUN -> FAIL +3
      fi-kbl-7500u:       PASS -> FAIL +5
      fi-snb-2600:        PASS -> FAIL +3
      fi-hsw-4770r:       PASS -> FAIL +5

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
      fi-kbl-7567u:       PASS -> FAIL +5
      fi-skl-6260u:       PASS -> FAIL +5
      fi-skl-6700k2:      PASS -> FAIL +5
      fi-skl-gvtdvm:      PASS -> FAIL +5
      {fi-skl-iommu}:     PASS -> FAIL +5
      fi-skl-6770hq:      PASS -> FAIL +5
      fi-bxt-j4205:       PASS -> FAIL +5
      fi-kbl-7560u:       PASS -> FAIL +5
      fi-whl-u:           PASS -> FAIL +5
      fi-kbl-r:           PASS -> FAIL +5

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@drv_selftest@live_hangcheck:
      fi-cfl-s3:          PASS -> DMESG-FAIL (fdo#106560)

    igt@drv_selftest@live_workarounds:
      fi-skl-6700k2:      PASS -> DMESG-FAIL (fdo#107292)

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
      fi-skl-guc:         PASS -> FAIL (fdo#103191)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-cfl-s3:          PASS -> FAIL (fdo#103481) +5

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      {fi-bsw-kefka}:     PASS -> FAIL (fdo#106211) +3
      {fi-cfl-8109u}:     PASS -> FAIL (fdo#103481) +5
      fi-cfl-guc:         PASS -> FAIL (fdo#103481) +5

    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-cfl-8700k:       PASS -> FAIL (fdo#103481) +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

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103191, fdo#107362)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s3:
      {fi-cfl-8109u}:     FAIL (fdo#103375) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-WARN (fdo#107372) -> DMESG-FAIL (fdo#107372)

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

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106211 https://bugs.freedesktop.org/show_bug.cgi?id=106211
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (53 -> 49) ==

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


== Build changes ==

    * Linux: CI_DRM_4663 -> Patchwork_9931

  CI_DRM_4663: 273ade7fd922e8074e5832fe5e014720aecba57c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4593: c88e219c6e890d89b7836c5e248ffedf334d55a2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9931: ecc1dc0d45ff4243c4c17e21cba1d5d15e317d92 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ecc1dc0d45ff Revert "drm: crc: Wait for a frame before returning from open()"
549435be9abe drm/crc: Cleanup crtc_crc_open function
8e68723ad195 drm/vkms/crc: Implement verify_crc_source callback

== Logs ==

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

[PATCH V2 1/3] drm/vkms/crc: Implement verify_crc_source callback

[Mahesh Kumar]

This patch implements "verify_crc_source" callback function for
Virtual KMS drm driver.

Changes Since V1:
- update values_cnt in verify_crc_source

Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/vkms/vkms_crc.c  | 38 ++++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/vkms/vkms_crtc.c |  1 +
 drivers/gpu/drm/vkms/vkms_drv.h  |  2 ++
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
index 37d717f38e3c..b2a484b4e2ad 100644
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_crc.c
@@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
 	drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, &crc32);
 }
 
+static int vkms_crc_parse_source(const char *src_name, bool *enabled)
+{
+	int ret = 0;
+
+	if (!src_name) {
+		*enabled = false;
+	} else if (strcmp(src_name, "auto") == 0) {
+		*enabled = true;
+	} else {
+		*enabled = false;
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
+			   size_t *values_cnt)
+{
+	bool enabled;
+
+	if (vkms_crc_parse_source(src_name, &enabled) < 0) {
+		DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
+		return -EINVAL;
+	}
+
+	*values_cnt = 1;
+
+	return 0;
+}
+
 int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 			size_t *values_cnt)
 {
@@ -78,10 +109,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 	unsigned long flags;
 	int ret = 0;
 
-	if (src_name && strcmp(src_name, "auto") == 0)
-		enabled = true;
-	else if (src_name)
-		ret = -EINVAL;
+	ret = vkms_crc_parse_source(src_name, &enabled);
+	if (ret)
+		return ret;
 
 	*values_cnt = 1;
 
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index bfe6e0312cc4..9d0b1a325a78 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
 	.enable_vblank		= vkms_enable_vblank,
 	.disable_vblank		= vkms_disable_vblank,
 	.set_crc_source		= vkms_set_crc_source,
+	.verify_crc_source	= vkms_verify_crc_source,
 };
 
 static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index f156c930366a..090c5e4f5544 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
 /* CRC Support */
 int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 			size_t *values_cnt);
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
+			   size_t *values_cnt);
 void vkms_crc_work_handle(struct work_struct *work);
 
 #endif /* _VKMS_DRV_H_ */
-- 
2.16.2

_______________________________________________
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 (rev12)

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4664 -> Patchwork_9932 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9932 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9932, 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/12/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

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

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

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-bdw-5557u:       PASS -> FAIL +5
      fi-pnv-d510:        PASS -> FAIL +3
      fi-skl-6600u:       PASS -> FAIL +5
      {fi-byt-clapper}:   PASS -> FAIL +3
      fi-bxt-dsi:         PASS -> FAIL +5
      fi-hsw-4770:        PASS -> FAIL +5
      fi-ivb-3770:        PASS -> FAIL +5
      fi-bwr-2160:        PASS -> FAIL +3

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      {fi-bdw-samus}:     PASS -> FAIL +5
      fi-hsw-peppy:       PASS -> FAIL +5
      fi-bdw-gvtdvm:      PASS -> FAIL +5
      fi-gdg-551:         PASS -> FAIL +3
      fi-kbl-7500u:       PASS -> FAIL +5
      fi-snb-2600:        PASS -> FAIL +3
      fi-hsw-4770r:       PASS -> FAIL +5

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
      fi-kbl-7567u:       PASS -> FAIL +5
      fi-skl-6260u:       PASS -> FAIL +5
      fi-skl-6700k2:      PASS -> FAIL +5
      fi-skl-gvtdvm:      PASS -> FAIL +5
      {fi-skl-iommu}:     PASS -> FAIL +5
      fi-skl-6770hq:      PASS -> FAIL +5
      fi-bxt-j4205:       PASS -> FAIL +5
      fi-kbl-7560u:       PASS -> FAIL +5
      fi-whl-u:           PASS -> FAIL +5
      fi-kbl-r:           PASS -> FAIL +5

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-skl-6600u:       PASS -> DMESG-FAIL (fdo#106560, fdo#107174)

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-cfl-s3:          PASS -> FAIL (fdo#103481) +5

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      {fi-bsw-kefka}:     PASS -> FAIL (fdo#106211) +3
      {fi-cfl-8109u}:     PASS -> FAIL (fdo#103481) +5
      fi-cfl-guc:         PASS -> FAIL (fdo#103481) +5

    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-cfl-8700k:       PASS -> FAIL (fdo#103481) +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

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         DMESG-FAIL (fdo#106685, fdo#107174) -> PASS

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#107139, fdo#105128) -> PASS

    igt@kms_chamelium@dp-crc-fast:
      fi-kbl-7500u:       DMESG-FAIL (fdo#103841) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

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

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#106211 https://bugs.freedesktop.org/show_bug.cgi?id=106211
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106685 https://bugs.freedesktop.org/show_bug.cgi?id=106685
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (54 -> 48) ==

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


== Build changes ==

    * Linux: CI_DRM_4664 -> Patchwork_9932

  CI_DRM_4664: 19e458884fe1d8d10e453529933199250cc8821f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4593: c88e219c6e890d89b7836c5e248ffedf334d55a2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9932: edc809370c62169b8cfaeee5a675f8c28e81f71b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

edc809370c62 Revert "drm: crc: Wait for a frame before returning from open()"
664027cdd442 drm/crc: Cleanup crtc_crc_open function
c1a160cb7674 drm/vkms/crc: Implement verify_crc_source callback

== Logs ==

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

Re: [PATCH V2 1/3] drm/vkms/crc: Implement verify_crc_source callback

[Haneen Mohammed]

On Tue, Aug 14, 2018 at 08:31:03AM +0530, Mahesh Kumar wrote:
> This patch implements "verify_crc_source" callback function for
> Virtual KMS drm driver.
> 
> Changes Since V1:
> - update values_cnt in verify_crc_source
> 
> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/vkms/vkms_crc.c  | 38 ++++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/vkms/vkms_crtc.c |  1 +
>  drivers/gpu/drm/vkms/vkms_drv.h  |  2 ++
>  3 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 37d717f38e3c..b2a484b4e2ad 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
>  	drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, &crc32);
>  }
>  
> +static int vkms_crc_parse_source(const char *src_name, bool *enabled)
> +{
> +	int ret = 0;
> +
> +	if (!src_name) {
> +		*enabled = false;
> +	} else if (strcmp(src_name, "auto") == 0) {
> +		*enabled = true;
> +	} else {
> +		*enabled = false;
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
> +			   size_t *values_cnt)
> +{
> +	bool enabled;
> +
> +	if (vkms_crc_parse_source(src_name, &enabled) < 0) {
> +		DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
> +		return -EINVAL;
> +	}
> +
> +	*values_cnt = 1;
> +
> +	return 0;
> +}
> +
>  int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>  			size_t *values_cnt)
>  {
> @@ -78,10 +109,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>  	unsigned long flags;
>  	int ret = 0;
>  
> -	if (src_name && strcmp(src_name, "auto") == 0)
> -		enabled = true;
> -	else if (src_name)
> -		ret = -EINVAL;
> +	ret = vkms_crc_parse_source(src_name, &enabled);
> +	if (ret)
> +		return ret;

I think the return value after vkms_crc_parse_source should be called
once instead at the end of vkms_set_crc_source otherwise crc_enabled
won't be updated?

- Haneen

>  
>  	*values_cnt = 1;
>  
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index bfe6e0312cc4..9d0b1a325a78 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
>  	.enable_vblank		= vkms_enable_vblank,
>  	.disable_vblank		= vkms_disable_vblank,
>  	.set_crc_source		= vkms_set_crc_source,
> +	.verify_crc_source	= vkms_verify_crc_source,
>  };
>  
>  static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index f156c930366a..090c5e4f5544 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
>  /* CRC Support */
>  int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>  			size_t *values_cnt);
> +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
> +			   size_t *values_cnt);
>  void vkms_crc_work_handle(struct work_struct *work);
>  
>  #endif /* _VKMS_DRV_H_ */
> -- 
> 2.16.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH V2 1/3] drm/vkms/crc: Implement verify_crc_source callback

[Kumar, Mahesh]

Hi Haneen,


On 8/21/2018 4:09 AM, Haneen Mohammed wrote:
> On Tue, Aug 14, 2018 at 08:31:03AM +0530, Mahesh Kumar wrote:
>> This patch implements "verify_crc_source" callback function for
>> Virtual KMS drm driver.
>>
>> Changes Since V1:
>> - update values_cnt in verify_crc_source
>>
>> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/vkms/vkms_crc.c  | 38 ++++++++++++++++++++++++++++++++++----
>>   drivers/gpu/drm/vkms/vkms_crtc.c |  1 +
>>   drivers/gpu/drm/vkms/vkms_drv.h  |  2 ++
>>   3 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
>> index 37d717f38e3c..b2a484b4e2ad 100644
>> --- a/drivers/gpu/drm/vkms/vkms_crc.c
>> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
>> @@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
>>   	drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, &crc32);
>>   }
>>   
>> +static int vkms_crc_parse_source(const char *src_name, bool *enabled)
>> +{
>> +	int ret = 0;
>> +
>> +	if (!src_name) {
>> +		*enabled = false;
>> +	} else if (strcmp(src_name, "auto") == 0) {
>> +		*enabled = true;
>> +	} else {
>> +		*enabled = false;
>> +		ret = -EINVAL;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
>> +			   size_t *values_cnt)
>> +{
>> +	bool enabled;
>> +
>> +	if (vkms_crc_parse_source(src_name, &enabled) < 0) {
>> +		DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
>> +		return -EINVAL;
>> +	}
>> +
>> +	*values_cnt = 1;
>> +
>> +	return 0;
>> +}
>> +
>>   int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>>   			size_t *values_cnt)
>>   {
>> @@ -78,10 +109,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>>   	unsigned long flags;
>>   	int ret = 0;
>>   
>> -	if (src_name && strcmp(src_name, "auto") == 0)
>> -		enabled = true;
>> -	else if (src_name)
>> -		ret = -EINVAL;
>> +	ret = vkms_crc_parse_source(src_name, &enabled);
>> +	if (ret)
>> +		return ret;
> I think the return value after vkms_crc_parse_source should be called
> once instead at the end of vkms_set_crc_source otherwise crc_enabled
> won't be updated?
Here I'm assuming if src_name is wrong we don't need to proceed further, 
let the CRC generation to continue whatever it was doing (enabled or 
disabled).
But anyway that is changing the original design, will remove above 
return to keep the behavior same.

-Mahesh
>
> - Haneen
>
>>   
>>   	*values_cnt = 1;
>>   
>> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
>> index bfe6e0312cc4..9d0b1a325a78 100644
>> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
>> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
>> @@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
>>   	.enable_vblank		= vkms_enable_vblank,
>>   	.disable_vblank		= vkms_disable_vblank,
>>   	.set_crc_source		= vkms_set_crc_source,
>> +	.verify_crc_source	= vkms_verify_crc_source,
>>   };
>>   
>>   static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
>> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
>> index f156c930366a..090c5e4f5544 100644
>> --- a/drivers/gpu/drm/vkms/vkms_drv.h
>> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
>> @@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
>>   /* CRC Support */
>>   int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>>   			size_t *values_cnt);
>> +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
>> +			   size_t *values_cnt);
>>   void vkms_crc_work_handle(struct work_struct *work);
>>   
>>   #endif /* _VKMS_DRV_H_ */
>> -- 
>> 2.16.2
>>

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

[PATCH V3 1/3] drm/vkms/crc: Implement verify_crc_source callback

[Mahesh Kumar]

This patch implements "verify_crc_source" callback function for
Virtual KMS drm driver.

Changes Since V1:
- update values_cnt in verify_crc_source
Changes Since V2:
- don't return early from set_crc_source to keep behavior same (Haneen)

Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/vkms/vkms_crc.c  | 36 ++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/vkms/vkms_crtc.c |  1 +
 drivers/gpu/drm/vkms/vkms_drv.h  |  2 ++
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
index 37d717f38e3c..68421d3d809a 100644
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_crc.c
@@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
 	drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, &crc32);
 }
 
+static int vkms_crc_parse_source(const char *src_name, bool *enabled)
+{
+	int ret = 0;
+
+	if (!src_name) {
+		*enabled = false;
+	} else if (strcmp(src_name, "auto") == 0) {
+		*enabled = true;
+	} else {
+		*enabled = false;
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
+			   size_t *values_cnt)
+{
+	bool enabled;
+
+	if (vkms_crc_parse_source(src_name, &enabled) < 0) {
+		DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
+		return -EINVAL;
+	}
+
+	*values_cnt = 1;
+
+	return 0;
+}
+
 int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 			size_t *values_cnt)
 {
@@ -78,10 +109,7 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 	unsigned long flags;
 	int ret = 0;
 
-	if (src_name && strcmp(src_name, "auto") == 0)
-		enabled = true;
-	else if (src_name)
-		ret = -EINVAL;
+	ret = vkms_crc_parse_source(src_name, &enabled);
 
 	*values_cnt = 1;
 
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index bfe6e0312cc4..9d0b1a325a78 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
 	.enable_vblank		= vkms_enable_vblank,
 	.disable_vblank		= vkms_disable_vblank,
 	.set_crc_source		= vkms_set_crc_source,
+	.verify_crc_source	= vkms_verify_crc_source,
 };
 
 static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index f156c930366a..090c5e4f5544 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
 /* CRC Support */
 int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 			size_t *values_cnt);
+int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
+			   size_t *values_cnt);
 void vkms_crc_work_handle(struct work_struct *work);
 
 #endif /* _VKMS_DRV_H_ */
-- 
2.16.2

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

✓ Fi.CI.BAT: success for Improve crc-core driver interface (rev13)

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4692 -> Patchwork_9980 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7560u:       PASS -> DMESG-FAIL (fdo#106560, fdo#106947)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-FAIL (fdo#102614)
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103167)

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

  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947


== Participating hosts (51 -> 47) ==

  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4692 -> Patchwork_9980

  CI_DRM_4692: d53f119472fc7daa532e46ea77098e9e9db2ac10 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9980: 70f9435337d30e26426ebaa311efb29f7d52051a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

70f9435337d3 Revert "drm: crc: Wait for a frame before returning from open()"
314b22dfdb36 drm/crc: Cleanup crtc_crc_open function
b30e97d93e8d drm/vkms/crc: Implement verify_crc_source callback

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9980/issues.html
_______________________________________________
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 (rev13)

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4692 -> Patchwork_9981 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9981 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9981, 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/13/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_hangcheck:
      fi-cfl-guc:         PASS -> DMESG-FAIL

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_guc:
      fi-skl-guc:         NOTRUN -> DMESG-WARN (fdo#107258, fdo#107175)

    igt@drv_selftest@live_hangcheck:
      fi-bdw-5557u:       PASS -> DMESG-FAIL (fdo#106560)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#107175 https://bugs.freedesktop.org/show_bug.cgi?id=107175
  fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258


== Participating hosts (51 -> 48) ==

  Additional (1): fi-skl-guc 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4692 -> Patchwork_9981

  CI_DRM_4692: d53f119472fc7daa532e46ea77098e9e9db2ac10 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9981: 16455237058ae7e10b0fdee464c5e49e5164df75 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

16455237058a Revert "drm: crc: Wait for a frame before returning from open()"
7a546236475c drm/crc: Cleanup crtc_crc_open function
1fa679413c80 drm/vkms/crc: Implement verify_crc_source callback

== Logs ==

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

✓ Fi.CI.IGT: success for Improve crc-core driver interface (rev13)

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4692_full -> Patchwork_9980_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_await@wide-contexts:
      shard-apl:          PASS -> FAIL (fdo#106680, fdo#105900)

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          PASS -> INCOMPLETE (fdo#106023, fdo#103665)

    igt@gem_workarounds@suspend-resume:
      shard-glk:          PASS -> FAIL (fdo#103375)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-glk:          FAIL (fdo#106886) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4692 -> Patchwork_9980

  CI_DRM_4692: d53f119472fc7daa532e46ea77098e9e9db2ac10 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9980: 70f9435337d30e26426ebaa311efb29f7d52051a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

✓ Fi.CI.BAT: success for Improve crc-core driver interface (rev13)

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4692 -> Patchwork_9982 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         PASS -> DMESG-FAIL (fdo#107164)

    igt@drv_selftest@live_guc:
      fi-skl-guc:         NOTRUN -> DMESG-WARN (fdo#107175, fdo#107258)

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

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

  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107175 https://bugs.freedesktop.org/show_bug.cgi?id=107175
  fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (51 -> 48) ==

  Additional (1): fi-skl-guc 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4692 -> Patchwork_9982

  CI_DRM_4692: d53f119472fc7daa532e46ea77098e9e9db2ac10 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9982: 9965c67bd8260b6b8ef1fd7fa7d0ef50ac4c68b2 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9965c67bd826 Revert "drm: crc: Wait for a frame before returning from open()"
ec08b989fbec drm/crc: Cleanup crtc_crc_open function
4152f023b866 drm/vkms/crc: Implement verify_crc_source callback

== Logs ==

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

Re: [PATCH V3 1/3] drm/vkms/crc: Implement verify_crc_source callback

[Haneen Mohammed]

On Tue, Aug 21, 2018 at 10:04:01AM +0530, Mahesh Kumar wrote:
> This patch implements "verify_crc_source" callback function for
> Virtual KMS drm driver.
> 
> Changes Since V1:
> - update values_cnt in verify_crc_source
> Changes Since V2:
> - don't return early from set_crc_source to keep behavior same (Haneen)
> 
> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---

Reviewed-by: Haneen Mohammed <hamohammed.sa@gmail.com>

>  drivers/gpu/drm/vkms/vkms_crc.c  | 36 ++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/vkms/vkms_crtc.c |  1 +
>  drivers/gpu/drm/vkms/vkms_drv.h  |  2 ++
>  3 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 37d717f38e3c..68421d3d809a 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
>  	drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, &crc32);
>  }
>  
> +static int vkms_crc_parse_source(const char *src_name, bool *enabled)
> +{
> +	int ret = 0;
> +
> +	if (!src_name) {
> +		*enabled = false;
> +	} else if (strcmp(src_name, "auto") == 0) {
> +		*enabled = true;
> +	} else {
> +		*enabled = false;
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
> +			   size_t *values_cnt)
> +{
> +	bool enabled;
> +
> +	if (vkms_crc_parse_source(src_name, &enabled) < 0) {
> +		DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
> +		return -EINVAL;
> +	}
> +
> +	*values_cnt = 1;
> +
> +	return 0;
> +}
> +
>  int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>  			size_t *values_cnt)
>  {
> @@ -78,10 +109,7 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>  	unsigned long flags;
>  	int ret = 0;
>  
> -	if (src_name && strcmp(src_name, "auto") == 0)
> -		enabled = true;
> -	else if (src_name)
> -		ret = -EINVAL;
> +	ret = vkms_crc_parse_source(src_name, &enabled);
>  
>  	*values_cnt = 1;
>  
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index bfe6e0312cc4..9d0b1a325a78 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
>  	.enable_vblank		= vkms_enable_vblank,
>  	.disable_vblank		= vkms_disable_vblank,
>  	.set_crc_source		= vkms_set_crc_source,
> +	.verify_crc_source	= vkms_verify_crc_source,
>  };
>  
>  static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index f156c930366a..090c5e4f5544 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
>  /* CRC Support */
>  int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>  			size_t *values_cnt);
> +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
> +			   size_t *values_cnt);
>  void vkms_crc_work_handle(struct work_struct *work);
>  
>  #endif /* _VKMS_DRV_H_ */
> -- 
> 2.16.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH V2 1/3] drm/vkms/crc: Implement verify_crc_source callback

[Haneen Mohammed]

On Tue, Aug 21, 2018 at 09:53:11AM +0530, Kumar, Mahesh wrote:
> Hi Haneen,
> 
> 
> On 8/21/2018 4:09 AM, Haneen Mohammed wrote:
> > On Tue, Aug 14, 2018 at 08:31:03AM +0530, Mahesh Kumar wrote:
> > > This patch implements "verify_crc_source" callback function for
> > > Virtual KMS drm driver.
> > > 
> > > Changes Since V1:
> > > - update values_cnt in verify_crc_source
> > > 
> > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> > > ---
> > >   drivers/gpu/drm/vkms/vkms_crc.c  | 38 ++++++++++++++++++++++++++++++++++----
> > >   drivers/gpu/drm/vkms/vkms_crtc.c |  1 +
> > >   drivers/gpu/drm/vkms/vkms_drv.h  |  2 ++
> > >   3 files changed, 37 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> > > index 37d717f38e3c..b2a484b4e2ad 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_crc.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> > > @@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
> > >   	drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, &crc32);
> > >   }
> > > +static int vkms_crc_parse_source(const char *src_name, bool *enabled)
> > > +{
> > > +	int ret = 0;
> > > +
> > > +	if (!src_name) {
> > > +		*enabled = false;
> > > +	} else if (strcmp(src_name, "auto") == 0) {
> > > +		*enabled = true;
> > > +	} else {
> > > +		*enabled = false;
> > > +		ret = -EINVAL;
> > > +	}
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
> > > +			   size_t *values_cnt)
> > > +{
> > > +	bool enabled;
> > > +
> > > +	if (vkms_crc_parse_source(src_name, &enabled) < 0) {
> > > +		DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	*values_cnt = 1;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
> > >   			size_t *values_cnt)
> > >   {
> > > @@ -78,10 +109,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
> > >   	unsigned long flags;
> > >   	int ret = 0;
> > > -	if (src_name && strcmp(src_name, "auto") == 0)
> > > -		enabled = true;
> > > -	else if (src_name)
> > > -		ret = -EINVAL;
> > > +	ret = vkms_crc_parse_source(src_name, &enabled);
> > > +	if (ret)
> > > +		return ret;
> > I think the return value after vkms_crc_parse_source should be called
> > once instead at the end of vkms_set_crc_source otherwise crc_enabled
> > won't be updated?
> Here I'm assuming if src_name is wrong we don't need to proceed further, let
> the CRC generation to continue whatever it was doing (enabled or disabled).
> But anyway that is changing the original design, will remove above return to
> keep the behavior same.
> 
> -Mahesh

hm I'm not really sure if my approach was the right way, but if it wasn't
then I think maybe this change would be better if it was either in a separate patch or
here with more details

Thank you,
Haneen

> > 
> > - Haneen
> > 
> > >   	*values_cnt = 1;
> > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > index bfe6e0312cc4..9d0b1a325a78 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > @@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
> > >   	.enable_vblank		= vkms_enable_vblank,
> > >   	.disable_vblank		= vkms_disable_vblank,
> > >   	.set_crc_source		= vkms_set_crc_source,
> > > +	.verify_crc_source	= vkms_verify_crc_source,
> > >   };
> > >   static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > > index f156c930366a..090c5e4f5544 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > > @@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
> > >   /* CRC Support */
> > >   int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
> > >   			size_t *values_cnt);
> > > +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
> > > +			   size_t *values_cnt);
> > >   void vkms_crc_work_handle(struct work_struct *work);
> > >   #endif /* _VKMS_DRV_H_ */
> > > -- 
> > > 2.16.2
> > > 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Improve crc-core driver interface (rev13)

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4692_full -> Patchwork_9982_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_workarounds@suspend-resume:
      shard-glk:          PASS -> FAIL (fdo#103375)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-glk:          FAIL (fdo#106886) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-wc:
      shard-glk:          FAIL (fdo#103167) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4692 -> Patchwork_9982

  CI_DRM_4692: d53f119472fc7daa532e46ea77098e9e9db2ac10 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4607: 6e0b3e7a2d241af36f8c6b1cc335aa1db3532d29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9982: 9965c67bd8260b6b8ef1fd7fa7d0ef50ac4c68b2 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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