[Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] tests/gem_exec_nop: Remove submission batching

[Janusz Krzysztofik]

Hi Chris,

On Fri, 2020-05-08 at 18:54 +0100, Chris Wilson wrote:
> Quoting Janusz Krzysztofik (2020-05-08 14:56:31)
> >  static double nop_on_ring(int fd, uint32_t handle,
> >                           const struct intel_execution_engine2 *e, int timeout,
> > -                         unsigned long *out)
> > +                         unsigned long *count)
> >  {
> >         struct drm_i915_gem_execbuffer2 execbuf;
> >         struct drm_i915_gem_exec_object2 obj;
> >         struct timespec start, now;
> > -       unsigned long count;
> > +       unsigned long total;
> > +
> > +       igt_assert(*count);
> >  
> >         memset(&obj, 0, sizeof(obj));
> >         obj.handle = handle;
> > @@ -93,18 +95,18 @@ static double nop_on_ring(int fd, uint32_t handle,
> >         }
> >         intel_detect_and_clear_missed_interrupts(fd);
> >  
> > -       count = 0;
> > +       total = 0;
> >         clock_gettime(CLOCK_MONOTONIC, &start);
> >         do {
> > -               for (int loop = 0; loop < 1024; loop++)
> > +               for (int loop = 0; loop < *count; loop++)
> 
> This unnerves me. I expect to get this wrong when writing new callers.
> 
> There's no great reason to even have 1024 here, we can survive with
> doing clock_gettime() every iteration, and just accept it as part of the
> systematic cost.

Thanks for fixing this and merging the change.

Following our discussion on IRC which I probably didn't understand
precisely enough but maybe have a better understanding now after having
it reread, how about still addressing the "headless" requirement for
better precision with the following hunk?

@@ -373,10 +372,12 @@ stable_nop_on_ring(int fd, uint32_t handle,
 	while (reps--) {
 		unsigned long count;
 		double time;
 
 		time = nop_on_ring(fd, handle, e, timeout, &count);
+		igt_skip_on_f(count < 1000 * timeout,
+			      "submicrosecond precision of time measurement\n");
 		igt_stats_push_float(&s, time / count);
 	}
 
 	n = igt_stats_get_median(&s);
 	igt_stats_fini(&s);

Thanks,
Janusz


> >  
> > -               count += 1024;
> > +               total += *count;
> >                 clock_gettime(CLOCK_MONOTONIC, &now);
> >         } while (elapsed(&start, &now) < timeout);
> >         igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
> >  
> > -       *out = count;
> > +       *count = total;
> >         return elapsed(&start, &now);