On Mon Jan 19, 2026 at 8:30 AM CET, Chaoyi Chen wrote:
> From: Chaoyi Chen <[email protected]>
>
> 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 <[email protected]>
> ---
>
> (no changes since v11)
>
> Changes in v10:
> - Fix refcount usage of drm_bridge.
> - Remove unused cdn_dp_next_bridge type.
>
> Changes in v9:
> - Select DRM_AUX_HPD_BRIDGE when using DP driver.
>
> (no changes since v7)
>
> Changes in v6:
> - Rename some variable names.
> - Attach the DP bridge to the next bridge.
>
> Changes in v5:
> - 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.
> ---
>
> drivers/gpu/drm/rockchip/Kconfig | 1 +
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 325 ++++++++++++++++++++-----
> drivers/gpu/drm/rockchip/cdn-dp-core.h | 18 +-
> 3 files changed, 287 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/Kconfig
> b/drivers/gpu/drm/rockchip/Kconfig
> index b7b025814e72..10d9f29a3d44 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -56,6 +56,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 1e27301584a4..0bc3d248c266 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -27,16 +27,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
> @@ -191,14 +192,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;
> }
>
> @@ -253,12 +267,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;
> }
>
> @@ -267,12 +314,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) {
> @@ -550,6 +598,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)
> {
> @@ -571,6 +667,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;
>
> @@ -580,7 +677,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;
> @@ -599,6 +696,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",
> @@ -631,6 +731,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);
> }
> @@ -638,9 +739,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);
> @@ -827,6 +930,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,
> @@ -837,6 +950,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,
> @@ -885,7 +999,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);
>
> @@ -944,9 +1059,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,
> @@ -966,28 +1084,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);
> @@ -1002,26 +1108,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);
> @@ -1029,8 +1144,100 @@ 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;
> + }
> +
> + drm_bridge_get(bridge);
> + dp->next_bridge_valid = true;
> + dp->next_bridge_list[count] = bridge;
Correct, but the drm_bridge_get() slightly far away from the assignement is
a bit misleading. I hadn't seen it initially so I suspected a missing get.
I suggest to do it in a single statement, for clarity:
dp->next_bridge_list[count] = bridgedrm_bridge_get(bridge);
With that changed, at least for the bridge lifetime aspects, you can add to
the next version my:
Reviewed-by: Luca Ceresoli <[email protected]>
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
