From f8a75153b79872741ba70cae94d9d24e1a607def Mon Sep 17 00:00:00 2001 From: Rajkumar Subbiah Date: Wed, 14 Oct 2020 15:37:57 -0400 Subject: [PATCH 1/2] disp: msm: dp: send hpd notification before updating mst_active When processing mst hpd low, the driver is clearing mst_active before triggering hpd notification. The hpd notifier is common for both sst and mst and since mst_active is cleared it incorrectly treats this as sst unplug. This change switches the order of these operations to trigger hpd notification before clearing mst_active. Change-Id: I28f90da699e4f2fe177a4e4cfd1d9f03957c3176 Signed-off-by: Rajkumar Subbiah --- msm/dp/dp_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/msm/dp/dp_display.c b/msm/dp/dp_display.c index 471a53c9b7..6e528c8ff9 100644 --- a/msm/dp/dp_display.c +++ b/msm/dp/dp_display.c @@ -1250,12 +1250,12 @@ static void dp_display_process_mst_hpd_low(struct dp_display_private *dp) if (dp->mst.cbs.hpd) dp->mst.cbs.hpd(&dp->dp_display, false); - dp_display_update_mst_state(dp, false); - if ((dp_display_state_is(DP_STATE_CONNECT_NOTIFIED) || dp_display_state_is(DP_STATE_ENABLED))) rc = dp_display_send_hpd_notification(dp); + dp_display_update_mst_state(dp, false); + if (dp->mst.cbs.set_mgr_state) { info.mst_protocol = dp->parser->has_mst_sideband; dp->mst.cbs.set_mgr_state(&dp->dp_display, false, From 2fc0439a4698f5845e83878946dd8daef0139dbd Mon Sep 17 00:00:00 2001 From: Rajkumar Subbiah Date: Mon, 2 Nov 2020 16:47:28 -0500 Subject: [PATCH 2/2] disp: msm: dp: unify hpd event for sst and mst Currently hpd uevent notification to usermode is triggered at different points in the hpd handler callflow. With respect to the SST callflow, the MST callflow has the following issues: * the completion event object is getting reinitialized after the uevent is sent * The NOTIFIED states are not updated properly. * dp_display_process_mst_hpd_high is overloaded to do two different functions in the same flow and is controlled by the mst_probe argument. This change cleans up the hpd callflows for MST and unifies the hpd event notification. Also moved the mst check logic from dp_display_process_mst_hpd_high to a separate function. Change-Id: I8fdc92d2f9aae16d248c74643cb93688786dfbd5 Signed-off-by: Rajkumar Subbiah --- msm/dp/dp_display.c | 148 ++++++++++++++++++++++---------------------- 1 file changed, 73 insertions(+), 75 deletions(-) diff --git a/msm/dp/dp_display.c b/msm/dp/dp_display.c index 6e528c8ff9..86c124f429 100644 --- a/msm/dp/dp_display.c +++ b/msm/dp/dp_display.c @@ -807,7 +807,7 @@ static const struct component_ops dp_display_comp_ops = { .unbind = dp_display_unbind, }; -static void dp_display_send_hpd_event(struct dp_display_private *dp) +static bool dp_display_send_hpd_event(struct dp_display_private *dp) { struct drm_device *dev = NULL; struct drm_connector *connector; @@ -816,24 +816,18 @@ static void dp_display_send_hpd_event(struct dp_display_private *dp) char *envp[5]; int rc = 0; - if (dp->mst.mst_active) { - DP_DEBUG("skip notification for mst mode\n"); - dp_display_state_remove(DP_STATE_DISCONNECT_NOTIFIED); - return; - } - connector = dp->dp_display.base_connector; if (!connector) { DP_ERR("connector not set\n"); - return; + return false; } connector->status = connector->funcs->detect(connector, false); if (dp->cached_connector_status == connector->status) { DP_DEBUG("connector status (%d) unchanged, skipping uevent\n", dp->cached_connector_status); - return; + return false; } dp->cached_connector_status = connector->status; @@ -842,7 +836,7 @@ static void dp_display_send_hpd_event(struct dp_display_private *dp) if (dp->debug->skip_uevent) { DP_INFO("skipping uevent\n"); - goto update_state; + return false; } snprintf(name, HPD_STRING_SIZE, "name=%s", connector->name); @@ -864,14 +858,7 @@ static void dp_display_send_hpd_event(struct dp_display_private *dp) rc = kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp); DP_INFO("uevent %s: %d\n", rc ? "failure" : "success", rc); -update_state: - if (connector->status == connector_status_connected) { - dp_display_state_add(DP_STATE_CONNECT_NOTIFIED); - dp_display_state_remove(DP_STATE_DISCONNECT_NOTIFIED); - } else { - dp_display_state_add(DP_STATE_DISCONNECT_NOTIFIED); - dp_display_state_remove(DP_STATE_CONNECT_NOTIFIED); - } + return true; } static int dp_display_send_hpd_notification(struct dp_display_private *dp) @@ -900,13 +887,29 @@ static int dp_display_send_hpd_notification(struct dp_display_private *dp) dp->aux->state |= DP_STATE_NOTIFICATION_SENT; - if (!dp->mst.mst_active) + reinit_completion(&dp->notification_comp); + + if (!dp->mst.mst_active) { dp->dp_display.is_sst_connected = hpd; - else + + if (!dp_display_send_hpd_event(dp)) + goto skip_wait; + } else { dp->dp_display.is_sst_connected = false; - reinit_completion(&dp->notification_comp); - dp_display_send_hpd_event(dp); + if (!dp->mst.cbs.hpd) + goto skip_wait; + + dp->mst.cbs.hpd(&dp->dp_display, true); + } + + if (hpd) { + dp_display_state_add(DP_STATE_CONNECT_NOTIFIED); + dp_display_state_remove(DP_STATE_DISCONNECT_NOTIFIED); + } else { + dp_display_state_add(DP_STATE_DISCONNECT_NOTIFIED); + dp_display_state_remove(DP_STATE_CONNECT_NOTIFIED); + } /* * Skip the wait if TUI is active considering that the user mode will @@ -942,11 +945,9 @@ static void dp_display_update_mst_state(struct dp_display_private *dp, dp->panel->mst_state = state; } -static void dp_display_process_mst_hpd_high(struct dp_display_private *dp, - bool mst_probe) +static void dp_display_mst_init(struct dp_display_private *dp) { bool is_mst_receiver; - struct dp_mst_hpd_info info; const unsigned long clear_mstm_ctrl_timeout_us = 100000; u8 old_mstm_ctrl; int ret; @@ -957,50 +958,53 @@ static void dp_display_process_mst_hpd_high(struct dp_display_private *dp, return; } - DP_MST_DEBUG("mst_hpd_high work. mst_probe:%d\n", mst_probe); + is_mst_receiver = dp->panel->read_mst_cap(dp->panel); - if (!dp->mst.mst_active) { - is_mst_receiver = dp->panel->read_mst_cap(dp->panel); - - if (!is_mst_receiver) { - DP_MST_DEBUG("sink doesn't support mst\n"); - return; - } - - /* clear sink mst state */ - drm_dp_dpcd_readb(dp->aux->drm_aux, DP_MSTM_CTRL, - &old_mstm_ctrl); - drm_dp_dpcd_writeb(dp->aux->drm_aux, DP_MSTM_CTRL, 0); - - /* add extra delay if MST state is not cleared */ - if (old_mstm_ctrl) { - DP_MST_DEBUG("MSTM_CTRL is not cleared, wait %dus\n", - clear_mstm_ctrl_timeout_us); - usleep_range(clear_mstm_ctrl_timeout_us, - clear_mstm_ctrl_timeout_us + 1000); - } - - ret = drm_dp_dpcd_writeb(dp->aux->drm_aux, DP_MSTM_CTRL, - DP_MST_EN | DP_UP_REQ_EN | DP_UPSTREAM_IS_SRC); - if (ret < 0) { - DP_ERR("sink mst enablement failed\n"); - return; - } - - dp_display_update_mst_state(dp, true); - } else if (dp->mst.mst_active && mst_probe) { - info.mst_protocol = dp->parser->has_mst_sideband; - info.mst_port_cnt = dp->debug->mst_port_cnt; - info.edid = dp->debug->get_edid(dp->debug); - - if (dp->mst.cbs.set_mgr_state) - dp->mst.cbs.set_mgr_state(&dp->dp_display, true, &info); - - if (dp->mst.cbs.hpd) - dp->mst.cbs.hpd(&dp->dp_display, true); + if (!is_mst_receiver) { + DP_MST_DEBUG("sink doesn't support mst\n"); + return; } - DP_MST_DEBUG("mst_hpd_high. mst_active:%d\n", dp->mst.mst_active); + /* clear sink mst state */ + drm_dp_dpcd_readb(dp->aux->drm_aux, DP_MSTM_CTRL, &old_mstm_ctrl); + drm_dp_dpcd_writeb(dp->aux->drm_aux, DP_MSTM_CTRL, 0); + + /* add extra delay if MST state is not cleared */ + if (old_mstm_ctrl) { + DP_MST_DEBUG("MSTM_CTRL is not cleared, wait %dus\n", + clear_mstm_ctrl_timeout_us); + usleep_range(clear_mstm_ctrl_timeout_us, + clear_mstm_ctrl_timeout_us + 1000); + } + + ret = drm_dp_dpcd_writeb(dp->aux->drm_aux, DP_MSTM_CTRL, + DP_MST_EN | DP_UP_REQ_EN | DP_UPSTREAM_IS_SRC); + if (ret < 0) { + DP_ERR("sink mst enablement failed\n"); + return; + } + + dp_display_update_mst_state(dp, true); +} + +static void dp_display_set_mst_mgr_state(struct dp_display_private *dp, + bool state) +{ + struct dp_mst_hpd_info info = {0}; + + if (!dp->mst.mst_active) + return; + + info.mst_protocol = dp->parser->has_mst_sideband; + if (state) { + info.mst_port_cnt = dp->debug->mst_port_cnt; + info.edid = dp->debug->get_edid(dp->debug); + } + + if (dp->mst.cbs.set_mgr_state) + dp->mst.cbs.set_mgr_state(&dp->dp_display, state, &info); + + DP_MST_DEBUG("mst_mgr_state: %d\n", state); } static void dp_display_host_init(struct dp_display_private *dp) @@ -1174,7 +1178,7 @@ static int dp_display_process_hpd_high(struct dp_display_private *dp) dp->link->process_request(dp->link); dp->panel->handle_sink_request(dp->panel); - dp_display_process_mst_hpd_high(dp, false); + dp_display_mst_init(dp); rc = dp->ctrl->on(dp->ctrl, dp->mst.mst_active, dp->panel->fec_en, dp->panel->dsc_en, false); @@ -1185,7 +1189,7 @@ static int dp_display_process_hpd_high(struct dp_display_private *dp) dp->process_hpd_connect = false; - dp_display_process_mst_hpd_high(dp, true); + dp_display_set_mst_mgr_state(dp, true); end: mutex_unlock(&dp->session_lock); @@ -1234,7 +1238,6 @@ skip_notify: static void dp_display_process_mst_hpd_low(struct dp_display_private *dp) { int rc = 0; - struct dp_mst_hpd_info info = {0}; if (dp->mst.mst_active) { DP_MST_DEBUG("mst_hpd_low work\n"); @@ -1255,12 +1258,7 @@ static void dp_display_process_mst_hpd_low(struct dp_display_private *dp) rc = dp_display_send_hpd_notification(dp); dp_display_update_mst_state(dp, false); - - if (dp->mst.cbs.set_mgr_state) { - info.mst_protocol = dp->parser->has_mst_sideband; - dp->mst.cbs.set_mgr_state(&dp->dp_display, false, - &info); - } + dp_display_set_mst_mgr_state(dp, false); } DP_MST_DEBUG("mst_hpd_low. mst_active:%d\n", dp->mst.mst_active);