On Wed, Sep 30, 2026 at 08:41:50PM +0800, Xilin Wu wrote:
> Atomic bridge enable callbacks cannot return an error to userspace.
> After a failed enable, leaving link-status unchanged gives userspace
> no indication that it needs to retry the configuration.
>
> Mark the connector link status bad from a work item after unwinding
> the failed enable. Send one connector hotplug notification per failure
> episode: fbdev can synchronously retry the modeset from the
> notification, so notifying on every failure would create an unbounded
> retry loop. Subsequent failures still restore BAD after a retry has
> set the property to GOOD.
>
> Allow notifications again after a successful enable or an external
> sink connection change. Do not reset the notification latch during
> eDP's internal plug and unplug handling, which runs on every retry.
> Ignore queued work superseded by recovery or an external unplug.
> Serialize the failure state with plugged_lock and update link-status
> under the connection mutex before notifying clients with both locks
> released.
>
> Initialize the work at probe and cancel it before unbinding the
> display.
>
> Assisted-by: LLM
> Signed-off-by: Xilin Wu <[email protected]>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 63
> +++++++++++++++++++++++++++++++++++--
> 1 file changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index ae967ca652c9..1bfa6696d904 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -12,9 +12,12 @@
> #include <linux/phy/phy.h>
> #include <linux/delay.h>
> #include <linux/string_choices.h>
> +#include <linux/workqueue.h>
> #include <drm/display/drm_dp_aux_bus.h>
> #include <drm/display/drm_hdmi_audio_helper.h>
> #include <drm/drm_edid.h>
> +#include <drm/drm_modeset_lock.h>
> +#include <drm/drm_probe_helper.h>
>
> #include "msm_drv.h"
> #include "msm_kms.h"
> @@ -54,9 +57,13 @@ struct msm_dp_display_private {
> bool audio_supported;
> bool stream_pm_active;
> bool stream_link_attempted;
> + struct work_struct link_status_work;
>
> struct mutex plugged_lock;
> bool plugged;
> + /* Protected by plugged_lock, including accesses from link_status_work.
> */
> + bool link_failed;
> + bool link_status_notified;
Do we need it? I think, it's easier to send several notifications.
>
> struct drm_device *drm_dev;
>
> @@ -204,6 +211,39 @@ void msm_dp_display_signal_audio_complete(struct msm_dp
> *msm_dp_display)
> complete_all(&dp->audio_comp);
> }
>
> +static void msm_dp_display_reset_link_status(struct msm_dp_display_private
> *dp)
> +{
> + lockdep_assert_held(&dp->plugged_lock);
> +
> + dp->link_failed = false;
> + dp->link_status_notified = false;
> +}
> +
> +static void msm_dp_display_link_status_work(struct work_struct *work)
> +{
> + struct msm_dp_display_private *dp = container_of(work,
> + struct msm_dp_display_private, link_status_work);
> + struct drm_connector *connector = dp->msm_dp_display.connector;
> + struct drm_device *dev = connector->dev;
> + bool notify = false;
> +
> + /* Match atomic check's connection_mutex -> plugged_lock ordering. */
> + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> + scoped_guard(mutex, &dp->plugged_lock) {
Please use drm_connector_set_link_status_property() here. See
intel_connector_modeset_retry_work_fn(). I think, there is a TODO in the
driver code, you can drop it too with this patch.
> + /* A successful enable or unplug may have superseded this work.
> */
> + if (dp->link_failed) {
> + connector->state->link_status =
> DRM_MODE_LINK_STATUS_BAD;
> + notify = !dp->link_status_notified;
> + dp->link_status_notified = true;
> + }
> + }
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +
> + /* fbdev can retry the modeset synchronously from this notification. */
> + if (notify)
> + drm_kms_helper_connector_hotplug_event(connector);
> +}
> +
> static int msm_dp_display_bind(struct device *dev, struct device *master,
> void *data)
> {
--
With best wishes
Dmitry
