On Thu, Jun 3, 2021 at 12:23 AM Jason Ekstrand <jason@xxxxxxxxxxxxxx> wrote:
>
> On Mon, May 31, 2021 at 4:12 AM Daniel Vetter <daniel@xxxxxxxx> 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?
> 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?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
