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@xxxxxxxxxxx>
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) {
