[Intel-gfx] [PATCH] drm/i915/GuC: Combine the two kernel parameter into one

[Srivatsa, Anusha]

>-----Original Message-----
>From: Tvrtko Ursulin [mailto:tvrtko.ursulin at linux.intel.com]
>Sent: Monday, November 14, 2016 1:35 AM
>To: Mcgee, Jeff <jeff.mcgee at intel.com>; Srivatsa, Anusha
><anusha.srivatsa at intel.com>
>Cc: Ursulin, Tvrtko <tvrtko.ursulin at intel.com>; intel-gfx at lists.freedesktop.org;
>Vivi, Rodrigo <rodrigo.vivi at intel.com>
>Subject: Re: [Intel-gfx] [PATCH] drm/i915/GuC: Combine the two kernel
>parameter into one
>
>
>[corrected my email in cc]

Thankyou!

>On 12/11/2016 02:21, Jeff McGee wrote:
>> On Wed, Nov 09, 2016 at 11:11:07AM -0800, Anusha Srivatsa wrote:
>>> Replace i915.enable_guc_loading and i915.enable_guc_submission with a
>>> single parameter - i915.enable_guc. Where:
>>>
>>> -1 : Platform default (Only load GuC)
>>> 0 : Do not use GuC
>>> 1 : Load GuC, do not use Command Submission through GuC
>>> 2 : Load and use GuC for Command Submission
>>>
>> I think this approach could get ugly as we add more GuC functionality
>> and the list of combinations under this one parameter grows.
Yes I see your point.

>> What is the issue we are trying to solve? I thought it was that folks
>> didn't like that we had an option to just load GuC and do nothing with it.
>> Can those folks please comment?

Two parameters was not desired. One parameter that could control the GuC functionality was something that led to this patch.

>I am not one of those folks but I also am sure about the proposed change. Same
>concern about extensibility and also usability.
>
>What is the difference between -1 and 1 for example? Is 1 equivalent to the
>current "must use" (2) option? And -1 is equivalent to the current "try to use" (1)?
Right now -1 is for platform default and 1 is for loading and no submission. -1: platform default for now is set to do only loading of GuC firmware, unless we change that. For now both 1 and -1 are the same.
In future say if for SKL we make loading and submission as default, the code has to be changes slightly.

>Then you got proposed 2 (load and use guc) but that does not capture the option
>for built-in GuC firmware Dave has planned for in his version. I don't know if that
>is real or not, just saying
>
>I am also not sure it is so imperative to change this at all. But if people do want to
>change it we should make it really good. :)
I agree. I sent this patch to see what people feel about it and if we have to go ahead with these changes.

>One idea could be to hide the guc loading form the user altogether and hence
>improve usability (decrease exposed complexity) by having only two parameters;
>i915.enable_guc_scheduling and i915.enable_huc.
That's a good point. But with this we will have two parameters (which kills the point of why the patch was written in the first place), then we can rather leave it the way it is. Right?

-Anusha

>Whether or not firmware would be loaded would depend on if either of the two is
>turned on. That would also preserve the option of current fallback or wedge
>behaviour for guc.
>
>Regards,
>
>Tvrtko