[PATCH 21/29] drm/i915/gem: Use the proto-context to handle create parameters (v2)

[Jason Ekstrand]

On Thu, Jun 3, 2021 at 2:32 AM Daniel Vetter <daniel at ffwll.ch> wrote:
>
> On Thu, Jun 3, 2021 at 12:23 AM Jason Ekstrand <jason at jlekstrand.net> wrote:
> >
> > On Mon, May 31, 2021 at 4:12 AM Daniel Vetter <daniel at ffwll.ch> wrote:
> > >
> > > On Thu, May 27, 2021 at 11:26:42AM -0500, Jason Ekstrand wrote:
> > > > +static int set_proto_ctx_engines(struct drm_i915_file_private *fpriv,
> > > > +                              struct i915_gem_proto_context *pc,
> > > > +                              const struct drm_i915_gem_context_param *args)
> > > > +{
> > > > +     struct drm_i915_private *i915 = fpriv->dev_priv;
> > > > +     struct set_proto_ctx_engines set = { .i915 = i915 };
> > > > +     struct i915_context_param_engines __user *user =
> > > > +             u64_to_user_ptr(args->value);
> > > > +     unsigned int n;
> > > > +     u64 extensions;
> > > > +     int err;
> > > > +
> > > > +     if (!args->size) {
> > > > +             proto_context_free_user_engines(pc);
> > > > +             memset(&pc->legacy_rcs_sseu, 0, sizeof(pc->legacy_rcs_sseu));
> > >
> > > Hm I wonder whether we shouldn't put this into the cleanup helper, and
> > > then maybe call it proto_context_reset_user_engines()? I think that makes
> > > the entire user engines vs sseu flow a notch clearer again.
> >
> > I fought with myself over this.  The other two callers of
> > free_user_engines() would be fine with clearing out the SSEU as well,
> > I think, but neither of them need it.  I erred on the side of putting
> > it in the one place it's actually needed to make it clear what's going
> > on here.  I can move it if you'd like.
>
> So I'm wondering about semantics here a bit, and whether this is all
> real, as in, used in real userspace:
>
> Instead of resetting engines here, shouldn't we just complain if
> there's more than one engines_set command, ever, on a context?

I don't think it's ever used.  Let's kill it.

> > As a bit of a P.S., I really hate the SSEU handling.  It's horrible.
> > If I had it to do all over again, SSEU would be a purly dynamic
> > context param that you aren't allowed to set at create time.  But,
> > sadly, we're in the mess we're in. :-(
>
> Yeah it's rather annoying. If we go with "only one engines_set per ctx
> create", then maybe we could streamline the SSEU stuff some more too?

It certainly gets rid of this weird corner.

--Jason