[igt-dev] [PATCH i-g-t] lib/debugfs: Sanity check even discarded CRCs

[Dhinakaran Pandiyan]

On Thu, 2018-12-13 at 12:57 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> We currently only spot check some of the CRCs. We should check them
> all. 

Sounds like the right thing to do
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>

> This will probably cause failures on machines with PSR as
> it looks like the CRC captured after PSR exit is bogus. Or at least
> it is on ICL. We should probably just disable PSR whenever CRC
> capturing is active.

The frame counter got updated, but the CRC is junk. Isn't it still
useful to see make sure we get the CRC we expect with PSR, even if the
CRC is delayed by a frame?

> 
> Cc: José Roberto de Souza <jose.souza at intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=106974
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  lib/igt_debugfs.c | 41 ++++++++++++++++++-----------------------
>  1 file changed, 18 insertions(+), 23 deletions(-)
> 
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index a3aca8466658..f6ffcbe67b7b 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -760,6 +760,22 @@ static bool
> pipe_crc_init_from_string(igt_pipe_crc_t *pipe_crc, igt_crc_t *crc,
>  	return true;
>  }
>  
> +static void crc_sanity_checks(igt_crc_t *crc)
> +{
> +	bool all_zero = true;
> +
> +	for (int i = 0; i < crc->n_words; i++) {
> +		igt_warn_on_f(crc->crc[i] == 0xffffffff,
> +			      "Suspicious CRC: it looks like the CRC "
> +			      "read back was from a register in a
> powered "
> +			      "down well\n");
> +		if (crc->crc[i])
> +			all_zero = false;
> +	}
> +
> +	igt_warn_on_f(all_zero, "Suspicious CRC: All values are 0.\n");
> +}
> +
>  static int read_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
>  {
>  	ssize_t bytes_read;
> @@ -777,6 +793,8 @@ static int read_crc(igt_pipe_crc_t *pipe_crc,
> igt_crc_t *out)
>  	if (bytes_read > 0 && !pipe_crc_init_from_string(pipe_crc, out,
> buf))
>  		return -EINVAL;
>  
> +	crc_sanity_checks(out);
> +
>  	return bytes_read;
>  }
>  
> @@ -884,23 +902,6 @@ igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc,
> int n_crcs,
>  	return n;
>  }
>  
> -static void crc_sanity_checks(igt_crc_t *crc)
> -{
> -	int i;
> -	bool all_zero = true;
> -
> -	for (i = 0; i < crc->n_words; i++) {
> -		igt_warn_on_f(crc->crc[i] == 0xffffffff,
> -			      "Suspicious CRC: it looks like the CRC "
> -			      "read back was from a register in a
> powered "
> -			      "down well\n");
> -		if (crc->crc[i])
> -			all_zero = false;
> -	}
> -
> -	igt_warn_on_f(all_zero, "Suspicious CRC: All values are 0.\n");
> -}
> -
>  /**
>   * igt_pipe_crc_collect_crc:
>   * @pipe_crc: pipe CRC object
> @@ -927,8 +928,6 @@ void igt_pipe_crc_collect_crc(igt_pipe_crc_t
> *pipe_crc, igt_crc_t *out_crc)
>  	igt_pipe_crc_start(pipe_crc);
>  	read_one_crc(pipe_crc, out_crc);
>  	igt_pipe_crc_stop(pipe_crc);
> -
> -	crc_sanity_checks(out_crc);
>  }
>  
>  /**
> @@ -971,8 +970,6 @@ void igt_pipe_crc_drain(igt_pipe_crc_t *pipe_crc)
>  void igt_pipe_crc_get_single(igt_pipe_crc_t *pipe_crc, igt_crc_t
> *crc)
>  {
>  	read_one_crc(pipe_crc, crc);
> -
> -	crc_sanity_checks(crc);
>  }
>  
>  /**
> @@ -1000,8 +997,6 @@ igt_pipe_crc_get_current(int drm_fd,
> igt_pipe_crc_t *pipe_crc, igt_crc_t *crc)
>  			return;
>  		}
>  	} while (crc->frame <= vblank);
> -
> -	crc_sanity_checks(crc);
>  }
>  
>  /*