Messages in this thread | | | Date | Thu, 13 Jan 2022 14:14:27 -0800 | Subject | Re: [PATCH v11 1/4] drm/msm/dp: do not initialize phy until plugin interrupt received | From | Kuogee Hsieh <> |
| |
On 1/13/2022 1:52 PM, Stephen Boyd wrote: > Quoting Kuogee Hsieh (2022-01-13 12:44:16) >> On 1/13/2022 11:47 AM, Stephen Boyd wrote: >>> Quoting Kuogee Hsieh (2022-01-13 09:51:42) >>>> On 1/12/2022 8:13 PM, Stephen Boyd wrote: >>>>>>>> - if (dp->usbpd->orientation == ORIENTATION_CC2) >>>>>>>> - flip = true; >>>>>>>> + dp_power_init(dp->power, false); >>>>>>>> + dp_ctrl_reset_irq_ctrl(dp->ctrl, true); >>>>>>>> + >>>>>>>> + /* >>>>>>>> + * eDP is the embedded primary display and has its own phy >>>>>>>> + * initialize phy immediately >>>>>>> Question still stands why we can't wait for hpd high from the eDP panel. >>>>>>> Also, I think "has its own phy" means that it's not part of a combo >>>>>>> USB+DP phy? Can you please clarify? >> Correct, eDP has its dedicated phy which is not part of combo phy. > Why does that mean we can't wait for hpd high from the eDP panel?
Sorry, missed this one.
Yes, it can wait for hpd high.
Since it always presented, I just think it is better to show display sooner not later until hdp high.
you like it to wait until hpd high?
>>>>>>>> + */ >>>>>>>> + if (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP) >>>>>>>> + dp_display_host_phy_init(dp); >>>>>>>> >>>>>>>> - dp_power_init(dp->power, flip); >>>>>>>> - dp_ctrl_host_init(dp->ctrl, flip, reset); >>>>>>>> dp_aux_init(dp->aux); >>>>>>>> dp->core_initialized = true; >>>>>>>> } >>>>>>>> @@ -1306,20 +1330,23 @@ 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); >>>>>>> Why is the phy exited on resume when the link is still connected? Is >>>>>>> this supposed to be done only when the sink_count is 0? And how does >>>>>>> this interact with eDP where the phy is initialized by the call to >>>>>>> dp_display_host_init() earlier in this function. >> At beginning of dp_pm_resume bot core_initialized and phy_initialized >> should be off. >> >> However at the case of dongle still connected to DUT, we have to read >> dongle dpcd to decided any hdmi connect to dongle (sink_count != 0). in >> this case, we have to turn on phy to perform dpcd read and turn off phy >> after read so the following plugged-in interrupt can be handled correctly. >> > That looks like a lot of wasted work. Why can't we turn on the core, > turn on the phy, check if it's connected, and then turn off the phy if > it isn't? At the least, please put a comment above this phy_exit() call > indicating that we'll turn the phy back on while processing a plugged in > interrupt.
| |