[igt-dev] [PATCH i-g-t] lib/igt_debugfs: Add igt_pipe_crc_get_single and igt_pipe_crc_drain, v2.

Mika Kahola mika.kahola at intel.com
Thu Feb 8 11:15:48 UTC 2018


On Wed, 2018-02-07 at 12:47 +0100, Maarten Lankhorst wrote:
> Collecting CRC may result in a modeset and extra vblank waits. On
> some
> tests this will increase the runtime a lot, so it makes sense to
> keep it enabled, and only collect the most recent CRC when needed.
> 
> Changes since v1:
> - Fix read_crc semantics. (Ville)
> 

Reviewed-by: Mika Kahola <mika.kahola at intel.com>

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
>  lib/igt_debugfs.c         | 74
> +++++++++++++++++++++++++++++++++++++++++++----
>  lib/igt_debugfs.h         |  2 ++
>  tests/kms_cursor_legacy.c | 21 ++++----------
>  3 files changed, 76 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 1aec079c52bb..dc75d6adbf09 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -710,11 +710,11 @@ static int read_crc(igt_pipe_crc_t *pipe_crc,
> igt_crc_t *out)
>  		igt_assert(pipe_crc->flags & O_NONBLOCK);
>  
>  	if (bytes_read < 0)
> -		bytes_read = 0;
> -
> -	buf[bytes_read] = '\0';
> +		bytes_read = -errno;
> +	else
> +		buf[bytes_read] = '\0';
>  
> -	if (bytes_read && !pipe_crc_init_from_string(pipe_crc, out,
> buf))
> +	if (bytes_read > 0 && !pipe_crc_init_from_string(pipe_crc,
> out, buf))
>  		return -EINVAL;
>  
>  	return bytes_read;
> @@ -722,7 +722,9 @@ static int read_crc(igt_pipe_crc_t *pipe_crc,
> igt_crc_t *out)
>  
>  static void read_one_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
>  {
> -	while (read_crc(pipe_crc, out) == 0)
> +	int ret;
> +
> +	while ((ret = read_crc(pipe_crc, out)) <= 0 && ret !=
> -EINVAL)
>  		usleep(1000);
>  }
>  
> @@ -854,6 +856,68 @@ void igt_pipe_crc_collect_crc(igt_pipe_crc_t
> *pipe_crc, igt_crc_t *out_crc)
>  	crc_sanity_checks(out_crc);
>  }
>  
> +/**
> + * igt_pipe_crc_drain:
> + * @pipe_crc: pipe CRC object
> + *
> + * Discards all currently queued CRC values from @pipe_crc. This
> function does
> + * not block, and is useful to flush @pipe_crc. Afterwards you can
> get a fresh
> + * CRC with igt_pipe_crc_get_single().
> + */
> +void igt_pipe_crc_drain(igt_pipe_crc_t *pipe_crc)
> +{
> +	int ret;
> +	igt_crc_t crc;
> +
> +	fcntl(pipe_crc->crc_fd, F_SETFL, pipe_crc->flags |
> O_NONBLOCK);
> +
> +	do {
> +		ret = read_crc(pipe_crc, &crc);
> +	} while (ret != 0);
> +
> +	fcntl(pipe_crc->crc_fd, F_SETFL, pipe_crc->flags);
> +}
> +
> +/**
> + * igt_pipe_crc_get_single:
> + * @pipe_crc: pipe CRC object
> + * @crc: buffer pointer for the captured CRC value
> + *
> + * Read a single @crc from @pipe_crc. This function does not block
> + * when nonblocking CRC is requested, and will return false if no
> CRC
> + * can be captured.
> + *
> + * If opened in blocking mode it will always block until a new CRC
> is read, like
> + * igt_pipe_crc_collect_crc().
> + *
> + * Callers must start and stop the capturing themselves by calling
> + * igt_pipe_crc_start() and igt_pipe_crc_stop(). For one-shot CRC
> collecting
> + * look at igt_pipe_crc_collect_crc().
> + *
> + * If capturing has been going on for a while and a fresh crc is
> required,
> + * you will need to call igt_pipe_crc_drain() first to remove stale
> entries.
> + *
> + * Returns:
> + * Whether a crc is captured, only false in non-blocking mode.
> + */
> +bool
> +igt_pipe_crc_get_single(igt_pipe_crc_t *pipe_crc, igt_crc_t *crc)
> +{
> +	bool found = true;
> +
> +	if (pipe_crc->flags & O_NONBLOCK) {
> +		int ret = read_crc(pipe_crc, crc);
> +
> +		found = ret > 0;
> +	} else
> +		read_one_crc(pipe_crc, crc);
> +
> +	if (found)
> +		crc_sanity_checks(crc);
> +
> +	return found;
> +}
> +
>  /*
>   * Drop caches
>   */
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index d90dd7a68bb0..8d25abfe43c1 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -130,6 +130,8 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
>  __attribute__((warn_unused_result))
>  int igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
>  			  igt_crc_t **out_crcs);
> +void igt_pipe_crc_drain(igt_pipe_crc_t *pipe_crc);
> +bool igt_pipe_crc_get_single(igt_pipe_crc_t *pipe_crc, igt_crc_t
> *out_crc);
>  void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t
> *out_crc);
>  
>  void igt_hpd_storm_set_threshold(int fd, unsigned int threshold);
> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> index 5011e78e5c2f..d0a28b3c442c 100644
> --- a/tests/kms_cursor_legacy.c
> +++ b/tests/kms_cursor_legacy.c
> @@ -1276,7 +1276,7 @@ static void
> flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  	enum pipe pipe = find_connected_pipe(display, false);
>  	igt_pipe_t *pipe_connected = &display->pipes[pipe];
>  	igt_plane_t *plane_primary =
> igt_pipe_get_plane_type(pipe_connected, DRM_PLANE_TYPE_PRIMARY);
> -	igt_crc_t crcs[2];
> +	igt_crc_t crcs[2], test_crc;
>  
>  	if (atomic)
>  		igt_require(display->is_atomic);
> @@ -1290,7 +1290,7 @@ static void
> flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  
>  	igt_display_commit2(display, display->is_atomic ?
> COMMIT_ATOMIC : COMMIT_LEGACY);
>  
> -	pipe_crc = igt_pipe_crc_new_nonblock(display->drm_fd, pipe,
> INTEL_PIPE_CRC_SOURCE_AUTO);
> +	pipe_crc = igt_pipe_crc_new(display->drm_fd, pipe,
> INTEL_PIPE_CRC_SOURCE_AUTO);
>  
>  	set_cursor_on_pipe(display, pipe, &cursor_fb);
>  	igt_display_commit2(display, COMMIT_UNIVERSAL);
> @@ -1322,9 +1322,6 @@ static void
> flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  	/* Disable cursor, and immediately queue a flip. Check if
> resulting crc is correct. */
>  	for (int i = 1; i >= 0; i--) {
>  		igt_spin_t *spin;
> -		igt_crc_t *received_crcs = NULL;
> -		int ncrcs;
> -		static const int max_crcs = 8;
>  
>  		spin = igt_spin_batch_new(display->drm_fd, 0, 0,
>  					  fb_info[1].gem_handle);
> @@ -1336,7 +1333,8 @@ static void
> flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  
>  		igt_assert_eq(get_vblank(display->drm_fd, pipe, 0),
> vblank_start);
>  
> -		ncrcs = igt_pipe_crc_get_crcs(pipe_crc, max_crcs,
> &received_crcs);
> +		igt_pipe_crc_drain(pipe_crc);
> +		igt_pipe_crc_get_single(pipe_crc, &test_crc);
>  
>  		igt_spin_batch_free(display->drm_fd, spin);
>  
> @@ -1349,16 +1347,7 @@ static void
> flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  		igt_plane_set_fb(plane_primary, &fb_info[0]);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> -		/*
> -		 * We collect the crc nonblockingly, and should have
> at least 1
> -		 * but not so many crcs that we overflow. Last CRC
> is the only
> -		 * one we care about here. Other CRCs may have been
> from before
> -		 * the cursor update and can contain garbage.
> -		 */
> -		igt_assert(ncrcs > 0 && ncrcs < max_crcs);
> -
> -		igt_assert_crc_equal(&crcs[i], &received_crcs[ncrcs
> - 1]);
> -		free(received_crcs);
> +		igt_assert_crc_equal(&crcs[i], &test_crc);
>  	}
>  
>  	igt_remove_fb(display->drm_fd, &fb_info[1]);
-- 
Mika Kahola - Intel OTC



More information about the igt-dev mailing list