| Message ID | 20230330231517.2747024-1-konrad.dybcio@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/msm/adreno: adreno_gpu: Use suspend() instead of idle() on load error | expand |
On Fri, Mar 31, 2023 at 01:15:16AM +0200, Konrad Dybcio wrote: > The adreno_load_gpu() path is guarded by an error check on > adreno_load_fw(). This function is responsible for loading > Qualcomm-only-signed binaries (e.g. SQE and GMU FW for A6XX), but it > does not take the vendor-signed ZAP blob into account. > > By embedding the SQE (and GMU, if necessary) firmware into the > initrd/kernel, we can trigger and unfortunate path that would not bail > out early and proceed with gpu->hw_init(). That will fail, as the ZAP > loader path will not find the firmware and return back to > adreno_load_gpu(). > > This error path involves pm_runtime_put_sync() which then calls idle() > instead of suspend(). This is suboptimal, as it means that we're not > going through the clean shutdown sequence. With at least A619_holi, this > makes the GPU not wake up until it goes through at least one more > start-fail-stop cycle. The pm_runtime_put_sync that appears in the error > path actually does not guarantee that because of the earlier enabling of > runtime autosuspend. > > Fix that by using pm_runtime_put_sync_suspend to force a clean shutdown. > > Test cases: > 1. All firmware baked into kernel > 2. error loading ZAP fw in initrd -> load from rootfs at DE start > > Both succeed on A619_holi (SM6375) and A630 (SDM845). > > Fixes: 0d997f95b70f ("drm/msm/adreno: fix runtime PM imbalance at gpu load") As this one is marked for stable, you also need: Cc: stable@vger.kernel.org # 6.0 > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> Reviewed-by: Johan Hovold <johan+linaro@kernel.org> > --- > v1 -> v2: > - Improve the commit message and the reasoning within > > drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c > index f61896629be6..59f3302e8167 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c > @@ -477,7 +477,7 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev) > return gpu; > > err_put_rpm: > - pm_runtime_put_sync(&pdev->dev); > + pm_runtime_put_sync_suspend(&pdev->dev); > err_disable_rpm: > pm_runtime_disable(&pdev->dev); Johan
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c index f61896629be6..59f3302e8167 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_device.c +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c @@ -477,7 +477,7 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev) return gpu; err_put_rpm: - pm_runtime_put_sync(&pdev->dev); + pm_runtime_put_sync_suspend(&pdev->dev); err_disable_rpm: pm_runtime_disable(&pdev->dev);
The adreno_load_gpu() path is guarded by an error check on adreno_load_fw(). This function is responsible for loading Qualcomm-only-signed binaries (e.g. SQE and GMU FW for A6XX), but it does not take the vendor-signed ZAP blob into account. By embedding the SQE (and GMU, if necessary) firmware into the initrd/kernel, we can trigger and unfortunate path that would not bail out early and proceed with gpu->hw_init(). That will fail, as the ZAP loader path will not find the firmware and return back to adreno_load_gpu(). This error path involves pm_runtime_put_sync() which then calls idle() instead of suspend(). This is suboptimal, as it means that we're not going through the clean shutdown sequence. With at least A619_holi, this makes the GPU not wake up until it goes through at least one more start-fail-stop cycle. The pm_runtime_put_sync that appears in the error path actually does not guarantee that because of the earlier enabling of runtime autosuspend. Fix that by using pm_runtime_put_sync_suspend to force a clean shutdown. Test cases: 1. All firmware baked into kernel 2. error loading ZAP fw in initrd -> load from rootfs at DE start Both succeed on A619_holi (SM6375) and A630 (SDM845). Fixes: 0d997f95b70f ("drm/msm/adreno: fix runtime PM imbalance at gpu load") Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- v1 -> v2: - Improve the commit message and the reasoning within drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)