Hi Kamil,

On Wednesday, 8 October 2025 16:03:30 CEST Kamil Konieczny wrote:
> Hi Janusz,
> On 2025-10-08 at 14:15:28 +0200, Janusz Krzysztofik wrote:
> > Hi Kamil,
> > 
> > Thanks for review.
> > 
> > On Wednesday, 8 October 2025 13:59:11 CEST Kamil Konieczny wrote:
> > > Hi Janusz,
> > > On 2025-10-07 at 13:38:24 +0200, Janusz Krzysztofik wrote:
> > > > Subtests that measure time of resume after engine reset compare a median
> > > > value calculated from the measurements against a presumed limit and fail
> > > > if the limit has been exceeded.  However, if it occurs not possible to
> > > > collect enough measurements required for stable median value 
> > > > calculation,
> > > > that condition is now ignored and success is reported, as if the 
> > > > measured
> > > > time fit below the limit.
> > > > 
> > > > Skip if not able to collect sufficient number of time measurements.  CI
> > > > results from slow platforms that always skip may be handled as expected
> > > > skips.
> > > > 
> > > > Signed-off-by: Janusz Krzysztofik <[email protected]>
> > > > ---
> > > >  tests/intel/gem_eio.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/tests/intel/gem_eio.c b/tests/intel/gem_eio.c
> > > > index b65b914faf..0a00ef026e 100644
> > > > --- a/tests/intel/gem_eio.c
> > > > +++ b/tests/intel/gem_eio.c
> > > > @@ -409,8 +409,9 @@ 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 */
> > > > +       igt_require_f(st->n_values > 8,
> > > > +                     "at least 9 resets completed for stable median 
> > > > calculation, %d is too few\n",
> > > 
> > > imho add 'Test needs' at begin of sentence:
> > >                 "Test needs at least 9 resets completed for stable median 
> > > calculation, %d is too few\n",
> > 
> > It was never clear enough to me if the message should explain the 
> > requirement, 
> > or describe the problem.  I've always been assuming the former, and my 
> > stderr 
> > messages usually looked like this one:
> > 
> >     Test requirement: st->n_values > 8
> >     at least 9 resets completed for stable median calculation, 8 is too few
> 
> Ok, looks reasonable, so up to you, you could keep your code
> as is with my r-b here, imho this is more readable:
> 
>      Test requirement: st->n_values >= 9
>      at least 9 completed resets are needed for stable median calculation, 8 
> is too few

I like your wording better then mine.

Thanks,
Janusz

> 
> Regards,
> Kamil
> 
> > 
> > see 
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_13866/shard-mtlp-8/igt@[email protected]
> > 
> > If you think my understanding is wrong then I'll fix this as you suggest.
> > 
> > Thanks,
> > Janusz
> > 
> > 
> > > 
> > > With this
> > > Reviewed-by: Kamil Konieczny <[email protected]>
> > > 
> > > > +                     st->n_values);
> > > >  
> > > >         /*
> > > >          * Older platforms need to reset the display (incl. modeset to 
> > > > off,
> > > 
> > 
> > 
> > 
> > 
> 




Reply via email to