[igt-dev] [PATCH i-g-t 60/79] tests/i915/gem_ctx_create: Convert benchmarks to intel_ctx_t

[Jason Ekstrand]

On Mon, Jun 21, 2021 at 10:30 PM Dixit, Ashutosh
<ashutosh.dixit at intel.com> wrote:
>
> On Thu, 17 Jun 2021 12:14:57 -0700, Jason Ekstrand wrote:
> >
> > -static void maximum(int fd, int ncpus, unsigned mode)
> > +static void maximum(int fd, const intel_ctx_cfg_t *cfg,
> > +                 int ncpus, unsigned mode)
> >  {
> >       const uint32_t bbe = MI_BATCH_BUFFER_END;
> >       struct drm_i915_gem_execbuffer2 execbuf;
> > @@ -300,9 +302,7 @@ static void maximum(int fd, int ncpus, unsigned mode)
> >
> >               err = -ENOMEM;
> >               if (avail_mem > (count + 1) * ctx_size)
> > -                     err =  __gem_context_clone(fd, 0,
> > -                                                I915_CONTEXT_CLONE_ENGINES,
> > -                                                0, &ctx_id);
> > +                     err =  __gem_context_create(fd, &ctx_id);
> >               if (err) {
> >                       igt_info("Created %lu contexts, before failing with '%s' [%d]\n",
> >                                count, strerror(-err), -err);
> > @@ -323,6 +323,7 @@ static void maximum(int fd, int ncpus, unsigned mode)
> >
> >       igt_fork(child, ncpus) {
> >               struct timespec start, end;
> > +             const intel_ctx_t *ctx;
> >               int i915;
> >
> >               i915 = gem_reopen_driver(fd);
>
> Just a minor nit here. I'm thinking if we should remove the i915 variable
> and the gem_reopen_driver() here and just use fd? Because it seems we have
> exhausted the memory creating contexts earlier on, and now the previous
> code was creating one additional context using gem_reopen_driver() whereas
> we are creating two, one with gem_reopen_driver() and a second with
> intel_ctx_create(). Not sure if it matters or not but just in case it does
> we can get rid of gem_reopen_driver() and i915.

Wait a second... I'm not sure how this was expected to ever work.  In
the first loop, the test creates a bunch of contexts with fd.  Then,
it forks off children.  Each of those children re-open the driver and
then try to execute using said contexts on i915.  But those contexts
don't exist on the newly opened DRM device.  I don't see how this ever
worked.

--Jason

> Apart from this, this is:
>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>
>
> > @@ -331,7 +332,7 @@ static void maximum(int fd, int ncpus, unsigned mode)
> >                * a nop execbuf and stalling for it.
> >                */
> >               gem_quiescent_gpu(i915);
> > -             gem_context_copy_engines(fd, 0, i915, 0);
> > +             ctx = intel_ctx_create(i915, cfg);
> >
> >               hars_petruska_f54_1_random_perturb(child);
> >               obj[0].handle = gem_create(i915, 4096);
> > @@ -352,6 +353,7 @@ static void maximum(int fd, int ncpus, unsigned mode)
> >               gem_sync(i915, obj[0].handle);
> >               clock_gettime(CLOCK_MONOTONIC, &end);
> >               gem_close(i915, obj[0].handle);
> > +             intel_ctx_destroy(i915, ctx);