[Intel-gfx] [PATCH v8 2/6] drm/i915/guc : Removing i915_modparams.enable_guc_loading module parameter

[Jani Nikula]

On Fri, 03 Nov 2017, Joonas Lahtinen <joonas.lahtinen at linux.intel.com> wrote:
> On Thu, 2017-11-02 at 09:34 -0700, Sujaritha wrote:
>> 
>> On 10/25/2017 08:26 AM, Michal Wajdeczko wrote:
>> > On Tue, 24 Oct 2017 19:21:21 +0200, Sujaritha Sundaresan 
>> > <sujaritha.sundaresan at intel.com> wrote:
>> > 
>> > > We currently have two module parameters that control GuC:
>> > > "enable_guc_loading" and "enable_guc_submission". Whenever
>> > > we need submission=1, we also need loading=1.We also need
>> > > loading=1 when we want to want to verify the HuC, which
>> > > is every time we have a HuC (but all platforms with HuC
>> > > have a GuC and viceversa).
>> > > 
>> > > Also if we have HuC have firmware to be loaded, we need to
>> > > have GuC to actually load it. So if the user wants to avoid
>> > > the GuC from getting loaded, they must not have a HuC
>> > > firmware to be loaded, in addition to not using submission.
>> > 
>> > Hmm, I'm not sure that removal of HuC firmware file is the best
>> > way for the user to stop undesired GuC loading.
>> > 
>> > I know that we want to minimize number of modparams, but maybe
>> > new i915.enable_huc=auto(-1)|never(0)|if available(1)|required(2)
>> > will solve here ...
>> > 
>> > Alternatively we can replace both existing modparams with single:
>> > 
>> > i915.enable_guc = off(0) | auto(1) | submission(2) | huc(4)
>> > 
>> > then we could cover almost all cases:
>> > 
>> > 0 = GuC loading disabled (no GuC submission, no HuC)
>> > 1 = GuC loading auto
>> > 2 = GuC loading enabled, GuC submission required, HuC disabled
>> > 3 = GuC loading enabled, GuC submission enabled,  HuC disabled
>> > 4 = GuC loading enabled, GuC submission disabled, HuC required
>> > 5 = GuC loading enabled, GuC submission disabled, HuC enabled
>> > 6 = GuC loading enabled, GuC submission required, HuC required
>> > 7 = GuC loading enabled, GuC submission enabled,  HuC enabled
>
> Do we really need all these combinations.

Ugh, I hope not. Pick the combinations you're committed to testing. If
it's not tested, it doesn't exist.

Side note, you also have guc_firmware_path and huc_firmware_path
options.

BR,
Jani.

> My understanding is that we got three cases:
>
> 1. Load and use GuC, HuC goes on the side
> 2. Load GuC, just to get HuC
> 3. Don't load GuC at all
>
> Which could be mapped to .enable_guc:
>
> -1 = default (driver does as sees fit)
> 0 = no GuC, no nothing
> 1 = load and use GuC, HuC comes on the side
> 2 = Load GuC only, for HuC
>
> Or if you want just the GuC without HuC at times, then
>
> 0x1 = Use GuC
> 0x2 = Use Huc
>
> Loading is then implied. Somebody could remind me why we should
> consider required, disabled vs. enabled options?
>
> Regards, Joonas

-- 
Jani Nikula, Intel Open Source Technology Center