On Mon, 2023-05-08 at 17:49 +0000, Teres Alexis, Alan Previn wrote: > On Fri, 2023-05-05 at 00:39 -0700, Justen, Jordan L wrote: > > On 2023-05-04 22:30:07, Teres Alexis, Alan Previn wrote: > > > On Thu, 2023-04-27 at 16:48 -0700, Teres Alexis, Alan Previn wrote: > > > > 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. > > > > > If this is correct, then I think that the change in > > i915_gem_supports_protected_context() is good, and probably we can > > skip the change in iris_create_hw_context(). > > > > Basically, we're timing out for multiple seconds either way. And, I'm > > hoping that the kernel will eventually get the PXP init done and > > create the context. > > > > I think there's 2 cases of interest here. > > > > First, and most likely, the application running doesn't care about > > protected content. In this case we can quickly advertise the support, > > but there will be no consequence because the application won't use the > > feature. > alan: yes - that was one of the goals of this UAPI. > > > > Second, the application does care about protected content. In this > > case we can quickly advertise the support, but if the feature is used > > too quickly, then the context create might take a long time. > alan: no, that isnt the case now, we started at 8-secs, then 2-secs, > and finally settled on the same timeout as ADL since this new UAPI > will be something that can be polled on to be sure we have readiness > to make the create-context-call. That's why i wanted to add the > polling wait in the actual create call - but not the get caps call > which can be quick (as you said above). > > > > > If I915_PARAM_PXP_STATUS returning 2 has a reasonable chance in a > > production environment of eventually finding out that pxp can't work, > > then perhaps more disussion is needed. I guess the worst case scenario > > is no worse than today though. (Except it is still somewhat better, > > because the common case would not involve protected content being > > used by the application.) > alan: hmmm... i guess it depends on the meaning of resonable: if 50% > of the CI platforms being run have incorrect bios config / hw fusing > does it mean this UAPI is only 50% useful? My opinion is the alternative: > in the case of platform that has correctly configured BIOS + fusing, > is the chance of 2 eventually leading to a failure high? The answer is no. > Hypothetically i have actually never seen this happen (note: this UAPI > is new but i know from past debug of customer issues). There are some very > corner-cases but those get into the weeds of pxp runtime state machine.. > I am sure we don't wanna discuss that rabbit hole right now. > > > > Another option besides from the timeout loop in > > iris_create_hw_context() might be to check I915_PARAM_PXP_STATUS after > > the context create fails to tweak the debug message. > alan: Yeah, that is an option - I'm thinking we can add a DBG that reads > either"PXP context failure expected due not ready" vs > "Unexpected PXP context failure" > > > > > -Jordan >
