On 3/14/26 2:09 AM, Dmitry Baryshkov wrote:
> Tracking when the DP link is ready isn't that useful from the driver
> point of view. It doesn't provide a direct information if the device
> should be suspended, etc. Replace it with the 'plugged' boolean, which
> is set when the driver knows that there is DPRX plugged.
>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
[...]
> -static void msm_dp_display_host_phy_init(struct msm_dp_display_private *dp)
> +static bool msm_dp_display_host_phy_init(struct msm_dp_display_private *dp)
> {
> drm_dbg_dp(dp->drm_dev, "type=%d core_init=%d phy_init=%d\n",
> dp->msm_dp_display.connector_type, dp->core_initialized,
> @@ -312,7 +313,10 @@ static void msm_dp_display_host_phy_init(struct
> msm_dp_display_private *dp)
> if (!dp->phy_initialized) {
> msm_dp_ctrl_phy_init(dp->ctrl);
> dp->phy_initialized = true;
> + return true;
> }
> +
> + return false;
> }
Please either rename this function or add kerneldoc - it's not obvious
what "msm_dp_display_host_phy_init() == true" is supposed to mean
>
> static void msm_dp_display_host_phy_exit(struct msm_dp_display_private *dp)
> @@ -366,14 +370,6 @@ 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->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);
> - DRM_ERROR("Disconnected, no DP_LINK_STATUS_UPDATED\n");
> - return -EINVAL;
> - }
> - }
>
> msm_dp_ctrl_handle_sink_request(dp->ctrl);
>
> @@ -392,11 +388,11 @@ static int msm_dp_hpd_plug_handle(struct
> msm_dp_display_private *dp)
> dp->msm_dp_display.connector_type,
> dp->link->sink_count);
>
> - if (dp->msm_dp_display.link_ready)
> - return 0;
> + mutex_lock(&dp->plugged_lock);
guard(mutex)(&dp->plugged_lock)
[...]
> static void msm_dp_display_handle_plugged_change(struct msm_dp
> *msm_dp_display,
> @@ -446,8 +440,12 @@ static int msm_dp_hpd_unplug_handle(struct
> msm_dp_display_private *dp)
> dp->msm_dp_display.connector_type,
> dp->link->sink_count);
>
> - if (!dp->msm_dp_display.link_ready)
> + mutex_lock(&dp->plugged_lock);
likewise
[...]
> end:
> - pm_runtime_put_sync(&dp->pdev->dev);
> + /*
> + * If we detected the DPRX, leave the controller on so that it doesn't
> + * loose the state.
loose -> lose
> + */
> + if (!priv->plugged) {
> + if (phy_deinit) {
> + msm_dp_aux_enable_xfers(priv->aux, false);
> + msm_dp_display_host_phy_exit(priv);
Should msm_dp_aux_enable_xfers() logically be called anywhere outside
phy_init/exit()?
> + }
> +
> + pm_runtime_put_sync(&dp->pdev->dev);
> + }
> +
> + mutex_unlock(&priv->plugged_lock);
> +
> return status;
> }
>
> @@ -1123,6 +1145,8 @@ static int msm_dp_display_probe(struct platform_device
> *pdev)
> (dp->msm_dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
> dp->hpd_isr_status = 0;
>
> + mutex_init(&dp->plugged_lock);
devm/drmm?
Konrad
