On 10/06/2017 07:32 PM, Daniel Vetter wrote:
On Fri, Oct 06, 2017 at 04:27:06PM +0530, Archit Taneja wrote:
The DSI runtime PM suspend/resume callbacks check whether
msm_host->cfg_hnd is non-NULL before trying to enable the bus clocks.
This is done to accommodate early calls to these functions that may
happen before the bus clocks are even initialized.
Calling pm_runtime_put_autosuspend() in dsi_host_init() can result in
racy behaviour since msm_host->cfg_hnd is set very soon after. If the
suspend callback happens too late, we end up trying to disable clocks
that were never enabled, resulting in a bunch of WARN_ON splats.
Sounds like the correct fix here is to block autosuspend until after
everything is set up, including bus clocks. This patch just makes the race
harder to hit in practice ...
Thanks for the review. pm_runtime_put_sync() ensures that the suspend handler
is called in the same context as that of the caller, right? msm_host->cfg_hnd
is set to a non-NULL value only when we return from dsi_get_config(). The race
would never happen in this case.
This call is a one time thing during DSI probe, we do a pm_runtime_get_sync()
just so that we can read the block revision number. Once we have the revision
number, we look at an internal table which maintains IP version specific
resources, like what bus clocks to get, etc. Having pm_runtime_put_autosuspend()
here didn't help much anyway.
Archit
-Daniel
Use pm_runtime_put_sync() so that the suspend callback is called
immediately.
Reported-by: Nicolas Dechesne <nicolas.dechesne@xxxxxxxxxx>
Signed-off-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index dbb31a014419..deaf869374ea 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -248,7 +248,7 @@ static const struct msm_dsi_cfg_handler *dsi_get_config(
clk_disable_unprepare(ahb_clk);
disable_gdsc:
regulator_disable(gdsc_reg);
- pm_runtime_put_autosuspend(dev);
+ pm_runtime_put_sync(dev);
put_clk:
clk_put(ahb_clk);
put_gdsc:
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
