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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
