Re: [PATCH 3/5] drm/i915/guc: Simplify intel_guc_load()

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

<snip>

>>> +
>>> +fail:
>>> +	/*
>>> +	 * We've failed to load the firmware :(
>>> +	 *
>>> +	 * Decide whether to disable GuC submission and fall back to
>>> +	 * execlist mode, and whether to hide the error by returning
>>> +	 * zero or to return -EIO, which the caller will treat as a
>>> +	 * nonfatal error (i.e. it doesn't prevent driver load, but
>>> +	 * marks the GPU as wedged until reset).
>>> +	 */
>>> +	if (i915.enable_guc_loading > 1 || i915.enable_guc_submission > 1)
>>> +		ret = -EIO;
>>> +	else
>>> +		ret = 0;
>>> +
>>> +	if (i915.enable_guc_submission) {
>>> +		i915.enable_guc_submission = 0;
>>> +		DRM_INFO("GuC submission without firmware not supported\n");
>>> +		DRM_NOTE("Falling back from GuC submission to execlist mode\n");
>>
>> If i915.enable_guc_submission > 1 we will mark the GPU as wedged so it might
>> be worth retaining an error level message here in that scenario.
>
> If we are wedging the GPU you do not really care about the fallback, so
> theres no real use in having that promoted + those are the original
> levels that were already here.
>
> Anyway, it seems like the `enable_guc_* > 1` are likely to be gone. I've
> discussed that on IRC yesterday and no one seems to really remember why
> we've got it in the first place.
>
> Anusha posted similar concern here with her HuC series as well.
>

Just to clarify (because as you said the case will probably go away), 
what I meant was an extra log for the > 1 case like we had in the 
original code, i.e:

	DRM_ERROR("GuC init failed: %d\n", ret);

as otherwise we would have declared the GPU wedged without printing any 
error-level message to explain why.

Daniele
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx