| Message ID | 20200501182533.19753-1-jcrouse@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: Check for powered down HW in the devfreq callbacks | expand |
On Fri, May 1, 2020 at 11:26 AM Jordan Crouse <jcrouse@codeaurora.org> wrote: > > Writing to the devfreq sysfs nodes while the GPU is powered down can > result in a system crash (on a5xx) or a nasty GMU error (on a6xx): > > $ /sys/class/devfreq/5000000.gpu# echo 500000000 > min_freq > [ 104.841625] platform 506a000.gmu: [drm:a6xx_gmu_set_oob] > *ERROR* Timeout waiting for GMU OOB set GPU_DCVS: 0x0 > > Despite the fact that we carefully try to suspend the devfreq device when > the hardware is powered down there are lots of holes in the governors that > don't check for the suspend state and blindly call into the devfreq > callbacks that end up triggering hardware reads in the GPU driver. > > Check the power state in the gpu_busy() and gpu_set_freq() callbacks for > a5xx and a6xx to make sure that the hardware is active before trying to > access it. Chatted on IRC -- while this avoids the instaboot on db820c when setting /sys/class/devfreq/devfreq1/min_freq, I think we should be using pm_runtime_get_if_in_use() to avoid the races while still avoiding bringing up the GPU.
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index 724024a2243a..9d8c7fe6377e 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c @@ -1404,6 +1404,10 @@ static unsigned long a5xx_gpu_busy(struct msm_gpu *gpu) { u64 busy_cycles, busy_time; + /* devfreq can get here while we are idle so do a quick check first */ + if (!pm_runtime_active(&gpu->pdev->dev)) + return ~0LU; + busy_cycles = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_RBBM_0_LO, REG_A5XX_RBBM_PERFCTR_RBBM_0_HI); diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index c4e71abbdd53..84618f2ff073 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -108,6 +108,10 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) struct msm_gpu *gpu = &adreno_gpu->base; int ret; + /* This can be called via devfreq even when the power is off */ + if (!pm_runtime_active(gmu->dev)) + return; + gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0); gmu_write(gmu, REG_A6XX_GMU_DCVS_PERF_SETTING, diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index 68af24150de5..443efc952f13 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -810,6 +810,10 @@ static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu) struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); u64 busy_cycles, busy_time; + /* devfreq can get here while we are idle so do a quick check first */ + if (!pm_runtime_active(a6xx_gpu->gmu.dev)) + return ~0LU; + busy_cycles = gmu_read64(&a6xx_gpu->gmu, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_XOCLK_0_L, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_XOCLK_0_H);
Writing to the devfreq sysfs nodes while the GPU is powered down can result in a system crash (on a5xx) or a nasty GMU error (on a6xx): $ /sys/class/devfreq/5000000.gpu# echo 500000000 > min_freq [ 104.841625] platform 506a000.gmu: [drm:a6xx_gmu_set_oob] *ERROR* Timeout waiting for GMU OOB set GPU_DCVS: 0x0 Despite the fact that we carefully try to suspend the devfreq device when the hardware is powered down there are lots of holes in the governors that don't check for the suspend state and blindly call into the devfreq callbacks that end up triggering hardware reads in the GPU driver. Check the power state in the gpu_busy() and gpu_set_freq() callbacks for a5xx and a6xx to make sure that the hardware is active before trying to access it. Cc: stable@vger.kernel.org Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org> --- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++++ drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 4 ++++ drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 ++++ 3 files changed, 12 insertions(+)