Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_fbcon_fbt: Make FBC wait_until_update() more reliable

[Imre Deak <imre.deak@intel.com>]

On Wed, Apr 08, 2020 at 01:19:24PM -0700, José Roberto de Souza wrote:
> We can't depend onto debugfs reads of the FBC status as compression
> takes one idle frame to recompress and we could easily miss that.
> 
> Instead lets use the pipe CRC as it keeps the CRC of each frame so we
> can compare each other until a blink in the FBCON causes it do change.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_debugfs.c     | 13 +++++++++++--
>  lib/igt_debugfs.h     |  1 +
>  tests/kms_fbcon_fbt.c | 39 ++++++++++++++++++++++++++++++---------
>  3 files changed, 42 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index bf6be552..9e6c5c99 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -368,8 +368,17 @@ bool igt_debugfs_search(int device, const char *filename, const char *substring)
>   * Pipe CRC
>   */
>  
> -static bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b,
> -				  int *index)
> +/**
> + * igt_find_crc_mismatch:
> + * @a: first pipe CRC value
> + * @b: second pipe CRC value
> + * @index: index of the first value that mismatched
> + *
> + * Check if CRC a and CRC b mismatch.
> + *
> + * Returns true if CRC values mismatch, false otherwise;
> + */
> +bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b, int *index)
>  {
>  	int nwords = min(a->n_words, b->n_words);
>  	int i;
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index 7d1a6175..722c5cc3 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -85,6 +85,7 @@ typedef struct {
>  #define INTEL_PIPE_CRC_SOURCE_AUTO "auto"
>  #define AMDGPU_PIPE_CRC_SOURCE_DPRX "dprx"
>  
> +bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b, int *index);
>  void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b);
>  bool igt_check_crc_equal(const igt_crc_t *a, const igt_crc_t *b);
>  char *igt_crc_to_string_extended(igt_crc_t *crc, char delimiter, int crc_size);
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index c2361730..546dff99 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -142,13 +142,31 @@ static bool fbc_wait_until_disabled(int debugfs_fd)
>  	return r;
>  }
>  
> -static bool fbc_not_compressing_enabled(int debugfs_fd)
> +static bool fbc_check_cursor_blinking(struct drm_info *drm)
>  {
> -	char buf[128];
> +	igt_pipe_crc_t *pipe_crc;
> +	igt_crc_t crc[2];
> +	bool ret;
> +	int i;
>  
> -	igt_debugfs_simple_read(debugfs_fd, "i915_fbc_status", buf,
> -				sizeof(buf));
> -	return strstr(buf, "FBC enabled\nCompressing: no");
> +	pipe_crc = igt_pipe_crc_new(drm->fd, PIPE_A, INTEL_PIPE_CRC_SOURCE_AUTO);

There doesn't seem to be a way to find the actual pipe for fbcon, but
since nothing else is enabled here, it's a reasonable assumption:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> +
> +	igt_pipe_crc_start(pipe_crc);
> +	igt_pipe_crc_drain(pipe_crc);
> +
> +	for (i = 0; i < 60; i++) {
> +		igt_pipe_crc_get_single(pipe_crc, &crc[i % 2]);
> +
> +		if (i > 0) {
> +			ret = igt_find_crc_mismatch(&crc[0], &crc[1], NULL);
> +			if (ret)
> +				break;
> +		}
> +	}
> +	igt_pipe_crc_stop(pipe_crc);
> +	igt_pipe_crc_free(pipe_crc);
> +
> +	return ret;
>  }
>  
>  static bool fbc_wait_until_update(struct drm_info *drm)
> @@ -161,11 +179,14 @@ static bool fbc_wait_until_update(struct drm_info *drm)
>  	 * For older GENs FBC is still expected to be disabled as it still
>  	 * relies on a tiled and fenceable framebuffer to track modifications.
>  	 */
> -	if (AT_LEAST_GEN(drm->devid, 9))
> -		return igt_wait(fbc_not_compressing_enabled(drm->debugfs_fd),
> -				2500, 1);
> -	else
> +	if (AT_LEAST_GEN(drm->devid, 9)) {
> +		if (!fbc_wait_until_enabled(drm->debugfs_fd))
> +			return false;
> +
> +		return fbc_check_cursor_blinking(drm);
> +	} else {
>  		return fbc_wait_until_disabled(drm->debugfs_fd);
> +	}
>  }
>  
>  typedef bool (*connector_possible_fn)(drmModeConnectorPtr connector);
> -- 
> 2.26.0
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev