Re: [PATCH v3] drm/xe/pxp: add termination on resume

On Mon, 2026-07-20 at 15:27 -0700, Daniele Ceraolo Spurio wrote:
> Suspend/resume causes the PXP keys to become invalid, but doesn't
> actually kill the session. The driver also doesn't explicitly kill and
> re-start the session until a new PXP request comes in, which means that
> the "zombie" session can potentially stick around if there are no new
> requests from userspace. While this is not an issue for PXP, HDCP has a
> new behavior starting on PTL where a communication is sent to GSC if a
> session is active at suspend time (even if it doesn't have a valid key),
> which can lead to delays in the suspend flow if we suspend while the
> zombie session is still active.
> To avoid this, we can trigger a termination on resume and kill the
> zombie session immediately, instead of delaying the termination to the
> 
[snip]
> diff --git a/drivers/gpu/drm/xe/xe_pxp.c b/drivers/gpu/drm/xe/xe_pxp.c
> index fea3d8ceeddb..2fef274a585e 100644
> --- a/drivers/gpu/drm/xe/xe_pxp.c
> +++ b/drivers/gpu/drm/xe/xe_pxp.c
> @@ -8,6 +8,8 @@
>  #include <drm/drm_managed.h>
>  #include <uapi/drm/xe_drm.h>
>  
> +#include <linux/device.h>
> +
>  #include "xe_bo.h"
>  #include "xe_bo_types.h"
>  #include "xe_device_types.h"
> @@ -174,16 +176,9 @@ static void mark_termination_in_progress(struct xe_pxp *pxp)
>   pxp->status = XE_PXP_TERMINATION_IN_PROGRESS;
>  }
>  
> -static void pxp_terminate(struct xe_pxp *pxp)
> +static bool pxp_prep_for_termination(struct xe_pxp *pxp)
>  {
> - int ret = 0;
> - struct xe_device *xe = pxp->xe;
> -
> - if (!wait_for_completion_timeout(&pxp->activation,
> - msecs_to_jiffies(PXP_ACTIVATION_TIMEOUT_MS)))
> - drm_err(&xe->drm, "failed to wait for PXP start before termination\n");
> -
> - mutex_lock(&pxp->mutex);
> + lockdep_assert_held(&pxp->mutex);
>  
>   if (pxp->status == XE_PXP_ACTIVE)
>   pxp->key_instance++;
> @@ -192,10 +187,8 @@ static void pxp_terminate(struct xe_pxp *pxp)
>   * we'll mark the status as needing termination on resume, so no need to
>   * emit a termination now.
>   */
> - if (pxp->status == XE_PXP_SUSPENDED) {
> - mutex_unlock(&pxp->mutex);
> - return;
> - }
> + if (pxp->status == XE_PXP_SUSPENDED)
> + return false;
alan: in xe_pxp_pm_resume, we assert "pxp->status == XE_PXP_SUSPENDED" before calling
prep_for_termination... this assertion will mostly be true (i.e. we did successfully
go into suspend before executing the resume code). In that case, above check will always
return false (i.e. the worker will not get fired). i am only guessing, but perhaps
above check (from original pxp_terminate) was meant to deal with the case of an incoming
termination IRQ while we are in the midst of suspending? (i.e. we just defer the termination
code). But for terminate-prep, i guess we should not check this? would just removing this work
fine since this patch adds "PXP_TERMINATION_REQUEST_ON_RESUME" into pxp->events.pending
and you handle this in the event-worker now (i.e. if it is a HW IRQ, u drop it if we
just started suspending and proceed with the new suspend-prep->resume-actual termination.
> 
[snip]