Both AUX_SW_RESET and DP_SW_RESET clear pending HPD interrupts. Therefore irq_hpd handler should not issues either aux or sw reset to avoid following unplug interrupt be cleared accidentally.
Kuogee Hsieh (2): drm/msm/dp: return fail when both link lane and rate are 0 at dpcd read drm/msm/dp: unplug interrupt missed after irq_hpd handler
drivers/gpu/drm/msm/dp/dp_aux.c | 7 ------- drivers/gpu/drm/msm/dp/dp_catalog.c | 24 ++++++++++++++++++++++++ drivers/gpu/drm/msm/dp/dp_ctrl.c | 15 ++++++++++----- drivers/gpu/drm/msm/dp/dp_display.c | 7 +++++++ drivers/gpu/drm/msm/dp/dp_panel.c | 12 +++++++++--- 5 files changed, 50 insertions(+), 15 deletions(-)
Some dongle set both link lane and rate to 0 during dpcd receiver capability read if there is no monitor attache to this dongle. Therefore return fail to prevent driver from trying to populate monitor further.
Changes in V2: -- split this patch into two. Move postpone irq_handle into next patch -- add Fixes tag
Fixes: 78f94fbb6122 ("drm/msm/dp: fix connect/disconnect handled at irq_hpd")
Signed-off-by: Kuogee Hsieh khsieh@codeaurora.org --- drivers/gpu/drm/msm/dp/dp_panel.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c index 97dca3e..d1780bc 100644 --- a/drivers/gpu/drm/msm/dp/dp_panel.c +++ b/drivers/gpu/drm/msm/dp/dp_panel.c @@ -167,12 +167,18 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel, panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
rc = dp_panel_read_dpcd(dp_panel); + if (rc) { + DRM_ERROR("read dpcd failed %d\n", rc); + return rc; + } + bw_code = drm_dp_link_rate_to_bw_code(dp_panel->link_info.rate); - if (rc || !is_link_rate_valid(bw_code) || + if (!is_link_rate_valid(bw_code) || !is_lane_count_valid(dp_panel->link_info.num_lanes) || (bw_code > dp_panel->max_bw_code)) { - DRM_ERROR("read dpcd failed %d\n", rc); - return rc; + DRM_ERROR("Illegal link rate=%d lane=%d\n", dp_panel->link_info.rate, + dp_panel->link_info.num_lanes); + return -EINVAL; }
if (dp_panel->dfp_present) {
There is HPD unplug interrupts missed at scenario of an irq_hpd followed by unplug interrupts with around 10 ms in between. Since both AUX_SW_RESET and DP_SW_RESET clear pending HPD interrupts, irq_hpd handler should not issues either aux or sw reset to avoid following unplug interrupt be cleared accidentally. This patch also postpone handling of irq_hpd until connected state if it happened at connection pending state.
Changes in V2: -- add postpone handling of irq_hpd until connected state -- check DP_TRAINING_1 instead of DP_TRAINING_NONE
Signed-off-by: Kuogee Hsieh khsieh@codeaurora.org --- drivers/gpu/drm/msm/dp/dp_aux.c | 7 ------- drivers/gpu/drm/msm/dp/dp_catalog.c | 24 ++++++++++++++++++++++++ drivers/gpu/drm/msm/dp/dp_ctrl.c | 15 ++++++++++----- drivers/gpu/drm/msm/dp/dp_display.c | 7 +++++++ 4 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c index 19b35ae..1c6e1d2 100644 --- a/drivers/gpu/drm/msm/dp/dp_aux.c +++ b/drivers/gpu/drm/msm/dp/dp_aux.c @@ -336,7 +336,6 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux, ssize_t ret; int const aux_cmd_native_max = 16; int const aux_cmd_i2c_max = 128; - int const retry_count = 5; struct dp_aux_private *aux = container_of(dp_aux, struct dp_aux_private, dp_aux);
@@ -378,12 +377,6 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux, ret = dp_aux_cmd_fifo_tx(aux, msg);
if (ret < 0) { - if (aux->native) { - aux->retry_cnt++; - if (!(aux->retry_cnt % retry_count)) - dp_catalog_aux_update_cfg(aux->catalog); - dp_catalog_aux_reset(aux->catalog); - } usleep_range(400, 500); /* at least 400us to next try */ goto unlock_exit; } diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c index 44f0c57..b1a9b1b 100644 --- a/drivers/gpu/drm/msm/dp/dp_catalog.c +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c @@ -190,6 +190,18 @@ int dp_catalog_aux_clear_hw_interrupts(struct dp_catalog *dp_catalog) return 0; }
+/** + * dp_catalog_aux_reset() - reset AUX controller + * + * @aux: DP catalog structure + * + * return: void + * + * This function reset AUX controller + * + * NOTE: reset AUX controller will also clear any pending HPD related interrupts + * + */ void dp_catalog_aux_reset(struct dp_catalog *dp_catalog) { u32 aux_ctrl; @@ -483,6 +495,18 @@ int dp_catalog_ctrl_set_pattern(struct dp_catalog *dp_catalog, return 0; }
+/** + * dp_catalog_ctrl_reset() - reset DP controller + * + * @dp_catalog: DP catalog structure + * + * return: void + * + * This function reset the DP controller + * + * NOTE: reset DP controller will also clear any pending HPD related interrupts + * + */ void dp_catalog_ctrl_reset(struct dp_catalog *dp_catalog) { u32 sw_reset; diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index e3462f5..5ac155d 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1296,7 +1296,8 @@ static int dp_ctrl_setup_main_link(struct dp_ctrl_private *ctrl, * transitioned to PUSH_IDLE. In order to start transmitting * a link training pattern, we have to first do soft reset. */ - dp_catalog_ctrl_reset(ctrl->catalog); + if (*training_step == DP_TRAINING_1) + dp_catalog_ctrl_reset(ctrl->catalog);
ret = dp_ctrl_link_train(ctrl, cr, training_step);
@@ -1491,15 +1492,18 @@ static int dp_ctrl_deinitialize_mainlink(struct dp_ctrl_private *ctrl) return 0; }
+static void dp_ctrl_link_idle_reset(struct dp_ctrl_private *ctrl) +{ + dp_ctrl_push_idle(&ctrl->dp_ctrl); + dp_catalog_ctrl_reset(ctrl->catalog); +} + static int dp_ctrl_link_maintenance(struct dp_ctrl_private *ctrl) { int ret = 0; struct dp_cr_status cr; int training_step = DP_TRAINING_NONE;
- dp_ctrl_push_idle(&ctrl->dp_ctrl); - dp_catalog_ctrl_reset(ctrl->catalog); - ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
ret = dp_ctrl_setup_main_link(ctrl, &cr, &training_step); @@ -1626,6 +1630,7 @@ void dp_ctrl_handle_sink_request(struct dp_ctrl *dp_ctrl)
if (sink_request & DP_TEST_LINK_TRAINING) { dp_link_send_test_response(ctrl->link); + dp_ctrl_link_idle_reset(ctrl); if (dp_ctrl_link_maintenance(ctrl)) { DRM_ERROR("LM failed: TEST_LINK_TRAINING\n"); return; @@ -1679,7 +1684,7 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl) break; }
- training_step = DP_TRAINING_NONE; + training_step = DP_TRAINING_1; rc = dp_ctrl_setup_main_link(ctrl, &cr, &training_step); if (rc == 0) { /* training completed successfully */ diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 6e971d5..3bc7ed2 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -693,6 +693,13 @@ 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; + } + ret = dp_display_usbpd_attention_cb(&dp->pdev->dev); if (ret == -ECONNRESET) { /* cable unplugged */ dp->core_initialized = false;
Quoting Kuogee Hsieh (2021-01-13 10:59:58)
Both AUX_SW_RESET and DP_SW_RESET clear pending HPD interrupts. Therefore irq_hpd handler should not issues either aux or sw reset to avoid following unplug interrupt be cleared accidentally.
Kuogee Hsieh (2): drm/msm/dp: return fail when both link lane and rate are 0 at dpcd read drm/msm/dp: unplug interrupt missed after irq_hpd handler
It won't apply to the drm msm tree. Please rebase and resend.
On 2021-01-13 12:25, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2021-01-13 10:59:58)
Both AUX_SW_RESET and DP_SW_RESET clear pending HPD interrupts. Therefore irq_hpd handler should not issues either aux or sw reset to avoid following unplug interrupt be cleared accidentally.
Kuogee Hsieh (2): drm/msm/dp: return fail when both link lane and rate are 0 at dpcd read drm/msm/dp: unplug interrupt missed after irq_hpd handler
It won't apply to the drm msm tree. Please rebase and resend.
Both V1 two patches are picked by Rob already. I will drop V2 patches.
Quoting khsieh@codeaurora.org (2021-01-13 15:52:37)
On 2021-01-13 12:25, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2021-01-13 10:59:58)
Both AUX_SW_RESET and DP_SW_RESET clear pending HPD interrupts. Therefore irq_hpd handler should not issues either aux or sw reset to avoid following unplug interrupt be cleared accidentally.
Kuogee Hsieh (2): drm/msm/dp: return fail when both link lane and rate are 0 at dpcd read drm/msm/dp: unplug interrupt missed after irq_hpd handler
It won't apply to the drm msm tree. Please rebase and resend.
Both V1 two patches are picked by Rob already. I will drop V2 patches.
I only see the first patch, not the second one. Rob?
dri-devel@lists.freedesktop.org