Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:20 EEST Mahesh Kumar wrote:

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.

Why do you need to verify the source in the open() operation ? Isn't it enough to verify it in the write() handler ? The answer to this question might lie in patch 08/10, in which case I'd add the .verify_crc_source() call in open() in that patch, not here.

Signed-off-by: Mahesh Kumar mahesh1.kumar@intel.com Cc: dri-devel@lists.freedesktop.org Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

---

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:

Hi Mahesh,

On Tuesday, 10 July 2018 14:54:11 EEST Kumar, Mahesh wrote:

On 7/10/2018 4:56 PM, Laurent Pinchart wrote:

...

On Monday, 2 July 2018 14:07:20 EEST Mahesh Kumar wrote:

...

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.

Why do you need to verify the source in the open() operation ? Isn't it enough to verify it in the write() handler ? The answer to this question might lie in patch 08/10, in which case I'd add the .verify_crc_source() call in open() in that patch, not here.

Yes, final goal is achieved by patch 08/10. But if crc_read is called before setting the source, it may have wrong source set in that case I wanted to return early at least for the drivers implemented verify_crc_source. If you still think otherwise I will modify accordingly in next series.

I don't disagree with you, but I don't think this issue is new. As I got a bit confused as to why the call is needed in open() (there's no explanation in this patch), I think fixing the problem in 08/10 would be better.

...

...

Signed-off-by: Mahesh Kumar mahesh1.kumar@intel.com Cc: dri-devel@lists.freedesktop.org Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

---

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:

Hi,

thanks for the review. On 7/10/2018 4:52 PM, Laurent Pinchart wrote:

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:21 EEST Mahesh Kumar wrote:

...

This patch introduce 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.

The commit message seems to be outdated, the callback doesn't take a seq_file anymore.

ops, will update.

...

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 Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

---

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 c6a725b79ac6..f4d76528d24c 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;

Count it only used within the if () {} block, you can declare it there.

agree.

...

• if (crtc->funcs->get_crc_sources) {

• const char *const *sources = crtc->funcs->get_crc_sources(crtc,
  

• 							&count);
  

• size_t values_cnt;
  

• int i;
  

I only takes positive values, it can be an unsigned int.

ok

...

• if (count <= 0 || !sources)
  

count is a size_t, it can't be negative.

The .get_crc_sources() documentation doesn't clearly specify whether sources should always be NULL when count is zero. I advise updating the documentation, and possibly updating this check accordingly.

ok will update.

...

• 	goto out;
  

• seq_puts(m, "[");
  

• for (i = 0; i < count; i++)
  

• 	if (!crtc->funcs->verify_crc_source(crtc, sources[i],
  

• 					    &values_cnt))
  

I assume that you verify sources one by one here to avoid having to create a list of sources dynamically in the .get_crc_sources() callback ? If so, I think the .get_crc_sources() callback should document that.

You should also document that .verify_crc_source() is required when get_crc_sources() is provided.

ok sure.

...

• 		seq_printf(m, "%s ", sources[i]);
  

• seq_puts(m, "] ");
  

This assumes that source names can't include a space. Isn't that too restrictive ? Shouldn't a different separator be used ? How about one source name per line ?

what about comma separated as I'm putting names inside square-brackets?

Additionally, shouldn't the active source be marked ?

active source is again printed by the code in next few lines. output will be of following format. [space separated list of valid sources] active_source

-Mahesh

...

• }

+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 1a6dcbf91744..ffaec138aeee 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -676,6 +676,22 @@ struct drm_crtc_funcs { */ int (*verify_crc_source)(struct drm_crtc *crtc, const char *source, size_t *values_cnt);

• /**

• * @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:

Hi,

On 7/10/2018 5:39 PM, Laurent Pinchart wrote:

Hi Mahesh,

On Tuesday, 10 July 2018 15:01:38 EEST Kumar, Mahesh wrote:

...

On 7/10/2018 4:52 PM, Laurent Pinchart wrote:

...

Hi Mahesh, On Monday, 2 July 2018 14:07:21 EEST Mahesh Kumar wrote:

...

This patch introduce 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.

The commit message seems to be outdated, the callback doesn't take a seq_file anymore.

ops, will update.

...

...

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 Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

---

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 c6a725b79ac6..f4d76528d24c 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;

Count it only used within the if () {} block, you can declare it there.

agree.

...

...

• if (crtc->funcs->get_crc_sources) {

• const char *const *sources = crtc->funcs->get_crc_sources(crtc,
  

• 							&count);
  

• size_t values_cnt;
  

• int i;
  

I only takes positive values, it can be an unsigned int.

ok

...

...

• if (count <= 0 || !sources)
  

count is a size_t, it can't be negative.

The .get_crc_sources() documentation doesn't clearly specify whether sources should always be NULL when count is zero. I advise updating the documentation, and possibly updating this check accordingly.

ok will update.

...

...

• 	goto out;
  

• seq_puts(m, "[");
  

• for (i = 0; i < count; i++)
  

• 	if (!crtc->funcs->verify_crc_source(crtc, sources[i],
  

• 					    &values_cnt))
  

I assume that you verify sources one by one here to avoid having to create a list of sources dynamically in the .get_crc_sources() callback ? If so, I think the .get_crc_sources() callback should document that.

You should also document that .verify_crc_source() is required when get_crc_sources() is provided.

ok sure.

...

...

• 		seq_printf(m, "%s ", sources[i]);
  

• seq_puts(m, "] ");
  

This assumes that source names can't include a space. Isn't that too restrictive ? Shouldn't a different separator be used ? How about one source name per line ?

what about comma separated as I'm putting names inside square-brackets?

...

Additionally, shouldn't the active source be marked ?

active source is again printed by the code in next few lines. output will be of following format. [space separated list of valid sources] active_source

I had missed that, my bad.

The proposed format seems a bit hackish to me, in the sense that it forbids both spaces and brackets in source names. One source per line would fix both and be easy to parse. We would then need to mark the active source, which could be done by adding a marker to the corresponding line (maybe a * at the end of the line ?).

sounds good, will do that. -Mahesh

...

...

...

• }

+out: seq_printf(m, "%s\n", crtc->crc.source);

• return 0; }

[snip]

Op 02-07-18 om 13:07 schreef 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 Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

---

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)

Ack for merging this and 8/10 through drm-misc?

On 2018-07-03 06:19 AM, Maarten Lankhorst wrote:

Op 02-07-18 om 13:07 schreef 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 Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

Acked-by: Leo Li sunpeng.li@amd.com

...

---

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 66bd3cc3e387..dd97cbca0527 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) {

Ack for merging this and 8/10 through drm-misc?

---

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

Op 02-07-18 om 13:07 schreef 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 Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

---

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,

};

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

Ack for merging this and 8/10 through drm-misc?

Hi,

thanks foe the review.

On 7/10/2018 5:07 PM, Laurent Pinchart wrote:

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:24 EEST Mahesh Kumar wrote:

...

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

Signed-off-by: Mahesh Kumar mahesh1.kumar@intel.com Cc: dri-devel@lists.freedesktop.org Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

---

drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 40 +++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 15dc9caa128b..24eeaa7e14d7 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c @@ -756,6 +756,45 @@ static void rcar_du_crtc_disable_vblank(struct drm_crtc *crtc) rcrtc->vblank_enable = false; }

+static int rcar_du_crtc_verify_crc_source(struct drm_crtc *crtc,

• 			  const char *source_name,
  

• 			  size_t *values_cnt)
  

+{

• struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
• unsigned int index = 0;
• unsigned int i;
• int ret;
• /*

• * Parse the source name. Supported values are "plane%u" to compute the
  

• * CRC on an input plane (%u is the plane ID), and "auto" to compute the
  

• * CRC on the composer (VSP) output.
  

• */
  

• if (!source_name || !strcmp(source_name, "auto")) {

• goto out;
  

• } else if (strstarts(source_name, "plane")) {

• ret = kstrtouint(source_name + strlen("plane"), 10, &index);
  

• if (ret < 0)
  

• 	return ret;
  

• for (i = 0; i < rcrtc->vsp->num_planes; ++i) {
  

• 	if (index == rcrtc->vsp->planes[i].plane.base.id) {
  

• 		index = i;
  

• 		break;
  

• 	}
  

• }
  

• if (i >= rcrtc->vsp->num_planes)
  

• 	return -EINVAL;
  

• } else {

• return -EINVAL;
  

• }

+out:

• *values_cnt = 1;
• return 0;

This duplicates lots of code from the rcar_du_crtc_set_crc_source() function. Could you please extract it to a shared function ?

Agree, it duplicates the code but "index" is needed by set_crc_source call anyway will create a wrapper to avoid duplication of code.

Could you please also implement support for the .get_crc_sources() operation ? I think doing so might show limitations in the current API, namely the fact that the list will need to be dynamically created for this driver.

for that I think rcar_du_crtc_create function can build a dynamic list during initializing crtc functions, unless plane can be dynamically allocated to any CRTC. this is the place where I need input from maintainer.

-Mahesh

...

+}

• static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, size_t *values_cnt)

@@ -861,6 +900,7 @@ static const struct drm_crtc_funcs crtc_funcs_gen3 = { .enable_vblank = rcar_du_crtc_enable_vblank, .disable_vblank = rcar_du_crtc_disable_vblank, .set_crc_source = rcar_du_crtc_set_crc_source,

• .verify_crc_source = rcar_du_crtc_verify_crc_source, };

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

On 2018-07-02 07:07 AM, Mahesh Kumar wrote:

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 Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

Acked-by: Leo Li sunpeng.li@amd.com

---

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 f4d76528d24c..739a813b4b09 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);

@@ -370,7 +362,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 ffaec138aeee..81780325f36f 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:

Hi,

Thanks for the review.

On 7/10/2018 5:19 PM, Laurent Pinchart wrote:

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:27 EEST Mahesh Kumar wrote:

...

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 Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

---

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

[snip]

...

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c index f4d76528d24c..739a813b4b09 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;
  

  }

If you moved allocation before the !crc->opened check, you could group the two code blocks protected by the crc->lock.

agree, will update in next version. -Mahesh

...

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);

@@ -370,7 +362,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);

[snip]

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:28 EEST Mahesh Kumar wrote:

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 739a813b4b09..add35b77165b 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);

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:29 EEST Mahesh Kumar wrote:

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.

Reviewing this is difficult with a user. Could you submit a patch that makes use of this callback in a driver ?

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 add35b77165b..7aeed89f934a 100644 --- a/drivers/gpu/drm/drm_debugfs_crc.c +++ b/drivers/gpu/drm/drm_debugfs_crc.c @@ -272,6 +272,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 81780325f36f..7e2eab9c2f52 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -691,6 +691,20 @@ struct drm_crtc_funcs { */ const char *const *(*get_crc_sources)(struct drm_crtc *crtc, size_t *count);

• /**

• * @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: