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

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

 




On 15/11/2016 22:46, Jeff McGee wrote:
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 was wondering about the failure mode - or in other words does it suggest in the error message what might be the problem so it is helpful for the user. Let me check.. I did not find anything - what it will say? Just generic firmware load fetch messages with a timeout / error?

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?

I think it is OK to change them. But there would have to be wider agreement on the end result which I think means soliciting opinions from maintainers and domain owners.

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)?

Yes I was wondering myself. Don't know really. Dave is not around any more so all we could do is trawl the mailing list archives in case there is something there.

Regards,

Tvrtko
_______________________________________________
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