Re: [PATCH] drm/i915/GuC: Combine the two kernel parameter into one

From: Jeff McGee <jeff.mcgee@intel.com>
To: "Srivatsa, Anusha" <anusha.srivatsa@intel.com>
Cc: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Subject: Re: [PATCH] drm/i915/GuC: Combine the two kernel parameter into one
Date: Tue, 15 Nov 2016 14:46:28 -0800	[thread overview]
Message-ID: <20161115224628.GC18646@jeffdesk> (raw)
In-Reply-To: <83F5C7385F545743AD4FB2A62F75B07301938452@ORSMSX108.amr.corp.intel.com>

On Tue, Nov 15, 2016 at 10:06:47AM -0800, Srivatsa, Anusha wrote:
> 
> 
> >-----Original Message-----
> >From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
> >Sent: Tuesday, November 15, 2016 2:31 AM
> >To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; Mcgee, Jeff
> ><jeff.mcgee@intel.com>
> >Cc: Ursulin, Tvrtko <tvrtko.ursulin@intel.com>; intel-gfx@lists.freedesktop.org;
> >Vivi, Rodrigo <rodrigo.vivi@intel.com>
> >Subject: Re: [Intel-gfx] [PATCH] drm/i915/GuC: Combine the two kernel
> >parameter into one
> >
> >
> >On 14/11/2016 17:34, Srivatsa, Anusha wrote:
> >
> >[snip]
> >
> >>> One idea could be to hide the guc loading form the user altogether
> >>> and hence improve usability (decrease exposed complexity) by having
> >>> only two parameters; i915.enable_guc_scheduling and i915.enable_huc.
> >> That's a good point. But with this we will have two parameters (which kills the
> >point of why the patch was written in the first place), then we can rather leave it
> >the way it is. Right?
> >
> >For some reason I thought the HuC patch series add a another module
> >parameter.
> >
> >What is the failure mode for HuC is GuC firmware loading is disabled btw?
> 
> 
> Hi Tvrtko, in the function intel_guc_auth_huc() there is a check to see if GuC is loaded or not. If GuC loading has failed or loading is disabled, then HuC authentication does not happen.
> 
Yes, GuC must authenticate HuC firmware.

I am in favor of Tvrtko's suggestion for dropping i915.enable_guc_loading,
keeping i915.enable_guc_submission, and adding i915.enable_huc. If either of
the last two are enabled, GuC loading is implied. So kernel parameters are tied
to enabling specific functionality. I think the specific parameter for loading
is legacy from the first hurdle for GuC long ago. I assume we are not bound by
ABI to keep that around if it is no longer needed, yes?

The other thing I would want to reconsider is the "casual" enable vs. "force"
enable options. Does anyone remember why these 2 levels of enable have been
used. Maybe this is also a legacy. Can we just do a auto (-1), disable (0),
and enable (1)?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx