Hi Janusz,
On 2025-09-30 at 14:49:01 +0200, Janusz Krzysztofik wrote:

could you improve subject? Now it is a little confusing why
there are two, very similar changes needed, see first and
second subjects:

[i-g-t,1/3] tests/gem_eio: Adjust for long reset-resume cycle on Gen12+

[i-g-t,2/3] tests/gem_eio: Adjust for slow resume after reset on Gen12+

At first I do not know why two very similar changes are made with
two different commits.

See also below.

> Subtests that measure time of resume after engine reset require results
> from at least 9 reset-resume cycles for reasonable calculation of a median
> value to be compared against presumed limits.  On most of Gen12+
> platforms, the limit of 5 seconds for collecting those results occurs too
> short for executing 9 reset-resum cycles.
> 
> Raise the limit to 20 seconds, and break the loop as soon as 9 results are
> collected.  Also, warn if less than 9 resets have been completed within
> the limit instead of silently succeeding despite the check being skipped.
> 
> Signed-off-by: Janusz Krzysztofik <[email protected]>
> ---
>  tests/intel/gem_eio.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/intel/gem_eio.c b/tests/intel/gem_eio.c
> index b65b914faf..b6155c7fc4 100644
> --- a/tests/intel/gem_eio.c
> +++ b/tests/intel/gem_eio.c
> @@ -409,8 +409,10 @@ static void check_wait_elapsed(const char *prefix, int 
> fd, igt_stats_t *st)
>                igt_stats_get_median(st)*1e-6,
>                igt_stats_get_max(st)*1e-6);
>  
> -     if (st->n_values < 9)
> -             return; /* too few for stable median */
> +     if (igt_warn_on_f(st->n_values < 9,
> +         "%d resets completed -- less than 9, too few for stable median\n",
> +         st->n_values))
> +             return;

imho this could be a separate change, as there was silence before.

>  
>       /*
>        * Older platforms need to reset the display (incl. modeset to off,
> @@ -928,7 +930,7 @@ static void reset_stress(int fd, uint64_t ahnd, const 
> intel_ctx_t *ctx0,
>       gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
>  
>       igt_stats_init(&stats);
> -     igt_until_timeout(5) {
> +     igt_until_timeout(20) {

Could you increase it only when needed?

Regards,
Kamil

>               const intel_ctx_t *ctx = context_create_safe(fd);
>               igt_spin_t *hang;
>               unsigned int i;
> @@ -977,6 +979,9 @@ static void reset_stress(int fd, uint64_t ahnd, const 
> intel_ctx_t *ctx0,
>               gem_sync(fd, obj.handle);
>               igt_spin_free(fd, hang);
>               intel_ctx_destroy(fd, ctx);
> +
> +             if (stats.n_values > 8)
> +                     break;
>       }
>       check_wait_elapsed(name, fd, &stats);
>       igt_stats_fini(&stats);
> -- 
> 2.51.0
> 

Reply via email to