Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Unbalanced GPU RSC state machine on GMU firmware initialization failure paths. - [High] Missing error check for pm_runtime_get_sync() in a6xx_gmu_resume(). -- commit cd0bcd058768ca73ba4d2dc9f94b51c0f5269a1d Author: Rob Clark <[email protected]> drm/msm/a6xx+: Increase GMU FW init timeout This commit increases the GMU firmware initialization timeout from 10ms to 100ms. This matches the timeout used by the kgsl driver and prevents sporadic initialization failures. > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 27cac853975f9..6d49c51df1a27 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -320,7 +320,7 @@ static int a6xx_gmu_start(struct a6xx_gmu *gmu) > gmu_write(gmu, REG_A6XX_GMU_CM3_SYSRESET, 0); > > ret = gmu_poll_timeout(gmu, REG_A6XX_GMU_CM3_FW_INIT_RESULT, val, > - (val & mask) == reset_val, 100, 10000); > + (val & mask) == reset_val, 100, 100000); > > if (ret) > DRM_DEV_ERROR(gmu->dev, "GMU firmware initialization timed > out\n"); [Severity: High] This is a pre-existing issue, but can the GPU RSC remain implicitly started if firmware loading fails here? Looking at a6xx_gmu_fw_start(), if a6xx_gmu_fw_load() fails, it returns an error before GMU_STATUS_FW_START is set: drivers/gpu/drm/msm/adreno/a6xx_gmu.c:a6xx_gmu_fw_start() { ... 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_gmu_fw_load(gmu); if (ret) return ret; } ... } In the error path, does a6xx_rpmh_stop() abort early because GMU_STATUS_FW_START is unset? drivers/gpu/drm/msm/adreno/a6xx_gmu.c:a6xx_rpmh_stop() { ... if (!test_and_clear_bit(GMU_STATUS_FW_START, &gmu->status)) return; ... } If the driver skips the RSC initialization sequence on the next resume attempt because GMU_STATUS_PDC_SLEEP remains cleared, could this leave the hardware uninitialized and cause subsequent operations to hang? [Severity: High] This isn't a bug introduced by this patch, but does this code ignore the return value of pm_runtime_get_sync()? drivers/gpu/drm/msm/adreno/a6xx_gmu.c:a6xx_gmu_resume() { ... pm_runtime_get_sync(gmu->dev); a6xx_gmu_gxpd_get(gmu); ... } If pm_runtime_get_sync() fails and returns a negative error code, could proceeding to interact with the GMU clocks and registers via a6xx_gmu_gxpd_get() lead to a bus stall or system crash because the PM domain is inaccessible? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
