On 5/24/26 12:33 PM, 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]>
> Tested-by: Val Packett <[email protected]> # x1e80100-dell-latitude-7455
> Tested-by: Yongxing Mou <[email protected]> # Hamoa IOT EVK,
> QCS8300 Ride
> Co-developed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
This is way too complex for me to grasp at once, so I have a couple nits
[...]
> + drm_dbg_dp(dp->drm_dev, "After, type=%d\n",
> + dp->msm_dp_display.connector_type);
Most debug prints added in this patch have odd indent and some
have vague messages
[...]
> + /* 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;
Return directly
[...]
> +}
> +
> static int msm_dp_display_request_irq(struct msm_dp_display_private *dp)
> {
> int rc = 0;
> @@ -1247,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,
I think this field expects IRQF_-prefixed flags too
Konrad
