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

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Tue Feb 6 16:30:00 UTC 2018


Op 06-02-18 om 17:11 schreef Ville Syrjälä:
> On Tue, Feb 06, 2018 at 11:14:10AM +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.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> ---
>>  lib/igt_debugfs.c         | 73 ++++++++++++++++++++++++++++++++++++++++++++---
>>  lib/igt_debugfs.h         |  2 ++
>>  tests/kms_cursor_legacy.c | 21 ++++----------
>>  3 files changed, 76 insertions(+), 20 deletions(-)
>>
>> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
>> index 1aec079c52bb..ad3e7bcf38fa 100644
>> --- a/lib/igt_debugfs.c
>> +++ b/lib/igt_debugfs.c
>> @@ -33,6 +33,7 @@
>>  #include <limits.h>
>>  #include <string.h>
>>  #include <fcntl.h>
>> +#include <poll.h>
>>  #include <unistd.h>
>>  #include <i915_drm.h>
>>  
>> @@ -697,7 +698,7 @@ static bool pipe_crc_init_from_string(igt_pipe_crc_t *pipe_crc, igt_crc_t *crc,
>>  	return true;
>>  }
>>  
>> -static int read_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
>> +static int read_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out, bool override_nonblock)
>>  {
>>  	ssize_t bytes_read;
>>  	char buf[MAX_LINE_LEN + 1];
>> @@ -706,7 +707,7 @@ static int read_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
>>  	bytes_read = read(pipe_crc->crc_fd, &buf, MAX_LINE_LEN);
>>  	igt_reset_timeout();
>>  
>> -	if (bytes_read < 0 && errno == EAGAIN)
>> +	if (bytes_read < 0 && errno == EAGAIN && !override_nonblock)
>>  		igt_assert(pipe_crc->flags & O_NONBLOCK);
>>  
>>  	if (bytes_read < 0)
>> @@ -722,7 +723,7 @@ 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)
>> +	while (read_crc(pipe_crc, out, false) == 0)
>>  		usleep(1000);
>>  }
>>  
>> @@ -794,7 +795,7 @@ igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
>>  		igt_crc_t *crc = &crcs[n];
>>  		int ret;
>>  
>> -		ret = read_crc(pipe_crc, crc);
>> +		ret = read_crc(pipe_crc, crc, false);
>>  		if (ret < 0)
>>  			continue;
>>  		if (ret == 0)
>> @@ -854,6 +855,70 @@ 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)
>> +{
>> +	igt_crc_t crc;
>> +	int ret;
>> +
>> +	fcntl(pipe_crc->crc_fd, F_SETFL, pipe_crc->flags | O_NONBLOCK);
>> +	do {
>> +		ret = read_crc(pipe_crc, &crc, true);
>> +	} while (ret > 0 || errno == EINTR);
> Is errno valid for ret==0?
Crap, whole read_crc has botched this part, -1 is never returned even for the -EINVAL branch, because it can never be triggered even with invalid CRC's...

I guess this patch needs more thought..
>
>> +	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;
>> +
>> +		do {
>> +			ret = read_crc(pipe_crc, crc, false);
>> +		} while (ret < 0);
>> +
>> +		found = ret;
>> +	} 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]);
>> -- 
>> 2.16.1
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev




More information about the igt-dev mailing list