| Message ID | 1636493893-7600-1-git-send-email-quic_khsieh@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4] drm/msm/dp: do not initialize phy until plugin interrupt received | expand |
On 2021-11-09 13:38, Kuogee Hsieh wrote: > From: Kuogee Hsieh <khsieh@codeaurora.org> > > Current DP drivers have regulators, clocks, irq and phy are grouped > together within a function and executed not in a symmetric manner. > This increase difficulty of code maintenance and limited code > scalability. > This patch divided the driver life cycle of operation into four states, > resume (including booting up), dongle plugin, dongle unplugged and > suspend. > Regulators, core clocks and irq are grouped together and enabled at > resume > (or booting up) so that the DP controller is armed and ready to receive > HPD > plugin interrupts. HPD plugin interrupt is generated when a dongle > plugs > into DUT (device under test). Once HPD plugin interrupt is received, DP > controller will initialize phy so that dpcd read/write will function > and > following link training can be proceeded successfully. DP phy will be > disabled after main link is teared down at end of unplugged HPD > interrupt > handle triggered by dongle unplugged out of DUT. Finally regulators, > code > clocks and irq are disabled at corresponding suspension. > > Changes in V2: > -- removed unnecessary dp_ctrl NULL check > -- removed unnecessary phy init_count and power_count DRM_DEBUG_DP logs > -- remove flip parameter out of dp_ctrl_irq_enable() > -- add fixes tag > > Changes in V3: > -- call dp_display_host_phy_init() instead of dp_ctrl_phy_init() at > dp_display_host_init() for eDP > > Changes in V4: > -- rewording commit text to match this commit changes > > Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on > Snapdragon Chipsets") > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- any more comments? > drivers/gpu/drm/msm/dp/dp_ctrl.c | 87 > ++++++++++++++++--------------------- > drivers/gpu/drm/msm/dp/dp_ctrl.h | 9 ++-- > drivers/gpu/drm/msm/dp/dp_display.c | 83 > ++++++++++++++++++++++++++--------- > 3 files changed, 105 insertions(+), 74 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c > b/drivers/gpu/drm/msm/dp/dp_ctrl.c > index 7ec155d..4788e8c 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c > @@ -1364,60 +1364,54 @@ static int dp_ctrl_enable_stream_clocks(struct > dp_ctrl_private *ctrl) > return ret; > } > > -int dp_ctrl_host_init(struct dp_ctrl *dp_ctrl, bool flip, bool reset) > +void dp_ctrl_irq_enable(struct dp_ctrl *dp_ctrl) > +{ > + struct dp_ctrl_private *ctrl; > + > + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); > + > + dp_catalog_ctrl_reset(ctrl->catalog); > + > + dp_catalog_ctrl_enable_irq(ctrl->catalog, true); > +} > + > +void dp_ctrl_irq_disable(struct dp_ctrl *dp_ctrl) > +{ > + struct dp_ctrl_private *ctrl; > + > + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); > + > + dp_catalog_ctrl_reset(ctrl->catalog); > + > + dp_catalog_ctrl_enable_irq(ctrl->catalog, false); > +} > + > +void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl) > { > struct dp_ctrl_private *ctrl; > struct dp_io *dp_io; > struct phy *phy; > > - if (!dp_ctrl) { > - DRM_ERROR("Invalid input data\n"); > - return -EINVAL; > - } > - > ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); > dp_io = &ctrl->parser->io; > phy = dp_io->phy; > > - ctrl->dp_ctrl.orientation = flip; > - > - if (reset) > - dp_catalog_ctrl_reset(ctrl->catalog); > - > - DRM_DEBUG_DP("flip=%d\n", flip); > dp_catalog_ctrl_phy_reset(ctrl->catalog); > phy_init(phy); > - dp_catalog_ctrl_enable_irq(ctrl->catalog, true); > - > - return 0; > } > > -/** > - * dp_ctrl_host_deinit() - Uninitialize DP controller > - * @dp_ctrl: Display Port Driver data > - * > - * Perform required steps to uninitialize DP controller > - * and its resources. > - */ > -void dp_ctrl_host_deinit(struct dp_ctrl *dp_ctrl) > +void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl) > { > struct dp_ctrl_private *ctrl; > struct dp_io *dp_io; > struct phy *phy; > > - if (!dp_ctrl) { > - DRM_ERROR("Invalid input data\n"); > - return; > - } > - > ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); > dp_io = &ctrl->parser->io; > phy = dp_io->phy; > > - dp_catalog_ctrl_enable_irq(ctrl->catalog, false); > + dp_catalog_ctrl_phy_reset(ctrl->catalog); > phy_exit(phy); > - > - DRM_DEBUG_DP("Host deinitialized successfully\n"); > } > > static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl) > @@ -1895,8 +1889,14 @@ int dp_ctrl_off_link_stream(struct dp_ctrl > *dp_ctrl) > return ret; > } > > + DRM_DEBUG_DP("Before, phy=%x init_count=%d power_on=%d\n", > + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); > + > phy_power_off(phy); > > + DRM_DEBUG_DP("After, phy=%x init_count=%d power_on=%d\n", > + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); > + > /* aux channel down, reinit phy */ > phy_exit(phy); > phy_init(phy); > @@ -1905,23 +1905,6 @@ int dp_ctrl_off_link_stream(struct dp_ctrl > *dp_ctrl) > return ret; > } > > -void dp_ctrl_off_phy(struct dp_ctrl *dp_ctrl) > -{ > - struct dp_ctrl_private *ctrl; > - struct dp_io *dp_io; > - struct phy *phy; > - > - ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); > - dp_io = &ctrl->parser->io; > - phy = dp_io->phy; > - > - dp_catalog_ctrl_reset(ctrl->catalog); > - > - phy_exit(phy); > - > - DRM_DEBUG_DP("DP off phy done\n"); > -} > - > int dp_ctrl_off(struct dp_ctrl *dp_ctrl) > { > struct dp_ctrl_private *ctrl; > @@ -1949,10 +1932,14 @@ int dp_ctrl_off(struct dp_ctrl *dp_ctrl) > DRM_ERROR("Failed to disable link clocks. ret=%d\n", ret); > } > > + DRM_DEBUG_DP("Before, phy=%x init_count=%d power_on=%d\n", > + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); > + > phy_power_off(phy); > - phy_exit(phy); > > - DRM_DEBUG_DP("DP off done\n"); > + DRM_DEBUG_DP("After, phy=%x init_count=%d power_on=%d\n", > + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); > + > return ret; > } > > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h > b/drivers/gpu/drm/msm/dp/dp_ctrl.h > index 2363a2d..30f9414 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h > @@ -19,12 +19,9 @@ struct dp_ctrl { > u32 pixel_rate; > }; > > -int dp_ctrl_host_init(struct dp_ctrl *dp_ctrl, bool flip, bool reset); > -void dp_ctrl_host_deinit(struct dp_ctrl *dp_ctrl); > int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl); > int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl); > int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl); > -void dp_ctrl_off_phy(struct dp_ctrl *dp_ctrl); > int dp_ctrl_off(struct dp_ctrl *dp_ctrl); > void dp_ctrl_push_idle(struct dp_ctrl *dp_ctrl); > void dp_ctrl_isr(struct dp_ctrl *dp_ctrl); > @@ -34,4 +31,10 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, > struct dp_link *link, > struct dp_power *power, struct dp_catalog *catalog, > struct dp_parser *parser); > > +void dp_ctrl_irq_enable(struct dp_ctrl *dp_ctrl); > +void dp_ctrl_irq_disable(struct dp_ctrl *dp_ctrl); > +void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl); > +void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl); > +void dp_ctrl_irq_phy_exit(struct dp_ctrl *dp_ctrl); > + > #endif /* _DP_CTRL_H_ */ > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index e41dd40..2f113ff 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -84,6 +84,7 @@ struct dp_display_private { > > /* state variables */ > bool core_initialized; > + bool phy_initialized; > bool hpd_irq_on; > bool audio_supported; > > @@ -387,7 +388,29 @@ static int dp_display_process_hpd_high(struct > dp_display_private *dp) > return rc; > } > > -static void dp_display_host_init(struct dp_display_private *dp, int > reset) > +static void dp_display_host_phy_init(struct dp_display_private *dp) > +{ > + DRM_DEBUG_DP("core_init=%d phy_init=%d\n", > + dp->core_initialized, dp->phy_initialized); > + > + if (!dp->phy_initialized) { > + dp_ctrl_phy_init(dp->ctrl); > + dp->phy_initialized = true; > + } > +} > + > +static void dp_display_host_phy_exit(struct dp_display_private *dp) > +{ > + DRM_DEBUG_DP("core_init=%d phy_init=%d\n", > + dp->core_initialized, dp->phy_initialized); > + > + if (dp->phy_initialized) { > + dp_ctrl_phy_exit(dp->ctrl); > + dp->phy_initialized = false; > + } > +} > + > +static void dp_display_host_init(struct dp_display_private *dp) > { > bool flip = false; > > @@ -400,8 +423,16 @@ static void dp_display_host_init(struct > dp_display_private *dp, int reset) > if (dp->usbpd->orientation == ORIENTATION_CC2) > flip = true; > > - dp_power_init(dp->power, flip); > - dp_ctrl_host_init(dp->ctrl, flip, reset); > + dp_power_init(dp->power, false); > + dp_ctrl_irq_enable(dp->ctrl); > + > + /* > + * eDP is the embedded primary display and has its own phy > + * initialize phy immediately > + */ > + if (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP) > + dp_display_host_phy_init(dp); > + > dp_aux_init(dp->aux); > dp->core_initialized = true; > } > @@ -413,7 +444,7 @@ static void dp_display_host_deinit(struct > dp_display_private *dp) > return; > } > > - dp_ctrl_host_deinit(dp->ctrl); > + dp_ctrl_irq_disable(dp->ctrl); > dp_aux_deinit(dp->aux); > dp_power_deinit(dp->power); > > @@ -424,7 +455,7 @@ static int dp_display_usbpd_configure_cb(struct > device *dev) > { > struct dp_display_private *dp = dev_get_dp_display_private(dev); > > - dp_display_host_init(dp, true); > + dp_display_host_phy_init(dp); > > return dp_display_process_hpd_high(dp); > } > @@ -551,7 +582,7 @@ static int dp_hpd_plug_handle(struct > dp_display_private *dp, u32 data) > dp->hpd_state = ST_DISCONNECTED; > > if (ret == -ECONNRESET) { /* cable unplugged */ > - dp->core_initialized = false; > + dp->phy_initialized = false; > } > > } else { > @@ -623,9 +654,8 @@ static int dp_hpd_unplug_handle(struct > dp_display_private *dp, u32 data) > if (state == ST_DISCONNECTED) { > /* triggered by irq_hdp with sink_count = 0 */ > if (dp->link->sink_count == 0) { > - dp_ctrl_off_phy(dp->ctrl); > + dp_display_host_phy_exit(dp); > hpd->hpd_high = 0; > - dp->core_initialized = false; > } > mutex_unlock(&dp->event_mutex); > return 0; > @@ -716,7 +746,7 @@ static int dp_irq_hpd_handle(struct > dp_display_private *dp, u32 data) > > ret = dp_display_usbpd_attention_cb(&dp->pdev->dev); > if (ret == -ECONNRESET) { /* cable unplugged */ > - dp->core_initialized = false; > + dp->phy_initialized = false; > } > DRM_DEBUG_DP("hpd_state=%d\n", state); > > @@ -918,12 +948,19 @@ static int dp_display_disable(struct > dp_display_private *dp, u32 data) > > dp_display->audio_enabled = false; > > - /* triggered by irq_hpd with sink_count = 0 */ > if (dp->link->sink_count == 0) { > + /* > + * irq_hpd with sink_count = 0 > + * hdmi unplugged out of dongle > + */ > dp_ctrl_off_link_stream(dp->ctrl); > } else { > + /* > + * unplugged interrupt > + * dongle unplugged out of DUT > + */ > dp_ctrl_off(dp->ctrl); > - dp->core_initialized = false; > + dp_display_host_phy_exit(dp); > } > > dp_power_panel_on(dp->power, false); > @@ -1059,7 +1096,7 @@ void msm_dp_snapshot(struct msm_disp_state > *disp_state, struct msm_dp *dp) > static void dp_display_config_hpd(struct dp_display_private *dp) > { > > - dp_display_host_init(dp, true); > + dp_display_host_init(dp); > dp_catalog_ctrl_hpd_config(dp->catalog); > > /* Enable interrupt first time > @@ -1338,20 +1375,22 @@ static int dp_pm_resume(struct device *dev) > dp->hpd_state = ST_DISCONNECTED; > > /* turn on dp ctrl/phy */ > - dp_display_host_init(dp, true); > + dp_display_host_init(dp); > > dp_catalog_ctrl_hpd_config(dp->catalog); > > - /* > - * set sink to normal operation mode -- D0 > - * before dpcd read > - */ > - dp_link_psm_config(dp->link, &dp->panel->link_info, false); > - > if (dp_catalog_link_is_connected(dp->catalog)) { > + /* > + * set sink to normal operation mode -- D0 > + * before dpcd read > + */ > + dp_display_host_phy_init(dp); > + dp_link_psm_config(dp->link, &dp->panel->link_info, false); > sink_count = drm_dp_read_sink_count(dp->aux); > if (sink_count < 0) > sink_count = 0; > + > + dp_display_host_phy_exit(dp); > } > > dp->link->sink_count = sink_count; > @@ -1399,6 +1438,8 @@ static int dp_pm_suspend(struct device *dev) > dp_display_host_deinit(dp); > } > > + dp_display_host_phy_exit(dp); > + > dp->hpd_state = ST_SUSPENDED; > > /* host_init will be called at pm_resume */ > @@ -1473,7 +1514,7 @@ void msm_dp_irq_postinstall(struct msm_dp > *dp_display) > enable_irq(dp->irq); > dp_hpd_connect(dp->usbpd, true); > } else { > - dp_add_event(dp, EV_HPD_INIT_SETUP, 0, dp->id * 10); > + dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 0); > } > } > > @@ -1567,7 +1608,7 @@ int msm_dp_display_enable(struct msm_dp *dp, > struct drm_encoder *encoder) > state = dp_display->hpd_state; > > if (state == ST_DISPLAY_OFF) > - dp_display_host_init(dp_display, true); > + dp_display_host_phy_init(dp_display); > > dp_display_enable(dp_display, 0);
On 11/15/2021 12:23 PM, khsieh@codeaurora.org wrote: > On 2021-11-09 13:38, Kuogee Hsieh wrote: >> From: Kuogee Hsieh <khsieh@codeaurora.org> >> >> Current DP drivers have regulators, clocks, irq and phy are grouped >> together within a function and executed not in a symmetric manner. >> This increase difficulty of code maintenance and limited code >> scalability. >> This patch divided the driver life cycle of operation into four states, >> resume (including booting up), dongle plugin, dongle unplugged and >> suspend. >> Regulators, core clocks and irq are grouped together and enabled at >> resume >> (or booting up) so that the DP controller is armed and ready to >> receive HPD >> plugin interrupts. HPD plugin interrupt is generated when a dongle plugs >> into DUT (device under test). Once HPD plugin interrupt is received, DP >> controller will initialize phy so that dpcd read/write will function and >> following link training can be proceeded successfully. DP phy will be >> disabled after main link is teared down at end of unplugged HPD >> interrupt >> handle triggered by dongle unplugged out of DUT. Finally regulators, >> code >> clocks and irq are disabled at corresponding suspension. >> >> Changes in V2: >> -- removed unnecessary dp_ctrl NULL check >> -- removed unnecessary phy init_count and power_count DRM_DEBUG_DP logs >> -- remove flip parameter out of dp_ctrl_irq_enable() >> -- add fixes tag >> >> Changes in V3: >> -- call dp_display_host_phy_init() instead of dp_ctrl_phy_init() at >> dp_display_host_init() for eDP >> >> Changes in V4: >> -- rewording commit text to match this commit changes >> >> Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on >> Snapdragon Chipsets") >> >> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> >> --- > > any more comments? > Hi Stephen/Bjorn, Any more comments? We like to merge this patch by the end of this month. Would you please give me your comments asap. Thanks, >> drivers/gpu/drm/msm/dp/dp_ctrl.c | 87 >> ++++++++++++++++--------------------- >> drivers/gpu/drm/msm/dp/dp_ctrl.h | 9 ++-- >> drivers/gpu/drm/msm/dp/dp_display.c | 83 >> ++++++++++++++++++++++++++--------- >> 3 files changed, 105 insertions(+), 74 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c >> b/drivers/gpu/drm/msm/dp/dp_ctrl.c >> index 7ec155d..4788e8c 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c >> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c >> @@ -1364,60 +1364,54 @@ static int dp_ctrl_enable_stream_clocks(struct >> dp_ctrl_private *ctrl) >> return ret; >> } >> >> -int dp_ctrl_host_init(struct dp_ctrl *dp_ctrl, bool flip, bool reset) >> +void dp_ctrl_irq_enable(struct dp_ctrl *dp_ctrl) >> +{ >> + struct dp_ctrl_private *ctrl; >> + >> + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); >> + >> + dp_catalog_ctrl_reset(ctrl->catalog); >> + >> + dp_catalog_ctrl_enable_irq(ctrl->catalog, true); >> +} >> + >> +void dp_ctrl_irq_disable(struct dp_ctrl *dp_ctrl) >> +{ >> + struct dp_ctrl_private *ctrl; >> + >> + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); >> + >> + dp_catalog_ctrl_reset(ctrl->catalog); >> + >> + dp_catalog_ctrl_enable_irq(ctrl->catalog, false); >> +} >> + >> +void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl) >> { >> struct dp_ctrl_private *ctrl; >> struct dp_io *dp_io; >> struct phy *phy; >> >> - if (!dp_ctrl) { >> - DRM_ERROR("Invalid input data\n"); >> - return -EINVAL; >> - } >> - >> ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); >> dp_io = &ctrl->parser->io; >> phy = dp_io->phy; >> >> - ctrl->dp_ctrl.orientation = flip; >> - >> - if (reset) >> - dp_catalog_ctrl_reset(ctrl->catalog); >> - >> - DRM_DEBUG_DP("flip=%d\n", flip); >> dp_catalog_ctrl_phy_reset(ctrl->catalog); >> phy_init(phy); >> - dp_catalog_ctrl_enable_irq(ctrl->catalog, true); >> - >> - return 0; >> } >> >> -/** >> - * dp_ctrl_host_deinit() - Uninitialize DP controller >> - * @dp_ctrl: Display Port Driver data >> - * >> - * Perform required steps to uninitialize DP controller >> - * and its resources. >> - */ >> -void dp_ctrl_host_deinit(struct dp_ctrl *dp_ctrl) >> +void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl) >> { >> struct dp_ctrl_private *ctrl; >> struct dp_io *dp_io; >> struct phy *phy; >> >> - if (!dp_ctrl) { >> - DRM_ERROR("Invalid input data\n"); >> - return; >> - } >> - >> ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); >> dp_io = &ctrl->parser->io; >> phy = dp_io->phy; >> >> - dp_catalog_ctrl_enable_irq(ctrl->catalog, false); >> + dp_catalog_ctrl_phy_reset(ctrl->catalog); >> phy_exit(phy); >> - >> - DRM_DEBUG_DP("Host deinitialized successfully\n"); >> } >> >> static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl) >> @@ -1895,8 +1889,14 @@ int dp_ctrl_off_link_stream(struct dp_ctrl >> *dp_ctrl) >> return ret; >> } >> >> + DRM_DEBUG_DP("Before, phy=%x init_count=%d power_on=%d\n", >> + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); >> + >> phy_power_off(phy); >> >> + DRM_DEBUG_DP("After, phy=%x init_count=%d power_on=%d\n", >> + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); >> + >> /* aux channel down, reinit phy */ >> phy_exit(phy); >> phy_init(phy); >> @@ -1905,23 +1905,6 @@ int dp_ctrl_off_link_stream(struct dp_ctrl >> *dp_ctrl) >> return ret; >> } >> >> -void dp_ctrl_off_phy(struct dp_ctrl *dp_ctrl) >> -{ >> - struct dp_ctrl_private *ctrl; >> - struct dp_io *dp_io; >> - struct phy *phy; >> - >> - ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); >> - dp_io = &ctrl->parser->io; >> - phy = dp_io->phy; >> - >> - dp_catalog_ctrl_reset(ctrl->catalog); >> - >> - phy_exit(phy); >> - >> - DRM_DEBUG_DP("DP off phy done\n"); >> -} >> - >> int dp_ctrl_off(struct dp_ctrl *dp_ctrl) >> { >> struct dp_ctrl_private *ctrl; >> @@ -1949,10 +1932,14 @@ int dp_ctrl_off(struct dp_ctrl *dp_ctrl) >> DRM_ERROR("Failed to disable link clocks. ret=%d\n", ret); >> } >> >> + DRM_DEBUG_DP("Before, phy=%x init_count=%d power_on=%d\n", >> + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); >> + >> phy_power_off(phy); >> - phy_exit(phy); >> >> - DRM_DEBUG_DP("DP off done\n"); >> + DRM_DEBUG_DP("After, phy=%x init_count=%d power_on=%d\n", >> + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); >> + >> return ret; >> } >> >> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h >> b/drivers/gpu/drm/msm/dp/dp_ctrl.h >> index 2363a2d..30f9414 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h >> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h >> @@ -19,12 +19,9 @@ struct dp_ctrl { >> u32 pixel_rate; >> }; >> >> -int dp_ctrl_host_init(struct dp_ctrl *dp_ctrl, bool flip, bool reset); >> -void dp_ctrl_host_deinit(struct dp_ctrl *dp_ctrl); >> int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl); >> int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl); >> int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl); >> -void dp_ctrl_off_phy(struct dp_ctrl *dp_ctrl); >> int dp_ctrl_off(struct dp_ctrl *dp_ctrl); >> void dp_ctrl_push_idle(struct dp_ctrl *dp_ctrl); >> void dp_ctrl_isr(struct dp_ctrl *dp_ctrl); >> @@ -34,4 +31,10 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, >> struct dp_link *link, >> struct dp_power *power, struct dp_catalog *catalog, >> struct dp_parser *parser); >> >> +void dp_ctrl_irq_enable(struct dp_ctrl *dp_ctrl); >> +void dp_ctrl_irq_disable(struct dp_ctrl *dp_ctrl); >> +void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl); >> +void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl); >> +void dp_ctrl_irq_phy_exit(struct dp_ctrl *dp_ctrl); >> + >> #endif /* _DP_CTRL_H_ */ >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c >> b/drivers/gpu/drm/msm/dp/dp_display.c >> index e41dd40..2f113ff 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_display.c >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c >> @@ -84,6 +84,7 @@ struct dp_display_private { >> >> /* state variables */ >> bool core_initialized; >> + bool phy_initialized; >> bool hpd_irq_on; >> bool audio_supported; >> >> @@ -387,7 +388,29 @@ static int dp_display_process_hpd_high(struct >> dp_display_private *dp) >> return rc; >> } >> >> -static void dp_display_host_init(struct dp_display_private *dp, int >> reset) >> +static void dp_display_host_phy_init(struct dp_display_private *dp) >> +{ >> + DRM_DEBUG_DP("core_init=%d phy_init=%d\n", >> + dp->core_initialized, dp->phy_initialized); >> + >> + if (!dp->phy_initialized) { >> + dp_ctrl_phy_init(dp->ctrl); >> + dp->phy_initialized = true; >> + } >> +} >> + >> +static void dp_display_host_phy_exit(struct dp_display_private *dp) >> +{ >> + DRM_DEBUG_DP("core_init=%d phy_init=%d\n", >> + dp->core_initialized, dp->phy_initialized); >> + >> + if (dp->phy_initialized) { >> + dp_ctrl_phy_exit(dp->ctrl); >> + dp->phy_initialized = false; >> + } >> +} >> + >> +static void dp_display_host_init(struct dp_display_private *dp) >> { >> bool flip = false; >> >> @@ -400,8 +423,16 @@ static void dp_display_host_init(struct >> dp_display_private *dp, int reset) >> if (dp->usbpd->orientation == ORIENTATION_CC2) >> flip = true; >> >> - dp_power_init(dp->power, flip); >> - dp_ctrl_host_init(dp->ctrl, flip, reset); >> + dp_power_init(dp->power, false); >> + dp_ctrl_irq_enable(dp->ctrl); >> + >> + /* >> + * eDP is the embedded primary display and has its own phy >> + * initialize phy immediately >> + */ >> + if (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP) >> + dp_display_host_phy_init(dp); >> + >> dp_aux_init(dp->aux); >> dp->core_initialized = true; >> } >> @@ -413,7 +444,7 @@ static void dp_display_host_deinit(struct >> dp_display_private *dp) >> return; >> } >> >> - dp_ctrl_host_deinit(dp->ctrl); >> + dp_ctrl_irq_disable(dp->ctrl); >> dp_aux_deinit(dp->aux); >> dp_power_deinit(dp->power); >> >> @@ -424,7 +455,7 @@ static int dp_display_usbpd_configure_cb(struct >> device *dev) >> { >> struct dp_display_private *dp = dev_get_dp_display_private(dev); >> >> - dp_display_host_init(dp, true); >> + dp_display_host_phy_init(dp); >> >> return dp_display_process_hpd_high(dp); >> } >> @@ -551,7 +582,7 @@ static int dp_hpd_plug_handle(struct >> dp_display_private *dp, u32 data) >> dp->hpd_state = ST_DISCONNECTED; >> >> if (ret == -ECONNRESET) { /* cable unplugged */ >> - dp->core_initialized = false; >> + dp->phy_initialized = false; >> } >> >> } else { >> @@ -623,9 +654,8 @@ static int dp_hpd_unplug_handle(struct >> dp_display_private *dp, u32 data) >> if (state == ST_DISCONNECTED) { >> /* triggered by irq_hdp with sink_count = 0 */ >> if (dp->link->sink_count == 0) { >> - dp_ctrl_off_phy(dp->ctrl); >> + dp_display_host_phy_exit(dp); >> hpd->hpd_high = 0; >> - dp->core_initialized = false; >> } >> mutex_unlock(&dp->event_mutex); >> return 0; >> @@ -716,7 +746,7 @@ static int dp_irq_hpd_handle(struct >> dp_display_private *dp, u32 data) >> >> ret = dp_display_usbpd_attention_cb(&dp->pdev->dev); >> if (ret == -ECONNRESET) { /* cable unplugged */ >> - dp->core_initialized = false; >> + dp->phy_initialized = false; >> } >> DRM_DEBUG_DP("hpd_state=%d\n", state); >> >> @@ -918,12 +948,19 @@ static int dp_display_disable(struct >> dp_display_private *dp, u32 data) >> >> dp_display->audio_enabled = false; >> >> - /* triggered by irq_hpd with sink_count = 0 */ >> if (dp->link->sink_count == 0) { >> + /* >> + * irq_hpd with sink_count = 0 >> + * hdmi unplugged out of dongle >> + */ >> dp_ctrl_off_link_stream(dp->ctrl); >> } else { >> + /* >> + * unplugged interrupt >> + * dongle unplugged out of DUT >> + */ >> dp_ctrl_off(dp->ctrl); >> - dp->core_initialized = false; >> + dp_display_host_phy_exit(dp); >> } >> >> dp_power_panel_on(dp->power, false); >> @@ -1059,7 +1096,7 @@ void msm_dp_snapshot(struct msm_disp_state >> *disp_state, struct msm_dp *dp) >> static void dp_display_config_hpd(struct dp_display_private *dp) >> { >> >> - dp_display_host_init(dp, true); >> + dp_display_host_init(dp); >> dp_catalog_ctrl_hpd_config(dp->catalog); >> >> /* Enable interrupt first time >> @@ -1338,20 +1375,22 @@ static int dp_pm_resume(struct device *dev) >> dp->hpd_state = ST_DISCONNECTED; >> >> /* turn on dp ctrl/phy */ >> - dp_display_host_init(dp, true); >> + dp_display_host_init(dp); >> >> dp_catalog_ctrl_hpd_config(dp->catalog); >> >> - /* >> - * set sink to normal operation mode -- D0 >> - * before dpcd read >> - */ >> - dp_link_psm_config(dp->link, &dp->panel->link_info, false); >> - >> if (dp_catalog_link_is_connected(dp->catalog)) { >> + /* >> + * set sink to normal operation mode -- D0 >> + * before dpcd read >> + */ >> + dp_display_host_phy_init(dp); >> + dp_link_psm_config(dp->link, &dp->panel->link_info, false); >> sink_count = drm_dp_read_sink_count(dp->aux); >> if (sink_count < 0) >> sink_count = 0; >> + >> + dp_display_host_phy_exit(dp); >> } >> >> dp->link->sink_count = sink_count; >> @@ -1399,6 +1438,8 @@ static int dp_pm_suspend(struct device *dev) >> dp_display_host_deinit(dp); >> } >> >> + dp_display_host_phy_exit(dp); >> + >> dp->hpd_state = ST_SUSPENDED; >> >> /* host_init will be called at pm_resume */ >> @@ -1473,7 +1514,7 @@ void msm_dp_irq_postinstall(struct msm_dp >> *dp_display) >> enable_irq(dp->irq); >> dp_hpd_connect(dp->usbpd, true); >> } else { >> - dp_add_event(dp, EV_HPD_INIT_SETUP, 0, dp->id * 10); >> + dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 0); >> } >> } >> >> @@ -1567,7 +1608,7 @@ int msm_dp_display_enable(struct msm_dp *dp, >> struct drm_encoder *encoder) >> state = dp_display->hpd_state; >> >> if (state == ST_DISPLAY_OFF) >> - dp_display_host_init(dp_display, true); >> + dp_display_host_phy_init(dp_display); >> >> dp_display_enable(dp_display, 0);
Quoting Kuogee Hsieh (2021-11-09 13:38:13) > From: Kuogee Hsieh <khsieh@codeaurora.org> > > Current DP drivers have regulators, clocks, irq and phy are grouped > together within a function and executed not in a symmetric manner. > This increase difficulty of code maintenance and limited code scalability. > This patch divided the driver life cycle of operation into four states, > resume (including booting up), dongle plugin, dongle unplugged and suspend. > Regulators, core clocks and irq are grouped together and enabled at resume > (or booting up) so that the DP controller is armed and ready to receive HPD > plugin interrupts. HPD plugin interrupt is generated when a dongle plugs > into DUT (device under test). Once HPD plugin interrupt is received, DP > controller will initialize phy so that dpcd read/write will function and > following link training can be proceeded successfully. DP phy will be > disabled after main link is teared down at end of unplugged HPD interrupt > handle triggered by dongle unplugged out of DUT. Finally regulators, code > clocks and irq are disabled at corresponding suspension. > > Changes in V2: > -- removed unnecessary dp_ctrl NULL check > -- removed unnecessary phy init_count and power_count DRM_DEBUG_DP logs > -- remove flip parameter out of dp_ctrl_irq_enable() > -- add fixes tag > > Changes in V3: > -- call dp_display_host_phy_init() instead of dp_ctrl_phy_init() at > dp_display_host_init() for eDP > > Changes in V4: > -- rewording commit text to match this commit changes > > Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon Chipsets") > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- What commit is this patch based on?
On 11/18/2021 5:20 PM, Stephen Boyd wrote: > Quoting Kuogee Hsieh (2021-11-09 13:38:13) >> From: Kuogee Hsieh <khsieh@codeaurora.org> >> >> Current DP drivers have regulators, clocks, irq and phy are grouped >> together within a function and executed not in a symmetric manner. >> This increase difficulty of code maintenance and limited code scalability. >> This patch divided the driver life cycle of operation into four states, >> resume (including booting up), dongle plugin, dongle unplugged and suspend. >> Regulators, core clocks and irq are grouped together and enabled at resume >> (or booting up) so that the DP controller is armed and ready to receive HPD >> plugin interrupts. HPD plugin interrupt is generated when a dongle plugs >> into DUT (device under test). Once HPD plugin interrupt is received, DP >> controller will initialize phy so that dpcd read/write will function and >> following link training can be proceeded successfully. DP phy will be >> disabled after main link is teared down at end of unplugged HPD interrupt >> handle triggered by dongle unplugged out of DUT. Finally regulators, code >> clocks and irq are disabled at corresponding suspension. >> >> Changes in V2: >> -- removed unnecessary dp_ctrl NULL check >> -- removed unnecessary phy init_count and power_count DRM_DEBUG_DP logs >> -- remove flip parameter out of dp_ctrl_irq_enable() >> -- add fixes tag >> >> Changes in V3: >> -- call dp_display_host_phy_init() instead of dp_ctrl_phy_init() at >> dp_display_host_init() for eDP >> >> Changes in V4: >> -- rewording commit text to match this commit changes >> >> Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon Chipsets") >> >> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> >> --- > What commit is this patch based on? It base on Bjorn's patch, https://patchwork.freedesktop.org/series/92992/
Quoting Kuogee Hsieh (2021-11-09 13:38:13) > From: Kuogee Hsieh <khsieh@codeaurora.org> > > Current DP drivers have regulators, clocks, irq and phy are grouped > together within a function and executed not in a symmetric manner. > This increase difficulty of code maintenance and limited code scalability. > This patch divided the driver life cycle of operation into four states, > resume (including booting up), dongle plugin, dongle unplugged and suspend. > Regulators, core clocks and irq are grouped together and enabled at resume > (or booting up) so that the DP controller is armed and ready to receive HPD > plugin interrupts. HPD plugin interrupt is generated when a dongle plugs > into DUT (device under test). Once HPD plugin interrupt is received, DP > controller will initialize phy so that dpcd read/write will function and > following link training can be proceeded successfully. DP phy will be > disabled after main link is teared down at end of unplugged HPD interrupt > handle triggered by dongle unplugged out of DUT. Finally regulators, code > clocks and irq are disabled at corresponding suspension. > > Changes in V2: > -- removed unnecessary dp_ctrl NULL check > -- removed unnecessary phy init_count and power_count DRM_DEBUG_DP logs > -- remove flip parameter out of dp_ctrl_irq_enable() > -- add fixes tag > > Changes in V3: > -- call dp_display_host_phy_init() instead of dp_ctrl_phy_init() at > dp_display_host_init() for eDP > > Changes in V4: > -- rewording commit text to match this commit changes > > Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon Chipsets") > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- Can you please resend this based on the msm-next branch[1]? It doesn't apply now that other patches have been applied. [1] https://gitlab.freedesktop.org/drm/msm.git msm-next > drivers/gpu/drm/msm/dp/dp_ctrl.c | 87 ++++++++++++++++--------------------- > drivers/gpu/drm/msm/dp/dp_ctrl.h | 9 ++-- > drivers/gpu/drm/msm/dp/dp_display.c | 83 ++++++++++++++++++++++++++--------- > 3 files changed, 105 insertions(+), 74 deletions(-) >
On 12/2/2021 6:41 PM, Stephen Boyd wrote: > Quoting Kuogee Hsieh (2021-11-09 13:38:13) >> From: Kuogee Hsieh <khsieh@codeaurora.org> >> >> Current DP drivers have regulators, clocks, irq and phy are grouped >> together within a function and executed not in a symmetric manner. >> This increase difficulty of code maintenance and limited code scalability. >> This patch divided the driver life cycle of operation into four states, >> resume (including booting up), dongle plugin, dongle unplugged and suspend. >> Regulators, core clocks and irq are grouped together and enabled at resume >> (or booting up) so that the DP controller is armed and ready to receive HPD >> plugin interrupts. HPD plugin interrupt is generated when a dongle plugs >> into DUT (device under test). Once HPD plugin interrupt is received, DP >> controller will initialize phy so that dpcd read/write will function and >> following link training can be proceeded successfully. DP phy will be >> disabled after main link is teared down at end of unplugged HPD interrupt >> handle triggered by dongle unplugged out of DUT. Finally regulators, code >> clocks and irq are disabled at corresponding suspension. >> >> Changes in V2: >> -- removed unnecessary dp_ctrl NULL check >> -- removed unnecessary phy init_count and power_count DRM_DEBUG_DP logs >> -- remove flip parameter out of dp_ctrl_irq_enable() >> -- add fixes tag >> >> Changes in V3: >> -- call dp_display_host_phy_init() instead of dp_ctrl_phy_init() at >> dp_display_host_init() for eDP >> >> Changes in V4: >> -- rewording commit text to match this commit changes >> >> Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon Chipsets") >> >> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> >> --- > Can you please resend this based on the msm-next branch[1]? It doesn't > apply now that other patches have been applied. > > [1] https://gitlab.freedesktop.org/drm/msm.git msm-next Stephen, below patches missed at msm-next branch. Are there merged soon? 1) WIP: drm/msm/dp: Detect the connector type based on reg property 2) WIP: handle no-hpd property in DP driver >> drivers/gpu/drm/msm/dp/dp_ctrl.c | 87 ++++++++++++++++--------------------- >> drivers/gpu/drm/msm/dp/dp_ctrl.h | 9 ++-- >> drivers/gpu/drm/msm/dp/dp_display.c | 83 ++++++++++++++++++++++++++--------- >> 3 files changed, 105 insertions(+), 74 deletions(-) >>
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 7ec155d..4788e8c 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1364,60 +1364,54 @@ static int dp_ctrl_enable_stream_clocks(struct dp_ctrl_private *ctrl) return ret; } -int dp_ctrl_host_init(struct dp_ctrl *dp_ctrl, bool flip, bool reset) +void dp_ctrl_irq_enable(struct dp_ctrl *dp_ctrl) +{ + struct dp_ctrl_private *ctrl; + + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); + + dp_catalog_ctrl_reset(ctrl->catalog); + + dp_catalog_ctrl_enable_irq(ctrl->catalog, true); +} + +void dp_ctrl_irq_disable(struct dp_ctrl *dp_ctrl) +{ + struct dp_ctrl_private *ctrl; + + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); + + dp_catalog_ctrl_reset(ctrl->catalog); + + dp_catalog_ctrl_enable_irq(ctrl->catalog, false); +} + +void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl) { struct dp_ctrl_private *ctrl; struct dp_io *dp_io; struct phy *phy; - if (!dp_ctrl) { - DRM_ERROR("Invalid input data\n"); - return -EINVAL; - } - ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); dp_io = &ctrl->parser->io; phy = dp_io->phy; - ctrl->dp_ctrl.orientation = flip; - - if (reset) - dp_catalog_ctrl_reset(ctrl->catalog); - - DRM_DEBUG_DP("flip=%d\n", flip); dp_catalog_ctrl_phy_reset(ctrl->catalog); phy_init(phy); - dp_catalog_ctrl_enable_irq(ctrl->catalog, true); - - return 0; } -/** - * dp_ctrl_host_deinit() - Uninitialize DP controller - * @dp_ctrl: Display Port Driver data - * - * Perform required steps to uninitialize DP controller - * and its resources. - */ -void dp_ctrl_host_deinit(struct dp_ctrl *dp_ctrl) +void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl) { struct dp_ctrl_private *ctrl; struct dp_io *dp_io; struct phy *phy; - if (!dp_ctrl) { - DRM_ERROR("Invalid input data\n"); - return; - } - ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); dp_io = &ctrl->parser->io; phy = dp_io->phy; - dp_catalog_ctrl_enable_irq(ctrl->catalog, false); + dp_catalog_ctrl_phy_reset(ctrl->catalog); phy_exit(phy); - - DRM_DEBUG_DP("Host deinitialized successfully\n"); } static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl) @@ -1895,8 +1889,14 @@ int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl) return ret; } + DRM_DEBUG_DP("Before, phy=%x init_count=%d power_on=%d\n", + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); + phy_power_off(phy); + DRM_DEBUG_DP("After, phy=%x init_count=%d power_on=%d\n", + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); + /* aux channel down, reinit phy */ phy_exit(phy); phy_init(phy); @@ -1905,23 +1905,6 @@ int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl) return ret; } -void dp_ctrl_off_phy(struct dp_ctrl *dp_ctrl) -{ - struct dp_ctrl_private *ctrl; - struct dp_io *dp_io; - struct phy *phy; - - ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); - dp_io = &ctrl->parser->io; - phy = dp_io->phy; - - dp_catalog_ctrl_reset(ctrl->catalog); - - phy_exit(phy); - - DRM_DEBUG_DP("DP off phy done\n"); -} - int dp_ctrl_off(struct dp_ctrl *dp_ctrl) { struct dp_ctrl_private *ctrl; @@ -1949,10 +1932,14 @@ int dp_ctrl_off(struct dp_ctrl *dp_ctrl) DRM_ERROR("Failed to disable link clocks. ret=%d\n", ret); } + DRM_DEBUG_DP("Before, phy=%x init_count=%d power_on=%d\n", + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); + phy_power_off(phy); - phy_exit(phy); - DRM_DEBUG_DP("DP off done\n"); + DRM_DEBUG_DP("After, phy=%x init_count=%d power_on=%d\n", + (u32)(uintptr_t)phy, phy->init_count, phy->power_count); + return ret; } diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h index 2363a2d..30f9414 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h @@ -19,12 +19,9 @@ struct dp_ctrl { u32 pixel_rate; }; -int dp_ctrl_host_init(struct dp_ctrl *dp_ctrl, bool flip, bool reset); -void dp_ctrl_host_deinit(struct dp_ctrl *dp_ctrl); int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl); int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl); int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl); -void dp_ctrl_off_phy(struct dp_ctrl *dp_ctrl); int dp_ctrl_off(struct dp_ctrl *dp_ctrl); void dp_ctrl_push_idle(struct dp_ctrl *dp_ctrl); void dp_ctrl_isr(struct dp_ctrl *dp_ctrl); @@ -34,4 +31,10 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, struct dp_power *power, struct dp_catalog *catalog, struct dp_parser *parser); +void dp_ctrl_irq_enable(struct dp_ctrl *dp_ctrl); +void dp_ctrl_irq_disable(struct dp_ctrl *dp_ctrl); +void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl); +void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl); +void dp_ctrl_irq_phy_exit(struct dp_ctrl *dp_ctrl); + #endif /* _DP_CTRL_H_ */ diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index e41dd40..2f113ff 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -84,6 +84,7 @@ struct dp_display_private { /* state variables */ bool core_initialized; + bool phy_initialized; bool hpd_irq_on; bool audio_supported; @@ -387,7 +388,29 @@ static int dp_display_process_hpd_high(struct dp_display_private *dp) return rc; } -static void dp_display_host_init(struct dp_display_private *dp, int reset) +static void dp_display_host_phy_init(struct dp_display_private *dp) +{ + DRM_DEBUG_DP("core_init=%d phy_init=%d\n", + dp->core_initialized, dp->phy_initialized); + + if (!dp->phy_initialized) { + dp_ctrl_phy_init(dp->ctrl); + dp->phy_initialized = true; + } +} + +static void dp_display_host_phy_exit(struct dp_display_private *dp) +{ + DRM_DEBUG_DP("core_init=%d phy_init=%d\n", + dp->core_initialized, dp->phy_initialized); + + if (dp->phy_initialized) { + dp_ctrl_phy_exit(dp->ctrl); + dp->phy_initialized = false; + } +} + +static void dp_display_host_init(struct dp_display_private *dp) { bool flip = false; @@ -400,8 +423,16 @@ static void dp_display_host_init(struct dp_display_private *dp, int reset) if (dp->usbpd->orientation == ORIENTATION_CC2) flip = true; - dp_power_init(dp->power, flip); - dp_ctrl_host_init(dp->ctrl, flip, reset); + dp_power_init(dp->power, false); + dp_ctrl_irq_enable(dp->ctrl); + + /* + * eDP is the embedded primary display and has its own phy + * initialize phy immediately + */ + if (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP) + dp_display_host_phy_init(dp); + dp_aux_init(dp->aux); dp->core_initialized = true; } @@ -413,7 +444,7 @@ static void dp_display_host_deinit(struct dp_display_private *dp) return; } - dp_ctrl_host_deinit(dp->ctrl); + dp_ctrl_irq_disable(dp->ctrl); dp_aux_deinit(dp->aux); dp_power_deinit(dp->power); @@ -424,7 +455,7 @@ static int dp_display_usbpd_configure_cb(struct device *dev) { struct dp_display_private *dp = dev_get_dp_display_private(dev); - dp_display_host_init(dp, true); + dp_display_host_phy_init(dp); return dp_display_process_hpd_high(dp); } @@ -551,7 +582,7 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data) dp->hpd_state = ST_DISCONNECTED; if (ret == -ECONNRESET) { /* cable unplugged */ - dp->core_initialized = false; + dp->phy_initialized = false; } } else { @@ -623,9 +654,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) if (state == ST_DISCONNECTED) { /* triggered by irq_hdp with sink_count = 0 */ if (dp->link->sink_count == 0) { - dp_ctrl_off_phy(dp->ctrl); + dp_display_host_phy_exit(dp); hpd->hpd_high = 0; - dp->core_initialized = false; } mutex_unlock(&dp->event_mutex); return 0; @@ -716,7 +746,7 @@ static int dp_irq_hpd_handle(struct dp_display_private *dp, u32 data) ret = dp_display_usbpd_attention_cb(&dp->pdev->dev); if (ret == -ECONNRESET) { /* cable unplugged */ - dp->core_initialized = false; + dp->phy_initialized = false; } DRM_DEBUG_DP("hpd_state=%d\n", state); @@ -918,12 +948,19 @@ static int dp_display_disable(struct dp_display_private *dp, u32 data) dp_display->audio_enabled = false; - /* triggered by irq_hpd with sink_count = 0 */ if (dp->link->sink_count == 0) { + /* + * irq_hpd with sink_count = 0 + * hdmi unplugged out of dongle + */ dp_ctrl_off_link_stream(dp->ctrl); } else { + /* + * unplugged interrupt + * dongle unplugged out of DUT + */ dp_ctrl_off(dp->ctrl); - dp->core_initialized = false; + dp_display_host_phy_exit(dp); } dp_power_panel_on(dp->power, false); @@ -1059,7 +1096,7 @@ void msm_dp_snapshot(struct msm_disp_state *disp_state, struct msm_dp *dp) static void dp_display_config_hpd(struct dp_display_private *dp) { - dp_display_host_init(dp, true); + dp_display_host_init(dp); dp_catalog_ctrl_hpd_config(dp->catalog); /* Enable interrupt first time @@ -1338,20 +1375,22 @@ static int dp_pm_resume(struct device *dev) dp->hpd_state = ST_DISCONNECTED; /* turn on dp ctrl/phy */ - dp_display_host_init(dp, true); + dp_display_host_init(dp); dp_catalog_ctrl_hpd_config(dp->catalog); - /* - * set sink to normal operation mode -- D0 - * before dpcd read - */ - dp_link_psm_config(dp->link, &dp->panel->link_info, false); - if (dp_catalog_link_is_connected(dp->catalog)) { + /* + * set sink to normal operation mode -- D0 + * before dpcd read + */ + dp_display_host_phy_init(dp); + dp_link_psm_config(dp->link, &dp->panel->link_info, false); sink_count = drm_dp_read_sink_count(dp->aux); if (sink_count < 0) sink_count = 0; + + dp_display_host_phy_exit(dp); } dp->link->sink_count = sink_count; @@ -1399,6 +1438,8 @@ static int dp_pm_suspend(struct device *dev) dp_display_host_deinit(dp); } + dp_display_host_phy_exit(dp); + dp->hpd_state = ST_SUSPENDED; /* host_init will be called at pm_resume */ @@ -1473,7 +1514,7 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display) enable_irq(dp->irq); dp_hpd_connect(dp->usbpd, true); } else { - dp_add_event(dp, EV_HPD_INIT_SETUP, 0, dp->id * 10); + dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 0); } } @@ -1567,7 +1608,7 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder) state = dp_display->hpd_state; if (state == ST_DISPLAY_OFF) - dp_display_host_init(dp_display, true); + dp_display_host_phy_init(dp_display); dp_display_enable(dp_display, 0);