Hi Janusz,
On 2025-10-07 at 13:38:25 +0200, Janusz Krzysztofik wrote:
> 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 a presumed limit. On most Gen12+ platforms,
> as well as on some older ones like JSL, CHV, ILK or ELK, the current limit
> of 5 seconds for collecting those results occurs too short.
>
> Raise the limit to an empirically determined value of 20 seconds and break
> the loop as soon as 9 results are collected.
>
> v2: Split out a change in handling of not enough measurements to a
> separate patch (Kamil),
> - reword commit message to be more distinct from other patches in
> series (Kamil),
> - reword commit message and description so they no longer state the
> scope of the issue is limited to Gen12+, and list other (non-Gen12+)
> platforms found also affected.
>
> Signed-off-by: Janusz Krzysztofik <[email protected]>
> ---
> tests/intel/gem_eio.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tests/intel/gem_eio.c b/tests/intel/gem_eio.c
> index 0a00ef026e..79dcef8fa6 100644
> --- a/tests/intel/gem_eio.c
> +++ b/tests/intel/gem_eio.c
> @@ -929,7 +929,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) {
What I wanted here was actually (in pseudocode):
mtime = gen < 5 || gen >= 12 ? 20 : 5;
igt_until_timeout(mtime) {
> const intel_ctx_t *ctx = context_create_safe(fd);
> igt_spin_t *hang;
> unsigned int i;
> @@ -978,6 +978,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)
Can it be a define as it is used in other places, for example:
#define NUMER_OF_MEASURED_CYCLES_NEEDED 9
so you will use it elsewhere, and here it will be:
if (stats.n_values >= NUMER_OF_MEASURED_CYCLES_NEEDED)
break;
> }
> check_wait_elapsed(name, fd, &stats);
I did give you r-b for patch 1/5 but I am not sure how
reliable are measurements, should it be an assert instead of skip?
Maybe function check_wait_elapsed() should return bool to tell if
median is ready, and in each place subtests itself decide if it
should skip or assert? Up to you.
Regards,
Kamil
> igt_stats_fini(&stats);
> --
> 2.51.0
>
