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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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:
>>> These two:
>>> e6177ec586d1 ("drm/i915/huc: stall media submission until HuC is loaded")
>>> b76c14c8fb2a ("drm/i915/huc: better define HuC status getparam possible return values.")
>>> They do not help here? It is not possible to use or extend the refined I915_PARAM_HUC_STATUS return values combined with huc load fence for this all to keep working?
>> Checking is-huc-loaded won't reflect is-pxp-available (in case fw/fusing isn't allowing it). But the connection between them is hw-internal so i915 asking PXP-fw to attempt a PXP
>> session depends on HuC (and the 3 other things i mentioned). However, Tvrtko's point on using fences-or-equivalent is the same thing John Harrison brought up offline with Daniele
>> as the proper kernel way to do this type of dependency checking. However, any form of dependency-checking won't improve UMD's experience. We still need to decide if i915-PXP should
>> wait-in-kernel or return some-new-spec-error. A useful data point: we are debugging a related issue on actual customer stack. The compositor using mesa is hitting this code path
>> very early.. even before gsc-proxy component is loaded and we are trying to figure out why delaying inside intel_pxp_start is not helping (more delays causes the gsc-proxy
>> component to also get delayed) but that is a different conversation. I'm only mentioning this because we have a strict requirement to get the desktop login window up within 1-2
>> seconds of bootloader->kernel handoff. That said, if use -EAGAIN, I'm not sure if that would work as it would delay the compositor startup beyond the typical end user experience
>> unless MESA has a timeout to give up on a cap-testing when seeing repeated -EAGAIN (doubt mesa folks like this?). Perhaps we could just immediately return with a different error
>> (instead of current PXP-UAPI spec of -EINVAL or -ENODEV)... perhaps use -ENXIO which apparently is already part of the original pxp code but was never mentioned in UAPI - but we
>> return this immediately and document it in UAPI as "hw/fw insfratructure is not yet ready to create pxp arb session, user space can retry but may need delays of up to x-seconds on
>> ADl/TGL or y-seconds on MTL, before getting a SUCCESS or one of the other errors).
> 
> fair enough. It looks like we need a new get_param! :)

To check I understood this - new parameter would be like "is pxp support 
present"?

And then later it can still fail to initialize due some parameters not 
easily detectable during boot/probe?

How will the context create path look like on those platforms:

1. Block, then potentially error out if the full initialization failed.

2. Error out "in progress" while initializing, error out "something 
else" if initialization failed.

?

Regards,

Tvrtko