Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] PDC sleep sequence is triggered even when GMU firmware initialization fails due to unconditional status flag setting. -- commit 82656c1e5d01b9521acaeb8de8ad4bf1a538f52b Author: Shivam Rawat <[email protected]> drm/msm/a6xx: Fix stale rpmh votes after suspend This commit addresses stale RPMH (BCM) votes observed after GMU suspend by ensuring RPMH stop sequences are not skipped. It also puts the GMU into a reset state to prevent further activity. > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 2e5d7b53a0c38..a2f6918c4f7f2 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -642,7 +642,7 @@ static void a6xx_rpmh_stop(struct a6xx_gmu *gmu) > int ret; > u32 val; > > - if (test_and_clear_bit(GMU_STATUS_FW_START, &gmu->status)) > + if (!test_and_clear_bit(GMU_STATUS_FW_START, &gmu->status)) > return; [Severity: High] This isn't a bug introduced by this patch, but this logic change appears to expose a pre-existing issue in a6xx_gmu_start(). If a6xx_gmu_start() times out waiting for the firmware, it logs an error but still sets the status bit unconditionally: drivers/gpu/drm/msm/adreno/a6xx_gmu.c:a6xx_gmu_start() { ... if (ret) DRM_DEV_ERROR(gmu->dev, "GMU firmware initialization timed out\n"); set_bit(GMU_STATUS_FW_START, &gmu->status); ... } When a6xx_gmu_start() returns this error, the caller jumps to its error handling path and calls a6xx_rpmh_stop() to clean up. Now that a6xx_rpmh_stop() correctly proceeds when the bit is set, won't it issue the RSCC sleep request even though the firmware failed to initialize? Could this risk hanging the PDC subsystem, since the sleep sequence requires the GMU firmware to have successfully initialized certain registers? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
