On Tue, Aug 9, 2022 at 4:23 PM Ceraolo Spurio, Daniele
<daniele.ceraolospurio@xxxxxxxxx> wrote:
>
>
>
> On 8/9/2022 4:20 PM, Juston Li wrote:
> > On Tue, Aug 9, 2022 at 4:10 PM Ceraolo Spurio, Daniele
> > <daniele.ceraolospurio@xxxxxxxxx> wrote:
> >>
> >>
> >> On 8/9/2022 3:57 PM, Juston Li wrote:
> >>> pxp will not start correctly until after mei_pxp bind completes and
> >>> intel_pxp_init_hw() is called.
> >>>
> >>> Signed-off-by: Juston Li <justonli@xxxxxxxxxx>
> >>> ---
> >>> drivers/gpu/drm/i915/pxp/intel_pxp.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> >>> index 15311eaed848..3ef9e4e1870b 100644
> >>> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> >>> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> >>> @@ -184,7 +184,7 @@ int intel_pxp_start(struct intel_pxp *pxp)
> >>> {
> >>> int ret = 0;
> >>>
> >>> - if (!intel_pxp_is_enabled(pxp))
> >>> + if (!intel_pxp_is_enabled(pxp) || !pxp->pxp_component_added)
> >> pxp_component_added being true only indicates that we've called
> >> component_add and not if the component is currently bound. For checking
> >> the current state of the component you can look at the
> >> pxp->pxp_component pointer, which is set/cleared on component
> >> bind/unbind. Note that pxp_component has to be accessed under
> >> pxp->tee_mutex.
> >>
> >> Are you actually seeing a scenario where the user manages to submit
> >> before the bind is complete? the bind is async to i915, but I've always
> >> seen it complete before control was given to the user to start submitting.
> >>
> >> Daniele
> > Opps! I made a typo sending it out. Indeed when I tested it was with
> > pxp->component_add.
> > I did not have pxp->tee_mutex though so I'll add that too.
> >
> > We moved initialization earlier so it does try to call pxp_start() before mei
> > finishes binding.
>
> I'd also suggest to use a different error code if that is the case, so
> userspace can differentiate between !enabled and !ready and only retry
> in the latter case.
Sounds good, I think I'll use -EAGAIN
Thanks!
Juston
> Daniele
>
> >
> > Thanks
> > Juston
> >
> >>> return -ENODEV;
> >>>
> >>> mutex_lock(&pxp->arb_mutex);
>
