Re: [PATCH 02/10] drm: crc: Introduce get_crc_sources callback

Hi Mahesh,

Thank you for the patch.

On Thursday, 12 July 2018 11:36:27 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 return a constant pointer to an array of crc sources list and update count according to the number of source in the list.

Changes Since V1: (Daniel)

• return const pointer to an array of crc sources list
• do validation of sources in CRC-core

Changes Since V2:

• update commit message
• update callback documentation
• print one source name per line

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

---

drivers/gpu/drm/drm_debugfs_crc.c | 23 ++++++++++++++++++++++- include/drm/drm_crtc.h | 22 ++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c index 1956acb61cc8..940b76a1ab71 100644 --- a/drivers/gpu/drm/drm_debugfs_crc.c +++ b/drivers/gpu/drm/drm_debugfs_crc.c @@ -68,8 +68,29 @@ static int crc_control_show(struct seq_file *m, void *data) { struct drm_crtc *crtc = m->private;

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

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

• size_t count;
  

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

• 							&count);
  

• size_t values_cnt;
  

• int i;
  

i only takes positive values, you can make it an unsigned int.

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

• 	goto out;
  

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

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

• 					    &values_cnt)) {
  

• 		if (strcmp(sources[i], crtc->crc.source))
  

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

• 		else
  

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

• 	}
  

• }
• return 0;

+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 3ce5d2fe133d..69886025e628 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -699,6 +699,28 @@ 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 depends upon verify_crc_source, So
  

s/So/so/

• * verify_crc_source callback should be implemented before implementing
  

• * this. Driver can pass full list of available crc sources, this
  

• * callback does the verification on each crc-source before passing it
  

• * to userspace.
  

• *
  

• * This callback is optional if the driver does not support exporting of
  

• * possible CRC sources list.
  

• *
  

• * RETURNS:
  

• *
  

• * a constant character pointer to the list of all the available CRC
  

• * sources. On failure driver should return NULL. count should be
  

• * updated with number of sources in list. if zero we don't process any
  

s/if zero/If zero/

Apart from that,

Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com

• * source from the list.
  

• */
  

• const char *const *(*get_crc_sources)(struct drm_crtc *crtc,

• 			      size_t *count);
  
  

  /**

  • @atomic_print_state: