Re: [PATCH i-g-t 1/3] lib/igt_psr: Add mechanism to check sink status as well

["Naladala, Ramanaidu" <ramanaidu.naladala@intel.com>]

On 7/1/2024 10:42 AM, Jouni Högander wrote:
> We have seen passing PSR testcases even though panel is not even aware of
> PSR being used. This can happen because we currently not checking sink PSR
> statuses at all. Also sink might have detected errors but our testcase
> currently don't care about that.
> 
> Help the gap described above by adding new interface to check sink error
> statuses. Also add sink status check to be part of psr_is_active check.
> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>   lib/igt_psr.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>   lib/igt_psr.h |  1 +
>   2 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index e3e7577eb..47517b5dc 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -52,6 +52,19 @@ bool selective_fetch_check(int debugfs_fd, igt_output_t *output)
>   
>   	return strstr(buf, "PSR2 selective fetch: enabled");
>   }
> +static bool psr_active_sink_check(int debugfs_fd, igt_output_t *output)
> +{
> +	char debugfs_file[128] = {0};
> +	char buf[PSR_STATUS_MAX_LEN];
> +	int ret;
> +
> +	sprintf(debugfs_file, "%s/i915_psr_sink_status", output->name);
> +	ret = igt_debugfs_simple_read(debugfs_fd, debugfs_file, buf,
> +				      sizeof(buf));
> +	igt_assert_f(ret >= 1, "Failed to read sink status\n");
> +
> +	return strstr(buf, "0x2 [active, display from RFB]");
> +}
>   
>   /*
>    * Checks if Early Transport is enabled in PSR status by reading the debugfs.
> @@ -72,6 +85,7 @@ static bool psr_active_check(int debugfs_fd, enum psr_mode mode, igt_output_t *o
>   	char buf[PSR_STATUS_MAX_LEN];
>   	drmModeConnector *c;
>   	const char *state;
> +	bool active;
>   	int ret;
>   
>   	if (mode == PR_MODE || mode == PR_MODE_SEL_FETCH) {
> @@ -100,7 +114,11 @@ static bool psr_active_check(int debugfs_fd, enum psr_mode mode, igt_output_t *o
>   
>   	igt_skip_on(strstr(buf, "PSR sink not reliable: yes"));
>   
> -	return strstr(buf, state);
> +	active = strstr(buf, state);
> +	if (active && output)
> +		active = psr_active_sink_check(debugfs_fd, output);
> +
> +	return active;
>   }
>   
>   /*
> @@ -297,6 +315,38 @@ bool psr_sink_support(int device, int debugfs_fd, enum psr_mode mode, igt_output
>   	}
>   }
>   
> +/**
> + * psr_sink_error_check
> + * Check and assert on PSR errors detected by panel
> + *
> + * Returns:
> + * None
> + */
> +void psr_sink_error_check(int debugfs_fd, enum psr_mode mode, igt_output_t *output)
> +{
> +	char *line;
> +	char debugfs_file[128] = {0};
> +	char buf[PSR_STATUS_MAX_LEN];
> +	int ret;
> +
> +	sprintf(debugfs_file, "%s/i915_psr_sink_status", output->name);
> +	ret = igt_debugfs_simple_read(debugfs_fd, debugfs_file, buf,
> +				      sizeof(buf));
> +	igt_assert_f(ret >= 1, "Failed to read sink status\n");
> +
> +	line = strstr(buf, "error status: 0x0");
> +
> +	/*
> +	 * On certain PSR1 panels we are seeing "PSR VSC SDP
> +	 * uncorrectable error" bit set even it is applicable for PSR1
> +	 * only
> +	 */
> +	if (!line && mode == PSR_MODE_1)
> +		line = strstr(buf, "Sink PSR error status: 0x4");
> +
> +	igt_assert_f(line, "Sink detected PSR error(s):\n%s\n", buf);
> +}
> +
LGTM,
Reviewed-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
>   #define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
>   
>   /* Return the the last or last but one su blocks */
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index a7ebd0739..7639f8d46 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -56,6 +56,7 @@ bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode, igt_output_t *outp
>   bool psr_enable(int device, int debugfs_fd, enum psr_mode, igt_output_t *output);
>   bool psr_disable(int device, int debugfs_fd, igt_output_t *output);
>   bool psr_sink_support(int device, int debugfs_fd, enum psr_mode mode, igt_output_t *output);
> +void psr_sink_error_check(int debugfs_fd, enum psr_mode mode, igt_output_t *output);
>   bool psr2_wait_su(int debugfs_fd, uint16_t *num_su_blocks);
>   void psr_print_debugfs(int debugfs_fd);
>   enum psr_mode psr_get_mode(int debugfs_fd, igt_output_t *output);