On 11/29/2017 5:44 PM, Michal Wajdeczko wrote:
> 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.
I think we wanted to enable HuC for some platforms but not enable GuC
submission. Anusha can you
please confirm if such cherry-picking is needed through module parameter.
>
> 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.
In that case we can just have 3 values for enable_guc (if cherry-picking
is not to be done)
-1: auto (whatever is available)
0: all disabled
1: all enabled if available else all disabled
>
> Michal