[PATCH v2 i-g-t] lib/igt_kms: add retry loop in igt_enable_connectors()

[Kamil Konieczny]

Hi Luca,
On 2024-05-20 at 12:22:16 +0300, Luca Coelho wrote:
> Sometimes the connector is not fully initialized yet when we try to
> read it in igt_enable_connectors() and this causes a warning to be
> issued and the test to fail.  This can happen in normal situations,
> because there's no synchronization between the driver and IGT.  In
> some tests we load the module and immediately start reading the
> information, but the driver may not be fully initialized at that
> point, causing the problem.
> 
> To prevent this, add a retry loop in igt_enable_connectors() so we
> retry a few times, after a small delay, to wait for the initialization
> to complete.
> 
> The number of retries and the delay between retries were chose after
> running lots of tests, but these values may have to be adjusted after
> running more tests on different machines.
> 
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10436
> Signed-off-by: Luca Coelho <luciano.coelho at intel.com>

Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

> ---
> 
> In v2:
>    * Changed igt_info() to igt_debug() in the retry loop (Kamil)
>    * Changed the retry logs to start from 1 instead of 0 to avoid
>      confusion.
> 
>  lib/igt_kms.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index f82102144ea1..af63d13b1da3 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -5386,7 +5386,10 @@ void igt_wait_for_vblank(int drm_fd, int crtc_offset)
>   */
>  void igt_enable_connectors(int drm_fd)
>  {
> +#define MAX_TRIES	10
> +#define SLEEP_DURATION	50000
>  	drmModeRes *res;
> +	int tries;
>  
>  	res = drmModeGetResources(drm_fd);
>  	if (!res)
> @@ -5395,10 +5398,28 @@ void igt_enable_connectors(int drm_fd)
>  	for (int i = 0; i < res->count_connectors; i++) {
>  		drmModeConnector *c;
>  
> -		/* Do a probe. This may be the first action after booting */
> -		c = drmModeGetConnector(drm_fd, res->connectors[i]);
> -		if (!c) {
> -			igt_warn("Could not read connector %u: %m\n", res->connectors[i]);
> +		/*
> +		 * The kernel returns the count of connectors before
> +		 * they're all fully set up, so we can have a race
> +		 * condition where we try to get the connector when
> +		 * it's not fully set up yet.  To avoid failing here
> +		 * in these cases, retry a few times.
> +		 */
> +		for (tries = 0; tries < MAX_TRIES; tries++) {
> +			/* Do a probe. This may be the first action after booting */
> +			c = drmModeGetConnector(drm_fd, res->connectors[i]);
> +			if (c)
> +				break;
> +
> +			igt_debug("Could not read connector %u: %m (try %d of %d)\n",
> +				  res->connectors[i], tries + 1, MAX_TRIES);
> +
> +			usleep(SLEEP_DURATION);
> +		}
> +
> +		if (tries == MAX_TRIES) {
> +			igt_warn("Could not read connector %u after %d tries, skipping\n",
> +				 res->connectors[i], MAX_TRIES);
>  			continue;
>  		}
>  
> -- 
> 2.39.2
>