diff mbox

[38/41] drm/bridge: analogix_dp: Reorder plat_data->power_off to happen sooner

Message ID 20170310043305.17216-39-seanpaul@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Sean Paul March 10, 2017, 4:32 a.m. UTC 
From: Douglas Anderson <dianders@chromium.org>

The current user of the analogix power_off is "analogix_dp-rockchip".
That driver does this:
- deactivate PSR
- turn off a clock

Both of these things (especially deactive PSR) should be done before
we turn the PHY power off and turn off analog power.  Let's move the
callback up.

Note that without this patch (and with https://patchwork.kernel.org/patch/9553349/
[seanpaul: this patch was not applied, but it seems like the race
can still occur]), I experienced an error in reboot testing where one
thread was at:

  rockchip_drm_psr_deactivate
  rockchip_dp_powerdown
  analogix_dp_bridge_disable
  drm_bridge_disable

...and the other thread was at:

  analogix_dp_send_psr_spd
  analogix_dp_enable_psr
  analogix_dp_psr_set
  psr_flush_handler

The flush handler thread was finding AUX channel errors and eventually
reported "Failed to apply PSR", where I had a kgdb breakpoint. Presumably
the device would have eventually given up and shut down anyway, but it
seems better to fix the order to be more correct.

Cc: Kristian H. Kristensen <hoegsberg@chromium.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Andrzej Hajda March 22, 2017, 10:34 a.m. UTC | #1 
On 10.03.2017 05:32, Sean Paul wrote:
> From: Douglas Anderson <dianders@chromium.org>
>
> The current user of the analogix power_off is "analogix_dp-rockchip".
> That driver does this:
> - deactivate PSR
> - turn off a clock
>
> Both of these things (especially deactive PSR) should be done before
> we turn the PHY power off and turn off analog power.  Let's move the
> callback up.
>
> Note that without this patch (and with https://patchwork.kernel.org/patch/9553349/
> [seanpaul: this patch was not applied, but it seems like the race
> can still occur]), I experienced an error in reboot testing where one
> thread was at:
>
>   rockchip_drm_psr_deactivate
>   rockchip_dp_powerdown
>   analogix_dp_bridge_disable
>   drm_bridge_disable
>
> ...and the other thread was at:
>
>   analogix_dp_send_psr_spd
>   analogix_dp_enable_psr
>   analogix_dp_psr_set
>   psr_flush_handler
>
> The flush handler thread was finding AUX channel errors and eventually
> reported "Failed to apply PSR", where I had a kgdb breakpoint. Presumably
> the device would have eventually given up and shut down anyway, but it
> seems better to fix the order to be more correct.
>
> Cc: Kristian H. Kristensen <hoegsberg@chromium.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej
diff mbox

Patch

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 1454075bce9a..a05bc0271188 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1329,12 +1329,13 @@  static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
 	}
 
 	disable_irq(dp->irq);
-	phy_power_off(dp->phy);
-	analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
 
 	if (dp->plat_data->power_off)
 		dp->plat_data->power_off(dp->plat_data);
 
+	phy_power_off(dp->phy);
+	analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
+
 	clk_disable_unprepare(dp->clock);
 
 	pm_runtime_put_sync(dp->dev);