[PATCH v2 00/10] Add HPD support for Rockchip Analogix DP
Damon Ding
damon.ding at rock-chips.com
Sun Sep 20 04:36:16 PDT 2026
Hi Heiko,
On 8/6/2026 7:42 AM, Heiko Stübner wrote:
> Am Mittwoch, 5. August 2026, 06:06:48 Mitteleuropäische Sommerzeit schrieb Damon Ding:
>> Hi Heiko,
>>
>> On 8/5/2026 6:39 AM, Heiko Stübner wrote:
>>> Hi Damon,
>>>
>>> Am Dienstag, 4. August 2026, 10:17:07 Mitteleuropäische Sommerzeit schrieb Damon Ding:
>>>> Display-connector mode (DP connector without HPD GPIO):
>>>>
>>>> &edp_out_conn {
>>>> remote-endpoint = <&dp_con_in>;
>>>> };
>>>>
>>>> dp-con {
>>>> compatible = "dp-connector";
>>>> label = "DP OUT";
>>>> type = "full-size";
>>>>
>>>> port {
>>>> dp_con_in: endpoint {
>>>> remote-endpoint = <&edp_out_conn>;
>>>> };
>>>> };
>>>> };
>>>>
>>>> Display-connector mode (DP connector with HPD GPIO):
>>>>
>>>> dp-con {
>>>> compatible = "dp-connector";
>>>> label = "DP OUT";
>>>> type = "full-size";
>>>> pinctrl-0 = <&edp0_hpd>;
>>>> pinctrl-names = "default";
>>>> hpd-gpios = <&gpio4 RK_PC1 GPIO_ACTIVE_HIGH>;
>>>>
>>>> port {
>>>> dp_con_in: endpoint {
>>>> remote-endpoint = <&edp_out_conn>;
>>>> };
>>>> };
>>>> };
>>>>
>>>> All four configurations detect cable plug/unplug events correctly.
>>
>> Thanks a lot for your testing and feedback. :-)
>>
>> It seems my test setup gave me the false impression that those cases
>> worked well.
>>
>>>
>>> hmm, it wasn't working entirely for me though and was still running into
>>> issues when the display was unplugged on boot.
>>>
>>
>> Are you seeing this boot‑unplug issue for both scenarios: DP‑connector
>> with HPD‑gpio paired with eDP without HPD‑gpio, and DP‑connector without
>> HPD‑gpio paired with eDP with HPD‑gpio? I.e. it wrongly reports
>> connected even with no display plugged in and proceeds into DRM
>> .atomic_enable()?
>
> Yep it's different. The analogix-dp gpio-hpd works correctly, because the
> analogix driver can do gpiod_get_value() in analogix_dp_get_plug_in_status()
>
> The dp-connector does not, as with the patch you pointed to, it lost
> that ability. So on boot a plugged in display is not detected correctly
> because there won't be a hotplug interrupt.
>
>>
>>> I wiggled around a bit like in the diff below and am now getting correct
>>> plug and unplug events.
>>
>> Oh, I see. For the GPIO HPD case, the IRQ needs to be enabled early so
>> that plug‑in interrupts can be properly responded to. However GPIO HPD
>> does not require a runtime PM get. I will better separate these two
>> scenarios in the next version.
>>
>>>
>>> But of course, as the dp-variant of the connector does not provide
>>> a "detect" and just the "hpd" functionality, it's missing the initial state.
>>>
>>> I'm currently not sure how to find out _if_ a panel is connected on boot.
>>>
>>
>> Based on Dmitry's commit cb640b2ca546 ("drm/bridge: display‑connector:
>> don't set OP_DETECT for DisplayPorts"), HPD events from DP‑variant
>> connector should be handled by the upstream DP controller. Hence I added
>> analogix_dp_bridge_notify() to retrieve HPD status coming from downstream.
>>
>> As expected, under the bridge‑connector framework, the detect result
>> from Analogix DP should in theory reflect the actual connection status.
>> Let's dig into this boot‑time initial‑state issue together.
>
> One idea I had was, is it possible to do a drm_dp_read_dpcd_caps()
> read during bind/... to see if something answers?
>
Sorry for the long delay to get back to this series. I was stuck on some
tricky local issues.
I'll test your suggestion and try to send out v3 within next week.
Best regards,
Damon
>
>>> In other review comments:
>>> - the bridge could use devm_drm_of_get_bridge() as suggested in
>>> the documentation of drm_of_find_panel_or_bridge(), as that would
>>> remove the separate panel_bridge creation
>>> - instead of using plat_data->next_bridge _inside_ the driver
>>> struct drm_bridge has a field next_bridge already.
>>
>> Great suggestions, I will incorporate them for the next version
>> alongside Sashiko's comments.
>>
>> Best regards,
>> Damon
>>
>>>
>>>
>>> Heiko
>>>
>>>
>>> ------- 8< -------
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> index 877e1b3ca7525..1388640a27de7 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> @@ -43,7 +43,7 @@ static const bool verify_fast_training;
>>> static bool analogix_dp_require_pm_for_hpd_irq(struct analogix_dp_device *dp)
>>> {
>>> return analogix_dp_is_rockchip(dp->plat_data->dev_type) && !dp->hpd_gpiod &&
>>> - !dp->force_hpd;
>>> + !dp->hpd_bridge && !dp->force_hpd;
>>> }
>>>
>>> static void analogix_dp_init_dp(struct analogix_dp_device *dp)
>>> @@ -72,7 +72,7 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
>>> * Trust connection status from downstream bridge (e.g.,
>>> * display-connector with hpd-gpios).
>>> */
>>> - if (dp->plat_data->next_bridge && dp->connection_notified)
>>> + if (dp->hpd_bridge && dp->connection_notified)
>>> return 0;
>>>
>>> while (timeout_loop < DP_TIMEOUT_LOOP_COUNT) {
>>> @@ -926,10 +926,10 @@ analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *conne
>>> */
>>> if (dp->plat_data->next_bridge && dp->last_bridge_is_panel)
>>> status = connector_status_connected;
>>> -
>>> - if (!analogix_dp_detect_hpd(dp))
>>> + else if (!analogix_dp_detect_hpd(dp))
>>> status = connector_status_connected;
>>>
>>> +printk("---> %s status %d\n", __func__, status);
>>> return status;
>>> }
>>>
>>> @@ -1044,7 +1044,7 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
>>> goto out_dp_init;
>>> }
>>>
>>> - if (!analogix_dp_require_pm_for_hpd_irq(dp))
>>> + if (!analogix_dp_require_pm_for_hpd_irq(dp) && !dp->hpd_bridge)
>>> enable_irq(dp->irq);
>>> return 0;
>>>
>>> @@ -1187,7 +1187,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
>>> if (dp->dpms_mode != DRM_MODE_DPMS_ON)
>>> return;
>>>
>>> - if (!analogix_dp_require_pm_for_hpd_irq(dp))
>>> + if (!analogix_dp_require_pm_for_hpd_irq(dp) && !dp->hpd_bridge)
>>> disable_irq(dp->irq);
>>>
>>> analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
>>> @@ -1264,6 +1264,7 @@ static void analogix_dp_bridge_notify(struct drm_bridge *bridge, struct drm_conn
>>> struct analogix_dp_device *dp = to_dp(bridge);
>>>
>>> dp->connection_notified = (status == connector_status_connected);
>>> +printk("---> %s connection_notified %d\n", __func__, dp->connection_notified);
>>> }
>>>
>>> static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
>>> @@ -1641,6 +1642,13 @@ static int analogix_dp_aux_done_probing(struct drm_dp_aux *aux)
>>> if (ret && ret != -ENODEV)
>>> return ret;
>>>
>>> + /*
>>> + * There is a next link in the chain which is not a panel, we should
>>> + * expect hotplug-information coming from there.
>>> + */
>>> + if (plat_data->next_bridge && !drm_bridge_is_panel(plat_data->next_bridge))
>>> + dp->hpd_bridge = true;
>>> +
>>> return component_add(dp->dev, plat_data->ops);
>>> }
>>>
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> index d0fb25e543ea0..ecca3b87b4456 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> @@ -169,6 +169,7 @@ struct analogix_dp_device {
>>> bool fast_train_enable;
>>> bool psr_supported;
>>> bool last_bridge_is_panel;
>>> + bool hpd_bridge;
>>> bool connection_notified;
>>>
>>> u8 dpcd[DP_RECEIVER_CAP_SIZE];
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>>> index ec5950066f838..6f0d642739ffd 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>>> @@ -182,7 +182,7 @@ void analogix_dp_config_interrupt(struct analogix_dp_device *dp)
>>> writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_2);
>>> writel(0, dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_3);
>>>
>>> - if (dp->hpd_gpiod) {
>>> + if (dp->hpd_gpiod || dp->hpd_bridge) {
>>> analogix_dp_mute_hpd_interrupt(dp, HPD_IRQ);
>>> } else {
>>> /*
>>> @@ -438,7 +438,7 @@ void analogix_dp_init_hpd(struct analogix_dp_device *dp)
>>> {
>>> u32 reg;
>>>
>>> - if (dp->hpd_gpiod)
>>> + if (dp->hpd_gpiod || dp->hpd_bridge)
>>> return;
>>>
>>> analogix_dp_clear_hotplug_interrupts(dp, HPD_IRQ);
>>> @@ -539,6 +539,9 @@ int analogix_dp_get_plug_in_status(struct analogix_dp_device *dp)
>>> if (dp->hpd_gpiod) {
>>> if (gpiod_get_value(dp->hpd_gpiod))
>>> return 0;
>>> + } else if (dp->hpd_bridge) {
>>> + if (dp->connection_notified)
>>> + return 0;
>>> } else {
>>> reg = readl(dp->reg_base + ANALOGIX_DP_SYS_CTL_3);
>>> if (reg & HPD_STATUS)
>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
>
>
>
>
More information about the linux-arm-kernel
mailing list