add fixes to pass DP Link Layer compliance test cases
Kuogee Hsieh (7):
drm/msm/dp: use dp_ctrl_off_link_stream during PHY compliance test run
drm/msm/dp: reduce link rate if failed at link training 1
drm/msm/dp: reset aux controller after dp_aux_cmd_fifo_tx() failed.
drm/msm/dp: replug event is converted into an unplug followed by an
plug events
drm/msm/dp: return correct edid checksum after corrupted edid checksum
read
drm/msm/dp: do not end dp link training until video is ready
drm/msm/dp: retrain link when loss of symbol lock detected
drivers/gpu/drm/msm/dp/dp_aux.c | 3 +
drivers/gpu/drm/msm/dp/dp_ctrl.c | 145 +++++++++++++++++++++++-------------
drivers/gpu/drm/msm/dp/dp_display.c | 13 ++--
drivers/gpu/drm/msm/dp/dp_panel.c | 9 ++-
4 files changed, 110 insertions(+), 60 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Remove special handling of replug interrupt and instead treat replug event
as a sequential unplug followed by a plugin event. This is needed to meet
the requirements of DP Link Layer CTS test case 4.2.1.3.
Changes in V2:
-- add fixes statement
Fixes: f21c8a276c2d ("drm/msm/dp: handle irq_hpd with sink_count = 0 correctly")
Signed-off-by: Kuogee Hsieh <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_display.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 78c5301..d089ada 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1146,9 +1146,6 @@ static int hpd_event_thread(void *data)
case EV_IRQ_HPD_INT:
dp_irq_hpd_handle(dp_priv, todo->data);
break;
- case EV_HPD_REPLUG_INT:
- /* do nothing */
- break;
case EV_USER_NOTIFICATION:
dp_display_send_hpd_notification(dp_priv,
todo->data);
@@ -1192,10 +1189,8 @@ static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
if (hpd_isr_status & 0x0F) {
/* hpd related interrupts */
- if (hpd_isr_status & DP_DP_HPD_PLUG_INT_MASK ||
- hpd_isr_status & DP_DP_HPD_REPLUG_INT_MASK) {
+ if (hpd_isr_status & DP_DP_HPD_PLUG_INT_MASK)
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 */
@@ -1203,8 +1198,10 @@ static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
dp_add_event(dp, EV_IRQ_HPD_INT, 0, 0);
}
- if (hpd_isr_status & DP_DP_HPD_REPLUG_INT_MASK)
- dp_add_event(dp, EV_HPD_REPLUG_INT, 0, 0);
+ if (hpd_isr_status & DP_DP_HPD_REPLUG_INT_MASK) {
+ dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+ dp_add_event(dp, EV_HPD_PLUG_INT, 0, 3);
+ }
if (hpd_isr_status & DP_DP_HPD_UNPLUG_INT_MASK)
dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Aux hardware calibration sequence requires resetting the aux controller
in order for the new setting to take effect. However resetting the AUX
controller will also clear HPD interrupt status which may accidentally
cause pending unplug interrupt to get lost. Therefore reset aux
controller only when link is in connection state when dp_aux_cmd_fifo_tx()
fail. This fixes Link Layer CTS cases 4.2.1.1 and 4.2.1.2.
Signed-off-by: Kuogee Hsieh <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_aux.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
index 4a3293b..eb40d84 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.c
+++ b/drivers/gpu/drm/msm/dp/dp_aux.c
@@ -353,6 +353,9 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
if (!(aux->retry_cnt % MAX_AUX_RETRIES))
dp_catalog_aux_update_cfg(aux->catalog);
}
+ /* reset aux if link is in connected state */
+ if (dp_catalog_link_is_connected(aux->catalog))
+ dp_catalog_aux_reset(aux->catalog);
} else {
aux->retry_cnt = 0;
switch (aux->aux_error_num) {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Main link symbol locked is achieved at end of link training 2. Some
dongle main link symbol may become unlocked again if host did not end
link training soon enough after completion of link training 2. Host
have to re train main link if loss of symbol lock detected before
end link training so that the coming video stream can be transmitted
to sink properly.
Signed-off-by: Kuogee Hsieh <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 6a013b0..20951c8 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1638,6 +1638,25 @@ static bool dp_ctrl_clock_recovery_any_ok(
return drm_dp_clock_recovery_ok(link_status, lane_count);
}
+static bool dp_ctrl_loss_symbol_lock(struct dp_ctrl_private *ctrl)
+{
+ u8 link_status[DP_LINK_STATUS_SIZE];
+ u8 status;
+ int i;
+ int num_lanes = ctrl->link->link_params.num_lanes;
+
+ dp_ctrl_read_link_status(ctrl, link_status);
+
+ for (i = 0; i < num_lanes; i++) {
+ status = link_status[i / 2];
+ status >>= ((i % 2) * 4);
+ if (!(status & DP_LANE_SYMBOL_LOCKED))
+ return true;
+ }
+
+ return false;
+}
+
int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
{
int rc = 0;
@@ -1761,6 +1780,13 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
return rc;
}
+static int dp_ctrl_link_retrain(struct dp_ctrl_private *ctrl)
+{
+ int training_step = DP_TRAINING_NONE;
+
+ return dp_ctrl_setup_main_link(ctrl, &training_step);
+}
+
int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
{
int ret = 0;
@@ -1786,6 +1812,9 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
}
}
+ if (dp_ctrl_loss_symbol_lock(ctrl))
+ dp_ctrl_link_retrain(ctrl);
+
/* stop txing train pattern to end link training */
dp_ctrl_clear_training_pattern(ctrl);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Initialize both pre-emphasis and voltage swing level to 0 before
start link training and do not end link training until video is
ready to reduce the period between end of link training and video
start to meet Link Layer CTS requirement. This fixes Link Layer
CTS cases 4.3.2.1, 4.3.2.2, 4.3.2.3 and 4.3.2.4.
Signed-off-by: Kuogee Hsieh <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 92cf331..6a013b0 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1484,6 +1484,9 @@ static int dp_ctrl_link_maintenance(struct dp_ctrl_private *ctrl)
dp_ctrl_push_idle(&ctrl->dp_ctrl);
+ ctrl->link->phy_params.p_level = 0;
+ ctrl->link->phy_params.v_level = 0;
+
ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
ret = dp_ctrl_setup_main_link(ctrl, &training_step);
@@ -1670,6 +1673,9 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
ctrl->link->link_params.rate,
ctrl->link->link_params.num_lanes, ctrl->dp_ctrl.pixel_rate);
+ ctrl->link->phy_params.p_level = 0;
+ ctrl->link->phy_params.v_level = 0;
+
rc = dp_ctrl_enable_mainlink_clocks(ctrl);
if (rc)
return rc;
@@ -1735,17 +1741,19 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN)
return rc;
- /* stop txing train pattern */
- dp_ctrl_clear_training_pattern(ctrl);
+ if (rc == 0) { /* link train successfully */
+ /*
+ * do not stop train pattern here
+ * stop link training at on_stream
+ * to pass compliance test
+ */
+ } else {
+ /*
+ * link training failed
+ * end txing train pattern here
+ */
+ dp_ctrl_clear_training_pattern(ctrl);
- /*
- * keep transmitting idle pattern until video ready
- * to avoid main link from loss of sync
- */
- if (rc == 0) /* link train successfully */
- dp_ctrl_push_idle(dp_ctrl);
- else {
- /* link training failed */
dp_ctrl_deinitialize_mainlink(ctrl);
rc = -ECONNRESET;
}
@@ -1755,7 +1763,6 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
{
- u32 rate = 0;
int ret = 0;
bool mainlink_ready = false;
struct dp_ctrl_private *ctrl;
@@ -1765,10 +1772,6 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
- rate = ctrl->panel->link_info.rate;
-
- ctrl->link->link_params.rate = rate;
- ctrl->link->link_params.num_lanes = ctrl->panel->link_info.num_lanes;
ctrl->dp_ctrl.pixel_rate = ctrl->panel->dp_mode.drm_mode.clock;
DRM_DEBUG_DP("rate=%d, num_lanes=%d, pixel_rate=%d\n",
@@ -1783,6 +1786,9 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
}
}
+ /* stop txing train pattern to end link training */
+ dp_ctrl_clear_training_pattern(ctrl);
+
ret = dp_ctrl_enable_stream_clocks(ctrl);
if (ret) {
DRM_ERROR("Failed to start pixel clocks. ret=%d\n", ret);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Response with correct edid checksum saved at connector after corrupted edid
checksum read. This fixes Link Layer CTS cases 4.2.2.3, 4.2.2.6.
Signed-off-by: Kuogee Hsieh <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_panel.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 88196f7..0fdb551 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -271,7 +271,7 @@ static u8 dp_panel_get_edid_checksum(struct edid *edid)
{
struct edid *last_block;
u8 *raw_edid;
- bool is_edid_corrupt;
+ bool is_edid_corrupt = false;
if (!edid) {
DRM_ERROR("invalid edid input\n");
@@ -303,7 +303,12 @@ void dp_panel_handle_sink_request(struct dp_panel *dp_panel)
panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
if (panel->link->sink_request & DP_TEST_LINK_EDID_READ) {
- u8 checksum = dp_panel_get_edid_checksum(dp_panel->edid);
+ u8 checksum;
+
+ if (dp_panel->edid)
+ checksum = dp_panel_get_edid_checksum(dp_panel->edid);
+ else
+ checksum = dp_panel->connector->real_edid_checksum;
dp_link_send_edid_checksum(panel->link, checksum);
dp_link_send_test_response(panel->link);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Quoting Kuogee Hsieh (2021-07-13 08:54:03)
> Aux hardware calibration sequence requires resetting the aux controller
> in order for the new setting to take effect. However resetting the AUX
> controller will also clear HPD interrupt status which may accidentally
> cause pending unplug interrupt to get lost. Therefore reset aux
> controller only when link is in connection state when dp_aux_cmd_fifo_tx()
> fail. This fixes Link Layer CTS cases 4.2.1.1 and 4.2.1.2.
>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
Quoting Kuogee Hsieh (2021-07-13 08:54:07)
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 6a013b0..20951c8 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1638,6 +1638,25 @@ static bool dp_ctrl_clock_recovery_any_ok(
> return drm_dp_clock_recovery_ok(link_status, lane_count);
> }
>
> +static bool dp_ctrl_loss_symbol_lock(struct dp_ctrl_private *ctrl)
> +{
> + u8 link_status[DP_LINK_STATUS_SIZE];
> + u8 status;
> + int i;
> + int num_lanes = ctrl->link->link_params.num_lanes;
> +
> + dp_ctrl_read_link_status(ctrl, link_status);
> +
> + for (i = 0; i < num_lanes; i++) {
> + status = link_status[i / 2];
> + status >>= ((i % 2) * 4);
> + if (!(status & DP_LANE_SYMBOL_LOCKED))
> + return true;
> + }
> +
> + return false;
> +}
Can this function move to drivers/gpu/drm/drm_dp_helper.c and be called
drm_dp_symbol_locked()?
> +
> int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
> {
> int rc = 0;
Quoting Kuogee Hsieh (2021-07-13 08:54:06)
> Initialize both pre-emphasis and voltage swing level to 0 before
> start link training and do not end link training until video is
> ready to reduce the period between end of link training and video
> start to meet Link Layer CTS requirement. This fixes Link Layer
> CTS cases 4.3.2.1, 4.3.2.2, 4.3.2.3 and 4.3.2.4.
>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> ---
Any fixes tag?
Reviewed-by: Stephen Boyd <[email protected]>
Quoting Kuogee Hsieh (2021-07-13 08:54:05)
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 88196f7..0fdb551 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -303,7 +303,12 @@ void dp_panel_handle_sink_request(struct dp_panel *dp_panel)
> panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
>
> if (panel->link->sink_request & DP_TEST_LINK_EDID_READ) {
> - u8 checksum = dp_panel_get_edid_checksum(dp_panel->edid);
> + u8 checksum;
> +
> + if (dp_panel->edid)
> + checksum = dp_panel_get_edid_checksum(dp_panel->edid);
> + else
> + checksum = dp_panel->connector->real_edid_checksum;
Is there any reason why we can't use connector->real_edid_checksum all
the time?
>
> dp_link_send_edid_checksum(panel->link, checksum);
> dp_link_send_test_response(panel->link);
Quoting Kuogee Hsieh (2021-07-13 08:54:04)
> Remove special handling of replug interrupt and instead treat replug event
> as a sequential unplug followed by a plugin event. This is needed to meet
> the requirements of DP Link Layer CTS test case 4.2.1.3.
>
> Changes in V2:
> -- add fixes statement
>
> Fixes: f21c8a276c2d ("drm/msm/dp: handle irq_hpd with sink_count = 0 correctly")
>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 78c5301..d089ada 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1146,9 +1146,6 @@ static int hpd_event_thread(void *data)
> case EV_IRQ_HPD_INT:
> dp_irq_hpd_handle(dp_priv, todo->data);
> break;
> - case EV_HPD_REPLUG_INT:
Please remove the enum as well.
> - /* do nothing */
> - break;
> case EV_USER_NOTIFICATION:
> dp_display_send_hpd_notification(dp_priv,
> todo->data);
On 2021-07-22 12:23, Stephen Boyd wrote:
> Quoting Kuogee Hsieh (2021-07-13 08:54:05)
>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
>> b/drivers/gpu/drm/msm/dp/dp_panel.c
>> index 88196f7..0fdb551 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
>> @@ -303,7 +303,12 @@ void dp_panel_handle_sink_request(struct dp_panel
>> *dp_panel)
>> panel = container_of(dp_panel, struct dp_panel_private,
>> dp_panel);
>>
>> if (panel->link->sink_request & DP_TEST_LINK_EDID_READ) {
>> - u8 checksum =
>> dp_panel_get_edid_checksum(dp_panel->edid);
>> + u8 checksum;
>> +
>> + if (dp_panel->edid)
>> + checksum =
>> dp_panel_get_edid_checksum(dp_panel->edid);
>> + else
>> + checksum =
>> dp_panel->connector->real_edid_checksum;
>
> Is there any reason why we can't use connector->real_edid_checksum all
> the time?
>
real_edid_checksum only calculated by drm when edid checksum
vitrification failed after edid read.
In the good edid checksum case (edid verification succeed),
real_edid_checksum is not calculated by drm.
>>
>> dp_link_send_edid_checksum(panel->link, checksum);
>> dp_link_send_test_response(panel->link);