[Intel-gfx] [PATCH v10 1/2] drm/i915/guc : Removing enable_guc_loading and enable_guc_submission module parameters

[Michal Wajdeczko]

On Tue, 28 Nov 2017 11:41:57 +0100, Sagar Arun Kamble  
<sagar.a.kamble at intel.com> wrote:

>
>
> On 11/28/2017 1:24 AM, Sujaritha Sundaresan 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).
>>

<SNIP>

>> +
>> +	/* Making sure that our auto is well defined */
>> +	GEM_BUG_ON(auto_enable_guc < 0);
>> +	GEM_BUG_ON((auto_enable_guc > 0) && !HAS_GUC_FW(dev_priv));
>> +	GEM_BUG_ON((auto_enable_guc & 2) && !HAS_HUC_FW(dev_priv));
>> +
>> +	if (i915_modparams.enable_guc < 0)
>> +		i915_modparams.enable_guc = auto_enable_guc;
>> +
>> +	if (i915_modparams.enable_guc > 0) {
>> +		if (!HAS_GUC(dev_priv) || !HAS_GUC_FW(dev_priv)) {
>> +			DRM_INFO("Ignoring option enable_guc=%d - %s\n",
>> +					 i915_modparams.enable_guc,
>> +				     !HAS_GUC(dev_priv) ? "no GuC hardware" :
>> +					 "no GuC firmware");
>> +			i915_modparams.enable_guc = 0;
>> +		}
>>   	}
>>   -	/* A negative value means "use platform default" */
>> -	if (i915_modparams.enable_guc_loading < 0)
>> -		i915_modparams.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
>> -
>> -	/* Verify firmware version */
>> -	if (i915_modparams.enable_guc_loading) {
>> -		if (HAS_HUC_UCODE(dev_priv))
>> -			intel_huc_select_fw(&dev_priv->huc);
>> -
>> -		if (intel_guc_fw_select(&dev_priv->guc))
>> -			i915_modparams.enable_guc_loading = 0;
>> +	if (i915_modparams.enable_guc & 2) {
>> +		if (!HAS_HUC_FW(dev_priv)) {
>> +			DRM_INFO("Ignoring option enable_guc=%d - %s\n",
>> +				i915_modparams.enable_guc, "no HuC firmware");
>> +			i915_modparams.enable_guc = 0;
> Clearing only HuC status from enable_guc would be proper I guess. Sorry  
> I did not see this earlier.

My understanding is that if user had specified non-zero positive value of
'enable_guc' then it means that he requests *all* GuC options to be  
available
(something like our old '2=required' mode). If any of required option is  
not
available then we should not try to cherry-pick/drop single option.

Note that if user don't care about specific option, then user should use  
-1(auto)
mode and rely on driver decision what is available and in which  
configuration.
And for 'auto' mode we try to make sure to do not select broken  
configurations.

Michal