Messages in this thread | | | Date | Mon, 13 Jun 2022 08:42:48 -0700 | Subject | Re: [PATCH v4] drm/msm/dp: force link training for display resolution change | From | Kuogee Hsieh <> |
| |
On 6/10/2022 2:04 PM, Stephen Boyd wrote: > Quoting Kuogee Hsieh (2022-05-27 14:32:13) >> During display resolution changes display have to be disabled first >> followed by display enabling with new resolution. Display disable >> will turn off both pixel clock and main link clock so that main link >> have to be re trained during display enable to have new video stream > s/re/re-/ > >> flow again. At current implementation, display enable function manually >> kicks up irq_hpd_handle which will read panel link status and start link >> training if link status is not in sync state. However, there is rare >> case that a particular panel links status keep staying in sync for >> some period of time after main link had been shut down previously at >> display disabled. Main link retraining will not be executed by >> irq_hdp_handle() if the link status read from pane shows it is in > s/pane/panel/ > >> sync state. If this was happen, then video stream of newer display >> resolution will fail to be transmitted to panel due to main link is >> not in sync between host and panel. This patch force main link always >> be retrained during display enable procedure to prevent this rare >> failed case from happening. Also this implementation are more >> efficient than manual kicking off irq_hpd_handle function. > The description makes it sound like the link status is not updated, > sometimes. Isn't that the real issue here? Not that link training needs > to be done again (which it always does apparently), but that disabling > the display doesn't wait for the link to go down. Or disabling the link > is causing some sort of glitch on the sink causing it to report the > status as OK when it really isn't.
As soon as mainlink teared down, sink has to reflect the mainlink at out of synch state in real time.
link re-training always required for display resolution changes since resolution changes will involve link configuration changes, such as 2 lanes change to 1 changes or 5.7G link rate changes to 2.7G or vise versa. Therefore there is no need to check sinker's main link status to decide re training is required or not.
>> Changes in v2: >> -- set force_link_train flag on DP only (is_edp == false) >> >> Changes in v3: >> -- revise commit text >> -- add Fixes tag >> >> Changes in v4: >> -- revise commit text >> >> Fixes: 62671d2ef24b ("drm/msm/dp: fixes wrong connection state caused by failure of link train") >> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >> index c388323..370348d 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_display.c >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c >> @@ -1688,10 +1689,14 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge) >> >> state = dp_display->hpd_state; >> >> - if (state == ST_DISPLAY_OFF) >> + if (state == ST_DISPLAY_OFF) { >> dp_display_host_phy_init(dp_display); >> >> - dp_display_enable(dp_display, 0); >> + if (!dp->is_edp) > Does this assume eDP has one resolution? I don't understand why eDP is > special here, especially if eDP has more than one resolution available > it seems like we would want to retrain the link regardless. > >> + force_link_train = true; >> + } >> + >> + dp_display_enable(dp_display, force_link_train); >> >> rc = dp_display_post_enable(dp); >> if (rc) {
| |