Re: [PATCH 13/39] drm/i915: Extend I915_CONTEXT_PARAM_SSEU to support local ctx->engine[]

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Tvrtko Ursulin (2019-03-14 16:49:54)
> 
> On 13/03/2019 14:43, Chris Wilson wrote:
> > Allow the user to specify a local engine index (as opposed to
> > class:index) that they can use to refer to a preset engine inside the
> > ctx->engine[] array defined by an earlier I915_CONTEXT_PARAM_ENGINES.
> > This will be useful for setting SSEU parameters on virtual engines that
> > are local to the context and do not have a valid global class:instance
> > lookup.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_gem_context.c | 24 ++++++++++++++++++++----
> >   include/uapi/drm/i915_drm.h             |  3 ++-
> >   2 files changed, 22 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 07377b75b563..7ae28622b709 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -1375,6 +1375,7 @@ static int set_sseu(struct i915_gem_context *ctx,
> >       struct drm_i915_gem_context_param_sseu user_sseu;
> >       struct intel_engine_cs *engine;
> >       struct intel_sseu sseu;
> > +     unsigned long lookup;
> 
> I'd prefer unsigned int.

Why not register size for what should only be in a register and matches
usage with BIT()?

> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index 6dde864e14e7..e17c7375248c 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -1567,9 +1567,10 @@ struct drm_i915_gem_context_param_sseu {
> >       __u16 engine_instance;
> >   
> >       /*
> > -      * Unused for now. Must be cleared to zero.
> > +      * Unknown flags must be cleared to zero.
> >        */
> >       __u32 flags;
> > +#define I915_CONTEXT_SSEU_FLAG_ENGINE_INDEX (1u << 0)
> >   
> >       /*
> >        * Mask of slices to enable for the context. Valid values are a subset
> > 
> 
> Which patch has this hunk:
> 
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -1185,7 +1185,6 @@ i915_gem_context_reconfigure_sseu(struct 
> i915_gem_context *ctx,
>         int ret = 0;
> 
>         GEM_BUG_ON(INTEL_GEN(ctx->i915) < 8);
> -       GEM_BUG_ON(engine->id != RCS0);
> 
> I think it should go into this one.

That's in load balancing (introduction of virtual engine) itself.
Consider it done.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx