| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACSVV0346j2y-1Jkj=wasekYy5syax_E495AQZv0bvrrqwCSRw@mail.gmail.com>
Date: Tue, 22 Jul 2025 10:26:58 -0700
From: Rob Clark <rob.clark@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Akhil P Oommen <akhilpo@....qualcomm.com>, Sean Paul <sean@...rly.run>,
Konrad Dybcio <konradybcio@...nel.org>,
Dmitry Baryshkov <lumag@...nel.org>,
Abhinav Kumar <abhinav.kumar@...ux.dev>,
Jessica Zhang <jessica.zhang@....qualcomm.com>,
Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/17] drm/msm/a6xx: Fix PDC sleep sequence
On Tue, Jul 22, 2025 at 6:33 AM Dmitry Baryshkov
<dmitry.baryshkov@....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
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.
>
> >
> > Signed-off-by: Akhil P Oommen <akhilpo@....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?
>
> > +
> > 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
Powered by blists - more mailing lists