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

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On Fri, 2017-11-03 at 12:33 +0200, Jani Nikula wrote:
> On Fri, 03 Nov 2017, Joonas Lahtinen <joonas.lahtinen@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@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.

Yep, I think I suggested them originally.

Then you only would have .enable_guc boolean for whether you want to
use GuC submission.

So I'm kinda looking forward to seeing a definitive list of what we
actually require by use-case and what we're committed to testing.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx