Re: [igt-dev] [PATCH 1/2] [i-g-t] lib/igt_kms: Re-try display_require if require output is not valid

["Modem, Bhanuprakash" <bhanuprakash.modem@intel.com>]

Hello Hersen,

On Sat-12-08-2023 03:42 am, Hersen Wu wrote:
> Add igt_display_require_retry to re-try igt_display_require.
> 
> At entry of each test, display setup is done by
> igt_display_require and igt_display_require_output.
> igt display require output may not be valid yet,
> this will let test be skipped. By re-try
> igt_display_require, require output could be valid.
> This will fix test case intermittent skip issue.

+ J-P & Kamil,

Few queries:

- Why is igt_display_require_output not valid & retry makes it valid?
- Is it applicable for all kms tests? Then you must update all kms tests 
to use this api. Otherwise if it's AMD specific, lib/igt_kms is not the 
right place.
- What is the impact on CI execution time? In headless configs it tries 
for "retry_times" and eventually sends a skip.

- Bhanu

> 
> Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
> ---
>   lib/igt_kms.c | 19 +++++++++++++++++++
>   lib/igt_kms.h | 15 +++++++++++++++
>   2 files changed, 34 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e0959ccff..5506e2eb2 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -2862,6 +2862,25 @@ void igt_display_require_output(igt_display_t *display)
>   	igt_skip("No valid crtc/connector combinations found.\n");
>   }
>   
> +/**
> + * is_igt_display_require_output_valid:
> + * @display: A pointer to an #igt_display_t structure
> + *
> + * Checks whether there's a valid @pipe/@output combination for the given @display
> + * Returns: true if a valid combination of @pipe and @output is found, or false
> + * if not found
> + */
> +bool is_igt_display_require_output_valid(igt_display_t *display)
> +{
> +	enum pipe pipe;
> +	igt_output_t *output;
> +
> +	for_each_pipe_with_valid_output(display, pipe, output)
> +		return true;
> +
> +	return false;
> +}
> +
>   /**
>    * igt_display_require_output_on_pipe:
>    * @display: A pointer to an #igt_display_t structure
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 91355c910..f801b87e6 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -485,6 +485,7 @@ int  igt_display_drop_events(igt_display_t *display);
>   int  igt_display_get_n_pipes(igt_display_t *display);
>   void igt_display_require_output(igt_display_t *display);
>   void igt_display_require_output_on_pipe(igt_display_t *display, enum pipe pipe);
> +bool is_igt_display_require_output_valid(igt_display_t *display);
>   
>   const char *igt_output_name(igt_output_t *output);
>   drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
> @@ -722,6 +723,20 @@ uint64_t igt_plane_get_prop(igt_plane_t *plane, enum igt_atomic_plane_properties
>   		igt_plane_set_prop_changed(plane, prop); \
>   	} while (0)
>   
> +#define igt_display_require_retry(display, fd, delay, retry_times) \
> +	for (int j__ = 0; j__ < retry_times + 1; j__++) { \
> +		igt_display_require(display, fd); \
> +		if (is_igt_display_require_output_valid(display)) { \
> +			if (j__ > 0) \
> +				igt_info("retry times:%d delay:%d seconds\n", j__, delay * j__); \
> +			break; \
> +		} \
> +		if (j__ == retry_times) \
> +			igt_skip("No valid crtc/connector combinations found.\n"); \
> +		else \
> +			sleep(delay); \
> +	}
> +
>   extern bool igt_plane_try_prop_enum(igt_plane_t *plane,
>   				    enum igt_atomic_plane_properties prop,
>   				    const char *val);