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

[Andi Shyti]

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

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