Re: [PATCH V1] accel/amdxdna: Fix runtime suspend deadlock when there is pending job

[Lizhi Hou <lizhi.hou@amd.com>]

On 3/10/26 10:50, Mario Limonciello wrote:
> On 3/10/26 12:49 PM, Lizhi Hou wrote:
>> The runtime suspend callback drains the running job workqueue before
>> suspending the device. If a job is still executing and calls
>> pm_runtime_resume_and_get(), it can deadlock with the runtime suspend
>> path.
>>
>> Fix this by moving pm_runtime_resume_and_get() from the job execution
>> routine to the job submission routine, ensuring the device is resumed
>> before the job is queued and avoiding the deadlock during runtime
>> suspend.
>>
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> Fixes tag?

I missed it again. I will send V2.

Thanks,

Lizhi

>> ---
>>   drivers/accel/amdxdna/aie2_ctx.c    | 14 ++------------
>>   drivers/accel/amdxdna/amdxdna_ctx.c | 10 ++++++++++
>>   2 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/aie2_ctx.c 
>> b/drivers/accel/amdxdna/aie2_ctx.c
>> index afee5e667f77..c0d348884f74 100644
>> --- a/drivers/accel/amdxdna/aie2_ctx.c
>> +++ b/drivers/accel/amdxdna/aie2_ctx.c
>> @@ -165,7 +165,6 @@ aie2_sched_notify(struct amdxdna_sched_job *job)
>>         trace_xdna_job(&job->base, job->hwctx->name, "signaled 
>> fence", job->seq);
>>   - amdxdna_pm_suspend_put(job->hwctx->client->xdna);
>>       job->hwctx->priv->completed++;
>>       dma_fence_signal(fence);
>>   @@ -290,19 +289,11 @@ aie2_sched_job_run(struct drm_sched_job 
>> *sched_job)
>>       struct dma_fence *fence;
>>       int ret;
>>   -    ret = amdxdna_pm_resume_get(hwctx->client->xdna);
>> -    if (ret)
>> +    if (!hwctx->priv->mbox_chann)
>>           return NULL;
>>   -    if (!hwctx->priv->mbox_chann) {
>> -        amdxdna_pm_suspend_put(hwctx->client->xdna);
>> -        return NULL;
>> -    }
>> -
>> -    if (!mmget_not_zero(job->mm)) {
>> -        amdxdna_pm_suspend_put(hwctx->client->xdna);
>> +    if (!mmget_not_zero(job->mm))
>>           return ERR_PTR(-ESRCH);
>> -    }
>>         kref_get(&job->refcnt);
>>       fence = dma_fence_get(job->fence);
>> @@ -333,7 +324,6 @@ aie2_sched_job_run(struct drm_sched_job *sched_job)
>>     out:
>>       if (ret) {
>> -        amdxdna_pm_suspend_put(hwctx->client->xdna);
>>           dma_fence_put(job->fence);
>>           aie2_job_put(job);
>>           mmput(job->mm);
>> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c 
>> b/drivers/accel/amdxdna/amdxdna_ctx.c
>> index 666dfd7b2a80..838430903a3e 100644
>> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
>> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
>> @@ -17,6 +17,7 @@
>>   #include "amdxdna_ctx.h"
>>   #include "amdxdna_gem.h"
>>   #include "amdxdna_pci_drv.h"
>> +#include "amdxdna_pm.h"
>>     #define MAX_HWCTX_ID        255
>>   #define MAX_ARG_COUNT        4095
>> @@ -445,6 +446,7 @@ amdxdna_arg_bos_lookup(struct amdxdna_client 
>> *client,
>>   void amdxdna_sched_job_cleanup(struct amdxdna_sched_job *job)
>>   {
>>       trace_amdxdna_debug_point(job->hwctx->name, job->seq, "job 
>> release");
>> +    amdxdna_pm_suspend_put(job->hwctx->client->xdna);
>>       amdxdna_arg_bos_put(job);
>>       amdxdna_gem_put_obj(job->cmd_bo);
>>       dma_fence_put(job->fence);
>> @@ -482,6 +484,12 @@ int amdxdna_cmd_submit(struct amdxdna_client 
>> *client,
>>           goto cmd_put;
>>       }
>>   +    ret = amdxdna_pm_resume_get(xdna);
>> +    if (ret) {
>> +        XDNA_ERR(xdna, "Resume failed, ret %d", ret);
>> +        goto put_bos;
>> +    }
>> +
>>       idx = srcu_read_lock(&client->hwctx_srcu);
>>       hwctx = xa_load(&client->hwctx_xa, hwctx_hdl);
>>       if (!hwctx) {
>> @@ -522,6 +530,8 @@ int amdxdna_cmd_submit(struct amdxdna_client 
>> *client,
>>       dma_fence_put(job->fence);
>>   unlock_srcu:
>>       srcu_read_unlock(&client->hwctx_srcu, idx);
>> +    amdxdna_pm_suspend_put(xdna);
>> +put_bos:
>>       amdxdna_arg_bos_put(job);
>>   cmd_put:
>>       amdxdna_gem_put_obj(job->cmd_bo);
>