On 8/7/2025 7:21 PM, Konrad Dybcio wrote:
> On 7/20/25 2:16 PM, Akhil P Oommen wrote:
>> Since the PDC resides out of the GPU subsystem and cannot be reset in
>> case it enters bad state, utmost care must be taken to trigger the PDC
>> wake/sleep routines in the correct order.
>>
>> The PDC wake sequence can be exercised only after a PDC sleep sequence.
>> Additionally, GMU firmware should initialize a few registers before the
>> KMD can trigger a PDC sleep sequence. So PDC sleep can't be done if the
>> GMU firmware has not initialized. Track these dependencies using a new
>> status variable and trigger PDC sleep/wake sequences appropriately.
>>
>> Signed-off-by: Akhil P Oommen <[email protected]>
>> ---
> 
> FWIW some time ago I made this patch, which tackles a similar issue,
> perhaps it's a good idea to merge both:
> 
> From 7d6441fc6ec5ee7fe723e1ad86d11fdd17bee922 Mon Sep 17 00:00:00 2001
> From: Konrad Dybcio <[email protected]>
> Date: Thu, 20 Feb 2025 10:28:51 +0100
> Subject: [PATCH] drm/msm/adreno: Delay the Adreno RPMh startup to HFI init
> 
> There's no use in trying to power up the GX logic before we're almost
> ready to fire up the GPU. In fact, with A8xx the PDC and RSC uCode are
> loaded by the GMU firmware, so we *must* wait for the GMU to fully
> initialize before trying to do so.
> 

iirc, this wake up sequence should be done before fw start. That aligns
with downstream sequence order too.

-Akhil

> Move it to right before HFI init.
> 
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 12 ++----------
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  1 +
>  drivers/gpu/drm/msm/adreno/a6xx_hfi.c |  2 +-
>  3 files changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 28e6705c6da6..3335583ada45 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -513,7 +513,7 @@ static int a6xx_gmu_notify_slumber(struct a6xx_gmu *gmu)
>       return ret;
>  }
>  
> -static int a6xx_rpmh_start(struct a6xx_gmu *gmu)
> +int a6xx_rpmh_start(struct a6xx_gmu *gmu)
>  {
>       int ret;
>       u32 val;
> @@ -842,19 +842,11 @@ static int a6xx_gmu_fw_start(struct a6xx_gmu *gmu, 
> unsigned int state)
>       else
>               gmu_write(gmu, REG_A6XX_GMU_GENERAL_7, 1);
>  
> -     if (state == GMU_WARM_BOOT) {
> -             ret = a6xx_rpmh_start(gmu);
> -             if (ret)
> -                     return ret;
> -     } else {
> +     if (state == GMU_COLD_BOOT) {
>               if (WARN(!adreno_gpu->fw[ADRENO_FW_GMU],
>                       "GMU firmware is not loaded\n"))
>                       return -ENOENT;
>  
> -             ret = a6xx_rpmh_start(gmu);
> -             if (ret)
> -                     return ret;
> -
>               ret = a6xx_gmu_fw_load(gmu);
>               if (ret)
>                       return ret;
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h 
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> index d1ce11131ba6..309305298a45 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> @@ -216,5 +216,6 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu);
>  bool a6xx_gmu_sptprac_is_on(struct a6xx_gmu *gmu);
>  void a6xx_sptprac_disable(struct a6xx_gmu *gmu);
>  int a6xx_sptprac_enable(struct a6xx_gmu *gmu);
> +int a6xx_rpmh_start(struct a6xx_gmu *gmu);
>  
>  #endif
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> index 8e69b1e84657..9ea01daf2995 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> @@ -910,7 +910,7 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int boot_state)
>       if (ret)
>               return ret;
>  
> -     return 0;
> +     return a6xx_rpmh_start(gmu);
>  }
>  
>  void a6xx_hfi_stop(struct a6xx_gmu *gmu)

Reply via email to