Currently, the DP link training is being done during HPD. Move
link training to atomic_enable() in accordance with the atomic_enable()
documentation.
Link disabling is already done in atomic_post_disable() (as part of the
dp_ctrl_off_link_stream() helper).
Finally, call the plug/unplug handlers directly in hpd_notify() instead
of queueing them in the event thread so that they aren't preempted by
other events.
Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index e2556de99894..c849befe58f0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -427,11 +427,6 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
msm_dp_link_psm_config(dp->link, &dp->panel->link_info, false);
msm_dp_link_reset_phy_params_vx_px(dp->link);
- rc = msm_dp_ctrl_on_link(dp->ctrl);
- if (rc) {
- DRM_ERROR("failed to complete DP link training\n");
- goto end;
- }
msm_dp_add_event(dp, EV_USER_NOTIFICATION, true, 0);
@@ -1680,6 +1675,12 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
force_link_train = true;
}
+ rc = msm_dp_ctrl_on_link(msm_dp_display->ctrl);
+ if (rc) {
+ DRM_ERROR("Failed link training (rc=%d)\n", rc);
+ dp->connector->state->link_status = DRM_LINK_STATUS_BAD;
+ }
+
msm_dp_display_enable(msm_dp_display, force_link_train);
rc = msm_dp_display_post_enable(dp);
@@ -1839,7 +1840,7 @@ void msm_dp_bridge_hpd_notify(struct drm_bridge *bridge,
return;
if (!msm_dp_display->link_ready && status == connector_status_connected)
- msm_dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+ msm_dp_hpd_plug_handle(dp, 0);
else if (msm_dp_display->link_ready && status == connector_status_disconnected)
- msm_dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+ msm_dp_hpd_unplug_handle(dp, 0);
}
--
2.50.1On 09/08/2025 03:35, Jessica Zhang wrote:
> Currently, the DP link training is being done during HPD. Move
> link training to atomic_enable() in accordance with the atomic_enable()
> documentation.
>
> Link disabling is already done in atomic_post_disable() (as part of the
> dp_ctrl_off_link_stream() helper).
>
> Finally, call the plug/unplug handlers directly in hpd_notify() instead
> of queueing them in the event thread so that they aren't preempted by
> other events.
>
> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index e2556de99894..c849befe58f0 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -427,11 +427,6 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
> msm_dp_link_psm_config(dp->link, &dp->panel->link_info, false);
>
> msm_dp_link_reset_phy_params_vx_px(dp->link);
> - rc = msm_dp_ctrl_on_link(dp->ctrl);
> - if (rc) {
> - DRM_ERROR("failed to complete DP link training\n");
> - goto end;
> - }
>
> msm_dp_add_event(dp, EV_USER_NOTIFICATION, true, 0);
Also at this point we enqueue event, which will set link_ready to true
(which is obviously incorrect).
>
> @@ -1680,6 +1675,12 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
> force_link_train = true;
> }
>
> + rc = msm_dp_ctrl_on_link(msm_dp_display->ctrl);
> + if (rc) {
> + DRM_ERROR("Failed link training (rc=%d)\n", rc);
> + dp->connector->state->link_status = DRM_LINK_STATUS_BAD;
Add a comment why it's set directly rather than through
drm_connector_set_link_status_property().
> + }
> +
> msm_dp_display_enable(msm_dp_display, force_link_train);
>
> rc = msm_dp_display_post_enable(dp);
> @@ -1839,7 +1840,7 @@ void msm_dp_bridge_hpd_notify(struct drm_bridge *bridge,
> return;
>
> if (!msm_dp_display->link_ready && status == connector_status_connected)
> - msm_dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
> + msm_dp_hpd_plug_handle(dp, 0);
> else if (msm_dp_display->link_ready && status == connector_status_disconnected)
> - msm_dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
> + msm_dp_hpd_unplug_handle(dp, 0);
> }
>
--
With best wishes
Dmitry
On 09/08/2025 03:35, Jessica Zhang wrote:
> Currently, the DP link training is being done during HPD. Move
> link training to atomic_enable() in accordance with the atomic_enable()
> documentation.
>
> Link disabling is already done in atomic_post_disable() (as part of the
> dp_ctrl_off_link_stream() helper).
>
> Finally, call the plug/unplug handlers directly in hpd_notify() instead
> of queueing them in the event thread so that they aren't preempted by
> other events.
This feels like two unrelated changes.
>
> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index e2556de99894..c849befe58f0 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -427,11 +427,6 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
> msm_dp_link_psm_config(dp->link, &dp->panel->link_info, false);
>
> msm_dp_link_reset_phy_params_vx_px(dp->link);
> - rc = msm_dp_ctrl_on_link(dp->ctrl);
> - if (rc) {
> - DRM_ERROR("failed to complete DP link training\n");
> - goto end;
> - }
>
> msm_dp_add_event(dp, EV_USER_NOTIFICATION, true, 0);
>
> @@ -1680,6 +1675,12 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
> force_link_train = true;
> }
>
> + rc = msm_dp_ctrl_on_link(msm_dp_display->ctrl);
> + if (rc) {
> + DRM_ERROR("Failed link training (rc=%d)\n", rc);
> + dp->connector->state->link_status = DRM_LINK_STATUS_BAD;
> + }
> +
> msm_dp_display_enable(msm_dp_display, force_link_train);
>
> rc = msm_dp_display_post_enable(dp);
> @@ -1839,7 +1840,7 @@ void msm_dp_bridge_hpd_notify(struct drm_bridge *bridge,
> return;
>
> if (!msm_dp_display->link_ready && status == connector_status_connected)
> - msm_dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
> + msm_dp_hpd_plug_handle(dp, 0);
> else if (msm_dp_display->link_ready && status == connector_status_disconnected)
> - msm_dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
> + msm_dp_hpd_unplug_handle(dp, 0);
> }
>
--
With best wishes
Dmitry
On 8/8/2025 5:45 PM, Dmitry Baryshkov wrote:
> On 09/08/2025 03:35, Jessica Zhang wrote:
>> Currently, the DP link training is being done during HPD. Move
>> link training to atomic_enable() in accordance with the atomic_enable()
>> documentation.
>>
>> Link disabling is already done in atomic_post_disable() (as part of the
>> dp_ctrl_off_link_stream() helper).
>>
>> Finally, call the plug/unplug handlers directly in hpd_notify() instead
>> of queueing them in the event thread so that they aren't preempted by
>> other events.
>
> This feels like two unrelated changes.
Hi Dmitry,
Ack. Would it make more sense to squash the hpd_notify part with the IRQ
thread change?
Thanks,
Jessica Zhang
>
>>
>> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
>> ---
>> drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++-------
>> 1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/
>> msm/dp/dp_display.c
>> index e2556de99894..c849befe58f0 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -427,11 +427,6 @@ static int msm_dp_display_process_hpd_high(struct
>> msm_dp_display_private *dp)
>> msm_dp_link_psm_config(dp->link, &dp->panel->link_info, false);
>> msm_dp_link_reset_phy_params_vx_px(dp->link);
>> - rc = msm_dp_ctrl_on_link(dp->ctrl);
>> - if (rc) {
>> - DRM_ERROR("failed to complete DP link training\n");
>> - goto end;
>> - }
>> msm_dp_add_event(dp, EV_USER_NOTIFICATION, true, 0);
>> @@ -1680,6 +1675,12 @@ void msm_dp_bridge_atomic_enable(struct
>> drm_bridge *drm_bridge,
>> force_link_train = true;
>> }
>> + rc = msm_dp_ctrl_on_link(msm_dp_display->ctrl);
>> + if (rc) {
>> + DRM_ERROR("Failed link training (rc=%d)\n", rc);
>> + dp->connector->state->link_status = DRM_LINK_STATUS_BAD;
>> + }
>> +
>> msm_dp_display_enable(msm_dp_display, force_link_train);
>> rc = msm_dp_display_post_enable(dp);
>> @@ -1839,7 +1840,7 @@ void msm_dp_bridge_hpd_notify(struct drm_bridge
>> *bridge,
>> return;
>> if (!msm_dp_display->link_ready && status ==
>> connector_status_connected)
>> - msm_dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
>> + msm_dp_hpd_plug_handle(dp, 0);
>> else if (msm_dp_display->link_ready && status ==
>> connector_status_disconnected)
>> - msm_dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
>> + msm_dp_hpd_unplug_handle(dp, 0);
>> }
>>
>
>
© 2016 - 2025 Red Hat, Inc.