On 1/14/2022 1:41 PM, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2022-01-14 13:11:47)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 7cc4d21..7cd6222 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -696,12 +699,9 @@ static int dp_irq_hpd_handle(struct dp_display_private *dp, u32 data)
* dp core (ahb/aux clks) must be initialized before
* irq_hpd be handled
*/
- if (dp->core_initialized) {
- ret = dp_display_usbpd_attention_cb(&dp->pdev->dev);
- if (ret == -ECONNRESET) { /* cable unplugged */
- dp->core_initialized = false;
- }
- }
+ if (dp->core_initialized)
When is this condition false? The irq isn't unmasked until the core has
been initialized. On the resume path I suppose the irq is enabled in
dp_display_host_init() calling dp_ctrl_reset_irq_ctrl(), and then we
could immediately get the interrupt but it will block on the event_mutex
lock.
This is left over form Lazor.
I remember that there is an extreme case that several irq_hpd interrupts
happen right after dongle plug in (happen at resume too) and sometime
cause system crash at dpcd read due to AHB clock is not enabled yet. It
took some time to debug it.
From looking into code, it does not look likely it will happen. But it
did happen at real world.
So that I would like to keep this condition checking.
+ dp_display_usbpd_attention_cb(&dp->pdev->dev);
+
DRM_DEBUG_DP("hpd_state=%d\n", state);
mutex_unlock(&dp->event_mutex);
@@ -1363,14 +1373,16 @@ static int dp_pm_suspend(struct device *dev)
if (dp_power_clk_status(dp->power, DP_CTRL_PM))
dp_ctrl_off_link_stream(dp->ctrl);
+ dp_display_host_phy_exit(dp);
+
+ /* host_init will be called at pm_resume */
dp_display_host_deinit(dp);
+ } else {
+ dp_display_host_phy_exit(dp);
I fail to see where this condition happens. Can we suspend the device
without the irq being installed?
Agree, with this new mechanism it should not happen.
Will remove it.
}
dp->hpd_state = ST_SUSPENDED;
- /* host_init will be called at pm_resume */
- dp->core_initialized = false;
-
DRM_DEBUG_DP("After, core_inited=%d power_on=%d\n",
dp->core_initialized, dp_display->power_on);
