[PATCH] drm/xe/pxp: do not queue unneeded terminations from debugfs

[Daniele Ceraolo Spurio]

On 4/15/2025 4:04 PM, John Harrison wrote:
> On 4/15/2025 3:48 PM, Daniele Ceraolo Spurio wrote:
>> The PXP terminate debugfs currently unconditionally simulates a
>> termination, no matter what the HW status is. This is unneeded if PXP is
>> not in use and can cause errors if the HW init hasn't completed yet.
>> To solve these issues, we can simply limit the terminations to the cases
>> where PXP is fully initialized and in use.
>>
>> Fixes: 385a8015b214 ("drm/xe/pxp: Add PXP debugfs support")
>> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/4749
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> Cc: John Harrison <John.C.Harrison at Intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_pxp_debugfs.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_pxp_debugfs.c 
>> b/drivers/gpu/drm/xe/xe_pxp_debugfs.c
>> index ccfbacf08efc..eb4a084103e3 100644
>> --- a/drivers/gpu/drm/xe/xe_pxp_debugfs.c
>> +++ b/drivers/gpu/drm/xe/xe_pxp_debugfs.c
>> @@ -66,9 +66,18 @@ static int pxp_terminate(struct seq_file *m, void 
>> *data)
>>   {
>>       struct xe_pxp *pxp = node_to_pxp(m->private);
>>       struct drm_printer p = drm_seq_file_printer(m);
>> +    int pxp_status = xe_pxp_get_readiness_status(pxp);
>>   -    if (!xe_pxp_is_enabled(pxp))
>> -        return -ENODEV;
>> +    if (pxp_status < 0)
>> +        return pxp_status; /* disabled or error occurred */
>> +    else if (!pxp_status)
>> +        return -EBUSY; /* init still in progress */
>> +
>> +    /* no need for a termination if PXP is not active */
>> +    if (pxp->status != XE_PXP_ACTIVE) {
> Having both 'pxp_status' and 'pxp->status' being testing in 
> back-to-back ifs seems unnecessarily confusing. Maybe rename 
> 'pxp_status' to 'ready'?

True, I hadn't realized they were that similar. Will wait for CI and 
then respin.

Daniele

>
> John.
>
>> +        drm_printf(&p, "PXP not active\n");
>> +        return 0;
>> +    }
>>         /* simulate a termination interrupt */
>>       spin_lock_irq(&pxp->xe->irq.lock);
>