Re: [Intel-gfx] [PATCH v9 6/8] drm/i915/uapi/pxp: Add a GET_PARAM for PXP

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

> > > > > Because of the additional firmware, component-driver and
> > > > > initialization depedencies required on MTL platform before a
> > > > > PXP context can be created, UMD calling for PXP creation as a
> > > > > way to get-caps can take a long time. An actual real world
> > > > > customer stack has seen this happen in the 4-to-8 second range
> > > > > after the kernel starts (which sees MESA's init appear in the
> > > > > middle of this range as the compositor comes up). To avoid
> > > > > unncessary delays experienced by the UMD for get-caps purposes,
> > > > > add a GET_PARAM for I915_PARAM_PXP_SUPPORT.
> > > > > 
> > > > alan:snip.
> > > > Progress update on the UMD side - I'm working on patch for PR here: 
> > > > https://gitlab.freedesktop.org/alan_previn_intel/mesa-alan-previn-features/-/commit/fb9d4fbfbef7dfd3f41df335cd31549fd39ddb57
> > > > but taking time to verify certain code paths
> > > 
> > > Just to confirm, if I915_PARAM_PXP_STATUS returns 2 ("will be ready
> > > soon"), then it is basically certain that in a production environment,
> > > then it will eventually return 1 meaning it's ready, right?
> > alan: yes - but not 100%. non-platform-state-machine could still
> > cause unexpected failures for example, [1] if hw was fused in a way
> > that doesnt permit PXP or [2] enabling certain BIOS debug knobs
> > doesnt allow PXP. However at the moment, there is no way for us
> > to know for sure without actually creating a protected context.
> > Of course having hw-fusing + bios-config that supports PXP have
> > always 100% succeeded for me.
> 
> In my opinion it is problematic mark that we support protected context but then it fails to create protected context.
> 
> It should check the I915_PARAM_PXP_STATUS in i915_gem_supports_protected_context() return earlier if know that protected context is not supported.
> Then create a protected context so we know that all other calls will succeed.

Hi Jose, I think your comment WRT how MESA change coule be implemented. Right now this UAPI
will provide all possible information: '-ENODEV'==no-support... '1'==supported-and-read...
'2'==supported-but-not-ready.

Unfortunately the '2'->'1' transition is not something the kernel driver has control over.
As per the earlier review comments in prior revs and weeks with others (Lionel, Jordan, Trvtko,
Daniele, etc), depending on certain scenarios (kernel configs with many components + interdependencies,
.. OR... a fresh platform-IFWI-update ... OR... a distro that is designed to boot under 1 sec),
the "2"->"1" can take up to 8-seconds-from-kernel-start. In almost all our internal CI testing
we are only seeing it take 2-seconds-from-kernel-start.

That said, perhaps we can discuss the MESA behavior on the MESA patch:
do we want to block on init time get-caps (i915_gem_supports_protected_context) or during
runtime context-creation (if the user explicitly requests it).. or never block from MESA
and report if the PXP context failed because it wasnt ready (so user knows it could retry).
<-- this last one was what Jordan wanted and what i posted here on MESA patch: 
https://gitlab.freedesktop.org/alan_previn_intel/mesa-alan-previn-features/-/commit/8728ab5a40c1a83f65afe0072f45a906ff26f706

However, as it stands today, this UAPI kernel patch has incorporated all
the requests from prior review comments and provides as much information as required.
Do you see other shortcoming this kernel side UAPI behavior? 
if not, an ack would be greatly appreciated :) since earlier ack from Lionel
was on the prior behavior before Jordan's request for this more
comprehensive response-set (-ENODEV vs '1' vs '2').