Hi Chris, On Tue, Oct 01, 2019 at 02:54:02PM +0100, Chris Wilson wrote: > Allow the user to restrict the available set of engines via a module > parameter.
what is the driving reason for wanting this? Could you explain it in the commit log? It feels a bit confusing, though. You are actually making a difference between engines we don't have and engines we don't want, and I don't see why. Wouldn't it make sense to either - define a new architecture (which could make things more confusing). or - have it specified in kernel configuration > Signed-off-by: Chris Wilson <[email protected]> > Cc: Stuart Summers <[email protected]> > Cc: Andi Shyti <[email protected]> > Cc: Mika Kuoppala <[email protected]> > Cc: Tvrtko Ursulin <[email protected]> > Cc: Joonas Lahtinen <[email protected]> > Cc: Martin Peres <[email protected]> > --- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 35 ++++++++++++++++------- > drivers/gpu/drm/i915/i915_gem.c | 5 ++++ > drivers/gpu/drm/i915/i915_params.c | 4 +++ > drivers/gpu/drm/i915/i915_params.h | 1 + > 4 files changed, 35 insertions(+), 10 deletions(-) Because this is a module option that is set by the user, I don't see any documentation about it. > +static bool engine_available(struct drm_i915_private *i915, int id) > +{ > + /* uAPI -- modparam bits must be consistent between kernels */ > + static const unsigned int param_bit[] = { > + [RCS0] = BIT(0), > + [VCS0] = BIT(1), > + [BCS0] = BIT(2), > + [VECS0] = BIT(3), > + [VCS1] = BIT(4), > + [VCS2] = BIT(5), > + [VCS3] = BIT(6), > + [VECS1] = BIT(7), > + }; Yet another way to refer to engines... this time inside a static function, without any visibility outside. Andi > + > + if (!HAS_ENGINE(i915, id)) > + return false; (engines we don't have... > + if (!(i915_modparams.engines & param_bit[id])) > + return false; ... engines we don't want) _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
