On Thu, Jan 15, 2026 at 09:29:12AM +0200, Dmitry Baryshkov wrote:
> From: Jessica Zhang <[email protected]>
> 
> Handling of the HPD events in the MSM DP driver is plagued with lots of
> problems. It tries to work aside of the main DRM framework, handling the
> HPD signals on its own. There are two separate paths, one for the HPD
> signals coming from the DP HPD pin and another path for signals coming
> from outside (e.g. from the Type-C AltMode). It lies about the connected
> state, returning the link established state instead. It is not easy to
> understand or modify it. Having a separate event machine doesn't add
> extra clarity.
> 
> Drop the whole event machine. When the DP receives a HPD event, send it
> to the DRM core. Then handle the events in the hpd_notify callback,
> unifying paths for HPD signals.
> 
> Signed-off-by: Jessica Zhang <[email protected]>
> Co-developed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Dmitry Baryshkov <[email protected]>

To the best of my ability...

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
>  drivers/gpu/drm/msm/dp/dp_ctrl.c    |  22 --
>  drivers/gpu/drm/msm/dp/dp_ctrl.h    |   1 -
>  drivers/gpu/drm/msm/dp/dp_display.c | 625 
> +++++++++---------------------------
>  drivers/gpu/drm/msm/dp/dp_display.h |   1 -
>  4 files changed, 148 insertions(+), 501 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c 
> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index aa2303d0e148..80796dd255fc 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -2581,28 +2581,6 @@ void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl 
> *msm_dp_ctrl)
>                       phy, phy->init_count, phy->power_count);
>  }
>  
> -void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl)
> -{
> -     struct msm_dp_ctrl_private *ctrl;
> -     struct phy *phy;
> -
> -     ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, 
> msm_dp_ctrl);
> -     phy = ctrl->phy;
> -
> -     msm_dp_ctrl_mainlink_disable(ctrl);
> -
> -     dev_pm_opp_set_rate(ctrl->dev, 0);
> -     msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
> -
> -     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);
> -}
> -
>  void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl)
>  {
>       struct msm_dp_ctrl_private *ctrl;
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h 
> b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index 124b9b21bb7f..f68bee62713f 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -19,7 +19,6 @@ struct phy;
>  int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl);
>  int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool 
> force_link_train);
>  void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl);
> -void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl);
>  void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl);
>  void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl);
>  irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl);
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index e93de362dd39..b26fba89e73a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -43,35 +43,6 @@ enum {
>       ISR_HPD_REPLUG_COUNT,
>  };
>  
> -/* event thread connection state */
> -enum {
> -     ST_DISCONNECTED,
> -     ST_MAINLINK_READY,
> -     ST_CONNECTED,
> -     ST_DISCONNECT_PENDING,
> -     ST_DISPLAY_OFF,
> -};
> -
> -enum {
> -     EV_NO_EVENT,
> -     /* hpd events */
> -     EV_HPD_PLUG_INT,
> -     EV_IRQ_HPD_INT,
> -     EV_HPD_UNPLUG_INT,
> -};
> -
> -#define EVENT_TIMEOUT        (HZ/10) /* 100ms */
> -#define DP_EVENT_Q_MAX       8
> -
> -#define DP_TIMEOUT_NONE              0
> -
> -#define WAIT_FOR_RESUME_TIMEOUT_JIFFIES (HZ / 2)
> -
> -struct msm_dp_event {
> -     u32 event_id;
> -     u32 delay;
> -};
> -
>  struct msm_dp_display_private {
>       int irq;
>  
> @@ -95,15 +66,9 @@ struct msm_dp_display_private {
>       /* wait for audio signaling */
>       struct completion audio_comp;
>  
> -     /* event related only access by event thread */
> -     struct mutex event_mutex;
> -     wait_queue_head_t event_q;
> -     u32 hpd_state;
> -     u32 event_pndx;
> -     u32 event_gndx;
> -     struct task_struct *ev_tsk;
> -     struct msm_dp_event event_list[DP_EVENT_Q_MAX];
> -     spinlock_t event_lock;
> +     /* HPD IRQ handling */
> +     spinlock_t irq_thread_lock;
> +     u32 hpd_isr_status;
>  
>       bool wide_bus_supported;
>  
> @@ -216,59 +181,6 @@ static struct msm_dp_display_private 
> *dev_get_dp_display_private(struct device *
>       return container_of(dp, struct msm_dp_display_private, msm_dp_display);
>  }
>  
> -static int msm_dp_add_event(struct msm_dp_display_private *msm_dp_priv, u32 
> event,
> -                         u32 delay)
> -{
> -     unsigned long flag;
> -     struct msm_dp_event *todo;
> -     int pndx;
> -
> -     spin_lock_irqsave(&msm_dp_priv->event_lock, flag);
> -     pndx = msm_dp_priv->event_pndx + 1;
> -     pndx %= DP_EVENT_Q_MAX;
> -     if (pndx == msm_dp_priv->event_gndx) {
> -             pr_err("event_q is full: pndx=%d gndx=%d\n",
> -                     msm_dp_priv->event_pndx, msm_dp_priv->event_gndx);
> -             spin_unlock_irqrestore(&msm_dp_priv->event_lock, flag);
> -             return -EPERM;
> -     }
> -     todo = &msm_dp_priv->event_list[msm_dp_priv->event_pndx++];
> -     msm_dp_priv->event_pndx %= DP_EVENT_Q_MAX;
> -     todo->event_id = event;
> -     todo->delay = delay;
> -     wake_up(&msm_dp_priv->event_q);
> -     spin_unlock_irqrestore(&msm_dp_priv->event_lock, flag);
> -
> -     return 0;
> -}
> -
> -static int msm_dp_del_event(struct msm_dp_display_private *msm_dp_priv, u32 
> event)
> -{
> -     unsigned long flag;
> -     struct msm_dp_event *todo;
> -     u32     gndx;
> -
> -     spin_lock_irqsave(&msm_dp_priv->event_lock, flag);
> -     if (msm_dp_priv->event_pndx == msm_dp_priv->event_gndx) {
> -             spin_unlock_irqrestore(&msm_dp_priv->event_lock, flag);
> -             return -ENOENT;
> -     }
> -
> -     gndx = msm_dp_priv->event_gndx;
> -     while (msm_dp_priv->event_pndx != gndx) {
> -             todo = &msm_dp_priv->event_list[gndx];
> -             if (todo->event_id == event) {
> -                     todo->event_id = EV_NO_EVENT;   /* deleted */
> -                     todo->delay = 0;
> -             }
> -             gndx++;
> -             gndx %= DP_EVENT_Q_MAX;
> -     }
> -     spin_unlock_irqrestore(&msm_dp_priv->event_lock, flag);
> -
> -     return 0;
> -}
> -
>  void msm_dp_display_signal_audio_start(struct msm_dp *msm_dp_display)
>  {
>       struct msm_dp_display_private *dp;
> @@ -287,8 +199,6 @@ void msm_dp_display_signal_audio_complete(struct msm_dp 
> *msm_dp_display)
>       complete_all(&dp->audio_comp);
>  }
>  
> -static int msm_dp_hpd_event_thread_start(struct msm_dp_display_private 
> *msm_dp_priv);
> -
>  static int msm_dp_display_bind(struct device *dev, struct device *master,
>                          void *data)
>  {
> @@ -308,12 +218,6 @@ static int msm_dp_display_bind(struct device *dev, 
> struct device *master,
>               goto end;
>       }
>  
> -     rc = msm_dp_hpd_event_thread_start(dp);
> -     if (rc) {
> -             DRM_ERROR("Event thread create failed\n");
> -             goto end;
> -     }
> -
>       return 0;
>  end:
>       return rc;
> @@ -325,8 +229,6 @@ static void msm_dp_display_unbind(struct device *dev, 
> struct device *master,
>       struct msm_dp_display_private *dp = dev_get_dp_display_private(dev);
>       struct msm_drm_private *priv = dev_get_drvdata(master);
>  
> -     kthread_stop(dp->ev_tsk);
> -
>       of_dp_aux_depopulate_bus(dp->aux);
>  
>       msm_dp_aux_unregister(dp->aux);
> @@ -340,38 +242,6 @@ static const struct component_ops 
> msm_dp_display_comp_ops = {
>       .unbind = msm_dp_display_unbind,
>  };
>  
> -static int msm_dp_display_send_hpd_notification(struct 
> msm_dp_display_private *dp,
> -                                         bool hpd)
> -{
> -     if ((hpd && dp->msm_dp_display.link_ready) ||
> -                     (!hpd && !dp->msm_dp_display.link_ready)) {
> -             drm_dbg_dp(dp->drm_dev, "HPD already %s\n", str_on_off(hpd));
> -             return 0;
> -     }
> -
> -     /* reset video pattern flag on disconnect */
> -     if (!hpd) {
> -             dp->panel->video_test = false;
> -             if (!dp->msm_dp_display.is_edp)
> -                     
> drm_dp_set_subconnector_property(dp->msm_dp_display.connector,
> -                                                      
> connector_status_disconnected,
> -                                                      dp->panel->dpcd,
> -                                                      
> dp->panel->downstream_ports);
> -     }
> -
> -     dp->msm_dp_display.link_ready = hpd;
> -
> -     drm_dbg_dp(dp->drm_dev, "type=%d hpd=%d\n",
> -                     dp->msm_dp_display.connector_type, hpd);
> -
> -     drm_bridge_hpd_notify(dp->msm_dp_display.bridge,
> -                           hpd ?
> -                           connector_status_connected :
> -                           connector_status_disconnected);
> -
> -     return 0;
> -}
> -
>  static int msm_dp_display_lttpr_init(struct msm_dp_display_private *dp, u8 
> *dpcd)
>  {
>       int rc, lttpr_count;
> @@ -414,6 +284,8 @@ static int msm_dp_display_process_hpd_high(struct 
> msm_dp_display_private *dp)
>                                                dp->panel->dpcd,
>                                                dp->panel->downstream_ports);
>  
> +     dp->msm_dp_display.link_ready = true;
> +
>       dp->msm_dp_display.psr_supported = dp->panel->psr_cap.version && 
> psr_enabled;
>  
>       dp->audio_supported = info->has_audio;
> @@ -427,8 +299,6 @@ static int msm_dp_display_process_hpd_high(struct 
> msm_dp_display_private *dp)
>  
>       msm_dp_link_reset_phy_params_vx_px(dp->link);
>  
> -     msm_dp_display_send_hpd_notification(dp, true);
> -
>  end:
>       return rc;
>  }
> @@ -483,24 +353,6 @@ static void msm_dp_display_host_deinit(struct 
> msm_dp_display_private *dp)
>       dp->core_initialized = false;
>  }
>  
> -static int msm_dp_display_usbpd_configure_cb(struct device *dev)
> -{
> -     struct msm_dp_display_private *dp = dev_get_dp_display_private(dev);
> -
> -     msm_dp_display_host_phy_init(dp);
> -
> -     return msm_dp_display_process_hpd_high(dp);
> -}
> -
> -static int msm_dp_display_notify_disconnect(struct device *dev)
> -{
> -     struct msm_dp_display_private *dp = dev_get_dp_display_private(dev);
> -
> -     msm_dp_display_send_hpd_notification(dp, false);
> -
> -     return 0;
> -}
> -
>  static void msm_dp_display_handle_video_request(struct 
> msm_dp_display_private *dp)
>  {
>       if (dp->link->sink_request & DP_TEST_LINK_VIDEO_PATTERN) {
> @@ -509,34 +361,12 @@ static void msm_dp_display_handle_video_request(struct 
> msm_dp_display_private *d
>       }
>  }
>  
> -static int msm_dp_display_handle_port_status_changed(struct 
> msm_dp_display_private *dp)
> -{
> -     int rc = 0;
> -
> -     if (drm_dp_is_branch(dp->panel->dpcd) && dp->link->sink_count == 0) {
> -             drm_dbg_dp(dp->drm_dev, "sink count is zero, nothing to do\n");
> -             if (dp->hpd_state != ST_DISCONNECTED) {
> -                     dp->hpd_state = ST_DISCONNECT_PENDING;
> -                     msm_dp_display_send_hpd_notification(dp, false);
> -             }
> -     } else {
> -             if (dp->hpd_state == ST_DISCONNECTED) {
> -                     dp->hpd_state = ST_MAINLINK_READY;
> -                     rc = msm_dp_display_process_hpd_high(dp);
> -                     if (rc)
> -                             dp->hpd_state = ST_DISCONNECTED;
> -             }
> -     }
> -
> -     return rc;
> -}
> -
>  static int msm_dp_display_handle_irq_hpd(struct msm_dp_display_private *dp)
>  {
>       u32 sink_request = dp->link->sink_request;
>  
>       drm_dbg_dp(dp->drm_dev, "%d\n", sink_request);
> -     if (dp->hpd_state == ST_DISCONNECTED) {
> +     if (!dp->msm_dp_display.link_ready) {
>               if (sink_request & DP_LINK_STATUS_UPDATED) {
>                       drm_dbg_dp(dp->drm_dev, "Disconnected sink_request: 
> %d\n",
>                                                       sink_request);
> @@ -553,76 +383,36 @@ static int msm_dp_display_handle_irq_hpd(struct 
> msm_dp_display_private *dp)
>       return 0;
>  }
>  
> -static int msm_dp_display_usbpd_attention_cb(struct device *dev)
> -{
> -     int rc = 0;
> -     u32 sink_request;
> -     struct msm_dp_display_private *dp = dev_get_dp_display_private(dev);
> -
> -     /* check for any test request issued by sink */
> -     rc = msm_dp_link_process_request(dp->link);
> -     if (!rc) {
> -             sink_request = dp->link->sink_request;
> -             drm_dbg_dp(dp->drm_dev, "hpd_state=%d sink_request=%d\n",
> -                                     dp->hpd_state, sink_request);
> -             if (sink_request & DS_PORT_STATUS_CHANGED)
> -                     rc = msm_dp_display_handle_port_status_changed(dp);
> -             else
> -                     rc = msm_dp_display_handle_irq_hpd(dp);
> -     }
> -
> -     return rc;
> -}
> -
>  static int msm_dp_hpd_plug_handle(struct msm_dp_display_private *dp)
>  {
> -     u32 state;
>       int ret;
>       struct platform_device *pdev = dp->msm_dp_display.pdev;
>  
> -     msm_dp_aux_enable_xfers(dp->aux, true);
> -
> -     mutex_lock(&dp->event_mutex);
> -
> -     state =  dp->hpd_state;
> -     drm_dbg_dp(dp->drm_dev, "Before, type=%d hpd_state=%d\n",
> -                     dp->msm_dp_display.connector_type, state);
> -
> -     if (state == ST_DISPLAY_OFF) {
> -             mutex_unlock(&dp->event_mutex);
> -             return 0;
> -     }
> +     drm_dbg_dp(dp->drm_dev, "Before, type=%d\n",
> +                     dp->msm_dp_display.connector_type);
>  
> -     if (state == ST_MAINLINK_READY || state == ST_CONNECTED) {
> -             mutex_unlock(&dp->event_mutex);
> +     if (dp->msm_dp_display.link_ready)
>               return 0;
> -     }
> -
> -     if (state == ST_DISCONNECT_PENDING) {
> -             /* wait until ST_DISCONNECTED */
> -             msm_dp_add_event(dp, EV_HPD_PLUG_INT, 1);
> -             mutex_unlock(&dp->event_mutex);
> -             return 0;
> -     }
>  
>       ret = pm_runtime_resume_and_get(&pdev->dev);
>       if (ret) {
>               DRM_ERROR("failed to pm_runtime_resume\n");
> -             mutex_unlock(&dp->event_mutex);
>               return ret;
>       }
>  
> -     ret = msm_dp_display_usbpd_configure_cb(&pdev->dev);
> +     msm_dp_aux_enable_xfers(dp->aux, true);
> +
> +     msm_dp_display_host_phy_init(dp);
> +
> +     ret = msm_dp_display_process_hpd_high(dp);
>       if (ret) {      /* link train failed */
> -             dp->hpd_state = ST_DISCONNECTED;
> +             dp->msm_dp_display.link_ready = false;
> +             msm_dp_aux_enable_xfers(dp->aux, false);
>               pm_runtime_put_sync(&pdev->dev);
> -     } else {
> -             dp->hpd_state = ST_MAINLINK_READY;
>       }
>  
> -     drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n",
> -                     dp->msm_dp_display.connector_type, state);
> -     mutex_unlock(&dp->event_mutex);
> +     drm_dbg_dp(dp->drm_dev, "After, type=%d\n",
> +                     dp->msm_dp_display.connector_type);
>  
>       /* uevent will complete connection part */
>       return 0;
> @@ -644,97 +434,69 @@ static void msm_dp_display_handle_plugged_change(struct 
> msm_dp *msm_dp_display,
>  
>  static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp)
>  {
> -     u32 state;
>       struct platform_device *pdev = dp->msm_dp_display.pdev;
>  
> -     msm_dp_aux_enable_xfers(dp->aux, false);
> -
> -     mutex_lock(&dp->event_mutex);
> -
> -     state = dp->hpd_state;
> +     dp->panel->video_test = false;
>  
> -     drm_dbg_dp(dp->drm_dev, "Before, type=%d hpd_state=%d\n",
> -                     dp->msm_dp_display.connector_type, state);
> +     msm_dp_aux_enable_xfers(dp->aux, false);
>  
> -     /* unplugged, no more irq_hpd handle */
> -     msm_dp_del_event(dp, EV_IRQ_HPD_INT);
> +     drm_dbg_dp(dp->drm_dev, "Before, type=%d\n",
> +                     dp->msm_dp_display.connector_type);
>  
> -     if (state == ST_DISCONNECTED) {
> -             /* triggered by irq_hdp with sink_count = 0 */
> -             if (dp->link->sink_count == 0) {
> -                     msm_dp_display_host_phy_exit(dp);
> -             }
> -             msm_dp_display_notify_disconnect(&dp->msm_dp_display.pdev->dev);
> -             mutex_unlock(&dp->event_mutex);
> +     if (!dp->msm_dp_display.link_ready)
>               return 0;
> -     } else if (state == ST_DISCONNECT_PENDING) {
> -             mutex_unlock(&dp->event_mutex);
> -             return 0;
> -     } else if (state == ST_MAINLINK_READY) {
> -             msm_dp_ctrl_off_link(dp->ctrl);
> +
> +     /* triggered by irq_hdp with sink_count = 0 */
> +     if (dp->link->sink_count == 0)
>               msm_dp_display_host_phy_exit(dp);
> -             dp->hpd_state = ST_DISCONNECTED;
> -             msm_dp_display_notify_disconnect(&dp->msm_dp_display.pdev->dev);
> -             pm_runtime_put_sync(&pdev->dev);
> -             mutex_unlock(&dp->event_mutex);
> -             return 0;
> -     }
>  
>       /*
>        * We don't need separate work for disconnect as
>        * connect/attention interrupts are disabled
>        */
> -     msm_dp_display_notify_disconnect(&dp->msm_dp_display.pdev->dev);
> +     if (!dp->msm_dp_display.is_edp)
> +             drm_dp_set_subconnector_property(dp->msm_dp_display.connector,
> +                                              connector_status_disconnected,
> +                                              dp->panel->dpcd,
> +                                              dp->panel->downstream_ports);
>  
> -     if (state == ST_DISPLAY_OFF) {
> -             dp->hpd_state = ST_DISCONNECTED;
> -     } else {
> -             dp->hpd_state = ST_DISCONNECT_PENDING;
> -     }
> +     dp->msm_dp_display.link_ready = false;
>  
>       /* signal the disconnect event early to ensure proper teardown */
>       msm_dp_display_handle_plugged_change(&dp->msm_dp_display, false);
>  
> -     drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n",
> -                     dp->msm_dp_display.connector_type, state);
> +     drm_dbg_dp(dp->drm_dev, "After, type=%d\n",
> +                     dp->msm_dp_display.connector_type);
>  
>       /* uevent will complete disconnection part */
>       pm_runtime_put_sync(&pdev->dev);
> -     mutex_unlock(&dp->event_mutex);
>       return 0;
>  }
>  
>  static int msm_dp_irq_hpd_handle(struct msm_dp_display_private *dp)
>  {
> -     u32 state;
> -
> -     mutex_lock(&dp->event_mutex);
> +     u32 sink_request;
> +     int rc = 0;
>  
>       /* irq_hpd can happen at either connected or disconnected state */
> -     state =  dp->hpd_state;
> -     drm_dbg_dp(dp->drm_dev, "Before, type=%d hpd_state=%d\n",
> -                     dp->msm_dp_display.connector_type, state);
> -
> -     if (state == ST_DISPLAY_OFF) {
> -             mutex_unlock(&dp->event_mutex);
> -             return 0;
> -     }
> +     drm_dbg_dp(dp->drm_dev, "Before, type=%d\n",
> +                     dp->msm_dp_display.connector_type);
>  
> -     if (state == ST_MAINLINK_READY || state == ST_DISCONNECT_PENDING) {
> -             /* wait until ST_CONNECTED */
> -             msm_dp_add_event(dp, EV_IRQ_HPD_INT, 1);
> -             mutex_unlock(&dp->event_mutex);
> -             return 0;
> +     /* check for any test request issued by sink */
> +     rc = msm_dp_link_process_request(dp->link);
> +     if (!rc) {
> +             sink_request = dp->link->sink_request;
> +             drm_dbg_dp(dp->drm_dev, "sink_request=%d\n", sink_request);
> +             if (sink_request & DS_PORT_STATUS_CHANGED)
> +                     rc = msm_dp_display_process_hpd_high(dp);
> +             else
> +                     rc = msm_dp_display_handle_irq_hpd(dp);
>       }
>  
> -     msm_dp_display_usbpd_attention_cb(&dp->msm_dp_display.pdev->dev);
> -
> -     drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n",
> -                     dp->msm_dp_display.connector_type, state);
> +     drm_dbg_dp(dp->drm_dev, "After, type=%d\n",
> +                     dp->msm_dp_display.connector_type);
>  
> -     mutex_unlock(&dp->event_mutex);
> -
> -     return 0;
> +     return rc;
>  }
>  
>  static void msm_dp_display_deinit_sub_modules(struct msm_dp_display_private 
> *dp)
> @@ -1010,12 +772,8 @@ void msm_dp_snapshot(struct msm_disp_state *disp_state, 
> struct msm_dp *dp)
>        * power_on status before dumping DP registers to avoid crash due
>        * to unclocked access
>        */
> -     mutex_lock(&msm_dp_display->event_mutex);
> -
> -     if (!dp->power_on) {
> -             mutex_unlock(&msm_dp_display->event_mutex);
> +     if (!dp->power_on)
>               return;
> -     }
>  
>       msm_disp_snapshot_add_block(disp_state, msm_dp_display->ahb_len,
>                                   msm_dp_display->ahb_base, "dp_ahb");
> @@ -1025,8 +783,6 @@ void msm_dp_snapshot(struct msm_disp_state *disp_state, 
> struct msm_dp *dp)
>                                   msm_dp_display->link_base, "dp_link");
>       msm_disp_snapshot_add_block(disp_state, msm_dp_display->p0_len,
>                                   msm_dp_display->p0_base, "dp_p0");
> -
> -     mutex_unlock(&msm_dp_display->event_mutex);
>  }
>  
>  void msm_dp_display_set_psr(struct msm_dp *msm_dp_display, bool enter)
> @@ -1042,95 +798,6 @@ void msm_dp_display_set_psr(struct msm_dp 
> *msm_dp_display, bool enter)
>       msm_dp_ctrl_set_psr(dp->ctrl, enter);
>  }
>  
> -static int hpd_event_thread(void *data)
> -{
> -     struct msm_dp_display_private *msm_dp_priv;
> -     unsigned long flag;
> -     struct msm_dp_event *todo;
> -     int timeout_mode = 0;
> -
> -     msm_dp_priv = (struct msm_dp_display_private *)data;
> -
> -     while (1) {
> -             if (timeout_mode) {
> -                     wait_event_timeout(msm_dp_priv->event_q,
> -                             (msm_dp_priv->event_pndx == 
> msm_dp_priv->event_gndx) ||
> -                                     kthread_should_stop(), EVENT_TIMEOUT);
> -             } else {
> -                     wait_event_interruptible(msm_dp_priv->event_q,
> -                             (msm_dp_priv->event_pndx != 
> msm_dp_priv->event_gndx) ||
> -                                     kthread_should_stop());
> -             }
> -
> -             if (kthread_should_stop())
> -                     break;
> -
> -             spin_lock_irqsave(&msm_dp_priv->event_lock, flag);
> -             todo = &msm_dp_priv->event_list[msm_dp_priv->event_gndx];
> -             if (todo->delay) {
> -                     struct msm_dp_event *todo_next;
> -
> -                     msm_dp_priv->event_gndx++;
> -                     msm_dp_priv->event_gndx %= DP_EVENT_Q_MAX;
> -
> -                     /* re enter delay event into q */
> -                     todo_next = 
> &msm_dp_priv->event_list[msm_dp_priv->event_pndx++];
> -                     msm_dp_priv->event_pndx %= DP_EVENT_Q_MAX;
> -                     todo_next->event_id = todo->event_id;
> -                     todo_next->delay = todo->delay - 1;
> -
> -                     /* clean up older event */
> -                     todo->event_id = EV_NO_EVENT;
> -                     todo->delay = 0;
> -
> -                     /* switch to timeout mode */
> -                     timeout_mode = 1;
> -                     spin_unlock_irqrestore(&msm_dp_priv->event_lock, flag);
> -                     continue;
> -             }
> -
> -             /* timeout with no events in q */
> -             if (msm_dp_priv->event_pndx == msm_dp_priv->event_gndx) {
> -                     spin_unlock_irqrestore(&msm_dp_priv->event_lock, flag);
> -                     continue;
> -             }
> -
> -             msm_dp_priv->event_gndx++;
> -             msm_dp_priv->event_gndx %= DP_EVENT_Q_MAX;
> -             timeout_mode = 0;
> -             spin_unlock_irqrestore(&msm_dp_priv->event_lock, flag);
> -
> -             switch (todo->event_id) {
> -             case EV_HPD_PLUG_INT:
> -                     msm_dp_hpd_plug_handle(msm_dp_priv);
> -                     break;
> -             case EV_HPD_UNPLUG_INT:
> -                     msm_dp_hpd_unplug_handle(msm_dp_priv);
> -                     break;
> -             case EV_IRQ_HPD_INT:
> -                     msm_dp_irq_hpd_handle(msm_dp_priv);
> -                     break;
> -             default:
> -                     break;
> -             }
> -     }
> -
> -     return 0;
> -}
> -
> -static int msm_dp_hpd_event_thread_start(struct msm_dp_display_private 
> *msm_dp_priv)
> -{
> -     /* set event q to empty */
> -     msm_dp_priv->event_gndx = 0;
> -     msm_dp_priv->event_pndx = 0;
> -
> -     msm_dp_priv->ev_tsk = kthread_run(hpd_event_thread, msm_dp_priv, 
> "dp_hpd_handler");
> -     if (IS_ERR(msm_dp_priv->ev_tsk))
> -             return PTR_ERR(msm_dp_priv->ev_tsk);
> -
> -     return 0;
> -}
> -
>  /**
>   * msm_dp_bridge_detect - callback to determine if connector is connected
>   * @bridge: Pointer to drm bridge structure
> @@ -1144,7 +811,7 @@ enum drm_connector_status msm_dp_bridge_detect(struct 
> drm_bridge *bridge,
>       struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
>       struct msm_dp_display_private *priv;
>       int ret = 0;
> -     int status = connector_status_disconnected;
> +     int status;
>       u8 dpcd[DP_RECEIVER_CAP_SIZE];
>       struct drm_dp_desc desc;
>  
> @@ -1153,77 +820,70 @@ enum drm_connector_status msm_dp_bridge_detect(struct 
> drm_bridge *bridge,
>       priv = container_of(dp, struct msm_dp_display_private, msm_dp_display);
>  
>       if (!dp->link_ready)
> -             return status;
> -
> -     msm_dp_aux_enable_xfers(priv->aux, true);
> +             return connector_status_disconnected;
>  
>       ret = pm_runtime_resume_and_get(&dp->pdev->dev);
>       if (ret) {
>               DRM_ERROR("failed to pm_runtime_resume\n");
> -             msm_dp_aux_enable_xfers(priv->aux, false);
> -             return status;
> +             return connector_status_disconnected;
>       }
>  
> +     msm_dp_aux_enable_xfers(priv->aux, true);
> +
>       ret = msm_dp_aux_is_link_connected(priv->aux);
> -     if (dp->internal_hpd && !ret)
> -             goto end;
> +     if (ret) {
> +             DRM_DEBUG_DP("aux not connected\n");
> +             goto err;
> +     }
>  
>       ret = drm_dp_read_dpcd_caps(priv->aux, dpcd);
> -     if (ret)
> -             goto end;
> +     if (ret) {
> +             DRM_DEBUG_DP("failed to read caps\n");
> +             goto err;
> +     }
>  
>       ret = drm_dp_read_desc(priv->aux, &desc, drm_dp_is_branch(dpcd));
> -     if (ret)
> -             goto end;
> +     if (ret) {
> +             DRM_DEBUG_DP("failed to read desc\n");
> +             goto err;
> +     }
>  
>       status = connector_status_connected;
>       if (drm_dp_read_sink_count_cap(connector, dpcd, &desc)) {
> -             int sink_count = drm_dp_read_sink_count(priv->aux);
> -
> -             drm_dbg_dp(dp->drm_dev, "sink_count = %d\n", sink_count);
> +             int sink_count;
>  
> +             sink_count = drm_dp_read_sink_count(priv->aux);
>               if (sink_count <= 0)
>                       status = connector_status_disconnected;
> +
> +             drm_dbg_dp(dp->drm_dev, "sink_count = %d\n", sink_count);
>       }
>  
> -end:
>       pm_runtime_put_sync(&dp->pdev->dev);
>       return status;
> +
> +err:
> +     pm_runtime_put_sync(&dp->pdev->dev);
> +     return connector_status_disconnected;
>  }
>  
>  static irqreturn_t msm_dp_display_irq_handler(int irq, void *dev_id)
>  {
>       struct msm_dp_display_private *dp = dev_id;
> -     irqreturn_t ret = IRQ_NONE;
>       u32 hpd_isr_status;
> -
> -     if (!dp) {
> -             DRM_ERROR("invalid data\n");
> -             return IRQ_NONE;
> -     }
> +     unsigned long flags;
> +     irqreturn_t ret = IRQ_HANDLED;
>  
>       hpd_isr_status = msm_dp_aux_get_hpd_intr_status(dp->aux);
>  
>       if (hpd_isr_status & 0x0F) {
>               drm_dbg_dp(dp->drm_dev, "type=%d isr=0x%x\n",
>                       dp->msm_dp_display.connector_type, hpd_isr_status);
> -             /* hpd related interrupts */
> -             if (hpd_isr_status & DP_DP_HPD_PLUG_INT_MASK)
> -                     msm_dp_add_event(dp, EV_HPD_PLUG_INT, 0);
> -
> -             if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK) {
> -                     msm_dp_add_event(dp, EV_IRQ_HPD_INT, 0);
> -             }
> -
> -             if (hpd_isr_status & DP_DP_HPD_REPLUG_INT_MASK) {
> -                     msm_dp_add_event(dp, EV_HPD_UNPLUG_INT, 0);
> -                     msm_dp_add_event(dp, EV_HPD_PLUG_INT, 3);
> -             }
>  
> -             if (hpd_isr_status & DP_DP_HPD_UNPLUG_INT_MASK)
> -                     msm_dp_add_event(dp, EV_HPD_UNPLUG_INT, 0);
> -
> -             ret = IRQ_HANDLED;
> +             spin_lock_irqsave(&dp->irq_thread_lock, flags);
> +             dp->hpd_isr_status |= hpd_isr_status;
> +             ret = IRQ_WAKE_THREAD;
> +             spin_unlock_irqrestore(&dp->irq_thread_lock, flags);
>       }
>  
>       /* DP controller isr */
> @@ -1232,6 +892,36 @@ static irqreturn_t msm_dp_display_irq_handler(int irq, 
> void *dev_id)
>       return ret;
>  }
>  
> +static irqreturn_t msm_dp_display_irq_thread(int irq, void *dev_id)
> +{
> +     struct msm_dp_display_private *dp = dev_id;
> +     irqreturn_t ret = IRQ_NONE;
> +     unsigned long flags;
> +     u32 hpd_isr_status;
> +
> +     spin_lock_irqsave(&dp->irq_thread_lock, flags);
> +     hpd_isr_status = dp->hpd_isr_status;
> +     dp->hpd_isr_status = 0;
> +     spin_unlock_irqrestore(&dp->irq_thread_lock, flags);
> +
> +     if (hpd_isr_status & DP_DP_HPD_UNPLUG_INT_MASK)
> +             drm_bridge_hpd_notify(dp->msm_dp_display.bridge,
> +                                   connector_status_disconnected);
> +
> +     if (hpd_isr_status & DP_DP_HPD_PLUG_INT_MASK)
> +             drm_bridge_hpd_notify(dp->msm_dp_display.bridge,
> +                                   connector_status_connected);
> +
> +     /* Send HPD as connected and distinguish it in the notifier */
> +     if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK)
> +             drm_bridge_hpd_notify(dp->msm_dp_display.bridge,
> +                                   connector_status_connected);
> +
> +     ret = IRQ_HANDLED;
> +
> +     return ret;
> +}
> +
>  static int msm_dp_display_request_irq(struct msm_dp_display_private *dp)
>  {
>       int rc = 0;
> @@ -1243,9 +933,13 @@ static int msm_dp_display_request_irq(struct 
> msm_dp_display_private *dp)
>               return dp->irq;
>       }
>  
> -     rc = devm_request_irq(&pdev->dev, dp->irq, msm_dp_display_irq_handler,
> -                           IRQF_TRIGGER_HIGH|IRQF_NO_AUTOEN,
> -                           "dp_display_isr", dp);
> +     spin_lock_init(&dp->irq_thread_lock);
> +     irq_set_status_flags(dp->irq, IRQ_NOAUTOEN);
> +     rc = devm_request_threaded_irq(&pdev->dev, dp->irq,
> +                                    msm_dp_display_irq_handler,
> +                                    msm_dp_display_irq_thread,
> +                                    IRQ_TYPE_LEVEL_HIGH,
> +                                    "dp_display_isr", dp);
>  
>       if (rc < 0) {
>               DRM_ERROR("failed to request IRQ%u: %d\n",
> @@ -1425,6 +1119,7 @@ static int msm_dp_display_probe(struct platform_device 
> *pdev)
>       dp->wide_bus_supported = desc->wide_bus_supported;
>       dp->msm_dp_display.is_edp =
>               (dp->msm_dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
> +     dp->hpd_isr_status = 0;
>  
>       rc = msm_dp_display_get_io(dp);
>       if (rc)
> @@ -1436,11 +1131,6 @@ static int msm_dp_display_probe(struct platform_device 
> *pdev)
>               return -EPROBE_DEFER;
>       }
>  
> -     /* setup event q */
> -     mutex_init(&dp->event_mutex);
> -     init_waitqueue_head(&dp->event_q);
> -     spin_lock_init(&dp->event_lock);
> -
>       /* Store DP audio handle inside DP display */
>       dp->msm_dp_display.msm_dp_audio = dp->audio;
>  
> @@ -1636,7 +1326,6 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge 
> *drm_bridge,
>       struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
>       int rc = 0;
>       struct msm_dp_display_private *msm_dp_display;
> -     u32 hpd_state;
>       bool force_link_train = false;
>  
>       msm_dp_display = container_of(dp, struct msm_dp_display_private, 
> msm_dp_display);
> @@ -1648,29 +1337,21 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge 
> *drm_bridge,
>       if (dp->is_edp)
>               msm_dp_hpd_plug_handle(msm_dp_display);
>  
> -     mutex_lock(&msm_dp_display->event_mutex);
>       if (pm_runtime_resume_and_get(&dp->pdev->dev)) {
>               DRM_ERROR("failed to pm_runtime_resume\n");
> -             mutex_unlock(&msm_dp_display->event_mutex);
>               return;
>       }
>  
> -     hpd_state = msm_dp_display->hpd_state;
> -     if (hpd_state != ST_DISPLAY_OFF && hpd_state != ST_MAINLINK_READY) {
> -             mutex_unlock(&msm_dp_display->event_mutex);
> +     if (msm_dp_display->link->sink_count == 0)
>               return;
> -     }
>  
>       rc = msm_dp_display_set_mode(dp, &msm_dp_display->msm_dp_mode);
>       if (rc) {
>               DRM_ERROR("Failed to perform a mode set, rc=%d\n", rc);
> -             mutex_unlock(&msm_dp_display->event_mutex);
>               return;
>       }
>  
> -     hpd_state =  msm_dp_display->hpd_state;
> -
> -     if (hpd_state == ST_DISPLAY_OFF) {
> +     if (dp->link_ready && !dp->power_on) {
>               msm_dp_display_host_phy_init(msm_dp_display);
>               force_link_train = true;
>       }
> @@ -1689,11 +1370,7 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge 
> *drm_bridge,
>               msm_dp_display_disable(msm_dp_display);
>       }
>  
> -     /* completed connection */
> -     msm_dp_display->hpd_state = ST_CONNECTED;
> -
>       drm_dbg_dp(dp->drm_dev, "type=%d Done\n", dp->connector_type);
> -     mutex_unlock(&msm_dp_display->event_mutex);
>  }
>  
>  void msm_dp_bridge_atomic_disable(struct drm_bridge *drm_bridge,
> @@ -1713,7 +1390,6 @@ void msm_dp_bridge_atomic_post_disable(struct 
> drm_bridge *drm_bridge,
>  {
>       struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge);
>       struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
> -     u32 hpd_state;
>       struct msm_dp_display_private *msm_dp_display;
>  
>       msm_dp_display = container_of(dp, struct msm_dp_display_private, 
> msm_dp_display);
> @@ -1721,27 +1397,14 @@ void msm_dp_bridge_atomic_post_disable(struct 
> drm_bridge *drm_bridge,
>       if (dp->is_edp)
>               msm_dp_hpd_unplug_handle(msm_dp_display);
>  
> -     mutex_lock(&msm_dp_display->event_mutex);
> -
> -     hpd_state = msm_dp_display->hpd_state;
> -     if (hpd_state != ST_DISCONNECT_PENDING && hpd_state != ST_CONNECTED)
> -             drm_dbg_dp(dp->drm_dev, "type=%d wrong hpd_state=%d\n",
> -                        dp->connector_type, hpd_state);
> +     if (!dp->link_ready)
> +             drm_dbg_dp(dp->drm_dev, "type=%d is disconnected\n", 
> dp->connector_type);
>  
>       msm_dp_display_disable(msm_dp_display);
>  
> -     hpd_state =  msm_dp_display->hpd_state;
> -     if (hpd_state == ST_DISCONNECT_PENDING) {
> -             /* completed disconnection */
> -             msm_dp_display->hpd_state = ST_DISCONNECTED;
> -     } else {
> -             msm_dp_display->hpd_state = ST_DISPLAY_OFF;
> -     }
> -
>       drm_dbg_dp(dp->drm_dev, "type=%d Done\n", dp->connector_type);
>  
>       pm_runtime_put_sync(&dp->pdev->dev);
> -     mutex_unlock(&msm_dp_display->event_mutex);
>  }
>  
>  void msm_dp_bridge_mode_set(struct drm_bridge *drm_bridge,
> @@ -1797,18 +1460,13 @@ void msm_dp_bridge_hpd_enable(struct drm_bridge 
> *bridge)
>        * step-4: DP PHY is initialized at plugin handler before link training
>        *
>        */
> -     mutex_lock(&dp->event_mutex);
>       if (pm_runtime_resume_and_get(&msm_dp_display->pdev->dev)) {
>               DRM_ERROR("failed to resume power\n");
> -             mutex_unlock(&dp->event_mutex);
>               return;
>       }
>  
>       msm_dp_aux_hpd_enable(dp->aux);
>       msm_dp_aux_hpd_intr_enable(dp->aux);
> -
> -     msm_dp_display->internal_hpd = true;
> -     mutex_unlock(&dp->event_mutex);
>  }
>  
>  void msm_dp_bridge_hpd_disable(struct drm_bridge *bridge)
> @@ -1817,15 +1475,10 @@ void msm_dp_bridge_hpd_disable(struct drm_bridge 
> *bridge)
>       struct msm_dp *msm_dp_display = msm_dp_bridge->msm_dp_display;
>       struct msm_dp_display_private *dp = container_of(msm_dp_display, struct 
> msm_dp_display_private, msm_dp_display);
>  
> -     mutex_lock(&dp->event_mutex);
> -
>       msm_dp_aux_hpd_intr_disable(dp->aux);
>       msm_dp_aux_hpd_disable(dp->aux);
>  
> -     msm_dp_display->internal_hpd = false;
> -
>       pm_runtime_put_sync(&msm_dp_display->pdev->dev);
> -     mutex_unlock(&dp->event_mutex);
>  }
>  
>  void msm_dp_bridge_hpd_notify(struct drm_bridge *bridge,
> @@ -1835,13 +1488,31 @@ void msm_dp_bridge_hpd_notify(struct drm_bridge 
> *bridge,
>       struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(bridge);
>       struct msm_dp *msm_dp_display = msm_dp_bridge->msm_dp_display;
>       struct msm_dp_display_private *dp = container_of(msm_dp_display, struct 
> msm_dp_display_private, msm_dp_display);
> +     u32 hpd_link_status = 0;
>  
> -     /* Without next_bridge interrupts are handled by the DP core directly */
> -     if (msm_dp_display->internal_hpd)
> +     if (pm_runtime_resume_and_get(&msm_dp_display->pdev->dev)) {
> +             DRM_ERROR("failed to pm_runtime_resume\n");
>               return;
> +     }
> +
> +     hpd_link_status = msm_dp_aux_is_link_connected(dp->aux);
>  
> -     if (!msm_dp_display->link_ready && status == connector_status_connected)
> -             msm_dp_add_event(dp, EV_HPD_PLUG_INT, 0);
> -     else if (msm_dp_display->link_ready && status == 
> connector_status_disconnected)
> -             msm_dp_add_event(dp, EV_HPD_UNPLUG_INT, 0);
> +     drm_dbg_dp(dp->drm_dev, "type=%d link hpd_link_status=0x%x, 
> link_ready=%d, status=%d\n",
> +                msm_dp_display->connector_type, hpd_link_status,
> +                msm_dp_display->link_ready, status);
> +
> +     if (status == connector_status_connected) {
> +             if (hpd_link_status == ISR_HPD_REPLUG_COUNT) {
> +                     msm_dp_hpd_plug_handle(dp);
> +                     msm_dp_hpd_unplug_handle(dp);
> +             } else if (hpd_link_status == ISR_IRQ_HPD_PULSE_COUNT) {
> +                     msm_dp_irq_hpd_handle(dp);
> +             } else {
> +                     msm_dp_hpd_plug_handle(dp);
> +             }
> +     } else {
> +             msm_dp_hpd_unplug_handle(dp);
> +     }
> +
> +     pm_runtime_put_sync(&msm_dp_display->pdev->dev);
>  }
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h 
> b/drivers/gpu/drm/msm/dp/dp_display.h
> index 60094061c102..d2d3d61eb0b0 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -22,7 +22,6 @@ struct msm_dp {
>       bool power_on;
>       unsigned int connector_type;
>       bool is_edp;
> -     bool internal_hpd;
>  
>       struct msm_dp_audio *msm_dp_audio;
>       bool psr_supported;
> 
> -- 
> 2.47.3
> 
> 

Reply via email to