[Intel-gfx] [PATCH 08/13] drm/i915: Allow a context to define its set of engines

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-03-11 16:34:41)
> 
> On 11/03/2019 16:22, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-03-11 16:16:27)
> >>
> >> On 11/03/2019 14:45, Chris Wilson wrote:
> >>> Quoting Chris Wilson (2019-03-11 09:45:17)
> >>>> Quoting Tvrtko Ursulin (2019-03-11 09:23:44)
> >>>>>
> >>>>> On 08/03/2019 16:47, Chris Wilson wrote:
> >>>>>> Quoting Tvrtko Ursulin (2019-03-08 16:27:22)
> >>>>>>>
> >>>>>>> On 08/03/2019 14:12, Chris Wilson wrote:
> >>>>>>>> +static int
> >>>>>>>> +set_engines(struct i915_gem_context *ctx,
> >>>>>>>> +         const struct drm_i915_gem_context_param *args)
> >>>>>>>> +{
> >>>>>>>> +     struct i915_context_param_engines __user *user;
> >>>>>>>> +     struct set_engines set = { .ctx = ctx };
> >>>>>>>> +     u64 size, extensions;
> >>>>>>>> +     unsigned int n;
> >>>>>>>> +     int err;
> >>>>>>>> +
> >>>>>>>> +     user = u64_to_user_ptr(args->value);
> >>>>>>>> +     size = args->size;
> >>>>>>>> +     if (!size)
> >>>>>>>> +             goto out;
> >>>>>>>
> >>>>>>> This prevents a hypothetical extension with empty map data.
> >>>>>>
> >>>>>> No... This is required for resetting and I think that's covered in what
> >>>>>> little docs there are. It's the set.nengine==0 test later
> >>>>>> that you mean to object to. But we can't do that as that's how we
> >>>>>> differentiate between modes at the moment.
> >>>>>>
> >>>>>> We could use ctx->nengine = 0 and ctx->engines = ZERO_PTR.
> >>>>>
> >>>>> size == sizeof(struct i915_context_param_engines) could mean reset -
> >>>>> meaning no map array provided.
> >>>>
> >>>> Nah, size=sizeof() => 0 [], size=0 => default map.
> >>>>    
> >>>>> Meaning one could reset the map and still pass in extensions.
> >>>>
> >>>> I missed that you were pointing out we didn't follow the extensions on
> >>>> resetting.
> >>>>
> >>>> I'm not sure if that makes sense tbh. The extensions are written around
> >>>> the concept of applying to the new engines[], and if the use has
> >>>> explicitly removed the engines[] (distinct from defining a zero array),
> >>>> what extensions can apply? One hopes they end up -EINVAL. As they should
> >>>> -EINVAL, I guess it is no harm done to apply them.
> >>>
> >>> Ok, the problem with the size=0 case is that quite obviously there is no
> >>> extension chain to follow. (That was silly of me.)
> >>>
> >>> I think
> >>>        .size = 0 => reset to default
> >>> and
> >>>        .size = sizeof(arg) => 0 engines ([])
> >>
> >> What is the difference between these two?
> > 
> > One uses the legacy ring mask, and the other is a context with no
> > engines.
> 
> The latter is hypothetical, no? Current patches don't have this ability 
> AFAIR. Why would we want this?

They did the moment you asked for it. Do you not recall? :-p
-Chris