[PATCH v2 08/17] drm/msm/adreno: Coredump on GPU/GMU init failures

Akhil P Oommen posted 17 patches 6 days, 18 hours ago
[PATCH v2 08/17] drm/msm/adreno: Coredump on GPU/GMU init failures
Posted by Akhil P Oommen 6 days, 18 hours ago
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 <akhilpo@oss.qualcomm.com>
---
 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);
+
 	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
Re: [PATCH v2 08/17] drm/msm/adreno: Coredump on GPU/GMU init failures
Posted by Rob Clark 2 days, 22 hours ago
On Thu, Mar 26, 2026 at 5:15 PM Akhil P Oommen <akhilpo@oss.qualcomm.com> 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 <akhilpo@oss.qualcomm.com>
> ---
>  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.

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
>
Re: [PATCH v2 08/17] drm/msm/adreno: Coredump on GPU/GMU init failures
Posted by Akhil P Oommen 2 days ago
On 3/31/2026 1:21 AM, Rob Clark wrote:
> On Thu, Mar 26, 2026 at 5:15 PM Akhil P Oommen <akhilpo@oss.qualcomm.com> 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 <akhilpo@oss.qualcomm.com>
>> ---
>>  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.

-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
>>

Re: [PATCH v2 08/17] drm/msm/adreno: Coredump on GPU/GMU init failures
Posted by Rob Clark 2 days ago
On Tue, Mar 31, 2026 at 11:05 AM Akhil P Oommen
<akhilpo@oss.qualcomm.com> wrote:
>
> On 3/31/2026 1:21 AM, Rob Clark wrote:
> > On Thu, Mar 26, 2026 at 5:15 PM Akhil P Oommen <akhilpo@oss.qualcomm.com> 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 <akhilpo@oss.qualcomm.com>
> >> ---
> >>  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
> >>
>
Re: [PATCH v2 08/17] drm/msm/adreno: Coredump on GPU/GMU init failures
Posted by Konrad Dybcio 6 days, 7 hours ago
On 3/27/26 1:13 AM, Akhil P Oommen 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 <akhilpo@oss.qualcomm.com>
> ---

[...]


> --- 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);

This is something to take care of in a separate patch, but get_sync may
fail and then the read could crash the device (GMU may be off too)

put_autosuspend could theroetically fail too, but perhaps -edontcarethatmuch

Konrad
Re: [PATCH v2 08/17] drm/msm/adreno: Coredump on GPU/GMU init failures
Posted by Akhil P Oommen 2 days, 21 hours ago
On 3/27/2026 5:05 PM, Konrad Dybcio wrote:
> On 3/27/26 1:13 AM, Akhil P Oommen 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 <akhilpo@oss.qualcomm.com>
>> ---
> 
> [...]
> 
> 
>> --- 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);
> 
> This is something to take care of in a separate patch, but get_sync may
> fail and then the read could crash the device (GMU may be off too)
> 
> put_autosuspend could theroetically fail too, but perhaps -edontcarethatmuch

Right. That is something we should fix everywhere separately.

-Akhil.

> 
> Konrad