Re: [igt-dev] [PATCH i-g-t 71/79] lib/i915: Rework engine API availability checks (v2)

[Jason Ekstrand <jason@jlekstrand.net>]

On Mon, Jun 28, 2021 at 2:56 PM Dixit, Ashutosh
<ashutosh.dixit@intel.com> wrote:
>
> On Thu, 17 Jun 2021 12:15:08 -0700, Jason Ekstrand wrote:
> >
> >  bool gem_has_engine_topology(int fd)
> >  {
> > -     struct drm_i915_gem_context_param param = {
> > -             .param = I915_CONTEXT_PARAM_ENGINES,
> > -     };
> > -
> > -     return !__gem_context_get_param(fd, &param);
> > +     struct intel_engine_data ed;
> > +     return !__query_engine_list(fd, &ed);
>
> Is this correct? Isn't this actually a check for the presence of
> I915_CONTEXT_PARAM_ENGINES (seems so looking from which this is called)?

You're right that the check here is slightly different.  The old check
queried the engine set on ctx0 but the new check queries the global
engine set.  The three callers are gem_ctx_engines, gem_exec_balancer,
and gem_exec_schedule, all of which really want "is the engines API
available?"  I don't think it matters which one we query from a kernel
support POV.  Every kernel that has the global engines query should
have the CONTEXT_PARAM_ENGINES and vice versa.

> So we could probably just create a new context and do a set_engines on it and
> return true if that succeeded?

Yes, we could do that.  It's just a lot more annoying to get right
because we first have to query the engine set to get a valid engine to
attempt to set.  I'm not sure there's a point.

Or do that in addition to
> __query_engine_list? Maybe intel_ctx_create_all_physical() will do both?

Not sure what you mean by this.

--Jason

> Thanks.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev