[Intel-gfx] [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check

[Tvrtko Ursulin]

On 07/06/16 09:41, Tvrtko Ursulin wrote:
>
> On 07/06/16 09:14, Dave Gordon wrote:
>> The last stage of the GuC loader also sanitises the GuC submission
>> settings, so should be called unconditionally (even on platforms
>> without a GuC) to ensure consistent settings; in particular, this
>> prevents any attempt to use GuC submission on GuCless platforms!
>>
>> Also fix error path handling and clarify DRM_INFO fallback message.
>>
>> Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_gem.c         |  8 +++-----
>>   drivers/gpu/drm/i915/intel_guc_loader.c | 12 ++++++++----
>>   2 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> b/drivers/gpu/drm/i915/i915_gem.c
>> index 1bfc260..eae8d7a 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -4930,11 +4930,9 @@ int i915_gem_init_engines(struct drm_device *dev)
>>       intel_mocs_init_l3cc_table(dev);
>>
>>       /* We can't enable contexts until all firmware is loaded */
>> -    if (HAS_GUC(dev)) {
>> -        ret = intel_guc_setup(dev);
>> -        if (ret)
>> -            goto out;
>> -    }
>> +    ret = intel_guc_setup(dev);
>> +    if (ret)
>> +        goto out;
>>
>>       /*
>>        * Increment the next seqno by 0x100 so we have a visible break
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index f2b88c7..4e34c2e 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -425,9 +425,13 @@ int intel_guc_setup(struct drm_device *dev)
>>       if (!i915.enable_guc_loading) {
>>           err = 0;
>>           goto fail;
>> -    } else if (fw_path == NULL || *fw_path == '\0') {
>> -        if (*fw_path == '\0')
>
> Ops. I can only assume I meant !fw_path.
>
>> -            DRM_INFO("No GuC firmware known for this platform\n");
>> +    } else if (fw_path == NULL) {
>> +        /* Device is known to have no uCode (e.g. no GuC) */
>> +        err = -ENXIO;
>> +        goto fail;
>> +    } else if (*fw_path == '\0') {
>> +        /* Device has a GuC but we don't know what f/w to load? */
>> +        DRM_INFO("No GuC firmware known for this platform\n");
>>           err = -ENODEV;
>>           goto fail;
>>       }
>> @@ -535,7 +539,7 @@ int intel_guc_setup(struct drm_device *dev)
>>           if (fw_path == NULL)
>>               DRM_INFO("GuC submission without firmware not
>> supported\n");
>>           if (ret == 0)
>> -            DRM_INFO("Falling back to execlist mode\n");
>> +            DRM_INFO("Falling back from GuC submission to execlist
>> mode\n");
>>           else
>>               DRM_ERROR("GuC init failed: %d\n", ret);
>>       }
>>
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Bah now on BDW we get on boot:

[drm:gen8_init_common_ring] Execlists enabled for render ring
[drm:gen8_init_common_ring] Execlists enabled for blitter ring
[drm:gen8_init_common_ring] Execlists enabled for bsd ring
[drm:gen8_init_common_ring] Execlists enabled for video enhancement ring
[drm:intel_guc_setup] GuC fw status: path (null), fetch NONE, load NONE
[drm] GuC firmware load skipped
[drm] GuC submission without firmware not supported
[drm] Falling back from GuC submission to execlist mode

Which is a bit untidy. :(

Regards,

Tvrtko