[Intel-gfx] [PATCH v6 5/8] drm/i915/pxp: Add ARB session creation and cleanup

[Tvrtko Ursulin]

On 30/03/2023 20:44, Teres Alexis, Alan Previn wrote:
> On Thu, 2023-03-30 at 13:25 +0100, Tvrtko Ursulin wrote:
>> On 30/03/2023 01:10, Teres Alexis, Alan Previn wrote:
>>> On Wed, 2023-03-29 at 08:43 +0100, Tvrtko Ursulin wrote:
>>>> On 28/03/2023 18:52, Rodrigo Vivi wrote:
>>>>> On Tue, Mar 28, 2023 at 05:01:36PM +0000, Teres Alexis, Alan Previn wrote:
>>>>>> On Mon, 2023-03-27 at 17:15 +0100, Tvrtko Ursulin wrote:
> alan:snip (excuse my snips - my evolution keeps inserting CRs - still looking for solution)
>> But intuitively I thought that what Mesa wants is a no-cost getparam
>> which would somewhat reliably tell it if the feature is supposed to be
>> there and context create at a later stage, with the protected flag set,
>> is supposed to work. AFAIU it can still fail at that point or probably
>> block until the required setup is done.
> Yes - that's right - i had another round of discussions with Daniele about a cleaner approach - below..
> alan:snip
>> Even 200ms is possibly not good enough since boot time targets (to UI
>> AFAIR) are pretty tight. Don't know... Maybe I'd need a timeline diagram
>> showing the involved components to understand this properly.
> Absolutely, my experiences in i915 on embedded products even had PORs of <1000milisec to first-fully-renderered-display from cold-boot so yes, we need to work with this requirement
> in mind and do testing on real customer stack.
> 
> I spoke to Daniele and we have another idea - but would also impact mesa, for the better:
> 
> 1. Introduce get-param (is_PXP_avail)
> 	- will return a simple yes or no
> 		- yes means : i915-device-info supports it, kernel configs supports it and required-firmwares were found (not necessarily loaded/init yet).
> 			(NOTE: this would be made to hook up to pxp helpers such as intel_pxp_is_supported)
> 2. Gem-pxp-context-creation continues blocking like today with minor tweak:
> 	(same)- success = all dependencies are in place, all firmware init completed, pxp arb session successfully completed.
> 	(same)- non-success -ENODEV = if any dependency wasnt available or fw failed to create arb-session due to fw-init-failure/BIOS/platform config.
> 	(tweak)- non-success -ENXIO (or some other -E'FOO') if component-driver-init or firmware-init is still pending after brief timeout.
> 		- on timeout - TBD - need testing/debug on real world stack.
> 		- UAPI spec needs update but pxp implementation currently uses -ENXIO for similiar reason inheritted first merge.
> 
> Thus, with this: Get param would always be immediate. Pxp-context-creation would only block when all dependencies are in place and we attempt to create the pxp arb session.
> (firmware can take up to 200-milisecs, according to MTL spec, so I'd say ~210 given other overheads between i915 and fw and back).
> We would need to change MESA-get-caps to use get-param (and not pxp-context-creation) as it would always return immediately with kernel side support.
> And if application explicitly requires PXP support, then it needs to call pxp-context-creation that may block or require retry.

The above sounds good to me.

I am only not 100% clear on the ENODEV option from context create, does 
it include even things which can be detected without any timeouts at 
probe time, or just failures which take time to learn about.

> WRT to fast-boot-to-first-frame, I am hoping real customer stack doesn't require PXP on the compositor and first mesa instance works fine without PXP caps.
> And when customer apps that needs PXP starts, it would create pxp context which would block but the app would not have a choice.

Yeah that sounds like an unlikely use case and one that we cannot 
improve on the kernel or uapi side.

(I can imagine resuming directly into a full screen video playback post 
suspend, but a cold boot into it is a stretch.)

Regards,

Tvrtko