Re: [PATCH 05/17] drm/msm/a6xx: Fix PDC sleep sequence

[Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>]

On Wed, Jul 23, 2025 at 02:35:31AM +0530, Akhil P Oommen wrote:
> On 7/22/2025 10:56 PM, Rob Clark wrote:
> > On Tue, Jul 22, 2025 at 6:33 AM Dmitry Baryshkov
> > <dmitry.baryshkov@oss.qualcomm.com> wrote:
> >>
> >> On Sun, Jul 20, 2025 at 05:46:06PM +0530, 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
> >>
> >> s/KMD/the driver/
> > 
> > IMHO for gpu things "KMD" makes sense, to differentiate between kernel
> > and user mode (UMD).. this is perhaps different from other areas where
> > there isn't a userspace component to the driver stack
> 
> Ack
> 
> > 
> > BR,
> > -R
> > 
> >>> GMU firmware has not initialized. Track these dependencies using a new
> >>> status variable and trigger PDC sleep/wake sequences appropriately.
> >>
> >> Again, it looks like there should be a Fixes tag here.
> 
> Ack. I guess it is not a bad idea to backport this one too.

Please add cc:stable to the patches that need to be backported and make
sure that all Fixes are at the start of the series.

> 
> >>
> >>>
> >>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> >>> ---
> >>>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 30 +++++++++++++++++++-----------
> >>>  drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  6 ++++++
> >>>  2 files changed, 25 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >>> index 3bebb6dd7059782ceca29f2efd2acee24d3fc930..4d6c70735e0892ed87d6a68d64f24bda844e5e16 100644
> >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >>> @@ -279,6 +279,8 @@ static int a6xx_gmu_start(struct a6xx_gmu *gmu)
> >>>       if (ret)
> >>>               DRM_DEV_ERROR(gmu->dev, "GMU firmware initialization timed out\n");
> >>>
> >>> +     set_bit(GMU_STATUS_FW_START, &gmu->status);
> >>> +
> >>>       return ret;
> >>>  }
> >>>
> >>> @@ -528,6 +530,9 @@ static int a6xx_rpmh_start(struct a6xx_gmu *gmu)
> >>>       int ret;
> >>>       u32 val;
> >>>
> >>> +     if (!test_and_clear_bit(GMU_STATUS_PDC_SLEEP, &gmu->status))
> >>> +             return 0;
> >>> +
> >>>       gmu_write(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ, BIT(1));
> >>>
> >>>       ret = gmu_poll_timeout(gmu, REG_A6XX_GMU_RSCC_CONTROL_ACK, val,
> >>> @@ -555,6 +560,11 @@ static void a6xx_rpmh_stop(struct a6xx_gmu *gmu)
> >>>       int ret;
> >>>       u32 val;
> >>>
> >>> +     if (test_and_clear_bit(GMU_STATUS_FW_START, &gmu->status))
> >>> +             return;
> >>> +
> >>> +     /* TODO: should we skip if IFPC is not enabled */
> >>
> >> Is this a question or a statement?
> 
> It was a reminder to myself which I forgot to revisit later. Will
> addresss this in the next revision.
> 
> -Akhil.
> 
> >>
> >>> +
> >>>       gmu_write(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ, 1);
> >>>
> >>>       ret = gmu_poll_timeout_rscc(gmu, REG_A6XX_GPU_RSCC_RSC_STATUS0_DRV0,
> >>> @@ -563,6 +573,8 @@ static void a6xx_rpmh_stop(struct a6xx_gmu *gmu)
> >>>               DRM_DEV_ERROR(gmu->dev, "Unable to power off the GPU RSC\n");
> >>>
> >>>       gmu_write(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ, 0);
> >>> +
> >>> +     set_bit(GMU_STATUS_PDC_SLEEP, &gmu->status);
> >>>  }
> >>>
> >>>  static inline void pdc_write(void __iomem *ptr, u32 offset, u32 value)
> >>> @@ -691,8 +703,6 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
> >>>       /* ensure no writes happen before the uCode is fully written */
> >>>       wmb();
> >>>
> >>> -     a6xx_rpmh_stop(gmu);
> >>> -
> >>>  err:
> >>>       if (!IS_ERR_OR_NULL(pdcptr))
> >>>               iounmap(pdcptr);
> >>> @@ -852,19 +862,15 @@ 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 {
> >>> +     ret = a6xx_rpmh_start(gmu);
> >>> +     if (ret)
> >>> +             return ret;
> >>> +
> >>> +     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;
> >>> @@ -1046,6 +1052,8 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
> >>>
> >>>       /* Reset GPU core blocks */
> >>>       a6xx_gpu_sw_reset(gpu, true);
> >>> +
> >>> +     a6xx_rpmh_stop(gmu);
> >>>  }
> >>>
> >>>  static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu)
> >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> >>> index b2d4489b40249b1916ab4a42c89e3f4bdc5c4af9..034f1b4e5a3fb9cd601bfbe6d06d64e5ace3b6e7 100644
> >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> >>> @@ -117,6 +117,12 @@ struct a6xx_gmu {
> >>>
> >>>       struct qmp *qmp;
> >>>       struct a6xx_hfi_msg_bw_table *bw_table;
> >>> +
> >>> +/* To check if we can trigger sleep seq at PDC. Cleared in a6xx_rpmh_stop() */
> >>> +#define GMU_STATUS_FW_START  0
> >>> +/* To track if PDC sleep seq was done */
> >>> +#define GMU_STATUS_PDC_SLEEP 1
> >>> +     unsigned long status;
> >>>  };
> >>>
> >>>  static inline u32 gmu_read(struct a6xx_gmu *gmu, u32 offset)
> >>>
> >>> --
> >>> 2.50.1
> >>>
> >>
> >> --
> >> With best wishes
> >> Dmitry
> 

-- 
With best wishes
Dmitry