Skip to content

Commit

Permalink
drm/msm/dp: tear down main link at unplug handle immediately
Browse files Browse the repository at this point in the history 
Two stages are required to setup up main link to be ready to transmit
video stream.
Stage 1: dp_hpd_plug_handle() perform link training to set up main link
stage 2: user space framework (msm_dp_display_enable()) to enable pixel
clock and transfer main link to video ready state.

At current implementation, when dongle unplugged dp_hdp_unplug_handle()
has to wait until stage 2 completed before it can send link down uevent
to user space framework to disable pixel clock followed by tearing down
main link.  This introduce unnecessary latency if dongle unplugged happen
after stage 1 and before stage 2. It also has possibility leave main link
stay at ready state after dongle unplugged if framework does not response
to link down uevent notification. This will prevent next dongle plug in
from working. This scenario could possibly happen when dongle unplug while
system in the middle of suspending.

This patch allow unplug handle to tear down main link and notify
framework link down immediately if dongle unplugged happen after
stage 1 and before stage 2. With this approach, dp driver is much
more resilient to any different scenarios. Also redundant both
dp_connect_pending_timeout() and dp_disconnect_pending_timeout()
are removed to reduce logic complexity.

Changes in V2:
-- return -EINVAL at msm_dp_display_enable() if not in correct state
-- replace ST_CONNECT_PENDING with ST_MAINLINK_READY

Fixes: 8ede2ec ("drm/msm/dp: Add DP compliance tests on Snapdragon Chipsets")
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/483391/
Link: https://lore.kernel.org/r/1650927382-22461-1-git-send-email-quic_khsieh@quicinc.com
[DB: fixed return values due to conversion to function merge]
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
  • Loading branch information
Kuogee Hsieh authored and Dmitry Baryshkov committed May 1, 2022
1 parent e92d0d9 commit 375a126
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 80 deletions.
29 changes: 29 additions & 0 deletions drivers/gpu/drm/msm/dp/dp_ctrl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1925,6 +1925,35 @@ int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl)
return ret;
}

int dp_ctrl_off_link(struct dp_ctrl *dp_ctrl)
{
struct dp_ctrl_private *ctrl;
struct dp_io *dp_io;
struct phy *phy;
int ret;

ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
dp_io = &ctrl->parser->io;
phy = dp_io->phy;

dp_catalog_ctrl_mainlink_ctrl(ctrl->catalog, false);

ret = dp_power_clk_enable(ctrl->power, DP_CTRL_PM, false);
if (ret) {
DRM_ERROR("Failed to disable link clocks. ret=%d\n", ret);
}

DRM_DEBUG_DP("Before, phy=%p init_count=%d power_on=%d\n",
phy, phy->init_count, phy->power_count);

phy_power_off(phy);

DRM_DEBUG_DP("After, phy=%p init_count=%d power_on=%d\n",
phy, phy->init_count, phy->power_count);

return ret;
}

int dp_ctrl_off(struct dp_ctrl *dp_ctrl)
{
struct dp_ctrl_private *ctrl;
Expand Down
1 change: 1 addition & 0 deletions drivers/gpu/drm/msm/dp/dp_ctrl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ struct 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);
int dp_ctrl_off_link(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);
Expand Down
113 changes: 33 additions & 80 deletions drivers/gpu/drm/msm/dp/dp_display.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ enum {
/* event thread connection state */
enum {
ST_DISCONNECTED,
ST_CONNECT_PENDING,
ST_MAINLINK_READY,
ST_CONNECTED,
ST_DISCONNECT_PENDING,
ST_DISPLAY_OFF,
Expand All @@ -56,14 +56,11 @@ enum {
EV_IRQ_HPD_INT,
EV_HPD_UNPLUG_INT,
EV_USER_NOTIFICATION,
EV_CONNECT_PENDING_TIMEOUT,
EV_DISCONNECT_PENDING_TIMEOUT,
};

#define EVENT_TIMEOUT (HZ/10) /* 100ms */
#define DP_EVENT_Q_MAX 8

#define DP_TIMEOUT_5_SECOND (5000/EVENT_TIMEOUT)
#define DP_TIMEOUT_NONE 0

#define WAIT_FOR_RESUME_TIMEOUT_JIFFIES (HZ / 2)
Expand Down Expand Up @@ -473,6 +470,11 @@ static int dp_display_usbpd_configure_cb(struct device *dev)
}

static int dp_display_usbpd_disconnect_cb(struct device *dev)
{
return 0;
}

static int dp_display_notify_disconnect(struct device *dev)
{
struct dp_display_private *dp = dev_get_dp_display_private(dev);

Expand Down Expand Up @@ -501,7 +503,7 @@ static int dp_display_handle_port_ststus_changed(struct dp_display_private *dp)
}
} else {
if (dp->hpd_state == ST_DISCONNECTED) {
dp->hpd_state = ST_CONNECT_PENDING;
dp->hpd_state = ST_MAINLINK_READY;
rc = dp_display_process_hpd_high(dp);
if (rc)
dp->hpd_state = ST_DISCONNECTED;
Expand Down Expand Up @@ -558,7 +560,6 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data)
{
struct dp_usbpd *hpd = dp->usbpd;
u32 state;
u32 tout = DP_TIMEOUT_5_SECOND;
int ret;

if (!hpd)
Expand All @@ -575,7 +576,7 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data)
return 0;
}

if (state == ST_CONNECT_PENDING || state == ST_CONNECTED) {
if (state == ST_MAINLINK_READY || state == ST_CONNECTED) {
mutex_unlock(&dp->event_mutex);
return 0;
}
Expand All @@ -587,14 +588,11 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data)
return 0;
}

dp->hpd_state = ST_CONNECT_PENDING;

ret = dp_display_usbpd_configure_cb(&dp->pdev->dev);
if (ret) { /* link train failed */
dp->hpd_state = ST_DISCONNECTED;
} else {
/* start sentinel checking in case of missing uevent */
dp_add_event(dp, EV_CONNECT_PENDING_TIMEOUT, 0, tout);
dp->hpd_state = ST_MAINLINK_READY;
}

/* enable HDP irq_hpd/replug interrupt */
Expand All @@ -612,23 +610,6 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data)
static int dp_display_enable(struct dp_display_private *dp, u32 data);
static int dp_display_disable(struct dp_display_private *dp, u32 data);

static int dp_connect_pending_timeout(struct dp_display_private *dp, u32 data)
{
u32 state;

mutex_lock(&dp->event_mutex);

state = dp->hpd_state;
if (state == ST_CONNECT_PENDING) {
dp->hpd_state = ST_CONNECTED;
drm_dbg_dp(dp->drm_dev, "type=%d\n", dp->dp_display.connector_type);
}

mutex_unlock(&dp->event_mutex);

return 0;
}

static void dp_display_handle_plugged_change(struct msm_dp *dp_display,
bool plugged)
{
Expand Down Expand Up @@ -670,35 +651,35 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
if (dp->link->sink_count == 0) {
dp_display_host_phy_exit(dp);
}
dp_display_notify_disconnect(&dp->pdev->dev);
mutex_unlock(&dp->event_mutex);
return 0;
}

if (state == ST_DISCONNECT_PENDING) {
} else if (state == ST_DISCONNECT_PENDING) {
mutex_unlock(&dp->event_mutex);
return 0;
}

if (state == ST_CONNECT_PENDING) {
/* wait until CONNECTED */
dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 1); /* delay = 1 */
} else if (state == ST_MAINLINK_READY) {
dp_ctrl_off_link(dp->ctrl);
dp_display_host_phy_exit(dp);
dp->hpd_state = ST_DISCONNECTED;
dp_display_notify_disconnect(&dp->pdev->dev);
mutex_unlock(&dp->event_mutex);
return 0;
}

dp->hpd_state = ST_DISCONNECT_PENDING;

/* disable HPD plug interrupts */
dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false);

/*
* We don't need separate work for disconnect as
* connect/attention interrupts are disabled
*/
dp_display_usbpd_disconnect_cb(&dp->pdev->dev);
dp_display_notify_disconnect(&dp->pdev->dev);

/* start sentinel checking in case of missing uevent */
dp_add_event(dp, EV_DISCONNECT_PENDING_TIMEOUT, 0, DP_TIMEOUT_5_SECOND);
if (state == ST_DISPLAY_OFF) {
dp->hpd_state = ST_DISCONNECTED;
} else {
dp->hpd_state = ST_DISCONNECT_PENDING;
}

/* signal the disconnect event early to ensure proper teardown */
dp_display_handle_plugged_change(&dp->dp_display, false);
Expand All @@ -714,23 +695,6 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
return 0;
}

static int dp_disconnect_pending_timeout(struct dp_display_private *dp, u32 data)
{
u32 state;

mutex_lock(&dp->event_mutex);

state = dp->hpd_state;
if (state == ST_DISCONNECT_PENDING) {
dp->hpd_state = ST_DISCONNECTED;
drm_dbg_dp(dp->drm_dev, "type=%d\n", dp->dp_display.connector_type);
}

mutex_unlock(&dp->event_mutex);

return 0;
}

static int dp_irq_hpd_handle(struct dp_display_private *dp, u32 data)
{
u32 state;
Expand All @@ -747,14 +711,7 @@ static int dp_irq_hpd_handle(struct dp_display_private *dp, u32 data)
return 0;
}

if (state == ST_CONNECT_PENDING) {
/* wait until ST_CONNECTED */
dp_add_event(dp, EV_IRQ_HPD_INT, 0, 1); /* delay = 1 */
mutex_unlock(&dp->event_mutex);
return 0;
}

if (state == ST_CONNECT_PENDING || state == ST_DISCONNECT_PENDING) {
if (state == ST_MAINLINK_READY || state == ST_DISCONNECT_PENDING) {
/* wait until ST_CONNECTED */
dp_add_event(dp, EV_IRQ_HPD_INT, 0, 1); /* delay = 1 */
mutex_unlock(&dp->event_mutex);
Expand Down Expand Up @@ -1192,14 +1149,6 @@ static int hpd_event_thread(void *data)
dp_display_send_hpd_notification(dp_priv,
todo->data);
break;
case EV_CONNECT_PENDING_TIMEOUT:
dp_connect_pending_timeout(dp_priv,
todo->data);
break;
case EV_DISCONNECT_PENDING_TIMEOUT:
dp_disconnect_pending_timeout(dp_priv,
todo->data);
break;
default:
break;
}
Expand Down Expand Up @@ -1242,8 +1191,6 @@ static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);

if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK) {
/* stop sentinel connect pending checking */
dp_del_event(dp, EV_CONNECT_PENDING_TIMEOUT);
dp_add_event(dp, EV_IRQ_HPD_INT, 0, 0);
}

Expand Down Expand Up @@ -1629,8 +1576,11 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)

mutex_lock(&dp_display->event_mutex);

/* stop sentinel checking */
dp_del_event(dp_display, EV_CONNECT_PENDING_TIMEOUT);
state = dp_display->hpd_state;
if (state != ST_DISPLAY_OFF && state != ST_MAINLINK_READY) {
mutex_unlock(&dp_display->event_mutex);
return;
}

rc = dp_display_set_mode(dp, &dp_display->dp_mode);
if (rc) {
Expand Down Expand Up @@ -1694,8 +1644,11 @@ void dp_bridge_post_disable(struct drm_bridge *drm_bridge)

mutex_lock(&dp_display->event_mutex);

/* stop sentinel checking */
dp_del_event(dp_display, EV_DISCONNECT_PENDING_TIMEOUT);
state = dp_display->hpd_state;
if (state != ST_DISCONNECT_PENDING && state != ST_CONNECTED) {
mutex_unlock(&dp_display->event_mutex);
return;
}

dp_display_disable(dp_display, 0);

Expand Down

0 comments on commit 375a126

Please sign in to comment.