[Intel-gfx] [PATCH 1/2] drm/i915: Use a modparam to restrict exposed engines

[Chris Wilson]

Quoting Andi Shyti (2019-10-05 11:10:45)
> 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's to allow the user to restrict the available set of engines, for
whatever reason they may have, presumably because they want to work
around some HW stability (or if that is the case to enable some HW
despite instability, since we should err on the side of caution).

> 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).

No, it's an optional overlay of an existing under the whim of the user.
 
> or
> 
>  - have it specified in kernel configuration

Is how we configure the defaults in reduced configurations.
 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Stuart Summers <stuart.summers at intel.com>
> > Cc: Andi Shyti <andi.shyti at intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Martin Peres <martin.peres at linux.intel.com>
> > ---
> >  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.

It's in i915_params.c

> > +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.

Sure. It's a static mapping table that has to stable for generations to
come because it is described in i915_params as part of the i915.engines
contract.
-Chris