* [PATCH 0/5] drm/bridge: Implement generic USB Type-C DP HPD bridge
@ 2026-05-21 3:28 Chaoyi Chen
2026-05-21 3:28 ` [PATCH 1/5] " Chaoyi Chen
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Chaoyi Chen @ 2026-05-21 3:28 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang,
Heiko Stübner, Andy Yan, Vinod Koul
Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel,
dri-devel, linux-arm-kernel, linux-rockchip, linux-phy,
Chaoyi Chen
From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
This series is split from the v15 "Add Type-C DP support for RK3399 EVB
IND board" series [1]. It focuses on the DRM bridge and Rockchip
platform CDN-DP controller changes.
[1] https://lore.kernel.org/all/20260304094152.92-1-kernel@airkyi.com/
====
1. Generic Type-C DP HPD bridge
Currently, several USB-C controller drivers register their own DP HPD
bridge via aux-hpd-bridge.c, each duplicating the same logic. For
devicetree based platforms, the USB-C controller may vary across boards,
and not every USB-C controller driver implements this feature. Patch 1
implements a generic DP HPD bridge that monitors Type-C bus events and
automatically creates an HPD bridge when a Type-C port device with DP
SVID is registered.
2. Multiple bridge model for CDN-DP
The RK3399 has two USB/DP combo PHY and one CDN-DP controller. Patch 5
introduces a multi-bridge model where each PHY port gets a separate
encoder and bridge, allowing flexible selection of the output PHY port.
This is based on the DRM AUX HPD bridge rather than extcon.
====
Patch 1 adds generic USB Type-C DP HPD bridge (Dmitry, Heikki).
Patch 2 adds new API drm_aux_bridge_register_from_node() (Neil).
Patch 3 adds DRM AUX bridge support for RK3399 USBDP PHY (Neil).
Patch 4 drops CDN-DP's extcon dependency when Type-C is present (Dmitry).
Patch 5 adds multiple bridges to support PHY port selection (Dmitry, Luca).
Chaoyi Chen (5):
drm/bridge: Implement generic USB Type-C DP HPD bridge
drm/bridge: aux: Add drm_aux_bridge_register_from_node()
phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge
drm/rockchip: cdn-dp: Support handle lane info without extcon
drm/rockchip: cdn-dp: Add multiple bridges to support PHY port
selection
drivers/gpu/drm/bridge/Kconfig | 10 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/aux-bridge.c | 24 +-
.../gpu/drm/bridge/aux-hpd-typec-dp-bridge.c | 49 +++
drivers/gpu/drm/rockchip/Kconfig | 1 +
drivers/gpu/drm/rockchip/cdn-dp-core.c | 349 ++++++++++++++----
drivers/gpu/drm/rockchip/cdn-dp-core.h | 18 +-
drivers/phy/rockchip/Kconfig | 2 +
drivers/phy/rockchip/phy-rockchip-typec.c | 13 +-
include/drm/bridge/aux-bridge.h | 6 +
10 files changed, 404 insertions(+), 69 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
--
2.53.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/5] drm/bridge: Implement generic USB Type-C DP HPD bridge 2026-05-21 3:28 [PATCH 0/5] drm/bridge: Implement generic USB Type-C DP HPD bridge Chaoyi Chen @ 2026-05-21 3:28 ` Chaoyi Chen 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 2026-05-21 3:28 ` [PATCH 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node() Chaoyi Chen ` (4 subsequent siblings) 5 siblings, 1 reply; 14+ messages in thread From: Chaoyi Chen @ 2026-05-21 3:28 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip, linux-phy, Chaoyi Chen From: Chaoyi Chen <chaoyi.chen@rock-chips.com> The HPD function of Type-C DP is implemented through drm_connector_oob_hotplug_event(). For embedded DP, it is required that the DRM connector fwnode corresponds to the Type-C port fwnode. To describe the relationship between the DP controller and the Type-C port device, we usually using drm_bridge to build a bridge chain. Now several USB-C controller drivers have already implemented the DP HPD bridge function provided by aux-hpd-bridge.c, it will build a DP HPD bridge on USB-C connector port device. But this requires the USB-C controller driver to manually register the HPD bridge. If the driver does not implement this feature, the bridge will not be create. So this patch implements a generic DP HPD bridge based on aux-hpd-bridge.c. It will monitor Type-C bus events, and when a Type-C port device containing the DP svid is registered, it will create an HPD bridge for it without the need for the USB-C controller driver to implement it. Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> --- drivers/gpu/drm/bridge/Kconfig | 10 ++++ drivers/gpu/drm/bridge/Makefile | 1 + .../gpu/drm/bridge/aux-hpd-typec-dp-bridge.c | 49 +++++++++++++++++++ 3 files changed, 60 insertions(+) create mode 100644 drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index c3209b0f4678..d92e93875793 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -30,6 +30,16 @@ config DRM_AUX_HPD_BRIDGE Simple bridge that terminates the bridge chain and provides HPD support. +if DRM_AUX_HPD_BRIDGE +config DRM_AUX_HPD_TYPEC_BRIDGE + tristate + depends on TYPEC || !TYPEC + default TYPEC + help + Simple bridge that terminates the bridge chain and provides HPD + support. It build bridge on each USB-C connector device node. +endif + menu "Display Interface Bridges" depends on DRM && DRM_BRIDGE diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index beab5b695a6e..c4761526ba0a 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_DRM_AUX_BRIDGE) += aux-bridge.o obj-$(CONFIG_DRM_AUX_HPD_BRIDGE) += aux-hpd-bridge.o +obj-$(CONFIG_DRM_AUX_HPD_TYPEC_BRIDGE) += aux-hpd-typec-dp-bridge.o obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o diff --git a/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c new file mode 100644 index 000000000000..d915e0fb0668 --- /dev/null +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-2.0+ +#include <linux/of.h> +#include <linux/usb/typec_altmode.h> +#include <linux/usb/typec_dp.h> + +#include <drm/bridge/aux-bridge.h> + +static int drm_typec_bus_event(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct device *dev = (struct device *)data; + struct typec_altmode *alt = to_typec_altmode(dev); + + if (action != BUS_NOTIFY_ADD_DEVICE) + goto done; + + /* + * alt->dev.parent->parent : USB-C controller device + * alt->dev.parent : USB-C connector device + */ + if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID) + drm_dp_hpd_bridge_register(alt->dev.parent->parent, + to_of_node(alt->dev.parent->fwnode)); + +done: + return NOTIFY_OK; +} + +static struct notifier_block drm_typec_event_nb = { + .notifier_call = drm_typec_bus_event, +}; + +static void drm_aux_hpd_typec_dp_bridge_module_exit(void) +{ + bus_unregister_notifier(&typec_bus, &drm_typec_event_nb); +} + +static int __init drm_aux_hpd_typec_dp_bridge_module_init(void) +{ + bus_register_notifier(&typec_bus, &drm_typec_event_nb); + + return 0; +} + +module_init(drm_aux_hpd_typec_dp_bridge_module_init); +module_exit(drm_aux_hpd_typec_dp_bridge_module_exit); + +MODULE_DESCRIPTION("DRM TYPEC DP HPD BRIDGE"); +MODULE_LICENSE("GPL"); -- 2.53.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: drm/bridge: Implement generic USB Type-C DP HPD bridge 2026-05-21 3:28 ` [PATCH 1/5] " Chaoyi Chen @ 2026-05-25 11:00 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-05-25 11:00 UTC (permalink / raw) To: dri-devel-reviews Patch Review This patch adds a new module that monitors the Type-C bus for altmode devices with DP SVID and automatically creates an HPD bridge for them. **Issue 1: No error handling for bridge registration failure** ```c if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID) drm_dp_hpd_bridge_register(alt->dev.parent->parent, to_of_node(alt->dev.parent->fwnode)); ``` `drm_dp_hpd_bridge_register()` returns a `struct device *` which can be an ERR_PTR. The return value is silently ignored. While there may not be much that can be done from a bus notifier, at least a `dev_warn()` would help debugging. **Issue 2: Fragile device hierarchy assumption** The comment documents the parent traversal (`alt->dev.parent->parent` = USB-C controller device), but this is a fragile assumption about the device hierarchy. If the hierarchy ever changes, this will silently break. Consider whether there's a more robust API to get the controller device. **Issue 3: Module exit doesn't clean up bridges** `drm_aux_hpd_typec_dp_bridge_module_exit()` only unregisters the notifier but doesn't clean up any bridges that were registered via `drm_dp_hpd_bridge_register()`. Those are `devm`-managed on the controller device, so they'll survive module unload. This may be intentional, but it means loading/unloading this module repeatedly could have unexpected interactions. **Minor: Missing MODULE_AUTHOR** --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node() 2026-05-21 3:28 [PATCH 0/5] drm/bridge: Implement generic USB Type-C DP HPD bridge Chaoyi Chen 2026-05-21 3:28 ` [PATCH 1/5] " Chaoyi Chen @ 2026-05-21 3:28 ` Chaoyi Chen 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 2026-05-21 3:28 ` [PATCH 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge Chaoyi Chen ` (3 subsequent siblings) 5 siblings, 1 reply; 14+ messages in thread From: Chaoyi Chen @ 2026-05-21 3:28 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip, linux-phy, Chaoyi Chen From: Chaoyi Chen <chaoyi.chen@rock-chips.com> The drm_aux_bridge_register() uses the device->of_node as the bridge->of_node. This patch adds drm_aux_bridge_register_from_node() to allow specifying the of_node corresponding to the bridge. Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> --- drivers/gpu/drm/bridge/aux-bridge.c | 24 ++++++++++++++++++++++-- include/drm/bridge/aux-bridge.h | 6 ++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/aux-bridge.c b/drivers/gpu/drm/bridge/aux-bridge.c index 1ed21a8713bf..f50283abed5f 100644 --- a/drivers/gpu/drm/bridge/aux-bridge.c +++ b/drivers/gpu/drm/bridge/aux-bridge.c @@ -35,6 +35,7 @@ static void drm_aux_bridge_unregister_adev(void *_adev) /** * drm_aux_bridge_register - Create a simple bridge device to link the chain * @parent: device instance providing this bridge + * @np: device node pointer corresponding to this bridge instance * * Creates a simple DRM bridge that doesn't implement any drm_bridge * operations. Such bridges merely fill a place in the bridge chain linking @@ -42,7 +43,7 @@ static void drm_aux_bridge_unregister_adev(void *_adev) * * Return: zero on success, negative error code on failure */ -int drm_aux_bridge_register(struct device *parent) +int drm_aux_bridge_register_from_node(struct device *parent, struct device_node *np) { struct auxiliary_device *adev; int ret; @@ -62,7 +63,10 @@ int drm_aux_bridge_register(struct device *parent) adev->dev.parent = parent; adev->dev.release = drm_aux_bridge_release; - device_set_of_node_from_dev(&adev->dev, parent); + if (np) + device_set_node(&adev->dev, of_fwnode_handle(np)); + else + device_set_of_node_from_dev(&adev->dev, parent); ret = auxiliary_device_init(adev); if (ret) { @@ -80,6 +84,22 @@ int drm_aux_bridge_register(struct device *parent) return devm_add_action_or_reset(parent, drm_aux_bridge_unregister_adev, adev); } +EXPORT_SYMBOL_GPL(drm_aux_bridge_register_from_node); + +/** + * drm_aux_bridge_register - Create a simple bridge device to link the chain + * @parent: device instance providing this bridge + * + * Creates a simple DRM bridge that doesn't implement any drm_bridge + * operations. Such bridges merely fill a place in the bridge chain linking + * surrounding DRM bridges. + * + * Return: zero on success, negative error code on failure + */ +int drm_aux_bridge_register(struct device *parent) +{ + return drm_aux_bridge_register_from_node(parent, NULL); +} EXPORT_SYMBOL_GPL(drm_aux_bridge_register); struct drm_aux_bridge_data { diff --git a/include/drm/bridge/aux-bridge.h b/include/drm/bridge/aux-bridge.h index c2f5a855512f..7dd1f17a1354 100644 --- a/include/drm/bridge/aux-bridge.h +++ b/include/drm/bridge/aux-bridge.h @@ -13,11 +13,17 @@ struct auxiliary_device; #if IS_ENABLED(CONFIG_DRM_AUX_BRIDGE) int drm_aux_bridge_register(struct device *parent); +int drm_aux_bridge_register_from_node(struct device *parent, struct device_node *np); #else static inline int drm_aux_bridge_register(struct device *parent) { return 0; } + +static inline int drm_aux_bridge_register_from_node(struct device *parent, struct device_node *np) +{ + return 0; +} #endif #if IS_ENABLED(CONFIG_DRM_AUX_HPD_BRIDGE) -- 2.53.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: drm/bridge: aux: Add drm_aux_bridge_register_from_node() 2026-05-21 3:28 ` [PATCH 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node() Chaoyi Chen @ 2026-05-25 11:00 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-05-25 11:00 UTC (permalink / raw) To: dri-devel-reviews Patch Review Clean patch. The refactoring to make `drm_aux_bridge_register()` a wrapper around the new `_from_node()` variant is the right approach. **Minor nit:** The kdoc on the original `drm_aux_bridge_register()` is updated to add `@np` but that parameter doesn't exist on the old function — the `@np` doc belongs only on the new `drm_aux_bridge_register_from_node()`. Looking more carefully: ```c /** * drm_aux_bridge_register - Create a simple bridge device to link the chain * @parent: device instance providing this bridge * @np: device node pointer corresponding to this bridge instance ``` This kdoc block is placed on `drm_aux_bridge_register_from_node()` after the rename, so the function name in the doc comment (`drm_aux_bridge_register`) is now wrong — it should say `drm_aux_bridge_register_from_node`. The `@np` parameter description should also mention that NULL falls back to the parent device's of_node. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge 2026-05-21 3:28 [PATCH 0/5] drm/bridge: Implement generic USB Type-C DP HPD bridge Chaoyi Chen 2026-05-21 3:28 ` [PATCH 1/5] " Chaoyi Chen 2026-05-21 3:28 ` [PATCH 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node() Chaoyi Chen @ 2026-05-21 3:28 ` Chaoyi Chen 2026-05-21 9:06 ` Heiko Stuebner 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 2026-05-21 3:28 ` [PATCH 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon Chaoyi Chen ` (2 subsequent siblings) 5 siblings, 2 replies; 14+ messages in thread From: Chaoyi Chen @ 2026-05-21 3:28 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip, linux-phy, Chaoyi Chen From: Chaoyi Chen <chaoyi.chen@rock-chips.com> Using the DRM_AUX_BRIDGE helper to create the transparent DRM bridge device. Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> --- drivers/phy/rockchip/Kconfig | 2 ++ drivers/phy/rockchip/phy-rockchip-typec.c | 13 +++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig index 14698571b607..9173d3b4fef4 100644 --- a/drivers/phy/rockchip/Kconfig +++ b/drivers/phy/rockchip/Kconfig @@ -119,6 +119,8 @@ config PHY_ROCKCHIP_SNPS_PCIE3 config PHY_ROCKCHIP_TYPEC tristate "Rockchip TYPEC PHY Driver" depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST) + depends on DRM || DRM=n + select DRM_AUX_BRIDGE if DRM_BRIDGE select EXTCON select GENERIC_PHY select RESET_CONTROLLER diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c index d9701b6106d5..48070b50416e 100644 --- a/drivers/phy/rockchip/phy-rockchip-typec.c +++ b/drivers/phy/rockchip/phy-rockchip-typec.c @@ -54,6 +54,7 @@ #include <linux/mfd/syscon.h> #include <linux/phy/phy.h> +#include <drm/bridge/aux-bridge.h> #define CMN_SSM_BANDGAP (0x21 << 2) #define CMN_SSM_BIAS (0x22 << 2) @@ -1162,16 +1163,24 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev) for_each_available_child_of_node(np, child_np) { struct phy *phy; + ret = 0; - if (of_node_name_eq(child_np, "dp-port")) + if (of_node_name_eq(child_np, "dp-port")) { phy = devm_phy_create(dev, child_np, &rockchip_dp_phy_ops); - else if (of_node_name_eq(child_np, "usb3-port")) + ret = drm_aux_bridge_register_from_node(dev, child_np); + } else if (of_node_name_eq(child_np, "usb3-port")) phy = devm_phy_create(dev, child_np, &rockchip_usb3_phy_ops); else continue; + if (ret) { + pm_runtime_disable(dev); + of_node_put(child_np); + return ret; + } + if (IS_ERR(phy)) { dev_err(dev, "failed to create phy: %pOFn\n", child_np); -- 2.53.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge 2026-05-21 3:28 ` [PATCH 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge Chaoyi Chen @ 2026-05-21 9:06 ` Heiko Stuebner 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 1 sibling, 0 replies; 14+ messages in thread From: Heiko Stuebner @ 2026-05-21 9:06 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang, Andy Yan, Vinod Koul, Chaoyi Chen Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip, linux-phy, Chaoyi Chen Hi, Am Donnerstag, 21. Mai 2026, 05:28:52 Mitteleuropäische Sommerzeit schrieb Chaoyi Chen: > From: Chaoyi Chen <chaoyi.chen@rock-chips.com> > > Using the DRM_AUX_BRIDGE helper to create the transparent DRM bridge > device. > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com> > Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> Reviewed-by: Heiko Stuebner <heiko@sntech.de> @Vinod: could you give this patch an Ack to go through the DRM tree please? It is independent of any other phy changes, but needs the drm-patches 1+2 from this series, so ideally would go through the drm tree together with them. Thanks a lot Heiko > --- > drivers/phy/rockchip/Kconfig | 2 ++ > drivers/phy/rockchip/phy-rockchip-typec.c | 13 +++++++++++-- > 2 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig > index 14698571b607..9173d3b4fef4 100644 > --- a/drivers/phy/rockchip/Kconfig > +++ b/drivers/phy/rockchip/Kconfig > @@ -119,6 +119,8 @@ config PHY_ROCKCHIP_SNPS_PCIE3 > config PHY_ROCKCHIP_TYPEC > tristate "Rockchip TYPEC PHY Driver" > depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST) > + depends on DRM || DRM=n > + select DRM_AUX_BRIDGE if DRM_BRIDGE > select EXTCON > select GENERIC_PHY > select RESET_CONTROLLER > diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c > index d9701b6106d5..48070b50416e 100644 > --- a/drivers/phy/rockchip/phy-rockchip-typec.c > +++ b/drivers/phy/rockchip/phy-rockchip-typec.c > @@ -54,6 +54,7 @@ > > #include <linux/mfd/syscon.h> > #include <linux/phy/phy.h> > +#include <drm/bridge/aux-bridge.h> > > #define CMN_SSM_BANDGAP (0x21 << 2) > #define CMN_SSM_BIAS (0x22 << 2) > @@ -1162,16 +1163,24 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev) > > for_each_available_child_of_node(np, child_np) { > struct phy *phy; > + ret = 0; > > - if (of_node_name_eq(child_np, "dp-port")) > + if (of_node_name_eq(child_np, "dp-port")) { > phy = devm_phy_create(dev, child_np, > &rockchip_dp_phy_ops); > - else if (of_node_name_eq(child_np, "usb3-port")) > + ret = drm_aux_bridge_register_from_node(dev, child_np); > + } else if (of_node_name_eq(child_np, "usb3-port")) > phy = devm_phy_create(dev, child_np, > &rockchip_usb3_phy_ops); > else > continue; > > + if (ret) { > + pm_runtime_disable(dev); > + of_node_put(child_np); > + return ret; > + } > + > if (IS_ERR(phy)) { > dev_err(dev, "failed to create phy: %pOFn\n", > child_np); > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Claude review: phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge 2026-05-21 3:28 ` [PATCH 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge Chaoyi Chen 2026-05-21 9:06 ` Heiko Stuebner @ 2026-05-25 11:00 ` Claude Code Review Bot 1 sibling, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-05-25 11:00 UTC (permalink / raw) To: dri-devel-reviews Patch Review Straightforward integration of the aux bridge into the Rockchip Type-C PHY driver. **Issue: Error check ordering** ```c if (of_node_name_eq(child_np, "dp-port")) { phy = devm_phy_create(dev, child_np, &rockchip_dp_phy_ops); ret = drm_aux_bridge_register_from_node(dev, child_np); } else if ... if (ret) { pm_runtime_disable(dev); of_node_put(child_np); return ret; } if (IS_ERR(phy)) { ``` If `devm_phy_create()` succeeds but `drm_aux_bridge_register_from_node()` returns `-EPROBE_DEFER`, we return the error but the PHY remains registered (via devm). This is fine because devm will clean it up, but it means the order of checking `ret` before `IS_ERR(phy)` could mask a PHY creation failure. If `devm_phy_create` fails AND `drm_aux_bridge_register_from_node` fails, the bridge error is returned while the PHY error is lost. Consider checking `IS_ERR(phy)` first. **Kconfig dependency:** ``` depends on DRM || DRM=n select DRM_AUX_BRIDGE if DRM_BRIDGE ``` This is a reasonable conditional select pattern for optional DRM dependencies. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon 2026-05-21 3:28 [PATCH 0/5] drm/bridge: Implement generic USB Type-C DP HPD bridge Chaoyi Chen ` (2 preceding siblings ...) 2026-05-21 3:28 ` [PATCH 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge Chaoyi Chen @ 2026-05-21 3:28 ` Chaoyi Chen 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 2026-05-21 3:28 ` [PATCH 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection Chaoyi Chen 2026-05-25 11:00 ` Claude review: drm/bridge: Implement generic USB Type-C DP HPD bridge Claude Code Review Bot 5 siblings, 1 reply; 14+ messages in thread From: Chaoyi Chen @ 2026-05-21 3:28 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip, linux-phy, Chaoyi Chen From: Chaoyi Chen <chaoyi.chen@rock-chips.com> This patch add support for get PHY lane info without help of extcon. There is no extcon needed if the Type-C controller is present. In this case, the lane info can be get from PHY instead of extcon. The extcon device should still be supported if Type-C controller is not present. Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com> Reviewed-by: Heiko Stuebner <heiko@sntech.de> --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 177e30445ee8..9068118859e2 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -157,6 +157,9 @@ static int cdn_dp_get_port_lanes(struct cdn_dp_port *port) int dptx; u8 lanes; + if (!edev) + return phy_get_bus_width(port->phy); + dptx = extcon_get_state(edev, EXTCON_DISP_DP); if (dptx > 0) { extcon_get_property(edev, EXTCON_DISP_DP, @@ -220,7 +223,7 @@ static bool cdn_dp_check_sink_connection(struct cdn_dp_device *dp) * some docks need more time to power up. */ while (time_before(jiffies, timeout)) { - if (!extcon_get_state(port->extcon, EXTCON_DISP_DP)) + if (port->extcon && !extcon_get_state(port->extcon, EXTCON_DISP_DP)) return false; if (!cdn_dp_get_sink_count(dp, &sink_count)) @@ -386,11 +389,14 @@ static int cdn_dp_enable_phy(struct cdn_dp_device *dp, struct cdn_dp_port *port) goto err_power_on; } - ret = extcon_get_property(port->extcon, EXTCON_DISP_DP, - EXTCON_PROP_USB_TYPEC_POLARITY, &property); - if (ret) { - DRM_DEV_ERROR(dp->dev, "get property failed\n"); - goto err_power_on; + property.intval = 0; + if (port->extcon) { + ret = extcon_get_property(port->extcon, EXTCON_DISP_DP, + EXTCON_PROP_USB_TYPEC_POLARITY, &property); + if (ret) { + DRM_DEV_ERROR(dp->dev, "get property failed\n"); + goto err_power_on; + } } port->lanes = cdn_dp_get_port_lanes(port); @@ -1029,6 +1035,9 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data) for (i = 0; i < dp->ports; i++) { port = dp->port[i]; + if (!port->extcon) + continue; + port->event_nb.notifier_call = cdn_dp_pd_event; ret = devm_extcon_register_notifier(dp->dev, port->extcon, EXTCON_DISP_DP, @@ -1121,14 +1130,14 @@ static int cdn_dp_probe(struct platform_device *pdev) PTR_ERR(phy) == -EPROBE_DEFER) return -EPROBE_DEFER; - if (IS_ERR(extcon) || IS_ERR(phy)) + if (IS_ERR(phy) || PTR_ERR(extcon) != -ENODEV) continue; port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL); if (!port) return -ENOMEM; - port->extcon = extcon; + port->extcon = IS_ERR(extcon) ? NULL : extcon; port->phy = phy; port->dp = dp; port->id = i; -- 2.53.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: drm/rockchip: cdn-dp: Support handle lane info without extcon 2026-05-21 3:28 ` [PATCH 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon Chaoyi Chen @ 2026-05-25 11:00 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-05-25 11:00 UTC (permalink / raw) To: dri-devel-reviews Patch Review Makes the extcon device optional in CDN-DP, falling back to `phy_get_bus_width()` for lane count. **Issue: Potentially incorrect filter condition in probe** ```c - if (IS_ERR(extcon) || IS_ERR(phy)) + if (IS_ERR(phy) || PTR_ERR(extcon) != -ENODEV) continue; ``` The original skips the port if either extcon OR phy is an error. The new code skips if phy is an error OR if `PTR_ERR(extcon) != -ENODEV`. But if `extcon` is **valid** (not an error), then `PTR_ERR(extcon)` is undefined behavior — it would cast a valid pointer to a long and compare it against `-ENODEV`. This condition should be: ```c if (IS_ERR(phy) || (IS_ERR(extcon) && PTR_ERR(extcon) != -ENODEV)) continue; ``` The intent is: skip if phy fails, or if extcon failed with something other than `-ENODEV` (meaning extcon is genuinely absent vs. a real error like `-EPROBE_DEFER`). The current code is **buggy for the case where extcon is valid**. Wait — re-reading: extcon is fetched above with `extcon_find_edev_by_node`. If it returns a valid pointer, `PTR_ERR()` of a valid pointer is not `-ENODEV`, so the `continue` would fire, skipping valid ports. This is a real bug. **Issue: `property.intval = 0` default for polarity** ```c property.intval = 0; if (port->extcon) { ret = extcon_get_property(port->extcon, EXTCON_DISP_DP, EXTCON_PROP_USB_TYPEC_POLARITY, &property); ``` When extcon is absent, polarity defaults to 0, which assumes normal polarity. This seems reasonable but should be documented or verified against the hardware behavior when the Type-C controller is managing polarity via the PHY directly. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection 2026-05-21 3:28 [PATCH 0/5] drm/bridge: Implement generic USB Type-C DP HPD bridge Chaoyi Chen ` (3 preceding siblings ...) 2026-05-21 3:28 ` [PATCH 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon Chaoyi Chen @ 2026-05-21 3:28 ` Chaoyi Chen 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 2026-05-25 11:00 ` Claude review: drm/bridge: Implement generic USB Type-C DP HPD bridge Claude Code Review Bot 5 siblings, 1 reply; 14+ messages in thread From: Chaoyi Chen @ 2026-05-21 3:28 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul Cc: Heikki Krogerus, Dmitry Baryshkov, Luca Ceresoli, linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip, linux-phy, Chaoyi Chen From: Chaoyi Chen <chaoyi.chen@rock-chips.com> The RK3399 has two USB/DP combo PHY and one CDN-DP controller. And the CDN-DP can be switched to output to one of the PHYs. If both ports are plugged into DP, DP will select the first port for output. This patch adds support for multiple bridges, enabling users to flexibly select the output port. For each PHY port, a separate encoder and bridge are registered. The change is based on the DRM AUX HPD bridge, rather than the extcon approach. This requires the DT to correctly describe the connections between the first bridge in bridge chain and DP controller. For example, the bridge chain may be like this: PHY aux birdge -> fsa4480 analog audio switch bridge -> onnn,nb7vpq904m USB reminder bridge -> USB-C controller AUX HPD bridge In this case, the connection relationships among the PHY aux bridge and the DP contorller need to be described in DT. In addition, the cdn_dp_parse_next_bridge_dt() will parses it and determines whether to register one or two bridges. Since there is only one DP controller, only one of the PHY ports can output at a time. The key is how to switch between different PHYs, which is handled by cdn_dp_switch_port() and cdn_dp_enable(). There are two cases: 1. Neither bridge is enabled. In this case, both bridges can independently read the EDID, and the PHY port may switch before reading the EDID. 2. One bridge is already enabled. In this case, other bridges are not allowed to read the EDID. So we will try to return the cached EDID. Since the scenario of two ports plug in at the same time is rare, I don't have a board which support two TypeC connector to test this. Therefore, I tested forced switching on a single PHY port, as well as output using a fake PHY port alongside a real PHY port. Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> Reviewed-by: Heiko Stuebner <heiko@sntech.de> --- drivers/gpu/drm/rockchip/Kconfig | 1 + drivers/gpu/drm/rockchip/cdn-dp-core.c | 324 ++++++++++++++++++++----- drivers/gpu/drm/rockchip/cdn-dp-core.h | 18 +- 3 files changed, 286 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig index 1479b8c4ed40..cb97690c5a5d 100644 --- a/drivers/gpu/drm/rockchip/Kconfig +++ b/drivers/gpu/drm/rockchip/Kconfig @@ -59,6 +59,7 @@ config ROCKCHIP_CDN_DP select DRM_DISPLAY_HELPER select DRM_BRIDGE_CONNECTOR select DRM_DISPLAY_DP_HELPER + select DRM_AUX_HPD_BRIDGE help This selects support for Rockchip SoC specific extensions for the cdn DP driver. If you want to enable Dp on diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 9068118859e2..b9ba279ca653 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -28,16 +28,17 @@ #include "cdn-dp-core.h" #include "cdn-dp-reg.h" -static inline struct cdn_dp_device *bridge_to_dp(struct drm_bridge *bridge) +static int cdn_dp_switch_port(struct cdn_dp_device *dp, struct cdn_dp_port *prev_port, + struct cdn_dp_port *port); + +static inline struct cdn_dp_bridge *bridge_to_dp_bridge(struct drm_bridge *bridge) { - return container_of(bridge, struct cdn_dp_device, bridge); + return container_of(bridge, struct cdn_dp_bridge, bridge); } -static inline struct cdn_dp_device *encoder_to_dp(struct drm_encoder *encoder) +static inline struct cdn_dp_device *bridge_to_dp(struct drm_bridge *bridge) { - struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder); - - return container_of(rkencoder, struct cdn_dp_device, encoder); + return bridge_to_dp_bridge(bridge)->parent; } #define GRF_SOC_CON9 0x6224 @@ -192,14 +193,27 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count) static struct cdn_dp_port *cdn_dp_connected_port(struct cdn_dp_device *dp) { struct cdn_dp_port *port; - int i, lanes; + int i, lanes[MAX_PHY]; for (i = 0; i < dp->ports; i++) { port = dp->port[i]; - lanes = cdn_dp_get_port_lanes(port); - if (lanes) + lanes[i] = cdn_dp_get_port_lanes(port); + if (!dp->next_bridge_valid) return port; } + + if (dp->next_bridge_valid) { + /* If more than one port is available, pick the last active port */ + if (dp->active_port > 0 && lanes[dp->active_port]) + return dp->port[dp->active_port]; + + /* If the last active port is not available, pick an available port in order */ + for (i = 0; i < dp->bridge_count; i++) { + if (lanes[i]) + return dp->port[i]; + } + } + return NULL; } @@ -254,12 +268,45 @@ static const struct drm_edid * cdn_dp_bridge_edid_read(struct drm_bridge *bridge, struct drm_connector *connector) { struct cdn_dp_device *dp = bridge_to_dp(bridge); - const struct drm_edid *drm_edid; + struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge); + struct cdn_dp_port *port = dp->port[dp_bridge->id]; + struct cdn_dp_port *prev_port; + const struct drm_edid *drm_edid = NULL; + int i, ret; mutex_lock(&dp->lock); + + /* More than one port is available */ + if (dp->bridge_count > 1 && !port->phy_enabled) { + for (i = 0; i < dp->bridge_count; i++) { + /* Another port already enable */ + if (dp->bridge_list[i] != dp_bridge && dp->bridge_list[i]->enabled) + goto get_cache; + /* Find already enabled port */ + if (dp->port[i]->phy_enabled) + prev_port = dp->port[i]; + } + + /* Switch to current port */ + if (prev_port) { + ret = cdn_dp_switch_port(dp, prev_port, port); + if (ret) + goto get_cache; + } + } + drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp); + /* replace edid cache */ + if (dp->edid_cache[dp_bridge->id]) + drm_edid_free(dp->edid_cache[dp_bridge->id]); + dp->edid_cache[dp_bridge->id] = drm_edid_dup(drm_edid); + mutex_unlock(&dp->lock); + return drm_edid; +get_cache: + drm_edid = drm_edid_dup(dp->edid_cache[dp_bridge->id]); + mutex_unlock(&dp->lock); return drm_edid; } @@ -268,12 +315,13 @@ cdn_dp_bridge_mode_valid(struct drm_bridge *bridge, const struct drm_display_info *display_info, const struct drm_display_mode *mode) { + struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge); struct cdn_dp_device *dp = bridge_to_dp(bridge); u32 requested, actual, rate, sink_max, source_max = 0; u8 lanes, bpc; /* If DP is disconnected, every mode is invalid */ - if (!dp->connected) + if (!dp_bridge->connected || !dp->connected) return MODE_BAD; switch (display_info->bpc) { @@ -551,6 +599,54 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp) return drm_dp_channel_eq_ok(link_status, min(port->lanes, sink_lanes)); } +static int cdn_dp_switch_port(struct cdn_dp_device *dp, struct cdn_dp_port *prev_port, + struct cdn_dp_port *port) +{ + int ret; + + if (dp->active) + return 0; + + ret = cdn_dp_disable_phy(dp, prev_port); + if (ret) + goto out; + ret = cdn_dp_enable_phy(dp, port); + if (ret) + goto out; + + ret = cdn_dp_get_sink_capability(dp); + if (ret) { + cdn_dp_disable_phy(dp, port); + goto out; + } + + dp->active = true; + dp->lanes = port->lanes; + + if (!cdn_dp_check_link_status(dp)) { + dev_info(dp->dev, "Connected with sink; re-train link\n"); + + ret = cdn_dp_train_link(dp); + if (ret) { + dev_err(dp->dev, "Training link failed: %d\n", ret); + goto out; + } + + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE); + if (ret) { + dev_err(dp->dev, "Failed to idle video %d\n", ret); + goto out; + } + + ret = cdn_dp_config_video(dp); + if (ret) + dev_err(dp->dev, "Failed to configure video: %d\n", ret); + } + +out: + return ret; +} + static void cdn_dp_display_info_update(struct cdn_dp_device *dp, struct drm_display_info *display_info) { @@ -572,6 +668,7 @@ static void cdn_dp_display_info_update(struct cdn_dp_device *dp, static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_atomic_state *state) { struct cdn_dp_device *dp = bridge_to_dp(bridge); + struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge); struct drm_connector *connector; int ret, val; @@ -581,7 +678,7 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at cdn_dp_display_info_update(dp, &connector->display_info); - ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, &dp->encoder.encoder); + ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, &dp_bridge->encoder.encoder); if (ret < 0) { DRM_DEV_ERROR(dp->dev, "Could not get vop id, %d", ret); return; @@ -600,6 +697,9 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at mutex_lock(&dp->lock); + if (dp->next_bridge_valid) + dp->active_port = dp_bridge->id; + ret = cdn_dp_enable(dp); if (ret) { DRM_DEV_ERROR(dp->dev, "Failed to enable bridge %d\n", @@ -632,6 +732,7 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at goto out; } + dp_bridge->enabled = true; out: mutex_unlock(&dp->lock); } @@ -639,9 +740,11 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at static void cdn_dp_bridge_atomic_disable(struct drm_bridge *bridge, struct drm_atomic_state *state) { struct cdn_dp_device *dp = bridge_to_dp(bridge); + struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge); int ret; mutex_lock(&dp->lock); + dp_bridge->enabled = false; if (dp->active) { ret = cdn_dp_disable(dp); @@ -828,6 +931,16 @@ static int cdn_dp_audio_mute_stream(struct drm_bridge *bridge, return ret; } +static void cdn_dp_bridge_hpd_notify(struct drm_bridge *bridge, + enum drm_connector_status status) +{ + struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge); + struct cdn_dp_device *dp = bridge_to_dp(bridge); + + dp->bridge_list[dp_bridge->id]->connected = status == connector_status_connected; + schedule_work(&dp->event_work); +} + static const struct drm_bridge_funcs cdn_dp_bridge_funcs = { .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, @@ -838,6 +951,7 @@ static const struct drm_bridge_funcs cdn_dp_bridge_funcs = { .atomic_disable = cdn_dp_bridge_atomic_disable, .mode_valid = cdn_dp_bridge_mode_valid, .mode_set = cdn_dp_bridge_mode_set, + .hpd_notify = cdn_dp_bridge_hpd_notify, .dp_audio_prepare = cdn_dp_audio_prepare, .dp_audio_mute_stream = cdn_dp_audio_mute_stream, @@ -886,7 +1000,8 @@ static void cdn_dp_pd_event_work(struct work_struct *work) { struct cdn_dp_device *dp = container_of(work, struct cdn_dp_device, event_work); - int ret; + bool connected; + int i, ret; mutex_lock(&dp->lock); @@ -945,9 +1060,12 @@ static void cdn_dp_pd_event_work(struct work_struct *work) out: mutex_unlock(&dp->lock); - drm_bridge_hpd_notify(&dp->bridge, - dp->connected ? connector_status_connected - : connector_status_disconnected); + for (i = 0; i < dp->bridge_count; i++) { + connected = dp->connected && dp->bridge_list[i]->connected; + drm_bridge_hpd_notify(&dp->bridge_list[i]->bridge, + connected ? connector_status_connected + : connector_status_disconnected); + } } static int cdn_dp_pd_event(struct notifier_block *nb, @@ -967,28 +1085,16 @@ static int cdn_dp_pd_event(struct notifier_block *nb, return NOTIFY_DONE; } -static int cdn_dp_bind(struct device *dev, struct device *master, void *data) +static int cdn_bridge_add(struct device *dev, + struct drm_bridge *bridge, + struct drm_bridge *next_bridge, + struct drm_encoder *encoder) { struct cdn_dp_device *dp = dev_get_drvdata(dev); - struct drm_encoder *encoder; + struct drm_device *drm_dev = dp->drm_dev; + struct drm_bridge *last_bridge __free(drm_bridge_put) = NULL; struct drm_connector *connector; - struct cdn_dp_port *port; - struct drm_device *drm_dev = data; - int ret, i; - - ret = cdn_dp_parse_dt(dp); - if (ret < 0) - return ret; - - dp->drm_dev = drm_dev; - dp->connected = false; - dp->active = false; - dp->active_port = -1; - dp->fw_loaded = false; - - INIT_WORK(&dp->event_work, cdn_dp_pd_event_work); - - encoder = &dp->encoder.encoder; + int ret; encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev, dev->of_node); @@ -1003,26 +1109,35 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data) drm_encoder_helper_add(encoder, &cdn_dp_encoder_helper_funcs); - dp->bridge.ops = - DRM_BRIDGE_OP_DETECT | - DRM_BRIDGE_OP_EDID | - DRM_BRIDGE_OP_HPD | - DRM_BRIDGE_OP_DP_AUDIO; - dp->bridge.of_node = dp->dev->of_node; - dp->bridge.type = DRM_MODE_CONNECTOR_DisplayPort; - dp->bridge.hdmi_audio_dev = dp->dev; - dp->bridge.hdmi_audio_max_i2s_playback_channels = 8; - dp->bridge.hdmi_audio_spdif_playback = 1; - dp->bridge.hdmi_audio_dai_port = -1; - - ret = devm_drm_bridge_add(dev, &dp->bridge); + bridge->ops = + DRM_BRIDGE_OP_DETECT | + DRM_BRIDGE_OP_EDID | + DRM_BRIDGE_OP_HPD | + DRM_BRIDGE_OP_DP_AUDIO; + bridge->of_node = dp->dev->of_node; + bridge->type = DRM_MODE_CONNECTOR_DisplayPort; + bridge->hdmi_audio_dev = dp->dev; + bridge->hdmi_audio_max_i2s_playback_channels = 8; + bridge->hdmi_audio_spdif_playback = 1; + bridge->hdmi_audio_dai_port = -1; + + ret = devm_drm_bridge_add(dev, bridge); if (ret) return ret; - ret = drm_bridge_attach(encoder, &dp->bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR); + ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR); if (ret) return ret; + if (next_bridge) { + ret = drm_bridge_attach(encoder, next_bridge, bridge, + DRM_BRIDGE_ATTACH_NO_CONNECTOR); + if (ret) + return ret; + + last_bridge = drm_bridge_chain_get_last_bridge(bridge->encoder); + } + connector = drm_bridge_connector_init(drm_dev, encoder); if (IS_ERR(connector)) { ret = PTR_ERR(connector); @@ -1030,8 +1145,99 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data) return ret; } + if (last_bridge) + connector->fwnode = fwnode_handle_get(of_fwnode_handle(last_bridge->of_node)); + drm_connector_attach_encoder(connector, encoder); + return 0; +} + +static int cdn_dp_parse_next_bridge_dt(struct cdn_dp_device *dp) +{ + struct device_node *np = dp->dev->of_node; + struct device_node *port __free(device_node) = of_graph_get_port_by_id(np, 1); + struct drm_bridge *bridge; + int count = 0; + int ret = 0; + int i; + + /* If device use extcon, do not use hpd bridge */ + for (i = 0; i < dp->ports; i++) { + if (dp->port[i]->extcon) { + dp->bridge_count = 1; + return 0; + } + } + + /* One endpoint may correspond to one next bridge. */ + for_each_of_graph_port_endpoint(port, dp_ep) { + struct device_node *next_bridge_node __free(device_node) = + of_graph_get_remote_port_parent(dp_ep); + + bridge = of_drm_find_bridge(next_bridge_node); + if (!bridge) { + ret = -EPROBE_DEFER; + goto out; + } + + dp->next_bridge_valid = true; + dp->next_bridge_list[count] = drm_bridge_get(bridge); + count++; + } + +out: + dp->bridge_count = count ? count : 1; + return ret; +} + +static int cdn_dp_bind(struct device *dev, struct device *master, void *data) +{ + struct cdn_dp_device *dp = dev_get_drvdata(dev); + struct drm_bridge *bridge, *next_bridge; + struct drm_encoder *encoder; + struct cdn_dp_port *port; + struct drm_device *drm_dev = data; + struct cdn_dp_bridge *dp_bridge; + int ret, i; + + ret = cdn_dp_parse_dt(dp); + if (ret < 0) + return ret; + + ret = cdn_dp_parse_next_bridge_dt(dp); + if (ret) + return ret; + + dp->drm_dev = drm_dev; + dp->connected = false; + dp->active = false; + dp->active_port = -1; + dp->fw_loaded = false; + + for (i = 0; i < dp->bridge_count; i++) { + dp_bridge = devm_drm_bridge_alloc(dev, struct cdn_dp_bridge, bridge, + &cdn_dp_bridge_funcs); + if (IS_ERR(dp_bridge)) + return PTR_ERR(dp_bridge); + dp_bridge->id = i; + dp_bridge->parent = dp; + if (!dp->next_bridge_valid) + dp_bridge->connected = true; + dp->bridge_list[i] = dp_bridge; + } + + for (i = 0; i < dp->bridge_count; i++) { + encoder = &dp->bridge_list[i]->encoder.encoder; + bridge = &dp->bridge_list[i]->bridge; + next_bridge = dp->next_bridge_list[i]; + ret = cdn_bridge_add(dev, bridge, next_bridge, encoder); + if (ret) + return ret; + } + + INIT_WORK(&dp->event_work, cdn_dp_pd_event_work); + for (i = 0; i < dp->ports; i++) { port = dp->port[i]; @@ -1059,10 +1265,18 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data) static void cdn_dp_unbind(struct device *dev, struct device *master, void *data) { struct cdn_dp_device *dp = dev_get_drvdata(dev); - struct drm_encoder *encoder = &dp->encoder.encoder; + struct drm_encoder *encoder; + int i; cancel_work_sync(&dp->event_work); - encoder->funcs->destroy(encoder); + for (i = 0; i < dp->bridge_count; i++) { + encoder = &dp->bridge_list[i]->encoder.encoder; + encoder->funcs->destroy(encoder); + drm_bridge_put(dp->next_bridge_list[i]); + } + + for (i = 0; i < MAX_PHY; i++) + drm_edid_free(dp->edid_cache[i]); pm_runtime_disable(dev); if (dp->fw_loaded) @@ -1113,10 +1327,10 @@ static int cdn_dp_probe(struct platform_device *pdev) int ret; int i; - dp = devm_drm_bridge_alloc(dev, struct cdn_dp_device, bridge, - &cdn_dp_bridge_funcs); - if (IS_ERR(dp)) - return PTR_ERR(dp); + dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL); + if (!dp) + return -ENOMEM; + dp->dev = dev; match = of_match_node(cdn_dp_dt_ids, pdev->dev.of_node); diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h index e9c30b9fd543..c10e423bbf06 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h @@ -38,6 +38,8 @@ enum vic_pxl_encoding_format { Y_ONLY = 0x10, }; +struct cdn_dp_device; + struct video_info { bool h_sync_polarity; bool v_sync_polarity; @@ -63,16 +65,28 @@ struct cdn_dp_port { u8 id; }; +struct cdn_dp_bridge { + struct cdn_dp_device *parent; + struct drm_bridge bridge; + struct rockchip_encoder encoder; + bool connected; + bool enabled; + int id; +}; + struct cdn_dp_device { struct device *dev; struct drm_device *drm_dev; - struct drm_bridge bridge; - struct rockchip_encoder encoder; + int bridge_count; + struct cdn_dp_bridge *bridge_list[MAX_PHY]; + struct drm_bridge *next_bridge_list[MAX_PHY]; + const struct drm_edid *edid_cache[MAX_PHY]; struct drm_display_mode mode; struct platform_device *audio_pdev; struct work_struct event_work; struct mutex lock; + bool next_bridge_valid; bool connected; bool active; bool suspended; -- 2.53.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection 2026-05-21 3:28 ` [PATCH 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection Chaoyi Chen @ 2026-05-25 11:00 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-05-25 11:00 UTC (permalink / raw) To: dri-devel-reviews Patch Review This is the largest and most complex patch in the series. **Bug: `prev_port` used uninitialized** In `cdn_dp_bridge_edid_read()`: ```c struct cdn_dp_port *prev_port; ... if (dp->bridge_count > 1 && !port->phy_enabled) { for (i = 0; i < dp->bridge_count; i++) { if (dp->bridge_list[i] != dp_bridge && dp->bridge_list[i]->enabled) goto get_cache; if (dp->port[i]->phy_enabled) prev_port = dp->port[i]; } /* Switch to current port */ if (prev_port) { ret = cdn_dp_switch_port(dp, prev_port, port); ``` `prev_port` is never initialized to `NULL`. If no port has `phy_enabled` set, `prev_port` contains stack garbage, and the `if (prev_port)` check passes with a random non-zero value, leading to a crash in `cdn_dp_switch_port()`. **Fix: initialize `prev_port = NULL`.** **Issue: `cdn_dp_switch_port` sets `dp->active = true` from EDID context** ```c static int cdn_dp_switch_port(struct cdn_dp_device *dp, ...) { if (dp->active) return 0; ... dp->active = true; dp->lanes = port->lanes; ``` This function is called from `cdn_dp_bridge_edid_read()`, which is a detect/probe path. Setting `dp->active = true` during EDID read means the subsequent `cdn_dp_enable()` call from `cdn_dp_bridge_atomic_enable()` will return early (`if (dp->active) return 0`), potentially skipping important initialization that normally happens during `cdn_dp_enable()` (firmware init, clock enable). This seems like it could leave the driver in an inconsistent state. **Issue: `cdn_dp_connected_port` logic change breaks non-bridge path** ```c for (i = 0; i < dp->ports; i++) { port = dp->port[i]; lanes[i] = cdn_dp_get_port_lanes(port); if (!dp->next_bridge_valid) return port; } ``` In the old code, the function returned the first port with non-zero lanes. Now with `!dp->next_bridge_valid`, it returns the first port **unconditionally** (even if `lanes[i]` is 0). This is a regression for the extcon path — a port with zero lanes would now be returned as "connected". Should be: ```c if (!dp->next_bridge_valid && lanes[i]) return port; ``` **Issue: `bridge_count` vs `ports` inconsistency** The `next_bridge_valid` path in `cdn_dp_connected_port()` iterates with `dp->bridge_count`: ```c for (i = 0; i < dp->bridge_count; i++) { if (lanes[i]) return dp->port[i]; } ``` But `lanes[]` was populated over `dp->ports` iterations. If `bridge_count != ports`, some `lanes[]` entries could be uninitialized or we could access `dp->port[i]` out of bounds. **Issue: `cdn_dp_bridge_detect` not updated for multi-bridge** The `cdn_dp_bridge_detect()` function still only checks `dp->connected`, but the per-bridge `dp_bridge->connected` flag is only checked in `cdn_dp_bridge_mode_valid()`. The detect callback should probably also consider the per-bridge connected state, otherwise all bridges report the same connection status. **Design: EDID caching** The EDID caching design (`edid_cache[MAX_PHY]`) is reasonable for the port-switching scenario. The `drm_edid_dup()`/`drm_edid_free()` calls appear correctly paired. **Good: `devm_drm_bridge_alloc` usage** Moving from a single bridge embedded in `cdn_dp_device` to dynamically allocated `cdn_dp_bridge` structs is the right structural change. **Cleanup needed in unbind:** ```c for (i = 0; i < dp->bridge_count; i++) { encoder = &dp->bridge_list[i]->encoder.encoder; encoder->funcs->destroy(encoder); drm_bridge_put(dp->next_bridge_list[i]); } ``` `drm_bridge_put(dp->next_bridge_list[i])` could be called with NULL if `next_bridge_list[i]` wasn't populated (e.g., extcon path where `bridge_count = 1` but `next_bridge_valid = false`). Should check for NULL before calling `drm_bridge_put()`. --- **Summary:** The series has the right architectural direction but patch 5 has several correctness issues that need fixing before merge, most critically the uninitialized `prev_port` and the `cdn_dp_connected_port` logic regression. Patch 4 also has a real bug in the probe filter condition. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* Claude review: drm/bridge: Implement generic USB Type-C DP HPD bridge 2026-05-21 3:28 [PATCH 0/5] drm/bridge: Implement generic USB Type-C DP HPD bridge Chaoyi Chen ` (4 preceding siblings ...) 2026-05-21 3:28 ` [PATCH 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection Chaoyi Chen @ 2026-05-25 11:00 ` Claude Code Review Bot 5 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-05-25 11:00 UTC (permalink / raw) To: dri-devel-reviews Overall Series Review Subject: drm/bridge: Implement generic USB Type-C DP HPD bridge Author: Chaoyi Chen <kernel@airkyi.com> Patches: 7 Reviewed: 2026-05-25T21:00:52.011314 --- This series implements a generic USB Type-C DP HPD bridge infrastructure and refactors the Rockchip CDN-DP controller to support multiple bridge/PHY port selection without depending on extcon. The design goal is sound: decouple DP hotplug detection from specific USB-C controller drivers, and allow the CDN-DP controller to work with either of the two combo PHYs on RK3399. The series has already collected Reviewed-by tags from Heikki Krogerus, Neil Armstrong, Luca Ceresoli, and Heiko Stuebner. The architecture is reasonable, but there are several correctness issues, most notably an **uninitialized variable bug** in patch 5 that could cause a kernel crash, and some logic concerns around the multi-bridge HPD/EDID switching path. **Key concerns:** 1. Patch 5: `prev_port` used uninitialized — potential NULL-pointer dereference or use of garbage pointer 2. Patch 5: `cdn_dp_switch_port` sets `dp->active = true` but is called from EDID read context, which could leave stale state 3. Patch 5: `cdn_dp_connected_port` changes semantics — the old non-`next_bridge_valid` path now falls through to the `next_bridge_valid` block 4. Patch 1: No error handling for `drm_dp_hpd_bridge_register` failure in bus notifier 5. Patch 4: Condition change in `cdn_dp_probe` may inadvertently filter out valid port configurations --- --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v15 0/9] Add Type-C DP support for RK3399 EVB IND board
@ 2026-03-04 9:41 Chaoyi Chen
2026-03-04 9:41 ` [PATCH v15 1/9] drm/bridge: Implement generic USB Type-C DP HPD bridge Chaoyi Chen
0 siblings, 1 reply; 14+ messages in thread
From: Chaoyi Chen @ 2026-03-04 9:41 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman, Dmitry Baryshkov, Peter Chen,
Luca Ceresoli, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner, Sandy Huang,
Andy Yan, Yubing Zhang, Frank Wang, Andrzej Hajda, Neil Armstrong,
Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Amit Sunil Dhamne, Dragan Simic, Johan Jonker,
Diederik de Haas, Peter Robinson, Hugh Cole-Baker
Cc: linux-usb, devicetree, linux-kernel, linux-phy, linux-arm-kernel,
linux-rockchip, dri-devel, Chaoyi Chen
From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
This series focuses on adding Type-C DP support for USBDP PHY and DP
driver. The USBDP PHY and DP will perceive the changes in cable status
based on the USB PD and Type-C state machines provided by TCPM. Before
this, the USBDP PHY and DP controller of RK3399 sensed cable state
changes through extcon, and devices such as the RK3399 Gru-Chromebook
rely on them. This series should not break them.
====
1. DisplayPort HPD status notify
Before v7, I implemented a variety of DP HPD status notify. However,
they all had various problems and it was difficult to become a generic
solution.
Under the guidance of Heikki and Dmitry, a decoupled notification
method between the TypeC and DRM subsystems was introduced in v7.
First, a notification is sent when TypeC registers a new altmode.
Then, a generic DP AUX HPD bridge is implemented on the DRM side.
During v7-v10, we added a new notifier in typec to notify the altmode
device register event. With the help of Greg and Heikki, we implemented
the reuse of notifiers for the type bus itself in patch1 of v11.
The USB subsystem related parts have already been merged into the
usb-next branch in v13 [0][1]. Therefore, this series no longer includes
these patches starting from v14. Thanks to Greg and Heikki!
[0]: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=67ab45426215c7fdccb65aecd4cac15bbe4dfcbb
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=4dee13db29de6dd869af9b3827e1ff569644e838
That makes it redundant for each Type-C controller driver to implement
a similar DP AUX HPD bridge in embedded scenarios.
====
2. Altmode switching and orientation switching for USBDP PHY
For USB Type-C interfaces, an external Type-C controller chip assists
by detecting cable attachment, determining plug orientation, and
reporting USB PD message. The USB/DP combo PHY supports software
configurable pin mapping and DisplayPort lane assignment. Based on
these message, the combo PHY can perform both altmode switching and
orientation switching via software.
The RK3399 EVB IND board has a Type-C interface DisplayPort. It use
fusb302 chip as Type-C controller. The connection diagram is shown below:
fusb302 chip +---> USB2.0 PHY ----> DWC3 USB controller
|
+---> USB/DP PHY0 +--> CDN-DP controller
|
+--> DWC3 USB controller
====
3. Multiple bridge model for RK3399 CDN-DP
The RK3399 has two USB/DP combo PHY and one CDN-DP controller. And
the CDN-DP can be switched to output to one of the PHYs.
USB/DP PHY0 ---+
| <----> CDN-DP controller
USB/DP PHY1 ---+
In previous versions, if both PHY ports were connected to DP,
the CDN-DP driver would select the first PHY port for output.
On Dmitry's suggestion, we introduced a multi-bridge model to support
flexible selection of the output PHY port. For each PHY port, a
separate encoder and bridge are registered.
The change is based on the DRM AUX HPD bridge, rather than the
extcon approach. This requires the DT to correctly describe the
connections between the first bridge in bridge chain and DP
controller. And Once the first bridge is obtained, we can get the
last bridge corresponding to the USB-C connector, and then set the
DRM connector's fwnode to the corresponding one to enable HPD
notification.
====
Patch1 add generic USB Type-C DP HPD bridge (Dmitry, Heikki).
Patch2 add new API drm_aux_bridge_register_from_node() (Neil) .
Patch3 add new Type-C mode switch for RK3399 USBDP phy binding (Krzysztof).
Patch4 add typec_mux and typec_switch for RK3399 USBDP PHY.
Patch5 add DRM AUX bridge support for RK3399 USBDP PHY (Neil).
Patch6 drops CDN-DP's extcon dependency when Type-C is present (Dmitry).
Patch7 add multiple bridges to support PHY port selection (Dmitry, Luca).
Patch8 add missing dp_out port for RK3399 CDN-DP.
Patch9 add Type-C DP support for RK3399 EVB IND board (Diederik, Peter).
Changes in v15:
- Link to V14: https://lore.kernel.org/all/20260119073100.143-1-kernel@airkyi.com/
- Improve clarity by inlining drm_bridge_get() in assignment (Luca).
Changes in v14:
- Link to V13: https://lore.kernel.org/all/20251208015500.94-1-kernel@airkyi.com/
- Drop the patches for the USB Type-C subsusytem part, as they have
already been merged into usb-next.
Changes in v13:
- Link to V12: https://lore.kernel.org/all/20251204063109.104-1-kernel@airkyi.com/
- Only register drm dp hpd bridge for typec port altmode device.
Changes in v12:
- Link to V11: https://lore.kernel.org/all/20251128020405.90-1-kernel@airkyi.com/
- Add missing Signed-off-by line.
Changes in v11:
- Link to V10: https://lore.kernel.org/all/20251120022343.250-1-kernel@airkyi.com/
- Switch to using typec bus notifiers.
Changes in v10:
- Link to V9: https://lore.kernel.org/all/20251111105040.94-1-kernel@airkyi.com/
- Notify TYPEC_ALTMODE_UNREGISTERED when altmode removed.
- Add drm_aux_bridge_register_from_node().
- Fix refcount usage of drm_bridge.
Changes in v9:
- Link to V8: https://lore.kernel.org/all/20251029071435.88-1-kernel@airkyi.com/
- Remove the exposed DRM_AUX_HPD_BRIDGE option, and select
DRM_AUX_HPD_TYPEC_BRIDGE when it is available.
- Add usb role switch for Type-C.
- Remove USB2 PHY in Type-C connection.
- ...
Changes in v8:
- Link to V7: https://lore.kernel.org/all/20251023033009.90-1-kernel@airkyi.com/
- Export all typec device types for identification.
- Merge generic DP HPD bridge into one module.
- Fix coding style.
Changes in v7:
- Link to V6: https://lore.kernel.org/all/20251016022741.91-1-kernel@airkyi.com/
- Add notifier functions for Type-C core.
- Add generic USB Type-C DP HPD bridge.
Changes in v6:
- Link to V5: https://lore.kernel.org/all/20251011033233.97-1-kernel@airkyi.com/
- Fix depend in Kconfig.
- Check DP svid in tcphy_typec_mux_set().
- Remove mode setting in tcphy_orien_sw_set().
- Rename some variable names.
- Attach the DP bridge to the next bridge.
Changes in v5:
- Link to V4: https://lore.kernel.org/all/20250922012039.323-1-kernel@airkyi.com/
- Remove the calls related to `drm_aux_hpd_bridge_notify()`.
- Place the helper functions in the same compilation unit.
- Add more comments about parent device.
- Add DRM AUX bridge support for RK3399 USBDP PHY
- By parsing the HPD bridge chain, set the connector's of_node to the
of_node corresponding to the USB-C connector.
- Return EDID cache when other port is already enabled.
Changes in v4:
- Link to V3: https://lore.kernel.org/all/20250729090032.97-1-kernel@airkyi.com/
- Add default HPD device for DisplayPort altmode.
- Introduce multiple bridges for CDN-DP.
- ...
Changes in v3:
- Link to V2: https://lore.kernel.org/all/20250718062619.99-1-kernel@airkyi.com/
- Add more descriptions to clarify the role of the PHY in switching.
- Fix wrong vdo value.
- Fix port node in usb-c-connector.
Changes in v2:
- Link to V1: https://lore.kernel.org/all/20250715112456.101-1-kernel@airkyi.com/
- Reuse dp-port/usb3-port in rk3399-typec-phy binding.
- Fix compile error when CONFIG_TYPEC is not enabled.
- Notify DP HPD state by USB/DP PHY.
- Ignore duplicate HPD events.
- Add endpoint to link DP PHY and DP controller.
- Fix devicetree coding style.
Chaoyi Chen (9):
drm/bridge: Implement generic USB Type-C DP HPD bridge
drm/bridge: aux: Add drm_aux_bridge_register_from_node()
dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch
phy: rockchip: phy-rockchip-typec: Add typec_mux/typec_switch support
phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge
drm/rockchip: cdn-dp: Support handle lane info without extcon
drm/rockchip: cdn-dp: Add multiple bridges to support PHY port
selection
arm64: dts: rockchip: Add missing dp_out port for RK3399 CDN-DP
arm64: dts: rockchip: rk3399-evb-ind: Add support for DisplayPort
.../phy/rockchip,rk3399-typec-phy.yaml | 6 +
arch/arm64/boot/dts/rockchip/rk3399-base.dtsi | 10 +-
.../boot/dts/rockchip/rk3399-evb-ind.dts | 147 +++++++
drivers/gpu/drm/bridge/Kconfig | 10 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/aux-bridge.c | 24 +-
.../gpu/drm/bridge/aux-hpd-typec-dp-bridge.c | 49 +++
drivers/gpu/drm/rockchip/Kconfig | 1 +
drivers/gpu/drm/rockchip/cdn-dp-core.c | 349 +++++++++++++---
drivers/gpu/drm/rockchip/cdn-dp-core.h | 18 +-
drivers/phy/rockchip/Kconfig | 3 +
drivers/phy/rockchip/phy-rockchip-typec.c | 373 +++++++++++++++++-
include/drm/bridge/aux-bridge.h | 6 +
13 files changed, 913 insertions(+), 84 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
--
2.51.1
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v15 1/9] drm/bridge: Implement generic USB Type-C DP HPD bridge 2026-03-04 9:41 [PATCH v15 0/9] Add Type-C DP support for RK3399 EVB IND board Chaoyi Chen @ 2026-03-04 9:41 ` Chaoyi Chen 2026-03-05 3:37 ` Claude review: " Claude Code Review Bot 0 siblings, 1 reply; 14+ messages in thread From: Chaoyi Chen @ 2026-03-04 9:41 UTC (permalink / raw) To: Heikki Krogerus, Greg Kroah-Hartman, Dmitry Baryshkov, Peter Chen, Luca Ceresoli, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner, Sandy Huang, Andy Yan, Yubing Zhang, Frank Wang, Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Amit Sunil Dhamne, Dragan Simic, Johan Jonker, Diederik de Haas, Peter Robinson, Hugh Cole-Baker Cc: linux-usb, devicetree, linux-kernel, linux-phy, linux-arm-kernel, linux-rockchip, dri-devel, Chaoyi Chen From: Chaoyi Chen <chaoyi.chen@rock-chips.com> The HPD function of Type-C DP is implemented through drm_connector_oob_hotplug_event(). For embedded DP, it is required that the DRM connector fwnode corresponds to the Type-C port fwnode. To describe the relationship between the DP controller and the Type-C port device, we usually using drm_bridge to build a bridge chain. Now several USB-C controller drivers have already implemented the DP HPD bridge function provided by aux-hpd-bridge.c, it will build a DP HPD bridge on USB-C connector port device. But this requires the USB-C controller driver to manually register the HPD bridge. If the driver does not implement this feature, the bridge will not be create. So this patch implements a generic DP HPD bridge based on aux-hpd-bridge.c. It will monitor Type-C bus events, and when a Type-C port device containing the DP svid is registered, it will create an HPD bridge for it without the need for the USB-C controller driver to implement it. Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> --- (no changes since v14) Changes in v13: - Only register drm dp hpd bridge for typec port altmode device. (no changes since v12) Changes in v11: - Switch to using typec bus notifiers. (no changes since v10) Changes in v9: - Remove the exposed DRM_AUX_HPD_BRIDGE option, and select DRM_AUX_HPD_TYPEC_BRIDGE when it is available. - Add more commit comment about problem background. Changes in v8: - Merge generic DP HPD bridge into one module. --- drivers/gpu/drm/bridge/Kconfig | 10 ++++ drivers/gpu/drm/bridge/Makefile | 1 + .../gpu/drm/bridge/aux-hpd-typec-dp-bridge.c | 49 +++++++++++++++++++ 3 files changed, 60 insertions(+) create mode 100644 drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index a250afd8d662..559487aa09a9 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -30,6 +30,16 @@ config DRM_AUX_HPD_BRIDGE Simple bridge that terminates the bridge chain and provides HPD support. +if DRM_AUX_HPD_BRIDGE +config DRM_AUX_HPD_TYPEC_BRIDGE + tristate + depends on TYPEC || !TYPEC + default TYPEC + help + Simple bridge that terminates the bridge chain and provides HPD + support. It build bridge on each USB-C connector device node. +endif + menu "Display Interface Bridges" depends on DRM && DRM_BRIDGE diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index c7dc03182e59..a3a0393d2e72 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_DRM_AUX_BRIDGE) += aux-bridge.o obj-$(CONFIG_DRM_AUX_HPD_BRIDGE) += aux-hpd-bridge.o +obj-$(CONFIG_DRM_AUX_HPD_TYPEC_BRIDGE) += aux-hpd-typec-dp-bridge.o obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o diff --git a/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c new file mode 100644 index 000000000000..d915e0fb0668 --- /dev/null +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-2.0+ +#include <linux/of.h> +#include <linux/usb/typec_altmode.h> +#include <linux/usb/typec_dp.h> + +#include <drm/bridge/aux-bridge.h> + +static int drm_typec_bus_event(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct device *dev = (struct device *)data; + struct typec_altmode *alt = to_typec_altmode(dev); + + if (action != BUS_NOTIFY_ADD_DEVICE) + goto done; + + /* + * alt->dev.parent->parent : USB-C controller device + * alt->dev.parent : USB-C connector device + */ + if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID) + drm_dp_hpd_bridge_register(alt->dev.parent->parent, + to_of_node(alt->dev.parent->fwnode)); + +done: + return NOTIFY_OK; +} + +static struct notifier_block drm_typec_event_nb = { + .notifier_call = drm_typec_bus_event, +}; + +static void drm_aux_hpd_typec_dp_bridge_module_exit(void) +{ + bus_unregister_notifier(&typec_bus, &drm_typec_event_nb); +} + +static int __init drm_aux_hpd_typec_dp_bridge_module_init(void) +{ + bus_register_notifier(&typec_bus, &drm_typec_event_nb); + + return 0; +} + +module_init(drm_aux_hpd_typec_dp_bridge_module_init); +module_exit(drm_aux_hpd_typec_dp_bridge_module_exit); + +MODULE_DESCRIPTION("DRM TYPEC DP HPD BRIDGE"); +MODULE_LICENSE("GPL"); -- 2.51.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Claude review: drm/bridge: Implement generic USB Type-C DP HPD bridge 2026-03-04 9:41 ` [PATCH v15 1/9] drm/bridge: Implement generic USB Type-C DP HPD bridge Chaoyi Chen @ 2026-03-05 3:37 ` Claude Code Review Bot 0 siblings, 0 replies; 14+ messages in thread From: Claude Code Review Bot @ 2026-03-05 3:37 UTC (permalink / raw) To: dri-devel-reviews Patch Review This creates a new module that listens for typec bus events and auto-registers DP HPD bridges when a DP altmode device appears on a Type-C port. **Issue (medium): Error return from `drm_dp_hpd_bridge_register()` is ignored.** ```c if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID) drm_dp_hpd_bridge_register(alt->dev.parent->parent, to_of_node(alt->dev.parent->fwnode)); ``` If bridge registration fails, the caller silently ignores it. At minimum a `dev_err()` or returning `NOTIFY_BAD` would help debugging. **Issue (minor): `to_typec_altmode()` is called unconditionally on every BUS_NOTIFY_ADD_DEVICE.** This is called for every device on the typec bus, not just altmode devices. If the cast is invalid for non-altmode devices (e.g. typec_port, typec_partner), this could access garbage. The `is_typec_port_altmode()` check happens after the cast. Normally this works because `container_of` is just pointer arithmetic, but it's fragile. Consider checking the device type first. **Observation:** The `module_exit` function is missing `__exit` annotation: ```c static void drm_aux_hpd_typec_dp_bridge_module_exit(void) ``` Should be `static void __exit`. **Observation:** The module has no `MODULE_AUTHOR()`. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-05-25 11:00 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-21 3:28 [PATCH 0/5] drm/bridge: Implement generic USB Type-C DP HPD bridge Chaoyi Chen 2026-05-21 3:28 ` [PATCH 1/5] " Chaoyi Chen 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 2026-05-21 3:28 ` [PATCH 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node() Chaoyi Chen 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 2026-05-21 3:28 ` [PATCH 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge Chaoyi Chen 2026-05-21 9:06 ` Heiko Stuebner 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 2026-05-21 3:28 ` [PATCH 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon Chaoyi Chen 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 2026-05-21 3:28 ` [PATCH 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection Chaoyi Chen 2026-05-25 11:00 ` Claude review: " Claude Code Review Bot 2026-05-25 11:00 ` Claude review: drm/bridge: Implement generic USB Type-C DP HPD bridge Claude Code Review Bot -- strict thread matches above, loose matches on Subject: below -- 2026-03-04 9:41 [PATCH v15 0/9] Add Type-C DP support for RK3399 EVB IND board Chaoyi Chen 2026-03-04 9:41 ` [PATCH v15 1/9] drm/bridge: Implement generic USB Type-C DP HPD bridge Chaoyi Chen 2026-03-05 3:37 ` Claude review: " Claude Code Review Bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox