Re: [PATCH] drm/i915/GuC: Combine the two kernel parameter into one

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>-----Original Message-----
>From: Mcgee, Jeff
>Sent: Tuesday, November 15, 2016 2:46 PM
>To: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>
>Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>; Ursulin, Tvrtko
><tvrtko.ursulin@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Vivi, Rodrigo
><rodrigo.vivi@xxxxxxxxx>
>Subject: Re:  [PATCH] drm/i915/GuC: Combine the two kernel
>parameter into one
>
>On Tue, Nov 15, 2016 at 10:06:47AM -0800, Srivatsa, Anusha wrote:
>>
>>
>> >-----Original Message-----
>> >From: Tvrtko Ursulin [mailto:tvrtko.ursulin@xxxxxxxxxxxxxxx]
>> >Sent: Tuesday, November 15, 2016 2:31 AM
>> >To: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>; Mcgee, Jeff
>> ><jeff.mcgee@xxxxxxxxx>
>> >Cc: Ursulin, Tvrtko <tvrtko.ursulin@xxxxxxxxx>;
>> >intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Vivi, Rodrigo
>> ><rodrigo.vivi@xxxxxxxxx>
>> >Subject: Re:  [PATCH] drm/i915/GuC: Combine the two kernel
>> >parameter into one
>> >
>> >
>> >On 14/11/2016 17:34, Srivatsa, Anusha wrote:
>> >
>> >[snip]
>> >
>> >>> 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?
>> >
>> >For some reason I thought the HuC patch series add a another module
>> >parameter.
>> >
>> >What is the failure mode for HuC is GuC firmware loading is disabled btw?
>>
>>
>> Hi Tvrtko, in the function intel_guc_auth_huc() there is a check to see if GuC is
>loaded or not. If GuC loading has failed or loading is disabled, then HuC
>authentication does not happen.
>>
>Yes, GuC must authenticate HuC firmware.
>
>I am in favor of Tvrtko's suggestion for dropping i915.enable_guc_loading,
>keeping i915.enable_guc_submission, and adding i915.enable_huc. If either of
>the last two are enabled, GuC loading is implied. So kernel parameters are tied to
>enabling specific functionality. I think the specific parameter for loading is legacy
>from the first hurdle for GuC long ago. I assume we are not bound by ABI to keep
>that around if it is no longer needed, yes?
 Right now we don't have any parameter for HuC, so why introduce one? We can keep guc_submission, if it is enabled then loading is implied which means HuC should have gotten authenticated. 
Also, why not just make guc loads default? Guc_submission should be the only parameter with which we can enable or disable submission.
In other words, if a platform has a guc, then load it. 
>The other thing I would want to reconsider is the "casual" enable vs. "force"
>enable options. Does anyone remember why these 2 levels of enable have been
>used. Maybe this is also a legacy. Can we just do a auto (-1), disable (0), and
>enable (1)?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux