On Tue, Mar 31, 2026 at 11:05 AM Akhil P Oommen
<[email protected]> wrote:
>
> On 3/31/2026 1:21 AM, Rob Clark wrote:
> > On Thu, Mar 26, 2026 at 5:15 PM Akhil P Oommen <[email protected]>
> > wrote:
> >>
> >> Capture coredump on GPU or GMU errors during initialization to help in
> >> debugging the issues. To be consistent with the locks while calling
> >> msm_gpu_crashstate_capture(), call pm_runtime_get(gpu) always with
> >> msm_gpu->lock.
> >>
> >> Signed-off-by: Akhil P Oommen <[email protected]>
> >> ---
> >> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 1 +
> >> drivers/gpu/drm/msm/adreno/adreno_device.c | 5 +++--
> >> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 5 ++++-
> >> drivers/gpu/drm/msm/msm_gpu.c | 5 +++--
> >> drivers/gpu/drm/msm/msm_gpu.h | 2 ++
> >> 5 files changed, 13 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >> index 1b44b9e21ad8..916c5d99c4d1 100644
> >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >> @@ -1335,6 +1335,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
> >>
> >> disable_irq:
> >> disable_irq(gmu->gmu_irq);
> >> + msm_gpu_crashstate_capture(gpu, NULL, NULL, NULL, NULL);
> >> a6xx_rpmh_stop(gmu);
> >> disable_clk:
> >> clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks);
> >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
> >> b/drivers/gpu/drm/msm/adreno/adreno_device.c
> >> index 4edfe80c5be7..ca5f96e16870 100644
> >> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> >> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> >> @@ -105,6 +105,8 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
> >> */
> >> pm_runtime_enable(&pdev->dev);
> >>
> >> + guard(mutex)(&gpu->lock);
> >
> > so, I am a fan of guard(), but I realized this means we are holding
> > gpu->lock across the debugfs_init() (only a5xx).. which I suspect will
> > anger lockdep, although I don't have a good way to test that atm.
>
> Shall we drop this patch for now? I will send a respin later.
I think I might, esp since the db820/a5xx ci runners seem to be down atm
BR,
-R
> -Akhil.
>
> >
> > BR,
> > -R
> >
> >> +
> >> ret = pm_runtime_get_sync(&pdev->dev);
> >> if (ret < 0) {
> >> pm_runtime_put_noidle(&pdev->dev);
> >> @@ -112,10 +114,9 @@ struct msm_gpu *adreno_load_gpu(struct drm_device
> >> *dev)
> >> goto err_disable_rpm;
> >> }
> >>
> >> - mutex_lock(&gpu->lock);
> >> ret = msm_gpu_hw_init(gpu);
> >> - mutex_unlock(&gpu->lock);
> >> if (ret) {
> >> + msm_gpu_crashstate_capture(gpu, NULL, NULL, NULL, NULL);
> >> DRM_DEV_ERROR(dev->dev, "gpu hw init failed: %d\n", ret);
> >> goto err_put_rpm;
> >> }
> >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> >> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> >> index 785e99fb5bd5..8475802fdde2 100644
> >> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> >> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> >> @@ -391,10 +391,13 @@ int adreno_get_param(struct msm_gpu *gpu, struct
> >> msm_context *ctx,
> >> return 0;
> >> case MSM_PARAM_TIMESTAMP:
> >> if (adreno_gpu->funcs->get_timestamp) {
> >> + mutex_lock(&gpu->lock);
> >> pm_runtime_get_sync(&gpu->pdev->dev);
> >> +
> >> *value = adreno_gpu->funcs->get_timestamp(gpu);
> >> - pm_runtime_put_autosuspend(&gpu->pdev->dev);
> >>
> >> + pm_runtime_put_autosuspend(&gpu->pdev->dev);
> >> + mutex_unlock(&gpu->lock);
> >> return 0;
> >> }
> >> return -EINVAL;
> >> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> >> index 995549d0bbbc..472db2c916f9 100644
> >> --- a/drivers/gpu/drm/msm/msm_gpu.c
> >> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> >> @@ -361,7 +361,7 @@ static void crashstate_get_vm_logs(struct
> >> msm_gpu_state *state, struct msm_gem_v
> >> mutex_unlock(&vm->mmu_lock);
> >> }
> >>
> >> -static void msm_gpu_crashstate_capture(struct msm_gpu *gpu,
> >> +void msm_gpu_crashstate_capture(struct msm_gpu *gpu,
> >> struct msm_gem_submit *submit, struct msm_gpu_fault_info
> >> *fault_info,
> >> char *comm, char *cmd)
> >> {
> >> @@ -886,7 +886,8 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct
> >> msm_gem_submit *submit)
> >>
> >> pm_runtime_get_sync(&gpu->pdev->dev);
> >>
> >> - msm_gpu_hw_init(gpu);
> >> + if (msm_gpu_hw_init(gpu))
> >> + msm_gpu_crashstate_capture(gpu, NULL, NULL, NULL, NULL);
> >>
> >> submit->seqno = submit->hw_fence->seqno;
> >>
> >> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> >> index 666cf499b7ec..eb5b3a7b81f9 100644
> >> --- a/drivers/gpu/drm/msm/msm_gpu.h
> >> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> >> @@ -747,6 +747,8 @@ static inline void msm_gpu_crashstate_put(struct
> >> msm_gpu *gpu)
> >> }
> >>
> >> void msm_gpu_fault_crashstate_capture(struct msm_gpu *gpu, struct
> >> msm_gpu_fault_info *fault_info);
> >> +void msm_gpu_crashstate_capture(struct msm_gpu *gpu, struct
> >> msm_gem_submit *submit,
> >> + struct msm_gpu_fault_info *fault_info, char *comm, char
> >> *cmd);
> >>
> >> /*
> >> * Simple macro to semi-cleanly add the MAP_PRIV flag for targets that can
> >>
> >> --
> >> 2.51.0
> >>
>
