[Intel-gfx] [PATCH 47/47] drm/i915/guc: Unblock GuC submission on Gen11+

[Martin Peres]

On 02/07/2021 16:06, Michal Wajdeczko wrote:
> 
> 
> On 02.07.2021 10:13, Martin Peres wrote:
>> On 01/07/2021 21:24, Martin Peres wrote:
>> [...]
>>>>
>>>>>
>>>>>> +        i915->params.enable_guc = ENABLE_GUC_LOAD_HUC;
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    /* Default: enable HuC authentication and GuC submission */
>>>>>> +    i915->params.enable_guc = ENABLE_GUC_LOAD_HUC |
>>>>>> ENABLE_GUC_SUBMISSION;
>>>>>
>>>>> This seems to be in contradiction with the GuC submission plan which
>>>>> states:
>>>>>
>>>>> "Not enabled by default on any current platforms but can be enabled via
>>>>> modparam enable_guc".
>>>>>
>>>>
>>>> I don't believe any current platform gets this point where GuC
>>>> submission would be enabled by default. The first would be ADL-P which
>>>> isn't out yet.
>>>
>>> Isn't that exactly what the line above does?
>>
>> In case you missed this crucial part of the review. Please answer the
>> above question.
> 
> I guess there is some misunderstanding here, and I must admit I had
> similar doubt, but if you look beyond patch diff and check function code
> you will find that the very condition is:
> 
> 	/* Don't enable GuC/HuC on pre-Gen12 */
> 	if (GRAPHICS_VER(i915) < 12) {
> 		i915->params.enable_guc = 0;
> 		return;
> 	}
> 
> so all pre-Gen12 platforms will continue to have GuC/HuC disabled.

Thanks Michal, but then the problem is the other way: how can one enable 
it on gen11?

I like what Daniele was going for here: separating the capability from 
the user-requested value, but then it seems the patch stopped half way. 
How about never touching the parameter, and having a AND between the two 
values to get the effective enable_guc?

Right now, the code is really confusing :s

Thanks,
Martin

> 
> Thanks,
> Michal
>