[PATCH v4 0/7] Add Type-C DP support for RK3399 EVB IND board

[PATCH v4 0/7] Add Type-C DP support for RK3399 EVB IND board

[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 v4, I implemented a variety of DP HPD status notify. However,
they all had various problems and it was difficult to become a common
solution.

Under Dmitry's guidance, I try to add default DRM AUX HPD device when
register DisplayPort altmode in patch 1. That makes it redundant for
each Type-C chip driver to implement a similar registration process
in embedded scenarios.

I'm not certain if the current implementation is appropriate.
Please let me know if there's a better way.
 
====
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 PHY, USB connector, and DP controller.
And cdn_dp_parse_hpd_bridge_dt() will parses it and determines
whether to register one or two bridges.

====
Patch1 add default HPD device when register Displayport altmode.
Patch2 add new Type-C mode switch for RK3399 USBDP phy binding.
Patch3 add typec_mux and typec_switch for RK3399 USBDP PHY.
Patch4 drops CDN-DP's extcon dependency when Type-C is present.
Patch5 add multiple bridges to support PHY port selection. 
Patch6 add missing dp_out port for RK3399 CDN-DP.
Patch7 add Type-C DP support for RK3399 EVB IND board.

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 (7):
  usb: typec: Add default HPD device when register DisplayPort altmode
  dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch
  phy: rockchip: phy-rockchip-typec: Add typec_mux/typec_switch support
  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      | 146 ++++++
 drivers/gpu/drm/rockchip/Kconfig              |   1 +
 drivers/gpu/drm/rockchip/cdn-dp-core.c        | 423 +++++++++++++++---
 drivers/gpu/drm/rockchip/cdn-dp-core.h        |  23 +-
 drivers/phy/rockchip/phy-rockchip-typec.c     | 365 ++++++++++++++-
 drivers/usb/typec/altmodes/displayport.c      |  27 ++
 drivers/usb/typec/altmodes/displayport.h      |   2 +
 drivers/usb/typec/class.c                     |   8 +
 include/linux/usb/typec_altmode.h             |   2 +
 11 files changed, 932 insertions(+), 81 deletions(-)

-- 
2.49.0

[PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

Add default DRM AUX HPD bridge device when register DisplayPort
altmode. That makes it redundant for each Type-C driver to implement
a similar registration process in embedded scenarios.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---
 drivers/usb/typec/altmodes/displayport.c | 27 ++++++++++++++++++++++++
 drivers/usb/typec/altmodes/displayport.h |  2 ++
 drivers/usb/typec/class.c                |  8 +++++++
 include/linux/usb/typec_altmode.h        |  2 ++
 4 files changed, 39 insertions(+)

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 1dcb77faf85d..e026dc6e5430 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -14,6 +14,7 @@
 #include <linux/property.h>
 #include <linux/usb/pd_vdo.h>
 #include <linux/usb/typec_dp.h>
+#include <drm/bridge/aux-bridge.h>
 #include <drm/drm_connector.h>
 #include "displayport.h"
 
@@ -182,6 +183,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
 				dp->pending_irq_hpd = true;
 		}
 	} else {
+		if (dp->port->hpd_dev)
+			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
+						  hpd ? connector_status_connected :
+							connector_status_disconnected);
 		drm_connector_oob_hotplug_event(dp->connector_fwnode,
 						hpd ? connector_status_connected :
 						      connector_status_disconnected);
@@ -206,6 +211,9 @@ static int dp_altmode_configured(struct dp_altmode *dp)
 	 * configuration is complete to signal HPD.
 	 */
 	if (dp->pending_hpd) {
+		if (dp->port->hpd_dev)
+			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
+						  connector_status_connected);
 		drm_connector_oob_hotplug_event(dp->connector_fwnode,
 						connector_status_connected);
 		sysfs_notify(&dp->alt->dev.kobj, "displayport", "hpd");
@@ -391,6 +399,9 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
 			dp->data.status = 0;
 			dp->data.conf = 0;
 			if (dp->hpd) {
+				if (dp->port->hpd_dev)
+					drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
+								  connector_status_disconnected);
 				drm_connector_oob_hotplug_event(dp->connector_fwnode,
 								connector_status_disconnected);
 				dp->hpd = false;
@@ -751,6 +762,18 @@ static const struct attribute_group *displayport_groups[] = {
 	NULL,
 };
 
+void dp_altmode_hpd_device_register(struct typec_altmode *alt)
+{
+	if (alt->svid != USB_TYPEC_DP_SID)
+		return;
+
+	alt->hpd_dev = drm_dp_hpd_bridge_register(alt->dev.parent->parent,
+						  dev_of_node(alt->dev.parent->parent));
+	if (IS_ERR(alt->hpd_dev))
+		alt->hpd_dev = NULL;
+}
+EXPORT_SYMBOL_GPL(dp_altmode_hpd_device_register);
+
 int dp_altmode_probe(struct typec_altmode *alt)
 {
 	const struct typec_altmode *port = typec_altmode_get_partner(alt);
@@ -812,6 +835,10 @@ void dp_altmode_remove(struct typec_altmode *alt)
 	cancel_work_sync(&dp->work);
 	typec_altmode_put_plug(dp->plug_prime);
 
+	if (dp->port->hpd_dev)
+		drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
+					  connector_status_disconnected);
+
 	if (dp->connector_fwnode) {
 		drm_connector_oob_hotplug_event(dp->connector_fwnode,
 						connector_status_disconnected);
diff --git a/drivers/usb/typec/altmodes/displayport.h b/drivers/usb/typec/altmodes/displayport.h
index e120364da9fd..9f3483ec10fb 100644
--- a/drivers/usb/typec/altmodes/displayport.h
+++ b/drivers/usb/typec/altmodes/displayport.h
@@ -2,7 +2,9 @@
 #if IS_ENABLED(CONFIG_TYPEC_DP_ALTMODE)
 int dp_altmode_probe(struct typec_altmode *alt);
 void dp_altmode_remove(struct typec_altmode *alt);
+void dp_altmode_hpd_device_register(struct typec_altmode *alt);
 #else
 int dp_altmode_probe(struct typec_altmode *alt) { return -ENOTSUPP; }
 void dp_altmode_remove(struct typec_altmode *alt) { }
+void dp_altmode_hpd_device_register(struct typec_altmode *alt) { }
 #endif /* CONFIG_TYPEC_DP_ALTMODE */
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 67a533e35150..95732b6d9a95 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -15,6 +15,7 @@
 #include <linux/usb/typec_mux.h>
 #include <linux/usb/typec_retimer.h>
 #include <linux/usb.h>
+#include "altmodes/displayport.h"
 
 #include "bus.h"
 #include "class.h"
@@ -600,6 +601,13 @@ typec_register_altmode(struct device *parent,
 		return ERR_PTR(ret);
 	}
 
+	/*
+	 * It is too late to register the HPD device when the DisplayPort
+	 * altmode device becomes ready. If the current altmode is DP,
+	 * register a static HPD device.
+	 */
+	dp_altmode_hpd_device_register(&alt->adev);
+
 	return &alt->adev;
 }
 
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index b3c0866ea70f..acb0af1b9d5d 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -21,6 +21,7 @@ struct typec_altmode_ops;
  * @desc: Optional human readable description of the mode
  * @ops: Operations vector from the driver
  * @cable_ops: Cable operations vector from the driver.
+ * @hpd_dev: HPD device for DisplayPort
  */
 struct typec_altmode {
 	struct device			dev;
@@ -32,6 +33,7 @@ struct typec_altmode {
 	char				*desc;
 	const struct typec_altmode_ops	*ops;
 	const struct typec_cable_ops	*cable_ops;
+	struct device			*hpd_dev;
 };
 
 #define to_typec_altmode(d) container_of(d, struct typec_altmode, dev)
-- 
2.49.0

[PATCH v4 2/7] dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

The RK3399 SoC integrates two USB/DP combo PHYs, each of which
supports software-configurable pin mapping and DisplayPort lane
assignment. These capabilities enable the PHY itself to handle both
mode switching and orientation switching, based on the Type-C plug
orientation and USB PD negotiation results.

While an external Type-C controller is still required to detect cable
attachment and report USB PD events, the actual mode and orientation
switching is performed internally by the PHY through software
configuration. This allows the PHY to act as a Type-C multiplexer for
both data role and DP altmode configuration.

To reflect this hardware design, this patch introduces a new
"mode-switch" property for the dp-port node in the device tree bindings.
This property indicates that the connected PHY is capable of handling
Type-C mode switching itself.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---

Changes in v4:
- Remove "|" in description.

Changes in v3:
- Add more descriptions to clarify the role of the PHY in switching.

Changes in v2:
- Reuse dp-port/usb3-port in rk3399-typec-phy binding.

 .../devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml  | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
index 91c011f68cd0..83ebcde096ea 100644
--- a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
@@ -51,6 +51,12 @@ properties:
       '#phy-cells':
         const: 0
 
+      mode-switch:
+        description:
+          Indicates the PHY can handle altmode switching. In this case,
+          requires an external USB Type-C controller to report USB PD message.
+        type: boolean
+
       port:
         $ref: /schemas/graph.yaml#/properties/port
         description: Connection to USB Type-C connector
-- 
2.49.0

[PATCH v4 3/7] phy: rockchip: phy-rockchip-typec: Add typec_mux/typec_switch support

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

This patch add support for Type-C Port Controller Manager. Each PHY
will register typec_mux and typec_switch when external Type-C
controller is present. Type-C events are handled by TCPM without
extcon.

The extcon device should still be supported.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---

Changes in v4:
- Remove notify DP HPD state by USB/DP PHY.

(no changes since v3)

Changes in v2:
- Fix compile error when CONFIG_TYPEC is not enabled.
- Notify DP HPD state by USB/DP PHY.

 drivers/phy/rockchip/phy-rockchip-typec.c | 365 +++++++++++++++++++++-
 1 file changed, 349 insertions(+), 16 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
index d9701b6106d5..4a1dfa8d65c7 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c
@@ -54,6 +54,8 @@
 
 #include <linux/mfd/syscon.h>
 #include <linux/phy/phy.h>
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_mux.h>
 
 #define CMN_SSM_BANDGAP			(0x21 << 2)
 #define CMN_SSM_BIAS			(0x22 << 2)
@@ -286,12 +288,23 @@
 #define RX_DIAG_SC2C_DELAY		(0x81e1 << 2)
 
 #define PMA_LANE_CFG			(0xc000 << 2)
+#define PMA_LANE3_DP_LANE_SEL(x)	(((x) & 0x3) << 14)
+#define PMA_LANE3_INTERFACE_SEL(x)	(((x) & 0x1) << 12)
+#define PMA_LANE2_DP_LANE_SEL(x)	(((x) & 0x3) << 10)
+#define PMA_LANE2_INTERFACE_SEL(x)	(((x) & 0x1) << 8)
+#define PMA_LANE1_DP_LANE_SEL(x)	(((x) & 0x3) << 6)
+#define PMA_LANE1_INTERFACE_SEL(x)	(((x) & 0x1) << 4)
+#define PMA_LANE0_DP_LANE_SEL(x)	(((x) & 0x3) << 2)
+#define PMA_LANE0_INTERFACE_SEL(x)	(((x) & 0x1) << 0)
 #define PIPE_CMN_CTRL1			(0xc001 << 2)
 #define PIPE_CMN_CTRL2			(0xc002 << 2)
 #define PIPE_COM_LOCK_CFG1		(0xc003 << 2)
 #define PIPE_COM_LOCK_CFG2		(0xc004 << 2)
 #define PIPE_RCV_DET_INH		(0xc005 << 2)
 #define DP_MODE_CTL			(0xc008 << 2)
+#define PHY_DP_POWER_STATE_ACK_MASK	GENMASK(7, 4)
+#define PHY_DP_POWER_STATE_ACK_SHIFT	4
+#define PHY_DP_POWER_STATE_MASK		GENMASK(3, 0)
 #define DP_CLK_CTL			(0xc009 << 2)
 #define STS				(0xc00F << 2)
 #define PHY_ISO_CMN_CTRL		(0xc010 << 2)
@@ -327,8 +340,15 @@
 
 #define DP_MODE_A0			BIT(4)
 #define DP_MODE_A2			BIT(6)
-#define DP_MODE_ENTER_A0		0xc101
-#define DP_MODE_ENTER_A2		0xc104
+
+#define DP_MODE_MASK			0xf
+#define DP_MODE_ENTER_A0		BIT(0)
+#define DP_MODE_ENTER_A2		BIT(2)
+#define DP_MODE_ENTER_A3		BIT(3)
+#define DP_MODE_A0_ACK			BIT(4)
+#define DP_MODE_A2_ACK			BIT(6)
+#define DP_MODE_A3_ACK			BIT(7)
+#define DP_LINK_RESET_DEASSERTED	BIT(8)
 
 #define PHY_MODE_SET_TIMEOUT		100000
 
@@ -340,6 +360,31 @@
 #define MODE_DFP_USB			BIT(1)
 #define MODE_DFP_DP			BIT(2)
 
+enum phy_dp_lane_num {
+	PHY_DP_LANE_0 = 0,
+	PHY_DP_LANE_1,
+	PHY_DP_LANE_2,
+	PHY_DP_LANE_3,
+};
+
+enum phy_pma_if {
+	PMA_IF_PIPE_PCS = 0,
+	PMA_IF_PHY_DP,
+};
+
+enum phy_typec_role {
+	TYPEC_PHY_USB = 0,
+	TYPEC_PHY_DP,
+	TYPEC_PHY_MAX,
+};
+
+enum phy_dp_power_state {
+	PHY_DP_POWER_STATE_A0 = 0,
+	PHY_DP_POWER_STATE_A1,
+	PHY_DP_POWER_STATE_A2,
+	PHY_DP_POWER_STATE_A3,
+};
+
 struct usb3phy_reg {
 	u32 offset;
 	u32 enable_bit;
@@ -372,18 +417,22 @@ struct rockchip_typec_phy {
 	struct device *dev;
 	void __iomem *base;
 	struct extcon_dev *extcon;
+	struct typec_mux_dev *mux;
+	struct typec_switch_dev *sw;
 	struct regmap *grf_regs;
 	struct clk *clk_core;
 	struct clk *clk_ref;
 	struct reset_control *uphy_rst;
 	struct reset_control *pipe_rst;
 	struct reset_control *tcphy_rst;
+	struct phy *phys[TYPEC_PHY_MAX];
 	const struct rockchip_usb3phy_port_cfg *port_cfgs;
 	/* mutex to protect access to individual PHYs */
 	struct mutex lock;
 
 	bool flip;
 	u8 mode;
+	u8 new_mode;
 };
 
 struct phy_reg {
@@ -454,6 +503,99 @@ static const struct rockchip_usb3phy_port_cfg rk3399_usb3phy_port_cfgs[] = {
 	{ /* sentinel */ }
 };
 
+static int tcphy_cfg_usb3_to_usb2_only(struct rockchip_typec_phy *tcphy,
+				       bool value);
+
+static int tcphy_dp_set_power_state(struct rockchip_typec_phy *tcphy,
+				    enum phy_dp_power_state state)
+{
+	u32 ack, reg, sts = BIT(state);
+	int ret;
+
+	/*
+	 * Power state changes must not be requested until after the cmn_ready
+	 * signal has gone active.
+	 */
+	reg = readl(tcphy->base + PMA_CMN_CTRL1);
+	if (!(reg & CMN_READY)) {
+		dev_err(tcphy->dev, "cmn_ready in the inactive state\n");
+		return -EINVAL;
+	}
+
+	reg = readl(tcphy->base + DP_MODE_CTL);
+	reg &= ~PHY_DP_POWER_STATE_MASK;
+	reg |= sts;
+	writel(reg, tcphy->base + DP_MODE_CTL);
+
+	ret = readl_poll_timeout(tcphy->base + DP_MODE_CTL,
+				 ack, (((ack & PHY_DP_POWER_STATE_ACK_MASK) >>
+				 PHY_DP_POWER_STATE_ACK_SHIFT) == sts), 10,
+				 PHY_MODE_SET_TIMEOUT);
+	if (ret < 0) {
+		dev_err(tcphy->dev, "failed to enter power state %d\n", state);
+		return ret;
+	}
+
+	return 0;
+}
+
+/*
+ * For the TypeC PHY, the 4 lanes are mapping to the USB TypeC receptacle pins
+ * as follows:
+ *   -------------------------------------------------------------------
+ *	PHY Lanes/Module Pins			TypeC Receptacle Pins
+ *   -------------------------------------------------------------------
+ *	Lane0 (tx_p/m_ln_0)			TX1+/TX1- (pins A2/A3)
+ *	Lane1 (tx_rx_p/m_ln_1)			RX1+/RX1- (pins B11/B10)
+ *	Lane2 (tx_rx_p/m_ln_2)			RX2+/RX2- (pins A11/A10)
+ *	Lane3 (tx_p/m_ln_3)			TX2+/TX2- (pins B2/B3)
+ *   -------------------------------------------------------------------
+ *
+ * USB and DP lanes mapping to TypeC PHY lanes for each of pin assignment
+ * options (normal connector orientation) described in the VESA DisplayPort
+ * Alt Mode on USB TypeC Standard as follows:
+ *
+ * ----------------------------------------------------------------------
+ *	PHY Lanes	A	B	C	D	E	F
+ * ----------------------------------------------------------------------
+ *	  0	       ML1     SSTX    ML2     SSTX    ML2     SSTX
+ *	  1	       ML3     SSRX    ML3     SSRX    ML3     SSRX
+ *	  2	       ML2     ML1     ML0     ML0     ML0     ML0
+ *	  3	       ML0     ML0     ML1     ML1     ML1     ML1
+ * ----------------------------------------------------------------------
+ */
+static void tcphy_set_lane_mapping(struct rockchip_typec_phy *tcphy, u8 mode)
+{
+	/*
+	 * The PMA_LANE_CFG register is used to select whether a PMA lane
+	 * is mapped for USB or PHY DP. The PMA_LANE_CFG register is
+	 * configured based on a normal connector orientation. Logic in the
+	 * PHY automatically handles the flipped connector case based on the
+	 * setting of orientation of TypeC PHY.
+	 */
+	if (mode == MODE_DFP_DP) {
+		/* This maps to VESA DP Alt Mode pin assignments C and E. */
+		writel(PMA_LANE3_DP_LANE_SEL(PHY_DP_LANE_1) |
+		       PMA_LANE3_INTERFACE_SEL(PMA_IF_PHY_DP) |
+		       PMA_LANE2_DP_LANE_SEL(PHY_DP_LANE_0) |
+		       PMA_LANE2_INTERFACE_SEL(PMA_IF_PHY_DP) |
+		       PMA_LANE1_DP_LANE_SEL(PHY_DP_LANE_3) |
+		       PMA_LANE1_INTERFACE_SEL(PMA_IF_PHY_DP) |
+		       PMA_LANE0_DP_LANE_SEL(PHY_DP_LANE_2) |
+		       PMA_LANE0_INTERFACE_SEL(PMA_IF_PHY_DP),
+		       tcphy->base + PMA_LANE_CFG);
+	} else {
+		/* This maps to VESA DP Alt Mode pin assignments D and F. */
+		writel(PMA_LANE3_DP_LANE_SEL(PHY_DP_LANE_1) |
+		       PMA_LANE3_INTERFACE_SEL(PMA_IF_PHY_DP) |
+		       PMA_LANE2_DP_LANE_SEL(PHY_DP_LANE_0) |
+		       PMA_LANE2_INTERFACE_SEL(PMA_IF_PHY_DP) |
+		       PMA_LANE1_INTERFACE_SEL(PMA_IF_PIPE_PCS) |
+		       PMA_LANE0_INTERFACE_SEL(PMA_IF_PIPE_PCS),
+		       tcphy->base + PMA_LANE_CFG);
+	}
+}
+
 static void tcphy_cfg_24m(struct rockchip_typec_phy *tcphy)
 {
 	u32 i, rdata;
@@ -743,8 +885,10 @@ static int tcphy_phy_init(struct rockchip_typec_phy *tcphy, u8 mode)
 	tcphy_dp_aux_set_flip(tcphy);
 
 	tcphy_cfg_24m(tcphy);
+	tcphy_set_lane_mapping(tcphy, mode);
 
 	if (mode == MODE_DFP_DP) {
+		tcphy_cfg_usb3_to_usb2_only(tcphy, true);
 		tcphy_cfg_dp_pll(tcphy);
 		for (i = 0; i < 4; i++)
 			tcphy_dp_cfg_lane(tcphy, i);
@@ -768,7 +912,10 @@ static int tcphy_phy_init(struct rockchip_typec_phy *tcphy, u8 mode)
 		writel(PIN_ASSIGN_D_F, tcphy->base + PMA_LANE_CFG);
 	}
 
-	writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
+	val = readl(tcphy->base + DP_MODE_CTL);
+	val &= ~DP_MODE_MASK;
+	val |= DP_MODE_ENTER_A2 | DP_LINK_RESET_DEASSERTED;
+	writel(val, tcphy->base + DP_MODE_CTL);
 
 	reset_control_deassert(tcphy->uphy_rst);
 
@@ -811,8 +958,9 @@ static int tcphy_get_mode(struct rockchip_typec_phy *tcphy)
 	u8 mode;
 	int ret, ufp, dp;
 
+	/* If extcon not exist, try to use tcpm mode */
 	if (!edev)
-		return MODE_DFP_USB;
+		return tcphy->new_mode;
 
 	ufp = extcon_get_state(edev, EXTCON_USB);
 	dp = extcon_get_state(edev, EXTCON_DISP_DP);
@@ -850,6 +998,72 @@ static int tcphy_get_mode(struct rockchip_typec_phy *tcphy)
 	return mode;
 }
 
+#if IS_ENABLED(CONFIG_TYPEC)
+static int tcphy_orien_sw_set(struct typec_switch_dev *sw,
+			      enum typec_orientation orien)
+{
+	struct rockchip_typec_phy *tcphy = typec_switch_get_drvdata(sw);
+
+	mutex_lock(&tcphy->lock);
+
+	if (orien == TYPEC_ORIENTATION_NONE) {
+		tcphy->new_mode = MODE_DISCONNECT;
+		goto unlock_ret;
+	}
+
+	tcphy->flip = (orien == TYPEC_ORIENTATION_REVERSE) ? true : false;
+	tcphy->new_mode = MODE_DFP_USB;
+
+unlock_ret:
+	mutex_unlock(&tcphy->lock);
+	return 0;
+}
+
+static void udphy_orien_switch_unregister(void *data)
+{
+	struct rockchip_typec_phy *tcphy = data;
+
+	typec_switch_unregister(tcphy->sw);
+}
+
+static int tcphy_setup_orien_switch(struct rockchip_typec_phy *tcphy)
+{
+	struct typec_switch_desc sw_desc = { };
+	struct device_node *np;
+	int ret = 0;
+
+	np = of_get_child_by_name(tcphy->dev->of_node, "usb3-port");
+	if (!np)
+		return 0;
+
+	if (!of_property_read_bool(np, "orientation-switch"))
+		goto put_np;
+
+	sw_desc.drvdata = tcphy;
+	sw_desc.fwnode = device_get_named_child_node(tcphy->dev, "usb3-port");
+	sw_desc.set = tcphy_orien_sw_set;
+
+	tcphy->sw = typec_switch_register(tcphy->dev, &sw_desc);
+	if (IS_ERR(tcphy->sw)) {
+		dev_err(tcphy->dev, "Error register typec orientation switch: %ld\n",
+			PTR_ERR(tcphy->sw));
+		ret = PTR_ERR(tcphy->sw);
+		goto put_np;
+	}
+
+	ret = devm_add_action_or_reset(tcphy->dev, udphy_orien_switch_unregister, tcphy);
+
+put_np:
+	of_node_put(np);
+	return ret;
+}
+#else
+static int tcphy_setup_orien_switch(struct rockchip_typec_phy *tcphy)
+{
+	return 0;
+}
+#endif
+
 static int tcphy_cfg_usb3_to_usb2_only(struct rockchip_typec_phy *tcphy,
 				       bool value)
 {
@@ -989,14 +1203,9 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
 
 	tcphy_dp_aux_calibration(tcphy);
 
-	writel(DP_MODE_ENTER_A0, tcphy->base + DP_MODE_CTL);
-
-	ret = readx_poll_timeout(readl, tcphy->base + DP_MODE_CTL,
-				 val, val & DP_MODE_A0, 1000,
-				 PHY_MODE_SET_TIMEOUT);
-	if (ret < 0) {
-		writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
-		dev_err(tcphy->dev, "failed to wait TCPHY enter A0\n");
+	ret = tcphy_dp_set_power_state(tcphy, PHY_DP_POWER_STATE_A0);
+	if (ret) {
+		dev_err(tcphy->dev, "failed to enter A0 power state\n");
 		goto power_on_finish;
 	}
 
@@ -1013,6 +1222,7 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
 static int rockchip_dp_phy_power_off(struct phy *phy)
 {
 	struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
+	int ret;
 
 	mutex_lock(&tcphy->lock);
 
@@ -1021,7 +1231,11 @@ static int rockchip_dp_phy_power_off(struct phy *phy)
 
 	tcphy->mode &= ~MODE_DFP_DP;
 
-	writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
+	ret = tcphy_dp_set_power_state(tcphy, PHY_DP_POWER_STATE_A2);
+	if (ret) {
+		dev_err(tcphy->dev, "failed to enter A2 power state\n");
+		goto unlock;
+	}
 
 	if (tcphy->mode == MODE_DISCONNECT)
 		tcphy_phy_deinit(tcphy);
@@ -1037,6 +1251,89 @@ static const struct phy_ops rockchip_dp_phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+#if IS_ENABLED(CONFIG_TYPEC)
+static int tcphy_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *state)
+{
+	struct rockchip_typec_phy *tcphy = typec_mux_get_drvdata(mux);
+	struct typec_displayport_data *data;
+	int hpd = 0;
+
+	mutex_lock(&tcphy->lock);
+
+	switch (state->mode) {
+	case TYPEC_STATE_SAFE:
+		fallthrough;
+	case TYPEC_STATE_USB:
+		tcphy->new_mode = MODE_DFP_USB;
+		phy_set_bus_width(tcphy->phys[TYPEC_PHY_DP], 0);
+		break;
+	case TYPEC_DP_STATE_C:
+	case TYPEC_DP_STATE_E:
+		tcphy->new_mode = MODE_DFP_DP;
+		data = state->data;
+		hpd = !!(data->status & DP_STATUS_HPD_STATE);
+		phy_set_bus_width(tcphy->phys[TYPEC_PHY_DP], hpd ? 4 : 0);
+		break;
+	case TYPEC_DP_STATE_D:
+		tcphy->new_mode = MODE_DFP_DP | MODE_DFP_USB;
+		data = state->data;
+		hpd = !!(data->status & DP_STATUS_HPD_STATE);
+		phy_set_bus_width(tcphy->phys[TYPEC_PHY_DP], hpd ? 2 : 0);
+		break;
+	default:
+		break;
+	}
+
+	mutex_unlock(&tcphy->lock);
+
+	return 0;
+}
+
+static void tcphy_typec_mux_unregister(void *data)
+{
+	struct rockchip_typec_phy *tcphy = data;
+
+	typec_mux_unregister(tcphy->mux);
+}
+
+static int tcphy_setup_typec_mux(struct rockchip_typec_phy *tcphy)
+{
+	struct typec_mux_desc mux_desc = {};
+	struct device_node *np;
+	int ret = 0;
+
+	np = of_get_child_by_name(tcphy->dev->of_node, "dp-port");
+	if (!np)
+		return 0;
+
+	if (!of_property_read_bool(np, "mode-switch"))
+		goto put_np;
+
+	mux_desc.drvdata = tcphy;
+	mux_desc.fwnode = device_get_named_child_node(tcphy->dev, "dp-port");
+	mux_desc.set = tcphy_typec_mux_set;
+
+	tcphy->mux = typec_mux_register(tcphy->dev, &mux_desc);
+	if (IS_ERR(tcphy->mux)) {
+		dev_err(tcphy->dev, "Error register typec mux: %ld\n",
+			PTR_ERR(tcphy->mux));
+		ret = PTR_ERR(tcphy->mux);
+		goto put_np;
+	}
+
+	ret = devm_add_action_or_reset(tcphy->dev, tcphy_typec_mux_unregister, tcphy);
+
+put_np:
+	of_node_put(np);
+	return ret;
+}
+#else
+static int tcphy_setup_typec_mux(struct rockchip_typec_phy *tcphy)
+{
+	return 0;
+}
+#endif
+
 static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
 			  struct device *dev)
 {
@@ -1095,6 +1392,25 @@ static void typec_phy_pre_init(struct rockchip_typec_phy *tcphy)
 	tcphy->mode = MODE_DISCONNECT;
 }
 
+static int typec_dp_lane_get(struct rockchip_typec_phy *tcphy)
+{
+	int dp_lanes;
+
+	switch (tcphy->new_mode) {
+	case MODE_DFP_DP:
+		dp_lanes = 4;
+		break;
+	case MODE_DFP_DP | MODE_DFP_USB:
+		dp_lanes = 2;
+		break;
+	default:
+		dp_lanes = 0;
+		break;
+	}
+
+	return dp_lanes;
+}
+
 static int rockchip_typec_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1142,6 +1458,7 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
 		return ret;
 
 	tcphy->dev = dev;
+	tcphy->new_mode = MODE_DFP_USB;
 	platform_set_drvdata(pdev, tcphy);
 	mutex_init(&tcphy->lock);
 
@@ -1151,6 +1468,7 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(tcphy->extcon)) {
 		if (PTR_ERR(tcphy->extcon) == -ENODEV) {
 			tcphy->extcon = NULL;
+			dev_info(dev, "extcon not exist, try to use typec mux\n");
 		} else {
 			if (PTR_ERR(tcphy->extcon) != -EPROBE_DEFER)
 				dev_err(dev, "Invalid or missing extcon\n");
@@ -1158,19 +1476,34 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
 		}
 	}
 
+	ret = tcphy_setup_orien_switch(tcphy);
+	if (ret)
+		return ret;
+
+	ret = tcphy_setup_typec_mux(tcphy);
+	if (ret)
+		return ret;
+
 	pm_runtime_enable(dev);
 
 	for_each_available_child_of_node(np, child_np) {
 		struct phy *phy;
 
-		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"))
+			if (!IS_ERR(phy)) {
+				tcphy->phys[TYPEC_PHY_DP] = phy;
+				phy_set_bus_width(phy, typec_dp_lane_get(tcphy));
+			}
+		} else if (of_node_name_eq(child_np, "usb3-port")) {
 			phy = devm_phy_create(dev, child_np,
 					      &rockchip_usb3_phy_ops);
-		else
+			if (!IS_ERR(phy))
+				tcphy->phys[TYPEC_PHY_USB] = phy;
+		} else {
 			continue;
+		}
 
 		if (IS_ERR(phy)) {
 			dev_err(dev, "failed to create phy: %pOFn\n",
-- 
2.49.0

[PATCH v4 4/7] drm/rockchip: cdn-dp: Support handle lane info without extcon

[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>
---

Changes in v4:
- Remove cdn_dp_hpd_notify().

(no changes since v3)

Changes in v2:
- Ignore duplicate HPD events.

 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 b7e3f5dcf8d5..1e27301584a4 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -156,6 +156,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,
@@ -219,7 +222,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))
@@ -385,11 +388,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);
@@ -1028,6 +1034,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,
@@ -1120,14 +1129,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.49.0

[PATCH v4 5/7] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection

[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 PHY, USB connector, and DP controller.
And cdn_dp_parse_hpd_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.

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>
---
 drivers/gpu/drm/rockchip/Kconfig       |   1 +
 drivers/gpu/drm/rockchip/cdn-dp-core.c | 398 +++++++++++++++++++++----
 drivers/gpu/drm/rockchip/cdn-dp-core.h |  23 +-
 3 files changed, 366 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index faf50d872be3..3a6266279323 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -55,6 +55,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..784f5656fcc4 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->hpd_bridge_valid)
 			return port;
 	}
+
+	if (dp->hpd_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;
 }
 
@@ -239,10 +253,11 @@ static enum drm_connector_status
 cdn_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
 {
 	struct cdn_dp_device *dp = bridge_to_dp(bridge);
+	struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
 	enum drm_connector_status status = connector_status_disconnected;
 
 	mutex_lock(&dp->lock);
-	if (dp->connected)
+	if (dp_bridge->connected && dp->connected)
 		status = connector_status_connected;
 	mutex_unlock(&dp->lock);
 
@@ -253,10 +268,36 @@ 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 unlock;
+			/* 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 unlock;
+		}
+	}
+
 	drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp);
+
+unlock:
 	mutex_unlock(&dp->lock);
 
 	return drm_edid;
@@ -267,12 +308,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) {
@@ -571,6 +613,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 +623,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 +642,9 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
 
 	mutex_lock(&dp->lock);
 
+	if (dp->hpd_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 +677,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 +685,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);
@@ -885,7 +934,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 +994,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 +1019,99 @@ 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 void cdn_dp_typec_hpd_cb(void *data, enum drm_connector_status status)
 {
-	struct cdn_dp_device *dp = dev_get_drvdata(dev);
-	struct drm_encoder *encoder;
-	struct drm_connector *connector;
-	struct cdn_dp_port *port;
-	struct drm_device *drm_dev = data;
-	int ret, i;
+	struct cdn_dp_hpd_bridge *hpd_bridge = data;
+	struct cdn_dp_device *dp = hpd_bridge->parent;
 
-	ret = cdn_dp_parse_dt(dp);
-	if (ret < 0)
-		return ret;
+	dp->bridge_list[hpd_bridge->id]->connected = status == connector_status_connected;
+	schedule_work(&dp->event_work);
+}
 
-	dp->drm_dev = drm_dev;
-	dp->connected = false;
-	dp->active = false;
-	dp->active_port = -1;
-	dp->fw_loaded = false;
+static void cdn_dp_bridge_hpd_enable(struct cdn_dp_device *dp)
+{
+	int i;
 
-	INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
+	if (!dp->hpd_bridge_valid)
+		return;
+
+	for (i = 0; i < dp->bridge_count; i++) {
+		drm_bridge_hpd_enable(dp->hpd_bridge_list[i].bridge, cdn_dp_typec_hpd_cb,
+				      &dp->hpd_bridge_list[i]);
+	}
+}
+
+static void cdn_dp_bridge_hpd_disable(struct cdn_dp_device *dp)
+{
+	int i;
+
+	if (!dp->hpd_bridge_valid)
+		return;
+
+	for (i = 0; i < dp->bridge_count; i++) {
+		drm_bridge_hpd_disable(dp->hpd_bridge_list[i].bridge);
+	}
+}
+
+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;
+
+	cdn_dp_bridge_hpd_disable(dp);
+
+	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);
+	}
 
-	encoder = &dp->encoder.encoder;
+out:
+	cdn_dp_bridge_hpd_enable(dp);
+	return ret;
+}
+
+static int cdn_bridge_add(struct device *dev,
+			  struct drm_bridge *bridge,
+			  struct drm_encoder *encoder)
+{
+	struct cdn_dp_device *dp = dev_get_drvdata(dev);
+	struct drm_device *drm_dev = dp->drm_dev;
+	struct drm_connector *connector;
+	int ret;
 
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
 							     dev->of_node);
@@ -1002,23 +1126,23 @@ 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;
 
@@ -1031,6 +1155,167 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
 
 	drm_connector_attach_encoder(connector, encoder);
 
+	return 0;
+}
+
+static int cdn_dp_parse_hpd_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;
+		}
+	}
+
+	/*
+	 *
+	 * &dp_out {
+	 *	dp_controller_output0: endpoint@0 {
+	 * 		remote-endpoint = <&dp_phy0_in>
+	 * 	};
+	 *
+	 * 	dp_controller_output1: endpoint@1 {
+	 * 		remote-endpoint = <&dp_phy1_in>
+	 * 	};
+	 * };
+	 *
+	 * &tcphy0_dp {
+	 * 	port {
+	 * 		tcphy0_typec_dp: endpoint@0 {
+	 * 			reg = <0>;
+	 * 			remote-endpoint = <&usbc0_dp>;
+	 * 		};
+	 *
+	 * 		dp_phy0_in: endpoint@1 {
+	 * 			reg = <1>;
+	 * 			remote-endpoint = <&dp_controller_output0>;
+	 * 		};
+	 * 	};
+	 * };
+	 *
+	 * &tcphy1_dp {
+	 * 	...
+	 * };
+	 *
+	 * usbcon0: connector {
+	 * 	...
+	 * 	ports {
+	 * 		...
+	 * 		port@2 {
+	 * 			reg = <2>;
+	 *
+	 * 			usbc0_dp: endpoint {
+	 * 				remote-endpoint = <&tcphy0_typec_dp>;
+	 * 			};
+	 * 		};
+	 * 	};
+	 * };
+	 *
+	 * usbcon1: connector {
+	 * 	...
+	 * };
+	 *
+	 */
+
+	/* One endpoint may correspond to one HPD bridge. */
+	for_each_of_graph_port_endpoint(port, dp_ep) {
+		/* Try to get "port" node of correspond PHY device */
+		struct device_node *phy_ep __free(device_node) =
+			of_graph_get_remote_endpoint(dp_ep);
+		struct device_node *phy_port __free(device_node) =
+			of_get_parent(phy_ep);
+
+		if (!phy_port) {
+			continue;
+		}
+
+		/*
+		 * A PHY port may contain two endpoints: USB connector port or CDN-DP port.
+		 * Try to find the node of USB connector.
+		 */
+		for_each_of_graph_port_endpoint(phy_port, typec_ep) {
+			struct device_node *connector_port __free(device_node) =
+				of_graph_get_remote_port_parent(typec_ep);
+			struct device_node *hpd_bridge_np __free(device_node) =
+				of_get_parent(connector_port);
+
+			if (typec_ep == phy_ep)
+				continue;
+
+			bridge = of_drm_find_bridge(hpd_bridge_np);
+			if (!bridge) {
+				ret = -EPROBE_DEFER;
+				goto out;
+			}
+
+			dp->hpd_bridge_valid = true;
+			dp->hpd_bridge_list[count].bridge = bridge;
+			dp->hpd_bridge_list[count].parent = dp;
+			dp->hpd_bridge_list[count].id = count;
+			count++;
+			break;
+		}
+	}
+
+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;
+	struct drm_encoder *encoder;
+	struct cdn_dp_port *port;
+	struct drm_device *drm_dev = data;
+	struct cdn_dp_bridge *bridge_list;
+	int ret, i;
+
+	ret = cdn_dp_parse_dt(dp);
+	if (ret < 0)
+		return ret;
+
+	ret = cdn_dp_parse_hpd_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++) {
+		bridge_list = devm_drm_bridge_alloc(dev, struct cdn_dp_bridge, bridge,
+						    &cdn_dp_bridge_funcs);
+		if (IS_ERR(bridge_list))
+			return PTR_ERR(bridge_list);
+		bridge_list->id = i;
+		bridge_list->parent = dp;
+		if (!dp->hpd_bridge_valid)
+			bridge_list->connected = true;
+		dp->bridge_list[i] = bridge_list;
+	}
+
+	for (i = 0; i < dp->bridge_count; i++) {
+		encoder = &dp->bridge_list[i]->encoder.encoder;
+		bridge = &dp->bridge_list[i]->bridge;
+		ret = cdn_bridge_add(dev, 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];
 
@@ -1050,6 +1335,7 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
 
 	pm_runtime_enable(dev);
 
+	cdn_dp_bridge_hpd_enable(dp);
 	schedule_work(&dp->event_work);
 
 	return 0;
@@ -1058,10 +1344,14 @@ 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);
+	}
 
 	pm_runtime_disable(dev);
 	if (dp->fw_loaded)
@@ -1112,10 +1402,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..215f3da61af2 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,33 @@ 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_hpd_bridge {
+	struct cdn_dp_device *parent;
+	struct drm_bridge *bridge;
+	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 cdn_dp_hpd_bridge hpd_bridge_list[MAX_PHY];
 	struct drm_display_mode mode;
 	struct platform_device *audio_pdev;
 	struct work_struct event_work;
 
 	struct mutex lock;
+	bool hpd_bridge_valid;
 	bool connected;
 	bool active;
 	bool suspended;
-- 
2.49.0

[PATCH v4 6/7] arm64: dts: rockchip: Add missing dp_out port for RK3399 CDN-DP

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

Let's make the ports nodes of cdn_dp in the same style as the other
display interface, and match the style of ports's yaml.

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---

Changes in v4:
- Remove unnecessary #address/#size-cells

(no changes since v1)

 arch/arm64/boot/dts/rockchip/rk3399-base.dtsi | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-base.dtsi
index 4dcceb9136b7..93b42820998f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-base.dtsi
@@ -618,7 +618,11 @@ cdn_dp: dp@fec00000 {
 		status = "disabled";
 
 		ports {
-			dp_in: port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			dp_in: port@0 {
+				reg = <0>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 
@@ -632,6 +636,10 @@ dp_in_vopl: endpoint@1 {
 					remote-endpoint = <&vopl_out_dp>;
 				};
 			};
+
+			dp_out: port@1 {
+				reg = <1>;
+			};
 		};
 	};
 
-- 
2.49.0

[PATCH v4 7/7] arm64: dts: rockchip: rk3399-evb-ind: Add support for DisplayPort

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

The RK3399 EVB IND board has a Type-C interface DisplayPort.
It use fusb302 chip as Type-C controller.

fusb302 chip ---> USB/DP PHY0 <----> CDN-DP controller

Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---

(no changes since v4)

Changes in v3:
- Fix wrong vdo value.
- Fix port node in usb-c-connector.

Changes in v2:
- Add endpoint to link DP PHY and DP controller.
- Fix devicetree coding style.

 .../boot/dts/rockchip/rk3399-evb-ind.dts      | 146 ++++++++++++++++++
 1 file changed, 146 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dts b/arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dts
index 70aee1ab904c..aeeee6bd2973 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dts
@@ -4,6 +4,7 @@
  */
 
 /dts-v1/;
+#include <dt-bindings/usb/pd.h>
 #include "rk3399.dtsi"
 
 / {
@@ -19,6 +20,21 @@ chosen {
 		stdout-path = "serial2:1500000n8";
 	};
 
+	sound: sound {
+		compatible = "rockchip,rk3399-gru-sound";
+		rockchip,cpu = <&i2s0 &spdif>;
+	};
+
+	vbus_typec: regulator-vbus-typec {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio1 RK_PC2 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&vcc5v0_typec0_en>;
+		regulator-name = "vbus_typec";
+		vin-supply = <&vcc5v0_sys>;
+	};
+
 	vcc5v0_sys: regulator-vcc5v0-sys {
 		compatible = "regulator-fixed";
 		enable-active-high;
@@ -31,6 +47,11 @@ vcc5v0_sys: regulator-vcc5v0-sys {
 	};
 };
 
+&cdn_dp {
+	phys = <&tcphy0_dp>;
+	status = "okay";
+};
+
 &cpu_b0 {
 	cpu-supply = <&vdd_cpu_b>;
 };
@@ -55,6 +76,12 @@ &cpu_l3 {
 	cpu-supply = <&vdd_cpu_l>;
 };
 
+&dp_out {
+	dp_controller_output: endpoint {
+		remote-endpoint = <&dp_phy_in>;
+	};
+};
+
 &emmc_phy {
 	status = "okay";
 };
@@ -341,6 +368,71 @@ regulator-state-mem {
 	};
 };
 
+&i2c4 {
+	i2c-scl-rising-time-ns = <475>;
+	i2c-scl-falling-time-ns = <26>;
+	status = "okay";
+
+	usbc0: typec-portc@22 {
+		compatible = "fcs,fusb302";
+		reg = <0x22>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <RK_PA2 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usbc0_int>;
+		vbus-supply = <&vbus_typec>;
+
+		usb_con: connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			data-role = "dual";
+			power-role = "dual";
+			try-power-role = "sink";
+			op-sink-microwatt = <1000000>;
+			sink-pdos =
+				<PDO_FIXED(5000, 2500, PDO_FIXED_USB_COMM)>;
+			source-pdos =
+				<PDO_FIXED(5000, 1500, PDO_FIXED_USB_COMM)>;
+
+			altmodes {
+				displayport {
+					svid = /bits/ 16 <0xff01>;
+					vdo = <0x00001c46>;
+				};
+			};
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+
+					usbc_hs: endpoint {
+						remote-endpoint = <&u2phy0_typec_hs>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+
+					usbc_ss: endpoint {
+						remote-endpoint = <&tcphy0_typec_ss>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+
+					usbc_dp: endpoint {
+						remote-endpoint = <&tcphy0_typec_dp>;
+					};
+				};
+			};
+		};
+	};
+};
+
 &i2s2 {
 	status = "okay";
 };
@@ -354,6 +446,16 @@ &io_domains {
 };
 
 &pinctrl {
+	usb-typec {
+		usbc0_int: usbc0-int {
+			rockchip,pins = <1 RK_PA2 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+
+		vcc5v0_typec0_en: vcc5v0-typec0-en {
+			rockchip,pins = <1 RK_PC2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	pmic {
 		pmic_int_l: pmic-int-l {
 			rockchip,pins = <1 RK_PC5 RK_FUNC_GPIO &pcfg_pull_up>;
@@ -400,10 +502,48 @@ &sdmmc {
 	status = "okay";
 };
 
+&sound {
+	rockchip,codec = <&cdn_dp>;
+	status = "okay";
+};
+
+&spdif {
+	status = "okay";
+};
+
 &tcphy0 {
 	status = "okay";
 };
 
+&tcphy0_dp {
+	mode-switch;
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		tcphy0_typec_dp: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&usbc_dp>;
+		};
+
+		dp_phy_in: endpoint@1 {
+			reg = <1>;
+			remote-endpoint = <&dp_controller_output>;
+		};
+	};
+};
+
+&tcphy0_usb3 {
+	orientation-switch;
+
+	port {
+		tcphy0_typec_ss: endpoint {
+			remote-endpoint = <&usbc_ss>;
+		};
+	};
+};
+
 &tcphy1 {
 	status = "okay";
 };
@@ -418,6 +558,12 @@ &tsadc {
 
 &u2phy0 {
 	status = "okay";
+
+	port {
+		u2phy0_typec_hs: endpoint {
+			remote-endpoint = <&usbc_hs>;
+		};
+	};
 };
 
 &u2phy0_host {
-- 
2.49.0

Re: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode

[kernel test robot]

Hi Chaoyi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus robh/for-next linus/master v6.17-rc7 next-20250919]
[cannot apply to rockchip/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chaoyi-Chen/usb-typec-Add-default-HPD-device-when-register-DisplayPort-altmode/20250922-092549
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20250922012039.323-2-kernel%40airkyi.com
patch subject: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode
config: i386-buildonly-randconfig-003-20250922 (https://download.01.org/0day-ci/archive/20250922/202509221516.1umvcXG6-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250922/202509221516.1umvcXG6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509221516.1umvcXG6-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/usb/typec/class.c:18:
>> drivers/usb/typec/altmodes/displayport.h:7:5: warning: no previous prototype for 'dp_altmode_probe' [-Wmissing-prototypes]
       7 | int dp_altmode_probe(struct typec_altmode *alt) { return -ENOTSUPP; }
         |     ^~~~~~~~~~~~~~~~
>> drivers/usb/typec/altmodes/displayport.h:8:6: warning: no previous prototype for 'dp_altmode_remove' [-Wmissing-prototypes]
       8 | void dp_altmode_remove(struct typec_altmode *alt) { }
         |      ^~~~~~~~~~~~~~~~~
>> drivers/usb/typec/altmodes/displayport.h:9:6: warning: no previous prototype for 'dp_altmode_hpd_device_register' [-Wmissing-prototypes]
       9 | void dp_altmode_hpd_device_register(struct typec_altmode *alt) { }
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/dp_altmode_probe +7 drivers/usb/typec/altmodes/displayport.h

d266e96820cc36 Ajay Gupta  2019-04-23 @7  int dp_altmode_probe(struct typec_altmode *alt) { return -ENOTSUPP; }
d266e96820cc36 Ajay Gupta  2019-04-23 @8  void dp_altmode_remove(struct typec_altmode *alt) { }
50d2edccce6bf3 Chaoyi Chen 2025-09-22 @9  void dp_altmode_hpd_device_register(struct typec_altmode *alt) { }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode

[kernel test robot]

Hi Chaoyi,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus robh/for-next linus/master v6.17-rc7 next-20250919]
[cannot apply to rockchip/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chaoyi-Chen/usb-typec-Add-default-HPD-device-when-register-DisplayPort-altmode/20250922-092549
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20250922012039.323-2-kernel%40airkyi.com
patch subject: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode
config: arm-randconfig-001-20250922 (https://download.01.org/0day-ci/archive/20250922/202509221607.rWZ3wNqm-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project cafc064fc7a96b3979a023ddae1da2b499d6c954)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250922/202509221607.rWZ3wNqm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509221607.rWZ3wNqm-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: dp_altmode_hpd_device_register
   >>> referenced by class.c:609 (drivers/usb/typec/class.c:609)
   >>>               drivers/usb/typec/class.o:(typec_register_altmode) in archive vmlinux.a

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 3/7] phy: rockchip: phy-rockchip-typec: Add typec_mux/typec_switch support

[Dmitry Baryshkov]

On Mon, Sep 22, 2025 at 09:20:35AM +0800, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> This patch add support for Type-C Port Controller Manager. Each PHY
> will register typec_mux and typec_switch when external Type-C
> controller is present. Type-C events are handled by TCPM without
> extcon.
> 
> The extcon device should still be supported.
> 
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> ---
> 
> Changes in v4:
> - Remove notify DP HPD state by USB/DP PHY.
> 
> (no changes since v3)
> 
> Changes in v2:
> - Fix compile error when CONFIG_TYPEC is not enabled.
> - Notify DP HPD state by USB/DP PHY.
> 
>  drivers/phy/rockchip/phy-rockchip-typec.c | 365 +++++++++++++++++++++-
>  1 file changed, 349 insertions(+), 16 deletions(-)
> 
> @@ -850,6 +998,72 @@ static int tcphy_get_mode(struct rockchip_typec_phy *tcphy)
>  	return mode;
>  }
>  
> +#if IS_ENABLED(CONFIG_TYPEC)
> +static int tcphy_orien_sw_set(struct typec_switch_dev *sw,
> +			      enum typec_orientation orien)
> +{
> +	struct rockchip_typec_phy *tcphy = typec_switch_get_drvdata(sw);
> +
> +	mutex_lock(&tcphy->lock);
> +
> +	if (orien == TYPEC_ORIENTATION_NONE) {
> +		tcphy->new_mode = MODE_DISCONNECT;
> +		goto unlock_ret;
> +	}
> +
> +	tcphy->flip = (orien == TYPEC_ORIENTATION_REVERSE) ? true : false;
> +	tcphy->new_mode = MODE_DFP_USB;

I don't think it is correct. Orientation defines only the cable (plug)
orientation. You should be getting the mux events for the mode
selection.

> +
> +unlock_ret:
> +	mutex_unlock(&tcphy->lock);
> +	return 0;
> +}
> +
> +static void udphy_orien_switch_unregister(void *data)
> +{
> +	struct rockchip_typec_phy *tcphy = data;
> +
> +	typec_switch_unregister(tcphy->sw);
> +}
> +

-- 
With best wishes
Dmitry

Re: [PATCH v4 4/7] drm/rockchip: cdn-dp: Support handle lane info without extcon

[Dmitry Baryshkov]

On Mon, Sep 22, 2025 at 09:20:36AM +0800, Chaoyi Chen wrote:
> 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>
> ---
> 
> Changes in v4:
> - Remove cdn_dp_hpd_notify().
> 
> (no changes since v3)
> 
> Changes in v2:
> - Ignore duplicate HPD events.
> 
>  drivers/gpu/drm/rockchip/cdn-dp-core.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> @@ -1120,14 +1129,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;

This will break the case when the extcon is present. It should be
(IS_ERR(extcon) && PTR_ERR(extcon) != -ENODEV)

>  
>  		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.49.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 0/7] Add Type-C DP support for RK3399 EVB IND board

[Rob Herring (Arm)]

On Mon, 22 Sep 2025 09:20:32 +0800, Chaoyi Chen wrote:
> 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 v4, I implemented a variety of DP HPD status notify. However,
> they all had various problems and it was difficult to become a common
> solution.
> 
> Under Dmitry's guidance, I try to add default DRM AUX HPD device when
> register DisplayPort altmode in patch 1. That makes it redundant for
> each Type-C chip driver to implement a similar registration process
> in embedded scenarios.
> 
> I'm not certain if the current implementation is appropriate.
> Please let me know if there's a better way.
> 
> ====
> 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 PHY, USB connector, and DP controller.
> And cdn_dp_parse_hpd_bridge_dt() will parses it and determines
> whether to register one or two bridges.
> 
> ====
> Patch1 add default HPD device when register Displayport altmode.
> Patch2 add new Type-C mode switch for RK3399 USBDP phy binding.
> Patch3 add typec_mux and typec_switch for RK3399 USBDP PHY.
> Patch4 drops CDN-DP's extcon dependency when Type-C is present.
> Patch5 add multiple bridges to support PHY port selection.
> Patch6 add missing dp_out port for RK3399 CDN-DP.
> Patch7 add Type-C DP support for RK3399 EVB IND board.
> 
> 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 (7):
>   usb: typec: Add default HPD device when register DisplayPort altmode
>   dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch
>   phy: rockchip: phy-rockchip-typec: Add typec_mux/typec_switch support
>   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      | 146 ++++++
>  drivers/gpu/drm/rockchip/Kconfig              |   1 +
>  drivers/gpu/drm/rockchip/cdn-dp-core.c        | 423 +++++++++++++++---
>  drivers/gpu/drm/rockchip/cdn-dp-core.h        |  23 +-
>  drivers/phy/rockchip/phy-rockchip-typec.c     | 365 ++++++++++++++-
>  drivers/usb/typec/altmodes/displayport.c      |  27 ++
>  drivers/usb/typec/altmodes/displayport.h      |   2 +
>  drivers/usb/typec/class.c                     |   8 +
>  include/linux/usb/typec_altmode.h             |   2 +
>  11 files changed, 932 insertions(+), 81 deletions(-)
> 
> --
> 2.49.0
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


This patch series was applied (using b4) to base:
 Base: attempting to guess base-commit...
 Base: tags/next-20250922 (best guess, 10/11 blobs matched)

If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)

New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/rockchip/' for 20250922012039.323-1-kernel@airkyi.com:

arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dtb: syscon@ff770000 (rockchip,rk3399-grf): usb2phy@e450: 'port' does not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/soc/rockchip/grf.yaml#
arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dtb: syscon@ff770000 (rockchip,rk3399-grf): usb2phy@e450: Unevaluated properties are not allowed ('port' was unexpected)
	from schema $id: http://devicetree.org/schemas/soc/rockchip/grf.yaml#
arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dtb: usb2phy@e450 (rockchip,rk3399-usb2phy): 'port' does not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/phy/rockchip,inno-usb2phy.yaml#
arch/arm64/boot/dts/rockchip/rk3399-evb-ind.dtb: /sound: failed to match any schema with compatible: ['rockchip,rk3399-gru-sound']

Re: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode

[Dmitry Baryshkov]

On Mon, Sep 22, 2025 at 09:20:33AM +0800, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> Add default DRM AUX HPD bridge device when register DisplayPort
> altmode. That makes it redundant for each Type-C driver to implement
> a similar registration process in embedded scenarios.
> 
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> ---
>  drivers/usb/typec/altmodes/displayport.c | 27 ++++++++++++++++++++++++
>  drivers/usb/typec/altmodes/displayport.h |  2 ++
>  drivers/usb/typec/class.c                |  8 +++++++
>  include/linux/usb/typec_altmode.h        |  2 ++
>  4 files changed, 39 insertions(+)
> 
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index 1dcb77faf85d..e026dc6e5430 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -14,6 +14,7 @@
>  #include <linux/property.h>
>  #include <linux/usb/pd_vdo.h>
>  #include <linux/usb/typec_dp.h>
> +#include <drm/bridge/aux-bridge.h>
>  #include <drm/drm_connector.h>
>  #include "displayport.h"
>  
> @@ -182,6 +183,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
>  				dp->pending_irq_hpd = true;
>  		}
>  	} else {
> +		if (dp->port->hpd_dev)
> +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> +						  hpd ? connector_status_connected :
> +							connector_status_disconnected);

There should be no need for these calls. Once the HPD bridge is added to
a correct fwnode, the drm_connector_oob_hotplug_event() calls should
deliver the signal as expected.

>  		drm_connector_oob_hotplug_event(dp->connector_fwnode,
>  						hpd ? connector_status_connected :
>  						      connector_status_disconnected);
> @@ -206,6 +211,9 @@ static int dp_altmode_configured(struct dp_altmode *dp)
>  	 * configuration is complete to signal HPD.
>  	 */
>  	if (dp->pending_hpd) {
> +		if (dp->port->hpd_dev)
> +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> +						  connector_status_connected);
>  		drm_connector_oob_hotplug_event(dp->connector_fwnode,
>  						connector_status_connected);
>  		sysfs_notify(&dp->alt->dev.kobj, "displayport", "hpd");
> @@ -391,6 +399,9 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
>  			dp->data.status = 0;
>  			dp->data.conf = 0;
>  			if (dp->hpd) {
> +				if (dp->port->hpd_dev)
> +					drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> +								  connector_status_disconnected);
>  				drm_connector_oob_hotplug_event(dp->connector_fwnode,
>  								connector_status_disconnected);
>  				dp->hpd = false;
> @@ -751,6 +762,18 @@ static const struct attribute_group *displayport_groups[] = {
>  	NULL,
>  };
>  
> +void dp_altmode_hpd_device_register(struct typec_altmode *alt)
> +{
> +	if (alt->svid != USB_TYPEC_DP_SID)
> +		return;
> +
> +	alt->hpd_dev = drm_dp_hpd_bridge_register(alt->dev.parent->parent,
> +						  dev_of_node(alt->dev.parent->parent));

This needs at least a comment, what is dev.parent->parent. Also, the
of_node is not correct here. It should be a node of the connector,
rather than the device itself. Consider USB-C controllers which handle
several USB-C connectors (e.g. UCSI). The DRM core won't be able to
identify the correct bridge.

> +	if (IS_ERR(alt->hpd_dev))
> +		alt->hpd_dev = NULL;
> +}
> +EXPORT_SYMBOL_GPL(dp_altmode_hpd_device_register);

Having the function here will bring a typec -> displayport dependency
between drivers (which you didn't document). It means it won't be
possible to build typec core into the kernel, having the DP AltMode
driver in the module (which also doesn't sound like a good idea).

> +
>  int dp_altmode_probe(struct typec_altmode *alt)
>  {
>  	const struct typec_altmode *port = typec_altmode_get_partner(alt);

-- 
With best wishes
Dmitry

Re: [PATCH v4 2/7] dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch

[Dmitry Baryshkov]

On Mon, Sep 22, 2025 at 09:20:34AM +0800, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> The RK3399 SoC integrates two USB/DP combo PHYs, each of which
> supports software-configurable pin mapping and DisplayPort lane
> assignment. These capabilities enable the PHY itself to handle both
> mode switching and orientation switching, based on the Type-C plug
> orientation and USB PD negotiation results.
> 
> While an external Type-C controller is still required to detect cable
> attachment and report USB PD events, the actual mode and orientation
> switching is performed internally by the PHY through software
> configuration. This allows the PHY to act as a Type-C multiplexer for
> both data role and DP altmode configuration.
> 
> To reflect this hardware design, this patch introduces a new
> "mode-switch" property for the dp-port node in the device tree bindings.
> This property indicates that the connected PHY is capable of handling
> Type-C mode switching itself.
> 
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> 
> Changes in v4:
> - Remove "|" in description.
> 
> Changes in v3:
> - Add more descriptions to clarify the role of the PHY in switching.
> 
> Changes in v2:
> - Reuse dp-port/usb3-port in rk3399-typec-phy binding.
> 
>  .../devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml  | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> index 91c011f68cd0..83ebcde096ea 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> @@ -51,6 +51,12 @@ properties:
>        '#phy-cells':
>          const: 0
>  
> +      mode-switch:

Having the mode-switch here is a bit strange. I think the whole PHY
device should be an orientation-switch and mode-switch. Otherwise it
feels weird to me.

> +        description:
> +          Indicates the PHY can handle altmode switching. In this case,
> +          requires an external USB Type-C controller to report USB PD message.
> +        type: boolean
> +
>        port:
>          $ref: /schemas/graph.yaml#/properties/port
>          description: Connection to USB Type-C connector
> -- 
> 2.49.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 4/7] drm/rockchip: cdn-dp: Support handle lane info without extcon

[Chaoyi Chen]

On 9/22/2025 6:01 PM, Dmitry Baryshkov wrote:

> On Mon, Sep 22, 2025 at 09:20:36AM +0800, Chaoyi Chen wrote:
>> 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>
>> ---
>>
>> Changes in v4:
>> - Remove cdn_dp_hpd_notify().
>>
>> (no changes since v3)
>>
>> Changes in v2:
>> - Ignore duplicate HPD events.
>>
>>   drivers/gpu/drm/rockchip/cdn-dp-core.c | 25 +++++++++++++++++--------
>>   1 file changed, 17 insertions(+), 8 deletions(-)
>>
>> @@ -1120,14 +1129,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;
> This will break the case when the extcon is present. It should be
> (IS_ERR(extcon) && PTR_ERR(extcon) != -ENODEV)

Yes, will fix in v5


>
>>   
>>   		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.49.0
>>
-- 
Best,
Chaoyi

Re: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode

[Chaoyi Chen]

On 9/23/2025 9:10 AM, Dmitry Baryshkov wrote:

> On Mon, Sep 22, 2025 at 09:20:33AM +0800, Chaoyi Chen wrote:
>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>
>> Add default DRM AUX HPD bridge device when register DisplayPort
>> altmode. That makes it redundant for each Type-C driver to implement
>> a similar registration process in embedded scenarios.
>>
>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>> ---
>>   drivers/usb/typec/altmodes/displayport.c | 27 ++++++++++++++++++++++++
>>   drivers/usb/typec/altmodes/displayport.h |  2 ++
>>   drivers/usb/typec/class.c                |  8 +++++++
>>   include/linux/usb/typec_altmode.h        |  2 ++
>>   4 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
>> index 1dcb77faf85d..e026dc6e5430 100644
>> --- a/drivers/usb/typec/altmodes/displayport.c
>> +++ b/drivers/usb/typec/altmodes/displayport.c
>> @@ -14,6 +14,7 @@
>>   #include <linux/property.h>
>>   #include <linux/usb/pd_vdo.h>
>>   #include <linux/usb/typec_dp.h>
>> +#include <drm/bridge/aux-bridge.h>
>>   #include <drm/drm_connector.h>
>>   #include "displayport.h"
>>   
>> @@ -182,6 +183,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
>>   				dp->pending_irq_hpd = true;
>>   		}
>>   	} else {
>> +		if (dp->port->hpd_dev)
>> +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>> +						  hpd ? connector_status_connected :
>> +							connector_status_disconnected);
> There should be no need for these calls. Once the HPD bridge is added to
> a correct fwnode, the drm_connector_oob_hotplug_event() calls should
> deliver the signal as expected.

It seems that only drm_bridge_connector can do this. I'm not sure if I remember correctly. I'll give it a try.


>
>>   		drm_connector_oob_hotplug_event(dp->connector_fwnode,
>>   						hpd ? connector_status_connected :
>>   						      connector_status_disconnected);
>> @@ -206,6 +211,9 @@ static int dp_altmode_configured(struct dp_altmode *dp)
>>   	 * configuration is complete to signal HPD.
>>   	 */
>>   	if (dp->pending_hpd) {
>> +		if (dp->port->hpd_dev)
>> +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>> +						  connector_status_connected);
>>   		drm_connector_oob_hotplug_event(dp->connector_fwnode,
>>   						connector_status_connected);
>>   		sysfs_notify(&dp->alt->dev.kobj, "displayport", "hpd");
>> @@ -391,6 +399,9 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
>>   			dp->data.status = 0;
>>   			dp->data.conf = 0;
>>   			if (dp->hpd) {
>> +				if (dp->port->hpd_dev)
>> +					drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>> +								  connector_status_disconnected);
>>   				drm_connector_oob_hotplug_event(dp->connector_fwnode,
>>   								connector_status_disconnected);
>>   				dp->hpd = false;
>> @@ -751,6 +762,18 @@ static const struct attribute_group *displayport_groups[] = {
>>   	NULL,
>>   };
>>   
>> +void dp_altmode_hpd_device_register(struct typec_altmode *alt)
>> +{
>> +	if (alt->svid != USB_TYPEC_DP_SID)
>> +		return;
>> +
>> +	alt->hpd_dev = drm_dp_hpd_bridge_register(alt->dev.parent->parent,
>> +						  dev_of_node(alt->dev.parent->parent));
> This needs at least a comment, what is dev.parent->parent. Also, the
> of_node is not correct here. It should be a node of the connector,
> rather than the device itself. Consider USB-C controllers which handle
> several USB-C connectors (e.g. UCSI). The DRM core won't be able to
> identify the correct bridge.

I think  alt.dev->parent->parent is the connector device. Am I missing something?



>
>> +	if (IS_ERR(alt->hpd_dev))
>> +		alt->hpd_dev = NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(dp_altmode_hpd_device_register);
> Having the function here will bring a typec -> displayport dependency
> between drivers (which you didn't document). It means it won't be
> possible to build typec core into the kernel, having the DP AltMode
> driver in the module (which also doesn't sound like a good idea).

It make sense. Perhaps moving it into class.c would be a good idea.


>
>> +
>>   int dp_altmode_probe(struct typec_altmode *alt)
>>   {
>>   	const struct typec_altmode *port = typec_altmode_get_partner(alt);

-- 
Best,
Chaoyi

Re: [PATCH v4 5/7] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection

[Dmitry Baryshkov]

On Mon, Sep 22, 2025 at 09:20:37AM +0800, Chaoyi Chen wrote:
> 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 PHY, USB connector, and DP controller.
> And cdn_dp_parse_hpd_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.
> 
> 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>
> ---
>  drivers/gpu/drm/rockchip/Kconfig       |   1 +
>  drivers/gpu/drm/rockchip/cdn-dp-core.c | 398 +++++++++++++++++++++----
>  drivers/gpu/drm/rockchip/cdn-dp-core.h |  23 +-
>  3 files changed, 366 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index faf50d872be3..3a6266279323 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -55,6 +55,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..784f5656fcc4 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->hpd_bridge_valid)
>  			return port;
>  	}
> +
> +	if (dp->hpd_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;
>  }
>  
> @@ -239,10 +253,11 @@ static enum drm_connector_status
>  cdn_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
>  {
>  	struct cdn_dp_device *dp = bridge_to_dp(bridge);
> +	struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
>  	enum drm_connector_status status = connector_status_disconnected;
>  
>  	mutex_lock(&dp->lock);
> -	if (dp->connected)
> +	if (dp_bridge->connected && dp->connected)
>  		status = connector_status_connected;
>  	mutex_unlock(&dp->lock);
>  
> @@ -253,10 +268,36 @@ 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 unlock;
> +			/* 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 unlock;
> +		}
> +	}
> +
>  	drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp);

So... If I try reading EDID for the PHY 2 while PHY 1 is enabled, will
it return NULL, even if there is a monitor there? It totally feels like
this is one of the rare cases when caching EDIDs might make sense.

> +
> +unlock:
>  	mutex_unlock(&dp->lock);
>  
>  	return drm_edid;
> @@ -267,12 +308,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) {
> @@ -571,6 +613,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 +623,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 +642,9 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
>  
>  	mutex_lock(&dp->lock);
>  
> +	if (dp->hpd_bridge_valid)
> +		dp->active_port = dp_bridge->id;
> +

Don't you need to switch port here? Consider reading EDID from port 1,
port 2 then enabling output on port 1.

>  	ret = cdn_dp_enable(dp);
>  	if (ret) {
>  		DRM_DEV_ERROR(dp->dev, "Failed to enable bridge %d\n",
> @@ -631,6 +677,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 +685,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);
> @@ -885,7 +934,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 +994,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 +1019,99 @@ 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 void cdn_dp_typec_hpd_cb(void *data, enum drm_connector_status status)
>  {
> -	struct cdn_dp_device *dp = dev_get_drvdata(dev);
> -	struct drm_encoder *encoder;
> -	struct drm_connector *connector;
> -	struct cdn_dp_port *port;
> -	struct drm_device *drm_dev = data;
> -	int ret, i;
> +	struct cdn_dp_hpd_bridge *hpd_bridge = data;
> +	struct cdn_dp_device *dp = hpd_bridge->parent;
>  
> -	ret = cdn_dp_parse_dt(dp);
> -	if (ret < 0)
> -		return ret;
> +	dp->bridge_list[hpd_bridge->id]->connected = status == connector_status_connected;
> +	schedule_work(&dp->event_work);
> +}
>  
> -	dp->drm_dev = drm_dev;
> -	dp->connected = false;
> -	dp->active = false;
> -	dp->active_port = -1;
> -	dp->fw_loaded = false;
> +static void cdn_dp_bridge_hpd_enable(struct cdn_dp_device *dp)
> +{
> +	int i;
>  
> -	INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
> +	if (!dp->hpd_bridge_valid)
> +		return;
> +
> +	for (i = 0; i < dp->bridge_count; i++) {
> +		drm_bridge_hpd_enable(dp->hpd_bridge_list[i].bridge, cdn_dp_typec_hpd_cb,
> +				      &dp->hpd_bridge_list[i]);
> +	}
> +}
> +
> +static void cdn_dp_bridge_hpd_disable(struct cdn_dp_device *dp)
> +{
> +	int i;
> +
> +	if (!dp->hpd_bridge_valid)
> +		return;
> +
> +	for (i = 0; i < dp->bridge_count; i++) {
> +		drm_bridge_hpd_disable(dp->hpd_bridge_list[i].bridge);
> +	}
> +}
> +
> +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;
> +
> +	cdn_dp_bridge_hpd_disable(dp);
> +
> +	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);
> +	}
>  
> -	encoder = &dp->encoder.encoder;
> +out:
> +	cdn_dp_bridge_hpd_enable(dp);
> +	return ret;
> +}
> +
> +static int cdn_bridge_add(struct device *dev,
> +			  struct drm_bridge *bridge,
> +			  struct drm_encoder *encoder)
> +{
> +	struct cdn_dp_device *dp = dev_get_drvdata(dev);
> +	struct drm_device *drm_dev = dp->drm_dev;
> +	struct drm_connector *connector;
> +	int ret;
>  
>  	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
>  							     dev->of_node);
> @@ -1002,23 +1126,23 @@ 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;
>  
> @@ -1031,6 +1155,167 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>  
>  	drm_connector_attach_encoder(connector, encoder);
>  
> +	return 0;
> +}
> +
> +static int cdn_dp_parse_hpd_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;
> +		}
> +	}
> +
> +	/*
> +	 *
> +	 * &dp_out {
> +	 *	dp_controller_output0: endpoint@0 {
> +	 * 		remote-endpoint = <&dp_phy0_in>
> +	 * 	};
> +	 *
> +	 * 	dp_controller_output1: endpoint@1 {
> +	 * 		remote-endpoint = <&dp_phy1_in>
> +	 * 	};
> +	 * };
> +	 *
> +	 * &tcphy0_dp {
> +	 * 	port {
> +	 * 		tcphy0_typec_dp: endpoint@0 {
> +	 * 			reg = <0>;
> +	 * 			remote-endpoint = <&usbc0_dp>;
> +	 * 		};
> +	 *
> +	 * 		dp_phy0_in: endpoint@1 {
> +	 * 			reg = <1>;
> +	 * 			remote-endpoint = <&dp_controller_output0>;
> +	 * 		};
> +	 * 	};
> +	 * };
> +	 *
> +	 * &tcphy1_dp {
> +	 * 	...
> +	 * };
> +	 *
> +	 * usbcon0: connector {
> +	 * 	...
> +	 * 	ports {
> +	 * 		...
> +	 * 		port@2 {
> +	 * 			reg = <2>;
> +	 *
> +	 * 			usbc0_dp: endpoint {
> +	 * 				remote-endpoint = <&tcphy0_typec_dp>;
> +	 * 			};
> +	 * 		};
> +	 * 	};
> +	 * };
> +	 *
> +	 * usbcon1: connector {
> +	 * 	...
> +	 * };
> +	 *
> +	 */
> +
> +	/* One endpoint may correspond to one HPD bridge. */
> +	for_each_of_graph_port_endpoint(port, dp_ep) {
> +		/* Try to get "port" node of correspond PHY device */
> +		struct device_node *phy_ep __free(device_node) =
> +			of_graph_get_remote_endpoint(dp_ep);
> +		struct device_node *phy_port __free(device_node) =
> +			of_get_parent(phy_ep);
> +
> +		if (!phy_port) {
> +			continue;
> +		}
> +
> +		/*
> +		 * A PHY port may contain two endpoints: USB connector port or CDN-DP port.
> +		 * Try to find the node of USB connector.

And then there can be a retimer between PHY and the USB-C connector. Or
some signal MUX. Or DP-to-HDMI bridge. Please, don't parse DT for other
devices. Instead you can add drm_aux_bridge to your PHY and let DRM core
build the bridge chain following OF graph.

> +		 */
> +		for_each_of_graph_port_endpoint(phy_port, typec_ep) {
> +			struct device_node *connector_port __free(device_node) =
> +				of_graph_get_remote_port_parent(typec_ep);
> +			struct device_node *hpd_bridge_np __free(device_node) =
> +				of_get_parent(connector_port);
> +
> +			if (typec_ep == phy_ep)
> +				continue;
> +
> +			bridge = of_drm_find_bridge(hpd_bridge_np);
> +			if (!bridge) {
> +				ret = -EPROBE_DEFER;
> +				goto out;
> +			}
> +
> +			dp->hpd_bridge_valid = true;
> +			dp->hpd_bridge_list[count].bridge = bridge;
> +			dp->hpd_bridge_list[count].parent = dp;
> +			dp->hpd_bridge_list[count].id = count;
> +			count++;
> +			break;
> +		}
> +	}
> +
> +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;
> +	struct drm_encoder *encoder;
> +	struct cdn_dp_port *port;
> +	struct drm_device *drm_dev = data;
> +	struct cdn_dp_bridge *bridge_list;
> +	int ret, i;
> +
> +	ret = cdn_dp_parse_dt(dp);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = cdn_dp_parse_hpd_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++) {
> +		bridge_list = devm_drm_bridge_alloc(dev, struct cdn_dp_bridge, bridge,
> +						    &cdn_dp_bridge_funcs);
> +		if (IS_ERR(bridge_list))
> +			return PTR_ERR(bridge_list);
> +		bridge_list->id = i;
> +		bridge_list->parent = dp;
> +		if (!dp->hpd_bridge_valid)
> +			bridge_list->connected = true;
> +		dp->bridge_list[i] = bridge_list;
> +	}
> +
> +	for (i = 0; i < dp->bridge_count; i++) {
> +		encoder = &dp->bridge_list[i]->encoder.encoder;
> +		bridge = &dp->bridge_list[i]->bridge;
> +		ret = cdn_bridge_add(dev, 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];
>  
> @@ -1050,6 +1335,7 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>  
>  	pm_runtime_enable(dev);
>  
> +	cdn_dp_bridge_hpd_enable(dp);
>  	schedule_work(&dp->event_work);
>  
>  	return 0;
> @@ -1058,10 +1344,14 @@ 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);
> +	}
>  
>  	pm_runtime_disable(dev);
>  	if (dp->fw_loaded)
> @@ -1112,10 +1402,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..215f3da61af2 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,33 @@ 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_hpd_bridge {
> +	struct cdn_dp_device *parent;
> +	struct drm_bridge *bridge;
> +	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 cdn_dp_hpd_bridge hpd_bridge_list[MAX_PHY];
>  	struct drm_display_mode mode;
>  	struct platform_device *audio_pdev;
>  	struct work_struct event_work;
>  
>  	struct mutex lock;
> +	bool hpd_bridge_valid;
>  	bool connected;
>  	bool active;
>  	bool suspended;
> -- 
> 2.49.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 2/7] dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch

[Chaoyi Chen]

Hi Dmitry,

On 9/23/2025 9:12 AM, Dmitry Baryshkov wrote:
> On Mon, Sep 22, 2025 at 09:20:34AM +0800, Chaoyi Chen wrote:
>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>
>> The RK3399 SoC integrates two USB/DP combo PHYs, each of which
>> supports software-configurable pin mapping and DisplayPort lane
>> assignment. These capabilities enable the PHY itself to handle both
>> mode switching and orientation switching, based on the Type-C plug
>> orientation and USB PD negotiation results.
>>
>> While an external Type-C controller is still required to detect cable
>> attachment and report USB PD events, the actual mode and orientation
>> switching is performed internally by the PHY through software
>> configuration. This allows the PHY to act as a Type-C multiplexer for
>> both data role and DP altmode configuration.
>>
>> To reflect this hardware design, this patch introduces a new
>> "mode-switch" property for the dp-port node in the device tree bindings.
>> This property indicates that the connected PHY is capable of handling
>> Type-C mode switching itself.
>>
>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>
>> Changes in v4:
>> - Remove "|" in description.
>>
>> Changes in v3:
>> - Add more descriptions to clarify the role of the PHY in switching.
>>
>> Changes in v2:
>> - Reuse dp-port/usb3-port in rk3399-typec-phy binding.
>>
>>   .../devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml  | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>> index 91c011f68cd0..83ebcde096ea 100644
>> --- a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>> @@ -51,6 +51,12 @@ properties:
>>         '#phy-cells':
>>           const: 0
>>   
>> +      mode-switch:
> Having the mode-switch here is a bit strange. I think the whole PHY
> device should be an orientation-switch and mode-switch. Otherwise it
> feels weird to me.

I think this is a difference in practice. In the previous binding, there was already an orientation-switch under the usb-port. I trying to add both an orientation-switch and a mode-switch to the top-level device in v2. And Krzysztof reminded me that adding a mode-switch under the dp-port would be better, so I changed it to the current form :)



>
>> +        description:
>> +          Indicates the PHY can handle altmode switching. In this case,
>> +          requires an external USB Type-C controller to report USB PD message.
>> +        type: boolean
>> +
>>         port:
>>           $ref: /schemas/graph.yaml#/properties/port
>>           description: Connection to USB Type-C connector
>> -- 
>> 2.49.0
>>
-- 
Best,
Chaoyi

Re: [PATCH v4 5/7] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection

[Chaoyi Chen]

On 9/23/2025 9:50 AM, Dmitry Baryshkov wrote:

> On Mon, Sep 22, 2025 at 09:20:37AM +0800, Chaoyi Chen wrote:
>> 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 PHY, USB connector, and DP controller.
>> And cdn_dp_parse_hpd_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.
>>
>> 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>
>> ---
>>   drivers/gpu/drm/rockchip/Kconfig       |   1 +
>>   drivers/gpu/drm/rockchip/cdn-dp-core.c | 398 +++++++++++++++++++++----
>>   drivers/gpu/drm/rockchip/cdn-dp-core.h |  23 +-
>>   3 files changed, 366 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
>> index faf50d872be3..3a6266279323 100644
>> --- a/drivers/gpu/drm/rockchip/Kconfig
>> +++ b/drivers/gpu/drm/rockchip/Kconfig
>> @@ -55,6 +55,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..784f5656fcc4 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->hpd_bridge_valid)
>>   			return port;
>>   	}
>> +
>> +	if (dp->hpd_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;
>>   }
>>   
>> @@ -239,10 +253,11 @@ static enum drm_connector_status
>>   cdn_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
>>   {
>>   	struct cdn_dp_device *dp = bridge_to_dp(bridge);
>> +	struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
>>   	enum drm_connector_status status = connector_status_disconnected;
>>   
>>   	mutex_lock(&dp->lock);
>> -	if (dp->connected)
>> +	if (dp_bridge->connected && dp->connected)
>>   		status = connector_status_connected;
>>   	mutex_unlock(&dp->lock);
>>   
>> @@ -253,10 +268,36 @@ 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 unlock;
>> +			/* 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 unlock;
>> +		}
>> +	}
>> +
>>   	drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp);
> So... If I try reading EDID for the PHY 2 while PHY 1 is enabled, will
> it return NULL, even if there is a monitor there? It totally feels like
> this is one of the rare cases when caching EDIDs might make sense.

Of course. I did consider using cache, but if the monitor changes, then caching the EDID doesn't seem to be of much use…


>
>> +
>> +unlock:
>>   	mutex_unlock(&dp->lock);
>>   
>>   	return drm_edid;
>> @@ -267,12 +308,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) {
>> @@ -571,6 +613,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 +623,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 +642,9 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
>>   
>>   	mutex_lock(&dp->lock);
>>   
>> +	if (dp->hpd_bridge_valid)
>> +		dp->active_port = dp_bridge->id;
>> +
> Don't you need to switch port here? Consider reading EDID from port 1,
> port 2 then enabling output on port 1.

Oh, I missed that. Thank you, I'll add it in v5.



>
>>   	ret = cdn_dp_enable(dp);
>>   	if (ret) {
>>   		DRM_DEV_ERROR(dp->dev, "Failed to enable bridge %d\n",
>> @@ -631,6 +677,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 +685,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);
>> @@ -885,7 +934,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 +994,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 +1019,99 @@ 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 void cdn_dp_typec_hpd_cb(void *data, enum drm_connector_status status)
>>   {
>> -	struct cdn_dp_device *dp = dev_get_drvdata(dev);
>> -	struct drm_encoder *encoder;
>> -	struct drm_connector *connector;
>> -	struct cdn_dp_port *port;
>> -	struct drm_device *drm_dev = data;
>> -	int ret, i;
>> +	struct cdn_dp_hpd_bridge *hpd_bridge = data;
>> +	struct cdn_dp_device *dp = hpd_bridge->parent;
>>   
>> -	ret = cdn_dp_parse_dt(dp);
>> -	if (ret < 0)
>> -		return ret;
>> +	dp->bridge_list[hpd_bridge->id]->connected = status == connector_status_connected;
>> +	schedule_work(&dp->event_work);
>> +}
>>   
>> -	dp->drm_dev = drm_dev;
>> -	dp->connected = false;
>> -	dp->active = false;
>> -	dp->active_port = -1;
>> -	dp->fw_loaded = false;
>> +static void cdn_dp_bridge_hpd_enable(struct cdn_dp_device *dp)
>> +{
>> +	int i;
>>   
>> -	INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
>> +	if (!dp->hpd_bridge_valid)
>> +		return;
>> +
>> +	for (i = 0; i < dp->bridge_count; i++) {
>> +		drm_bridge_hpd_enable(dp->hpd_bridge_list[i].bridge, cdn_dp_typec_hpd_cb,
>> +				      &dp->hpd_bridge_list[i]);
>> +	}
>> +}
>> +
>> +static void cdn_dp_bridge_hpd_disable(struct cdn_dp_device *dp)
>> +{
>> +	int i;
>> +
>> +	if (!dp->hpd_bridge_valid)
>> +		return;
>> +
>> +	for (i = 0; i < dp->bridge_count; i++) {
>> +		drm_bridge_hpd_disable(dp->hpd_bridge_list[i].bridge);
>> +	}
>> +}
>> +
>> +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;
>> +
>> +	cdn_dp_bridge_hpd_disable(dp);
>> +
>> +	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);
>> +	}
>>   
>> -	encoder = &dp->encoder.encoder;
>> +out:
>> +	cdn_dp_bridge_hpd_enable(dp);
>> +	return ret;
>> +}
>> +
>> +static int cdn_bridge_add(struct device *dev,
>> +			  struct drm_bridge *bridge,
>> +			  struct drm_encoder *encoder)
>> +{
>> +	struct cdn_dp_device *dp = dev_get_drvdata(dev);
>> +	struct drm_device *drm_dev = dp->drm_dev;
>> +	struct drm_connector *connector;
>> +	int ret;
>>   
>>   	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
>>   							     dev->of_node);
>> @@ -1002,23 +1126,23 @@ 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;
>>   
>> @@ -1031,6 +1155,167 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>>   
>>   	drm_connector_attach_encoder(connector, encoder);
>>   
>> +	return 0;
>> +}
>> +
>> +static int cdn_dp_parse_hpd_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;
>> +		}
>> +	}
>> +
>> +	/*
>> +	 *
>> +	 * &dp_out {
>> +	 *	dp_controller_output0: endpoint@0 {
>> +	 * 		remote-endpoint = <&dp_phy0_in>
>> +	 * 	};
>> +	 *
>> +	 * 	dp_controller_output1: endpoint@1 {
>> +	 * 		remote-endpoint = <&dp_phy1_in>
>> +	 * 	};
>> +	 * };
>> +	 *
>> +	 * &tcphy0_dp {
>> +	 * 	port {
>> +	 * 		tcphy0_typec_dp: endpoint@0 {
>> +	 * 			reg = <0>;
>> +	 * 			remote-endpoint = <&usbc0_dp>;
>> +	 * 		};
>> +	 *
>> +	 * 		dp_phy0_in: endpoint@1 {
>> +	 * 			reg = <1>;
>> +	 * 			remote-endpoint = <&dp_controller_output0>;
>> +	 * 		};
>> +	 * 	};
>> +	 * };
>> +	 *
>> +	 * &tcphy1_dp {
>> +	 * 	...
>> +	 * };
>> +	 *
>> +	 * usbcon0: connector {
>> +	 * 	...
>> +	 * 	ports {
>> +	 * 		...
>> +	 * 		port@2 {
>> +	 * 			reg = <2>;
>> +	 *
>> +	 * 			usbc0_dp: endpoint {
>> +	 * 				remote-endpoint = <&tcphy0_typec_dp>;
>> +	 * 			};
>> +	 * 		};
>> +	 * 	};
>> +	 * };
>> +	 *
>> +	 * usbcon1: connector {
>> +	 * 	...
>> +	 * };
>> +	 *
>> +	 */
>> +
>> +	/* One endpoint may correspond to one HPD bridge. */
>> +	for_each_of_graph_port_endpoint(port, dp_ep) {
>> +		/* Try to get "port" node of correspond PHY device */
>> +		struct device_node *phy_ep __free(device_node) =
>> +			of_graph_get_remote_endpoint(dp_ep);
>> +		struct device_node *phy_port __free(device_node) =
>> +			of_get_parent(phy_ep);
>> +
>> +		if (!phy_port) {
>> +			continue;
>> +		}
>> +
>> +		/*
>> +		 * A PHY port may contain two endpoints: USB connector port or CDN-DP port.
>> +		 * Try to find the node of USB connector.
> And then there can be a retimer between PHY and the USB-C connector. Or
> some signal MUX. Or DP-to-HDMI bridge. Please, don't parse DT for other
> devices. Instead you can add drm_aux_bridge to your PHY and let DRM core
> build the bridge chain following OF graph.

Okay, I will try to do it in v5.


>
>> +		 */
>> +		for_each_of_graph_port_endpoint(phy_port, typec_ep) {
>> +			struct device_node *connector_port __free(device_node) =
>> +				of_graph_get_remote_port_parent(typec_ep);
>> +			struct device_node *hpd_bridge_np __free(device_node) =
>> +				of_get_parent(connector_port);
>> +
>> +			if (typec_ep == phy_ep)
>> +				continue;
>> +
>> +			bridge = of_drm_find_bridge(hpd_bridge_np);
>> +			if (!bridge) {
>> +				ret = -EPROBE_DEFER;
>> +				goto out;
>> +			}
>> +
>> +			dp->hpd_bridge_valid = true;
>> +			dp->hpd_bridge_list[count].bridge = bridge;
>> +			dp->hpd_bridge_list[count].parent = dp;
>> +			dp->hpd_bridge_list[count].id = count;
>> +			count++;
>> +			break;
>> +		}
>> +	}
>> +
>> +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;
>> +	struct drm_encoder *encoder;
>> +	struct cdn_dp_port *port;
>> +	struct drm_device *drm_dev = data;
>> +	struct cdn_dp_bridge *bridge_list;
>> +	int ret, i;
>> +
>> +	ret = cdn_dp_parse_dt(dp);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = cdn_dp_parse_hpd_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++) {
>> +		bridge_list = devm_drm_bridge_alloc(dev, struct cdn_dp_bridge, bridge,
>> +						    &cdn_dp_bridge_funcs);
>> +		if (IS_ERR(bridge_list))
>> +			return PTR_ERR(bridge_list);
>> +		bridge_list->id = i;
>> +		bridge_list->parent = dp;
>> +		if (!dp->hpd_bridge_valid)
>> +			bridge_list->connected = true;
>> +		dp->bridge_list[i] = bridge_list;
>> +	}
>> +
>> +	for (i = 0; i < dp->bridge_count; i++) {
>> +		encoder = &dp->bridge_list[i]->encoder.encoder;
>> +		bridge = &dp->bridge_list[i]->bridge;
>> +		ret = cdn_bridge_add(dev, 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];
>>   
>> @@ -1050,6 +1335,7 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>>   
>>   	pm_runtime_enable(dev);
>>   
>> +	cdn_dp_bridge_hpd_enable(dp);
>>   	schedule_work(&dp->event_work);
>>   
>>   	return 0;
>> @@ -1058,10 +1344,14 @@ 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);
>> +	}
>>   
>>   	pm_runtime_disable(dev);
>>   	if (dp->fw_loaded)
>> @@ -1112,10 +1402,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..215f3da61af2 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,33 @@ 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_hpd_bridge {
>> +	struct cdn_dp_device *parent;
>> +	struct drm_bridge *bridge;
>> +	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 cdn_dp_hpd_bridge hpd_bridge_list[MAX_PHY];
>>   	struct drm_display_mode mode;
>>   	struct platform_device *audio_pdev;
>>   	struct work_struct event_work;
>>   
>>   	struct mutex lock;
>> +	bool hpd_bridge_valid;
>>   	bool connected;
>>   	bool active;
>>   	bool suspended;
>> -- 
>> 2.49.0
>>
-- 
Best,
Chaoyi

Re: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode

[Dmitry Baryshkov]

On Tue, Sep 23, 2025 at 09:34:39AM +0800, Chaoyi Chen wrote:
> On 9/23/2025 9:10 AM, Dmitry Baryshkov wrote:
> 
> > On Mon, Sep 22, 2025 at 09:20:33AM +0800, Chaoyi Chen wrote:
> > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > 
> > > Add default DRM AUX HPD bridge device when register DisplayPort
> > > altmode. That makes it redundant for each Type-C driver to implement
> > > a similar registration process in embedded scenarios.
> > > 
> > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > ---
> > >   drivers/usb/typec/altmodes/displayport.c | 27 ++++++++++++++++++++++++
> > >   drivers/usb/typec/altmodes/displayport.h |  2 ++
> > >   drivers/usb/typec/class.c                |  8 +++++++
> > >   include/linux/usb/typec_altmode.h        |  2 ++
> > >   4 files changed, 39 insertions(+)
> > > 
> > > diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> > > index 1dcb77faf85d..e026dc6e5430 100644
> > > --- a/drivers/usb/typec/altmodes/displayport.c
> > > +++ b/drivers/usb/typec/altmodes/displayport.c
> > > @@ -14,6 +14,7 @@
> > >   #include <linux/property.h>
> > >   #include <linux/usb/pd_vdo.h>
> > >   #include <linux/usb/typec_dp.h>
> > > +#include <drm/bridge/aux-bridge.h>
> > >   #include <drm/drm_connector.h>
> > >   #include "displayport.h"
> > > @@ -182,6 +183,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
> > >   				dp->pending_irq_hpd = true;
> > >   		}
> > >   	} else {
> > > +		if (dp->port->hpd_dev)
> > > +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> > > +						  hpd ? connector_status_connected :
> > > +							connector_status_disconnected);
> > There should be no need for these calls. Once the HPD bridge is added to
> > a correct fwnode, the drm_connector_oob_hotplug_event() calls should
> > deliver the signal as expected.
> 
> It seems that only drm_bridge_connector can do this. I'm not sure if I remember correctly. I'll give it a try.

Other connectors can implement the .oob_hotplug_event call. Calling
drm_bridge_hpd_notify() also depends on the connector setting the
callbacks via drm_bridge_hpd_enable(), a step which is done by only a
few drivers.

> 
> 
> > 
> > >   		drm_connector_oob_hotplug_event(dp->connector_fwnode,
> > >   						hpd ? connector_status_connected :
> > >   						      connector_status_disconnected);
> > > @@ -206,6 +211,9 @@ static int dp_altmode_configured(struct dp_altmode *dp)
> > >   	 * configuration is complete to signal HPD.
> > >   	 */
> > >   	if (dp->pending_hpd) {
> > > +		if (dp->port->hpd_dev)
> > > +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> > > +						  connector_status_connected);
> > >   		drm_connector_oob_hotplug_event(dp->connector_fwnode,
> > >   						connector_status_connected);
> > >   		sysfs_notify(&dp->alt->dev.kobj, "displayport", "hpd");
> > > @@ -391,6 +399,9 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
> > >   			dp->data.status = 0;
> > >   			dp->data.conf = 0;
> > >   			if (dp->hpd) {
> > > +				if (dp->port->hpd_dev)
> > > +					drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> > > +								  connector_status_disconnected);
> > >   				drm_connector_oob_hotplug_event(dp->connector_fwnode,
> > >   								connector_status_disconnected);
> > >   				dp->hpd = false;
> > > @@ -751,6 +762,18 @@ static const struct attribute_group *displayport_groups[] = {
> > >   	NULL,
> > >   };
> > > +void dp_altmode_hpd_device_register(struct typec_altmode *alt)
> > > +{
> > > +	if (alt->svid != USB_TYPEC_DP_SID)
> > > +		return;
> > > +
> > > +	alt->hpd_dev = drm_dp_hpd_bridge_register(alt->dev.parent->parent,
> > > +						  dev_of_node(alt->dev.parent->parent));
> > This needs at least a comment, what is dev.parent->parent. Also, the
> > of_node is not correct here. It should be a node of the connector,
> > rather than the device itself. Consider USB-C controllers which handle
> > several USB-C connectors (e.g. UCSI). The DRM core won't be able to
> > identify the correct bridge.
> 
> I think  alt.dev->parent->parent is the connector device. Am I missing something?

As I wrote, it needs a comment (in the source file). No, it's not a
connector device, it's a USB-C controller device. There is no guarantee
that there is a separate struct device for the USB-C connector. On
Qualcomm platforms, the device will point to the USB-C controller (TCPM
or UCSI), which contain usb-c-connector(s) as child node(s) in DT.

> 
> 
> 
> > 
> > > +	if (IS_ERR(alt->hpd_dev))
> > > +		alt->hpd_dev = NULL;
> > > +}
> > > +EXPORT_SYMBOL_GPL(dp_altmode_hpd_device_register);
> > Having the function here will bring a typec -> displayport dependency
> > between drivers (which you didn't document). It means it won't be
> > possible to build typec core into the kernel, having the DP AltMode
> > driver in the module (which also doesn't sound like a good idea).
> 
> It make sense. Perhaps moving it into class.c would be a good idea.
> 
> 
> > 
> > > +
> > >   int dp_altmode_probe(struct typec_altmode *alt)
> > >   {
> > >   	const struct typec_altmode *port = typec_altmode_get_partner(alt);
> 
> -- 
> Best,
> Chaoyi
> 
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
With best wishes
Dmitry

Re: [PATCH v4 2/7] dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch

[Dmitry Baryshkov]

On Tue, Sep 23, 2025 at 09:53:06AM +0800, Chaoyi Chen wrote:
> Hi Dmitry,
> 
> On 9/23/2025 9:12 AM, Dmitry Baryshkov wrote:
> > On Mon, Sep 22, 2025 at 09:20:34AM +0800, Chaoyi Chen wrote:
> > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > 
> > > The RK3399 SoC integrates two USB/DP combo PHYs, each of which
> > > supports software-configurable pin mapping and DisplayPort lane
> > > assignment. These capabilities enable the PHY itself to handle both
> > > mode switching and orientation switching, based on the Type-C plug
> > > orientation and USB PD negotiation results.
> > > 
> > > While an external Type-C controller is still required to detect cable
> > > attachment and report USB PD events, the actual mode and orientation
> > > switching is performed internally by the PHY through software
> > > configuration. This allows the PHY to act as a Type-C multiplexer for
> > > both data role and DP altmode configuration.
> > > 
> > > To reflect this hardware design, this patch introduces a new
> > > "mode-switch" property for the dp-port node in the device tree bindings.
> > > This property indicates that the connected PHY is capable of handling
> > > Type-C mode switching itself.
> > > 
> > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > 
> > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > ---
> > > 
> > > Changes in v4:
> > > - Remove "|" in description.
> > > 
> > > Changes in v3:
> > > - Add more descriptions to clarify the role of the PHY in switching.
> > > 
> > > Changes in v2:
> > > - Reuse dp-port/usb3-port in rk3399-typec-phy binding.
> > > 
> > >   .../devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml  | 6 ++++++
> > >   1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> > > index 91c011f68cd0..83ebcde096ea 100644
> > > --- a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> > > +++ b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> > > @@ -51,6 +51,12 @@ properties:
> > >         '#phy-cells':
> > >           const: 0
> > > +      mode-switch:
> > Having the mode-switch here is a bit strange. I think the whole PHY
> > device should be an orientation-switch and mode-switch. Otherwise it
> > feels weird to me.
> 
> I think this is a difference in practice. In the previous binding, there was already an orientation-switch under the usb-port. I trying to add both an orientation-switch and a mode-switch to the top-level device in v2. And Krzysztof reminded me that adding a mode-switch under the dp-port would be better, so I changed it to the current form :)

I couldn't find the comment on lore. Could you please point it out?

> 
> 
> 
> > 
> > > +        description:
> > > +          Indicates the PHY can handle altmode switching. In this case,
> > > +          requires an external USB Type-C controller to report USB PD message.
> > > +        type: boolean
> > > +
> > >         port:
> > >           $ref: /schemas/graph.yaml#/properties/port
> > >           description: Connection to USB Type-C connector
> > > -- 
> > > 2.49.0
> > > 
> -- 
> Best,
> Chaoyi
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 5/7] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection

[Dmitry Baryshkov]

On Tue, Sep 23, 2025 at 10:09:38AM +0800, Chaoyi Chen wrote:
> On 9/23/2025 9:50 AM, Dmitry Baryshkov wrote:
> 
> > On Mon, Sep 22, 2025 at 09:20:37AM +0800, Chaoyi Chen wrote:
> > > 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 PHY, USB connector, and DP controller.
> > > And cdn_dp_parse_hpd_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.
> > > 
> > > 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>
> > > ---
> > >   drivers/gpu/drm/rockchip/Kconfig       |   1 +
> > >   drivers/gpu/drm/rockchip/cdn-dp-core.c | 398 +++++++++++++++++++++----
> > >   drivers/gpu/drm/rockchip/cdn-dp-core.h |  23 +-
> > >   3 files changed, 366 insertions(+), 56 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> > > index faf50d872be3..3a6266279323 100644
> > > --- a/drivers/gpu/drm/rockchip/Kconfig
> > > +++ b/drivers/gpu/drm/rockchip/Kconfig
> > > @@ -55,6 +55,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..784f5656fcc4 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->hpd_bridge_valid)
> > >   			return port;
> > >   	}
> > > +
> > > +	if (dp->hpd_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;
> > >   }
> > > @@ -239,10 +253,11 @@ static enum drm_connector_status
> > >   cdn_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
> > >   {
> > >   	struct cdn_dp_device *dp = bridge_to_dp(bridge);
> > > +	struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
> > >   	enum drm_connector_status status = connector_status_disconnected;
> > >   	mutex_lock(&dp->lock);
> > > -	if (dp->connected)
> > > +	if (dp_bridge->connected && dp->connected)
> > >   		status = connector_status_connected;
> > >   	mutex_unlock(&dp->lock);
> > > @@ -253,10 +268,36 @@ 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 unlock;
> > > +			/* 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 unlock;
> > > +		}
> > > +	}
> > > +
> > >   	drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp);
> > So... If I try reading EDID for the PHY 2 while PHY 1 is enabled, will
> > it return NULL, even if there is a monitor there? It totally feels like
> > this is one of the rare cases when caching EDIDs might make sense.
> 
> Of course. I did consider using cache, but if the monitor changes, then caching the EDID doesn't seem to be of much use…

Yes... It might still be better to invalidate the cache on the plug
event rather than always reporting empty EDID when another monitor is
enabled.

-- 
With best wishes
Dmitry

Re: [PATCH v4 5/7] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection

[Chaoyi Chen]

On 9/23/2025 11:22 AM, Dmitry Baryshkov wrote:

> On Tue, Sep 23, 2025 at 10:09:38AM +0800, Chaoyi Chen wrote:
>> On 9/23/2025 9:50 AM, Dmitry Baryshkov wrote:
>>
>>> On Mon, Sep 22, 2025 at 09:20:37AM +0800, Chaoyi Chen wrote:
>>>> 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 PHY, USB connector, and DP controller.
>>>> And cdn_dp_parse_hpd_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.
>>>>
>>>> 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>
>>>> ---
>>>>    drivers/gpu/drm/rockchip/Kconfig       |   1 +
>>>>    drivers/gpu/drm/rockchip/cdn-dp-core.c | 398 +++++++++++++++++++++----
>>>>    drivers/gpu/drm/rockchip/cdn-dp-core.h |  23 +-
>>>>    3 files changed, 366 insertions(+), 56 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
>>>> index faf50d872be3..3a6266279323 100644
>>>> --- a/drivers/gpu/drm/rockchip/Kconfig
>>>> +++ b/drivers/gpu/drm/rockchip/Kconfig
>>>> @@ -55,6 +55,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..784f5656fcc4 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->hpd_bridge_valid)
>>>>    			return port;
>>>>    	}
>>>> +
>>>> +	if (dp->hpd_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;
>>>>    }
>>>> @@ -239,10 +253,11 @@ static enum drm_connector_status
>>>>    cdn_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
>>>>    {
>>>>    	struct cdn_dp_device *dp = bridge_to_dp(bridge);
>>>> +	struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
>>>>    	enum drm_connector_status status = connector_status_disconnected;
>>>>    	mutex_lock(&dp->lock);
>>>> -	if (dp->connected)
>>>> +	if (dp_bridge->connected && dp->connected)
>>>>    		status = connector_status_connected;
>>>>    	mutex_unlock(&dp->lock);
>>>> @@ -253,10 +268,36 @@ 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 unlock;
>>>> +			/* 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 unlock;
>>>> +		}
>>>> +	}
>>>> +
>>>>    	drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp);
>>> So... If I try reading EDID for the PHY 2 while PHY 1 is enabled, will
>>> it return NULL, even if there is a monitor there? It totally feels like
>>> this is one of the rare cases when caching EDIDs might make sense.
>> Of course. I did consider using cache, but if the monitor changes, then caching the EDID doesn't seem to be of much use…
> Yes... It might still be better to invalidate the cache on the plug
> event rather than always reporting empty EDID when another monitor is
> enabled.

Hmm, I will add cache in v5.


>
-- 
Best,
Chaoyi

Re: [PATCH v4 2/7] dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch

[Chaoyi Chen]

On 9/23/2025 11:17 AM, Dmitry Baryshkov wrote:

> On Tue, Sep 23, 2025 at 09:53:06AM +0800, Chaoyi Chen wrote:
>> Hi Dmitry,
>>
>> On 9/23/2025 9:12 AM, Dmitry Baryshkov wrote:
>>> On Mon, Sep 22, 2025 at 09:20:34AM +0800, Chaoyi Chen wrote:
>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>
>>>> The RK3399 SoC integrates two USB/DP combo PHYs, each of which
>>>> supports software-configurable pin mapping and DisplayPort lane
>>>> assignment. These capabilities enable the PHY itself to handle both
>>>> mode switching and orientation switching, based on the Type-C plug
>>>> orientation and USB PD negotiation results.
>>>>
>>>> While an external Type-C controller is still required to detect cable
>>>> attachment and report USB PD events, the actual mode and orientation
>>>> switching is performed internally by the PHY through software
>>>> configuration. This allows the PHY to act as a Type-C multiplexer for
>>>> both data role and DP altmode configuration.
>>>>
>>>> To reflect this hardware design, this patch introduces a new
>>>> "mode-switch" property for the dp-port node in the device tree bindings.
>>>> This property indicates that the connected PHY is capable of handling
>>>> Type-C mode switching itself.
>>>>
>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>
>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>
>>>> Changes in v4:
>>>> - Remove "|" in description.
>>>>
>>>> Changes in v3:
>>>> - Add more descriptions to clarify the role of the PHY in switching.
>>>>
>>>> Changes in v2:
>>>> - Reuse dp-port/usb3-port in rk3399-typec-phy binding.
>>>>
>>>>    .../devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml  | 6 ++++++
>>>>    1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>>>> index 91c011f68cd0..83ebcde096ea 100644
>>>> --- a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>>>> +++ b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>>>> @@ -51,6 +51,12 @@ properties:
>>>>          '#phy-cells':
>>>>            const: 0
>>>> +      mode-switch:
>>> Having the mode-switch here is a bit strange. I think the whole PHY
>>> device should be an orientation-switch and mode-switch. Otherwise it
>>> feels weird to me.
>> I think this is a difference in practice. In the previous binding, there was already an orientation-switch under the usb-port. I trying to add both an orientation-switch and a mode-switch to the top-level device in v2. And Krzysztof reminded me that adding a mode-switch under the dp-port would be better, so I changed it to the current form :)
> I couldn't find the comment on lore. Could you please point it out?

Sorry, it is v1. I added an orientation-switch and a mode-switch in the top-level PHY [0]. Comment is here: [1]


[0] https://lore.kernel.org/all/20250715112456.101-4-kernel@airkyi.com/

[1] https://lore.kernel.org/all/4dfed94c-665d-4e04-b527-ddd34fd3db8f@kernel.org/



>>
>>
>>>> +        description:
>>>> +          Indicates the PHY can handle altmode switching. In this case,
>>>> +          requires an external USB Type-C controller to report USB PD message.
>>>> +        type: boolean
>>>> +
>>>>          port:
>>>>            $ref: /schemas/graph.yaml#/properties/port
>>>>            description: Connection to USB Type-C connector
>>>> -- 
>>>> 2.49.0
>>>>
>> -- 
>> Best,
>> Chaoyi
>>
-- 
Best,
Chaoyi

Re: [PATCH v4 2/7] dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch

[Chaoyi Chen]

On 9/23/2025 11:17 AM, Dmitry Baryshkov wrote:

> On Tue, Sep 23, 2025 at 09:53:06AM +0800, Chaoyi Chen wrote:
>> Hi Dmitry,
>>
>> On 9/23/2025 9:12 AM, Dmitry Baryshkov wrote:
>>> On Mon, Sep 22, 2025 at 09:20:34AM +0800, Chaoyi Chen wrote:
>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>
>>>> The RK3399 SoC integrates two USB/DP combo PHYs, each of which
>>>> supports software-configurable pin mapping and DisplayPort lane
>>>> assignment. These capabilities enable the PHY itself to handle both
>>>> mode switching and orientation switching, based on the Type-C plug
>>>> orientation and USB PD negotiation results.
>>>>
>>>> While an external Type-C controller is still required to detect cable
>>>> attachment and report USB PD events, the actual mode and orientation
>>>> switching is performed internally by the PHY through software
>>>> configuration. This allows the PHY to act as a Type-C multiplexer for
>>>> both data role and DP altmode configuration.
>>>>
>>>> To reflect this hardware design, this patch introduces a new
>>>> "mode-switch" property for the dp-port node in the device tree bindings.
>>>> This property indicates that the connected PHY is capable of handling
>>>> Type-C mode switching itself.
>>>>
>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>
>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>
>>>> Changes in v4:
>>>> - Remove "|" in description.
>>>>
>>>> Changes in v3:
>>>> - Add more descriptions to clarify the role of the PHY in switching.
>>>>
>>>> Changes in v2:
>>>> - Reuse dp-port/usb3-port in rk3399-typec-phy binding.
>>>>
>>>>    .../devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml  | 6 ++++++
>>>>    1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>>>> index 91c011f68cd0..83ebcde096ea 100644
>>>> --- a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>>>> +++ b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>>>> @@ -51,6 +51,12 @@ properties:
>>>>          '#phy-cells':
>>>>            const: 0
>>>> +      mode-switch:
>>> Having the mode-switch here is a bit strange. I think the whole PHY
>>> device should be an orientation-switch and mode-switch. Otherwise it
>>> feels weird to me.
>> I think this is a difference in practice. In the previous binding, there was already an orientation-switch under the usb-port. I trying to add both an orientation-switch and a mode-switch to the top-level device in v2. And Krzysztof reminded me that adding a mode-switch under the dp-port would be better, so I changed it to the current form :)
> I couldn't find the comment on lore. Could you please point it out?

Sorry, it is v1. I added an orientation-switch and a mode-switch in the top-level PHY [0]. Comment is here: [1]


[0] https://lore.kernel.org/all/20250715112456.101-4-kernel@airkyi.com/

[1] https://lore.kernel.org/all/4dfed94c-665d-4e04-b527-ddd34fd3db8f@kernel.org/



>>
>>
>>>> +        description:
>>>> +          Indicates the PHY can handle altmode switching. In this case,
>>>> +          requires an external USB Type-C controller to report USB PD message.
>>>> +        type: boolean
>>>> +
>>>>          port:
>>>>            $ref: /schemas/graph.yaml#/properties/port
>>>>            description: Connection to USB Type-C connector
>>>> -- 
>>>> 2.49.0
>>>>
>> -- 
>> Best,
>> Chaoyi
>>
-- 
Best,
Chaoyi

Re: [PATCH v4 2/7] dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch

[Dmitry Baryshkov]

On Tue, Sep 23, 2025 at 11:40:33AM +0800, Chaoyi Chen wrote:
> On 9/23/2025 11:17 AM, Dmitry Baryshkov wrote:
> 
> > On Tue, Sep 23, 2025 at 09:53:06AM +0800, Chaoyi Chen wrote:
> > > Hi Dmitry,
> > > 
> > > On 9/23/2025 9:12 AM, Dmitry Baryshkov wrote:
> > > > On Mon, Sep 22, 2025 at 09:20:34AM +0800, Chaoyi Chen wrote:
> > > > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > 
> > > > > The RK3399 SoC integrates two USB/DP combo PHYs, each of which
> > > > > supports software-configurable pin mapping and DisplayPort lane
> > > > > assignment. These capabilities enable the PHY itself to handle both
> > > > > mode switching and orientation switching, based on the Type-C plug
> > > > > orientation and USB PD negotiation results.
> > > > > 
> > > > > While an external Type-C controller is still required to detect cable
> > > > > attachment and report USB PD events, the actual mode and orientation
> > > > > switching is performed internally by the PHY through software
> > > > > configuration. This allows the PHY to act as a Type-C multiplexer for
> > > > > both data role and DP altmode configuration.
> > > > > 
> > > > > To reflect this hardware design, this patch introduces a new
> > > > > "mode-switch" property for the dp-port node in the device tree bindings.
> > > > > This property indicates that the connected PHY is capable of handling
> > > > > Type-C mode switching itself.
> > > > > 
> > > > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > 
> > > > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > > ---
> > > > > 
> > > > > Changes in v4:
> > > > > - Remove "|" in description.
> > > > > 
> > > > > Changes in v3:
> > > > > - Add more descriptions to clarify the role of the PHY in switching.
> > > > > 
> > > > > Changes in v2:
> > > > > - Reuse dp-port/usb3-port in rk3399-typec-phy binding.
> > > > > 
> > > > >    .../devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml  | 6 ++++++
> > > > >    1 file changed, 6 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> > > > > index 91c011f68cd0..83ebcde096ea 100644
> > > > > --- a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> > > > > +++ b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> > > > > @@ -51,6 +51,12 @@ properties:
> > > > >          '#phy-cells':
> > > > >            const: 0
> > > > > +      mode-switch:
> > > > Having the mode-switch here is a bit strange. I think the whole PHY
> > > > device should be an orientation-switch and mode-switch. Otherwise it
> > > > feels weird to me.
> > > I think this is a difference in practice. In the previous binding, there was already an orientation-switch under the usb-port. I trying to add both an orientation-switch and a mode-switch to the top-level device in v2. And Krzysztof reminded me that adding a mode-switch under the dp-port would be better, so I changed it to the current form :)
> > I couldn't find the comment on lore. Could you please point it out?
> 
> Sorry, it is v1. I added an orientation-switch and a mode-switch in the top-level PHY [0]. Comment is here: [1]

My interpretation of [1] doesn't quite match yours. It doesn't say
anything about moving mode-switch to dp-port. It basically pointed out
that you already have two ports.

Also, I'm not sure how the current construction works: you register
switch and mux for the dev_fwnode(tcphy->dev), however the lookfup
functions should be looking for a device corresponding to the port OF
node (which doesn't exist).

> 
> 
> [0] https://lore.kernel.org/all/20250715112456.101-4-kernel@airkyi.com/
> 
> [1] https://lore.kernel.org/all/4dfed94c-665d-4e04-b527-ddd34fd3db8f@kernel.org/
> 
> 
> 
> > > 
> > > 
> > > > > +        description:
> > > > > +          Indicates the PHY can handle altmode switching. In this case,
> > > > > +          requires an external USB Type-C controller to report USB PD message.
> > > > > +        type: boolean
> > > > > +
> > > > >          port:
> > > > >            $ref: /schemas/graph.yaml#/properties/port
> > > > >            description: Connection to USB Type-C connector
> > > > > -- 
> > > > > 2.49.0
> > > > > 
> > > -- 
> > > Best,
> > > Chaoyi
> > > 
> -- 
> Best,
> Chaoyi
> 
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
With best wishes
Dmitry

Re: [PATCH v4 2/7] dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch

[Chaoyi Chen]

On 9/23/2025 12:51 PM, Dmitry Baryshkov wrote:

> On Tue, Sep 23, 2025 at 11:40:33AM +0800, Chaoyi Chen wrote:
>> On 9/23/2025 11:17 AM, Dmitry Baryshkov wrote:
>>
>>> On Tue, Sep 23, 2025 at 09:53:06AM +0800, Chaoyi Chen wrote:
>>>> Hi Dmitry,
>>>>
>>>> On 9/23/2025 9:12 AM, Dmitry Baryshkov wrote:
>>>>> On Mon, Sep 22, 2025 at 09:20:34AM +0800, Chaoyi Chen wrote:
>>>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>
>>>>>> The RK3399 SoC integrates two USB/DP combo PHYs, each of which
>>>>>> supports software-configurable pin mapping and DisplayPort lane
>>>>>> assignment. These capabilities enable the PHY itself to handle both
>>>>>> mode switching and orientation switching, based on the Type-C plug
>>>>>> orientation and USB PD negotiation results.
>>>>>>
>>>>>> While an external Type-C controller is still required to detect cable
>>>>>> attachment and report USB PD events, the actual mode and orientation
>>>>>> switching is performed internally by the PHY through software
>>>>>> configuration. This allows the PHY to act as a Type-C multiplexer for
>>>>>> both data role and DP altmode configuration.
>>>>>>
>>>>>> To reflect this hardware design, this patch introduces a new
>>>>>> "mode-switch" property for the dp-port node in the device tree bindings.
>>>>>> This property indicates that the connected PHY is capable of handling
>>>>>> Type-C mode switching itself.
>>>>>>
>>>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>
>>>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>> ---
>>>>>>
>>>>>> Changes in v4:
>>>>>> - Remove "|" in description.
>>>>>>
>>>>>> Changes in v3:
>>>>>> - Add more descriptions to clarify the role of the PHY in switching.
>>>>>>
>>>>>> Changes in v2:
>>>>>> - Reuse dp-port/usb3-port in rk3399-typec-phy binding.
>>>>>>
>>>>>>     .../devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml  | 6 ++++++
>>>>>>     1 file changed, 6 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>>>>>> index 91c011f68cd0..83ebcde096ea 100644
>>>>>> --- a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
>>>>>> @@ -51,6 +51,12 @@ properties:
>>>>>>           '#phy-cells':
>>>>>>             const: 0
>>>>>> +      mode-switch:
>>>>> Having the mode-switch here is a bit strange. I think the whole PHY
>>>>> device should be an orientation-switch and mode-switch. Otherwise it
>>>>> feels weird to me.
>>>> I think this is a difference in practice. In the previous binding, there was already an orientation-switch under the usb-port. I trying to add both an orientation-switch and a mode-switch to the top-level device in v2. And Krzysztof reminded me that adding a mode-switch under the dp-port would be better, so I changed it to the current form :)
>>> I couldn't find the comment on lore. Could you please point it out?
>> Sorry, it is v1. I added an orientation-switch and a mode-switch in the top-level PHY [0]. Comment is here: [1]
> My interpretation of [1] doesn't quite match yours. It doesn't say
> anything about moving mode-switch to dp-port. It basically pointed out
> that you already have two ports.

Yes, I think this can be easily changed, as long as the issue you mentioned below is resolved.


>
> Also, I'm not sure how the current construction works: you register
> switch and mux for the dev_fwnode(tcphy->dev), however the lookfup
> functions should be looking for a device corresponding to the port OF
> node (which doesn't exist).

In v1, that is fwnode = dev_fwnode(tcphy->dev) .  And dt like is:


&tcphy {
     port {
         tcphy0_orientation_switch: endpoint@0 { ... };
         tcphy_dp_altmode_switch: endpoint@1 { ... };
     };
};


Since the binding already includes a "usb3-port" and a "dp-port", it can not add another new port.

So after v1, that is fwnode = device_get_named_child_node(tcphy->dev, "usb3-port") . And dt like this:


&tcphy0_dp {
     port { ... };
};

&tcphy0_usb3 {
     port { ... };
};


Sorry, this looks a bit hacky. Do you have a better idea? Thank you.


>
>>
>> [0] https://lore.kernel.org/all/20250715112456.101-4-kernel@airkyi.com/
>>
>> [1] https://lore.kernel.org/all/4dfed94c-665d-4e04-b527-ddd34fd3db8f@kernel.org/
>>
>>
>>
>>>>
>>>>>> +        description:
>>>>>> +          Indicates the PHY can handle altmode switching. In this case,
>>>>>> +          requires an external USB Type-C controller to report USB PD message.
>>>>>> +        type: boolean
>>>>>> +
>>>>>>           port:
>>>>>>             $ref: /schemas/graph.yaml#/properties/port
>>>>>>             description: Connection to USB Type-C connector
>>>>>> -- 
>>>>>> 2.49.0
>>>>>>
>>>> -- 
>>>> Best,
>>>> Chaoyi
>>>>
>> -- 
>> Best,
>> Chaoyi
>>
>>
>> -- 
>> linux-phy mailing list
>> linux-phy@lists.infradead.org
>> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
Best,
Chaoyi

Re: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode

[Chaoyi Chen]

On 9/23/2025 11:11 AM, Dmitry Baryshkov wrote:

> On Tue, Sep 23, 2025 at 09:34:39AM +0800, Chaoyi Chen wrote:
>> On 9/23/2025 9:10 AM, Dmitry Baryshkov wrote:
>>
>>> On Mon, Sep 22, 2025 at 09:20:33AM +0800, Chaoyi Chen wrote:
>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>
>>>> Add default DRM AUX HPD bridge device when register DisplayPort
>>>> altmode. That makes it redundant for each Type-C driver to implement
>>>> a similar registration process in embedded scenarios.
>>>>
>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>> ---
>>>>    drivers/usb/typec/altmodes/displayport.c | 27 ++++++++++++++++++++++++
>>>>    drivers/usb/typec/altmodes/displayport.h |  2 ++
>>>>    drivers/usb/typec/class.c                |  8 +++++++
>>>>    include/linux/usb/typec_altmode.h        |  2 ++
>>>>    4 files changed, 39 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
>>>> index 1dcb77faf85d..e026dc6e5430 100644
>>>> --- a/drivers/usb/typec/altmodes/displayport.c
>>>> +++ b/drivers/usb/typec/altmodes/displayport.c
>>>> @@ -14,6 +14,7 @@
>>>>    #include <linux/property.h>
>>>>    #include <linux/usb/pd_vdo.h>
>>>>    #include <linux/usb/typec_dp.h>
>>>> +#include <drm/bridge/aux-bridge.h>
>>>>    #include <drm/drm_connector.h>
>>>>    #include "displayport.h"
>>>> @@ -182,6 +183,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
>>>>    				dp->pending_irq_hpd = true;
>>>>    		}
>>>>    	} else {
>>>> +		if (dp->port->hpd_dev)
>>>> +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>>>> +						  hpd ? connector_status_connected :
>>>> +							connector_status_disconnected);
>>> There should be no need for these calls. Once the HPD bridge is added to
>>> a correct fwnode, the drm_connector_oob_hotplug_event() calls should
>>> deliver the signal as expected.
>> It seems that only drm_bridge_connector can do this. I'm not sure if I remember correctly. I'll give it a try.
> Other connectors can implement the .oob_hotplug_event call. Calling
> drm_bridge_hpd_notify() also depends on the connector setting the
> callbacks via drm_bridge_hpd_enable(), a step which is done by only a
> few drivers.

Hmm, let's go over this again. First, drm_connector_oob_hotplug_event() requires a connector fwnode.

On the Qualcomm platforms, the fwnode corresponds to the USB-C controller device node, so

drm_connector_oob_hotplug_event(dp->connector_fwnode, ..) can handle them directly.

But our platform doesn't use the USB-C controller device node as drm connector fwnode :(

So I use drm_dp_hpd_bridge_register() and drm_aux_hpd_bridge_notify() here, I think it just create a simple hpd bridge to bridge_list.

But drm_connector_oob_hotplug_event() use connector_list instead of bridge_list.



>
>>
>>>>    		drm_connector_oob_hotplug_event(dp->connector_fwnode,
>>>>    						hpd ? connector_status_connected :
>>>>    						      connector_status_disconnected);
>>>> @@ -206,6 +211,9 @@ static int dp_altmode_configured(struct dp_altmode *dp)
>>>>    	 * configuration is complete to signal HPD.
>>>>    	 */
>>>>    	if (dp->pending_hpd) {
>>>> +		if (dp->port->hpd_dev)
>>>> +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>>>> +						  connector_status_connected);
>>>>    		drm_connector_oob_hotplug_event(dp->connector_fwnode,
>>>>    						connector_status_connected);
>>>>    		sysfs_notify(&dp->alt->dev.kobj, "displayport", "hpd");
>>>> @@ -391,6 +399,9 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
>>>>    			dp->data.status = 0;
>>>>    			dp->data.conf = 0;
>>>>    			if (dp->hpd) {
>>>> +				if (dp->port->hpd_dev)
>>>> +					drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>>>> +								  connector_status_disconnected);
>>>>    				drm_connector_oob_hotplug_event(dp->connector_fwnode,
>>>>    								connector_status_disconnected);
>>>>    				dp->hpd = false;
>>>> @@ -751,6 +762,18 @@ static const struct attribute_group *displayport_groups[] = {
>>>>    	NULL,
>>>>    };
>>>> +void dp_altmode_hpd_device_register(struct typec_altmode *alt)
>>>> +{
>>>> +	if (alt->svid != USB_TYPEC_DP_SID)
>>>> +		return;
>>>> +
>>>> +	alt->hpd_dev = drm_dp_hpd_bridge_register(alt->dev.parent->parent,
>>>> +						  dev_of_node(alt->dev.parent->parent));
>>> This needs at least a comment, what is dev.parent->parent. Also, the
>>> of_node is not correct here. It should be a node of the connector,
>>> rather than the device itself. Consider USB-C controllers which handle
>>> several USB-C connectors (e.g. UCSI). The DRM core won't be able to
>>> identify the correct bridge.
>> I think  alt.dev->parent->parent is the connector device. Am I missing something?
> As I wrote, it needs a comment (in the source file). No, it's not a
> connector device, it's a USB-C controller device. There is no guarantee
> that there is a separate struct device for the USB-C connector. On
> Qualcomm platforms, the device will point to the USB-C controller (TCPM
> or UCSI), which contain usb-c-connector(s) as child node(s) in DT.

Thanks for the clarification.



>
>>
>>
>>>> +	if (IS_ERR(alt->hpd_dev))
>>>> +		alt->hpd_dev = NULL;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(dp_altmode_hpd_device_register);
>>> Having the function here will bring a typec -> displayport dependency
>>> between drivers (which you didn't document). It means it won't be
>>> possible to build typec core into the kernel, having the DP AltMode
>>> driver in the module (which also doesn't sound like a good idea).
>> It make sense. Perhaps moving it into class.c would be a good idea.
>>
>>
>>>> +
>>>>    int dp_altmode_probe(struct typec_altmode *alt)
>>>>    {
>>>>    	const struct typec_altmode *port = typec_altmode_get_partner(alt);
>> -- 
>> Best,
>> Chaoyi
>>
>>
>> -- 
>> linux-phy mailing list
>> linux-phy@lists.infradead.org
>> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
Best,
Chaoyi

Re: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode

[Dmitry Baryshkov]

On Tue, Sep 23, 2025 at 05:07:25PM +0800, Chaoyi Chen wrote:
> On 9/23/2025 11:11 AM, Dmitry Baryshkov wrote:
> 
> > On Tue, Sep 23, 2025 at 09:34:39AM +0800, Chaoyi Chen wrote:
> > > On 9/23/2025 9:10 AM, Dmitry Baryshkov wrote:
> > > 
> > > > On Mon, Sep 22, 2025 at 09:20:33AM +0800, Chaoyi Chen wrote:
> > > > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > 
> > > > > Add default DRM AUX HPD bridge device when register DisplayPort
> > > > > altmode. That makes it redundant for each Type-C driver to implement
> > > > > a similar registration process in embedded scenarios.
> > > > > 
> > > > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > ---
> > > > >    drivers/usb/typec/altmodes/displayport.c | 27 ++++++++++++++++++++++++
> > > > >    drivers/usb/typec/altmodes/displayport.h |  2 ++
> > > > >    drivers/usb/typec/class.c                |  8 +++++++
> > > > >    include/linux/usb/typec_altmode.h        |  2 ++
> > > > >    4 files changed, 39 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> > > > > index 1dcb77faf85d..e026dc6e5430 100644
> > > > > --- a/drivers/usb/typec/altmodes/displayport.c
> > > > > +++ b/drivers/usb/typec/altmodes/displayport.c
> > > > > @@ -14,6 +14,7 @@
> > > > >    #include <linux/property.h>
> > > > >    #include <linux/usb/pd_vdo.h>
> > > > >    #include <linux/usb/typec_dp.h>
> > > > > +#include <drm/bridge/aux-bridge.h>
> > > > >    #include <drm/drm_connector.h>
> > > > >    #include "displayport.h"
> > > > > @@ -182,6 +183,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
> > > > >    				dp->pending_irq_hpd = true;
> > > > >    		}
> > > > >    	} else {
> > > > > +		if (dp->port->hpd_dev)
> > > > > +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> > > > > +						  hpd ? connector_status_connected :
> > > > > +							connector_status_disconnected);
> > > > There should be no need for these calls. Once the HPD bridge is added to
> > > > a correct fwnode, the drm_connector_oob_hotplug_event() calls should
> > > > deliver the signal as expected.
> > > It seems that only drm_bridge_connector can do this. I'm not sure if I remember correctly. I'll give it a try.
> > Other connectors can implement the .oob_hotplug_event call. Calling
> > drm_bridge_hpd_notify() also depends on the connector setting the
> > callbacks via drm_bridge_hpd_enable(), a step which is done by only a
> > few drivers.
> 
> Hmm, let's go over this again. First, drm_connector_oob_hotplug_event() requires a connector fwnode.
> 
> On the Qualcomm platforms, the fwnode corresponds to the USB-C controller device node, so
> 
> drm_connector_oob_hotplug_event(dp->connector_fwnode, ..) can handle them directly.
> 
> But our platform doesn't use the USB-C controller device node as drm connector fwnode :(

This sounds like an issue to be fixed. Alternative option would be make
the AltMode code find your fwnode and report OOB events against it.
But... I reallly think that using connector's fwnode is the cleanest
solution. In the end, your final 'display' connector is the USB-C
connector present on the board. If your display has a USB-C connector,
that will be the socket that gets the cable from the display, etc.

> 
> So I use drm_dp_hpd_bridge_register() and drm_aux_hpd_bridge_notify() here, I think it just create a simple hpd bridge to bridge_list.
> 
> But drm_connector_oob_hotplug_event() use connector_list instead of bridge_list.

The OOB interface was created by x86 people, but we successfully reused
it. I think that addign drm_bridge_hpd_notify() calls just duplicates
the effort unnecessarily.

> 
> 
> 
> > 
> > > 
> > > > >    		drm_connector_oob_hotplug_event(dp->connector_fwnode,
> > > > >    						hpd ? connector_status_connected :
> > > > >    						      connector_status_disconnected);
> > > > > @@ -206,6 +211,9 @@ static int dp_altmode_configured(struct dp_altmode *dp)
> > > > >    	 * configuration is complete to signal HPD.
> > > > >    	 */
> > > > >    	if (dp->pending_hpd) {
> > > > > +		if (dp->port->hpd_dev)
> > > > > +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> > > > > +						  connector_status_connected);
> > > > >    		drm_connector_oob_hotplug_event(dp->connector_fwnode,
> > > > >    						connector_status_connected);
> > > > >    		sysfs_notify(&dp->alt->dev.kobj, "displayport", "hpd");
> > > > > @@ -391,6 +399,9 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
> > > > >    			dp->data.status = 0;
> > > > >    			dp->data.conf = 0;
> > > > >    			if (dp->hpd) {
> > > > > +				if (dp->port->hpd_dev)
> > > > > +					drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> > > > > +								  connector_status_disconnected);
> > > > >    				drm_connector_oob_hotplug_event(dp->connector_fwnode,
> > > > >    								connector_status_disconnected);
> > > > >    				dp->hpd = false;
> > > > > @@ -751,6 +762,18 @@ static const struct attribute_group *displayport_groups[] = {
> > > > >    	NULL,
> > > > >    };
> > > > > +void dp_altmode_hpd_device_register(struct typec_altmode *alt)
> > > > > +{
> > > > > +	if (alt->svid != USB_TYPEC_DP_SID)
> > > > > +		return;
> > > > > +
> > > > > +	alt->hpd_dev = drm_dp_hpd_bridge_register(alt->dev.parent->parent,
> > > > > +						  dev_of_node(alt->dev.parent->parent));
> > > > This needs at least a comment, what is dev.parent->parent. Also, the
> > > > of_node is not correct here. It should be a node of the connector,
> > > > rather than the device itself. Consider USB-C controllers which handle
> > > > several USB-C connectors (e.g. UCSI). The DRM core won't be able to
> > > > identify the correct bridge.
> > > I think  alt.dev->parent->parent is the connector device. Am I missing something?
> > As I wrote, it needs a comment (in the source file). No, it's not a
> > connector device, it's a USB-C controller device. There is no guarantee
> > that there is a separate struct device for the USB-C connector. On
> > Qualcomm platforms, the device will point to the USB-C controller (TCPM
> > or UCSI), which contain usb-c-connector(s) as child node(s) in DT.
> 
> Thanks for the clarification.

I think it should be fine to pass the fwnode of the usb-c connector that
is outside of the USB-C controller device (if that's what your platform
uses). But I think this should be:
- the usb-c-connector node
- it should be coming from the Type-C controller driver, you can't guess
  it here.

> 
> 
> 
> > 
> > > 
> > > 
> > > > > +	if (IS_ERR(alt->hpd_dev))
> > > > > +		alt->hpd_dev = NULL;
> > > > > +}
> > > > > +EXPORT_SYMBOL_GPL(dp_altmode_hpd_device_register);
> > > > Having the function here will bring a typec -> displayport dependency
> > > > between drivers (which you didn't document). It means it won't be
> > > > possible to build typec core into the kernel, having the DP AltMode
> > > > driver in the module (which also doesn't sound like a good idea).
> > > It make sense. Perhaps moving it into class.c would be a good idea.
> > > 
> > > 
> > > > > +
> > > > >    int dp_altmode_probe(struct typec_altmode *alt)
> > > > >    {
> > > > >    	const struct typec_altmode *port = typec_altmode_get_partner(alt);
> > > -- 
> > > Best,
> > > Chaoyi
> > > 
> > > 
> > > -- 
> > > linux-phy mailing list
> > > linux-phy@lists.infradead.org
> > > https://lists.infradead.org/mailman/listinfo/linux-phy
> 
> -- 
> Best,
> Chaoyi
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 2/7] dt-bindings: phy: rockchip: rk3399-typec-phy: Support mode-switch

[Dmitry Baryshkov]

On Tue, Sep 23, 2025 at 03:17:20PM +0800, Chaoyi Chen wrote:
> On 9/23/2025 12:51 PM, Dmitry Baryshkov wrote:
> 
> > On Tue, Sep 23, 2025 at 11:40:33AM +0800, Chaoyi Chen wrote:
> > > On 9/23/2025 11:17 AM, Dmitry Baryshkov wrote:
> > > 
> > > > On Tue, Sep 23, 2025 at 09:53:06AM +0800, Chaoyi Chen wrote:
> > > > > Hi Dmitry,
> > > > > 
> > > > > On 9/23/2025 9:12 AM, Dmitry Baryshkov wrote:
> > > > > > On Mon, Sep 22, 2025 at 09:20:34AM +0800, Chaoyi Chen wrote:
> > > > > > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > > 
> > > > > > > The RK3399 SoC integrates two USB/DP combo PHYs, each of which
> > > > > > > supports software-configurable pin mapping and DisplayPort lane
> > > > > > > assignment. These capabilities enable the PHY itself to handle both
> > > > > > > mode switching and orientation switching, based on the Type-C plug
> > > > > > > orientation and USB PD negotiation results.
> > > > > > > 
> > > > > > > While an external Type-C controller is still required to detect cable
> > > > > > > attachment and report USB PD events, the actual mode and orientation
> > > > > > > switching is performed internally by the PHY through software
> > > > > > > configuration. This allows the PHY to act as a Type-C multiplexer for
> > > > > > > both data role and DP altmode configuration.
> > > > > > > 
> > > > > > > To reflect this hardware design, this patch introduces a new
> > > > > > > "mode-switch" property for the dp-port node in the device tree bindings.
> > > > > > > This property indicates that the connected PHY is capable of handling
> > > > > > > Type-C mode switching itself.
> > > > > > > 
> > > > > > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > > 
> > > > > > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > > > > ---
> > > > > > > 
> > > > > > > Changes in v4:
> > > > > > > - Remove "|" in description.
> > > > > > > 
> > > > > > > Changes in v3:
> > > > > > > - Add more descriptions to clarify the role of the PHY in switching.
> > > > > > > 
> > > > > > > Changes in v2:
> > > > > > > - Reuse dp-port/usb3-port in rk3399-typec-phy binding.
> > > > > > > 
> > > > > > >     .../devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml  | 6 ++++++
> > > > > > >     1 file changed, 6 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> > > > > > > index 91c011f68cd0..83ebcde096ea 100644
> > > > > > > --- a/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/phy/rockchip,rk3399-typec-phy.yaml
> > > > > > > @@ -51,6 +51,12 @@ properties:
> > > > > > >           '#phy-cells':
> > > > > > >             const: 0
> > > > > > > +      mode-switch:
> > > > > > Having the mode-switch here is a bit strange. I think the whole PHY
> > > > > > device should be an orientation-switch and mode-switch. Otherwise it
> > > > > > feels weird to me.
> > > > > I think this is a difference in practice. In the previous binding, there was already an orientation-switch under the usb-port. I trying to add both an orientation-switch and a mode-switch to the top-level device in v2. And Krzysztof reminded me that adding a mode-switch under the dp-port would be better, so I changed it to the current form :)
> > > > I couldn't find the comment on lore. Could you please point it out?
> > > Sorry, it is v1. I added an orientation-switch and a mode-switch in the top-level PHY [0]. Comment is here: [1]
> > My interpretation of [1] doesn't quite match yours. It doesn't say
> > anything about moving mode-switch to dp-port. It basically pointed out
> > that you already have two ports.
> 
> Yes, I think this can be easily changed, as long as the issue you mentioned below is resolved.
> 
> 
> > 
> > Also, I'm not sure how the current construction works: you register
> > switch and mux for the dev_fwnode(tcphy->dev), however the lookfup
> > functions should be looking for a device corresponding to the port OF
> > node (which doesn't exist).
> 
> In v1, that is fwnode = dev_fwnode(tcphy->dev) .  And dt like is:
> 
> 
> &tcphy {
>     port {
>         tcphy0_orientation_switch: endpoint@0 { ... };
>         tcphy_dp_altmode_switch: endpoint@1 { ... };
>     };
> };
> 
> 
> Since the binding already includes a "usb3-port" and a "dp-port", it can not add another new port.
> 
> So after v1, that is fwnode = device_get_named_child_node(tcphy->dev, "usb3-port") . And dt like this:

I see it now, I was probably looking at the different revision, sorry
about it.

> 
> 
> &tcphy0_dp {
>     port { ... };
> };
> 
> &tcphy0_usb3 {
>     port { ... };
> };
> 
> 
> Sorry, this looks a bit hacky. Do you have a better idea? Thank you.

No, it's fine from my POV now.

> 
> 
> > 
> > > 
> > > [0] https://lore.kernel.org/all/20250715112456.101-4-kernel@airkyi.com/
> > > 
> > > [1] https://lore.kernel.org/all/4dfed94c-665d-4e04-b527-ddd34fd3db8f@kernel.org/
> > > 
> > > 
> > > 
> > > > > 
> > > > > > > +        description:
> > > > > > > +          Indicates the PHY can handle altmode switching. In this case,
> > > > > > > +          requires an external USB Type-C controller to report USB PD message.
> > > > > > > +        type: boolean
> > > > > > > +
> > > > > > >           port:
> > > > > > >             $ref: /schemas/graph.yaml#/properties/port
> > > > > > >             description: Connection to USB Type-C connector
> > > > > > > -- 
> > > > > > > 2.49.0
> > > > > > > 
> > > > > -- 
> > > > > Best,
> > > > > Chaoyi
> > > > > 
> > > -- 
> > > Best,
> > > Chaoyi
> > > 
> > > 
> > > -- 
> > > linux-phy mailing list
> > > linux-phy@lists.infradead.org
> > > https://lists.infradead.org/mailman/listinfo/linux-phy
> 
> -- 
> Best,
> Chaoyi
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode

[Chaoyi Chen]

On 9/23/2025 6:40 PM, Dmitry Baryshkov wrote:

> On Tue, Sep 23, 2025 at 05:07:25PM +0800, Chaoyi Chen wrote:
>> On 9/23/2025 11:11 AM, Dmitry Baryshkov wrote:
>>
>>> On Tue, Sep 23, 2025 at 09:34:39AM +0800, Chaoyi Chen wrote:
>>>> On 9/23/2025 9:10 AM, Dmitry Baryshkov wrote:
>>>>
>>>>> On Mon, Sep 22, 2025 at 09:20:33AM +0800, Chaoyi Chen wrote:
>>>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>
>>>>>> Add default DRM AUX HPD bridge device when register DisplayPort
>>>>>> altmode. That makes it redundant for each Type-C driver to implement
>>>>>> a similar registration process in embedded scenarios.
>>>>>>
>>>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>> ---
>>>>>>     drivers/usb/typec/altmodes/displayport.c | 27 ++++++++++++++++++++++++
>>>>>>     drivers/usb/typec/altmodes/displayport.h |  2 ++
>>>>>>     drivers/usb/typec/class.c                |  8 +++++++
>>>>>>     include/linux/usb/typec_altmode.h        |  2 ++
>>>>>>     4 files changed, 39 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
>>>>>> index 1dcb77faf85d..e026dc6e5430 100644
>>>>>> --- a/drivers/usb/typec/altmodes/displayport.c
>>>>>> +++ b/drivers/usb/typec/altmodes/displayport.c
>>>>>> @@ -14,6 +14,7 @@
>>>>>>     #include <linux/property.h>
>>>>>>     #include <linux/usb/pd_vdo.h>
>>>>>>     #include <linux/usb/typec_dp.h>
>>>>>> +#include <drm/bridge/aux-bridge.h>
>>>>>>     #include <drm/drm_connector.h>
>>>>>>     #include "displayport.h"
>>>>>> @@ -182,6 +183,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
>>>>>>     				dp->pending_irq_hpd = true;
>>>>>>     		}
>>>>>>     	} else {
>>>>>> +		if (dp->port->hpd_dev)
>>>>>> +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>>>>>> +						  hpd ? connector_status_connected :
>>>>>> +							connector_status_disconnected);
>>>>> There should be no need for these calls. Once the HPD bridge is added to
>>>>> a correct fwnode, the drm_connector_oob_hotplug_event() calls should
>>>>> deliver the signal as expected.
>>>> It seems that only drm_bridge_connector can do this. I'm not sure if I remember correctly. I'll give it a try.
>>> Other connectors can implement the .oob_hotplug_event call. Calling
>>> drm_bridge_hpd_notify() also depends on the connector setting the
>>> callbacks via drm_bridge_hpd_enable(), a step which is done by only a
>>> few drivers.
>> Hmm, let's go over this again. First, drm_connector_oob_hotplug_event() requires a connector fwnode.
>>
>> On the Qualcomm platforms, the fwnode corresponds to the USB-C controller device node, so
>>
>> drm_connector_oob_hotplug_event(dp->connector_fwnode, ..) can handle them directly.
>>
>> But our platform doesn't use the USB-C controller device node as drm connector fwnode :(
> This sounds like an issue to be fixed. Alternative option would be make
> the AltMode code find your fwnode and report OOB events against it.
> But... I reallly think that using connector's fwnode is the cleanest
> solution. In the end, your final 'display' connector is the USB-C
> connector present on the board. If your display has a USB-C connector,
> that will be the socket that gets the cable from the display, etc.
>
>> So I use drm_dp_hpd_bridge_register() and drm_aux_hpd_bridge_notify() here, I think it just create a simple hpd bridge to bridge_list.
>>
>> But drm_connector_oob_hotplug_event() use connector_list instead of bridge_list.
> The OOB interface was created by x86 people, but we successfully reused
> it. I think that addign drm_bridge_hpd_notify() calls just duplicates
> the effort unnecessarily.

Yes, that commit comment said,  "It was proposed to add the displayport OF property to the DT bindings, but it was rejected in favor of properly describing the electrical signal path using of_graph."

But in the embedded case, we don't seem to have the opportunity to describe this kind of of_graph relationship between drm connector and usb connector in usb-connector.yaml. On the Qualcomm platform, the DRM connector fwnode to correspond to the USB-C controller, which is a clean solution.

However, on our platform we are using external USB-C controllers. In v4 and the previous versions, I focused on directly linking the USB-C controller with the DP controller. Referring to your suggest in [0], I think maybe this can be achieved with the help of the drm bridge chain. Assuming the bridge chain is like this:


Other birdges ... ->PHY drm aux hpd bridge -> CDN-DP bridge -> DP to HDMI bridge or other bridge or nothing...


We can use drm_bridge_chain_get_first_bridge() to get first bridge. In this case, that is drm aux hpd bridge from USB-C controller device. Next, we can obtain the fwnode corresponding to this bridge, and once we have it, we can set the connector's fwnode to it. In this way, drm_connector_oob_hotplug_event() can take effect.


Would this be a good idea? Thanks.


[0] https://lore.kernel.org/all/p3kgqn3euumhysckh4yyqavqv5y6any5zcrgkrcg3j5a7z7cyw@lfpkla5p3put/


>
>>
>>
>>>>>>     		drm_connector_oob_hotplug_event(dp->connector_fwnode,
>>>>>>     						hpd ? connector_status_connected :
>>>>>>     						      connector_status_disconnected);
>>>>>> @@ -206,6 +211,9 @@ static int dp_altmode_configured(struct dp_altmode *dp)
>>>>>>     	 * configuration is complete to signal HPD.
>>>>>>     	 */
>>>>>>     	if (dp->pending_hpd) {
>>>>>> +		if (dp->port->hpd_dev)
>>>>>> +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>>>>>> +						  connector_status_connected);
>>>>>>     		drm_connector_oob_hotplug_event(dp->connector_fwnode,
>>>>>>     						connector_status_connected);
>>>>>>     		sysfs_notify(&dp->alt->dev.kobj, "displayport", "hpd");
>>>>>> @@ -391,6 +399,9 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
>>>>>>     			dp->data.status = 0;
>>>>>>     			dp->data.conf = 0;
>>>>>>     			if (dp->hpd) {
>>>>>> +				if (dp->port->hpd_dev)
>>>>>> +					drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
>>>>>> +								  connector_status_disconnected);
>>>>>>     				drm_connector_oob_hotplug_event(dp->connector_fwnode,
>>>>>>     								connector_status_disconnected);
>>>>>>     				dp->hpd = false;
>>>>>> @@ -751,6 +762,18 @@ static const struct attribute_group *displayport_groups[] = {
>>>>>>     	NULL,
>>>>>>     };
>>>>>> +void dp_altmode_hpd_device_register(struct typec_altmode *alt)
>>>>>> +{
>>>>>> +	if (alt->svid != USB_TYPEC_DP_SID)
>>>>>> +		return;
>>>>>> +
>>>>>> +	alt->hpd_dev = drm_dp_hpd_bridge_register(alt->dev.parent->parent,
>>>>>> +						  dev_of_node(alt->dev.parent->parent));
>>>>> This needs at least a comment, what is dev.parent->parent. Also, the
>>>>> of_node is not correct here. It should be a node of the connector,
>>>>> rather than the device itself. Consider USB-C controllers which handle
>>>>> several USB-C connectors (e.g. UCSI). The DRM core won't be able to
>>>>> identify the correct bridge.
>>>> I think  alt.dev->parent->parent is the connector device. Am I missing something?
>>> As I wrote, it needs a comment (in the source file). No, it's not a
>>> connector device, it's a USB-C controller device. There is no guarantee
>>> that there is a separate struct device for the USB-C connector. On
>>> Qualcomm platforms, the device will point to the USB-C controller (TCPM
>>> or UCSI), which contain usb-c-connector(s) as child node(s) in DT.
>> Thanks for the clarification.
> I think it should be fine to pass the fwnode of the usb-c connector that
> is outside of the USB-C controller device (if that's what your platform
> uses). But I think this should be:
> - the usb-c-connector node
> - it should be coming from the Type-C controller driver, you can't guess
>    it here.
>
>>
>>
>>>>
>>>>>> +	if (IS_ERR(alt->hpd_dev))
>>>>>> +		alt->hpd_dev = NULL;
>>>>>> +}
>>>>>> +EXPORT_SYMBOL_GPL(dp_altmode_hpd_device_register);
>>>>> Having the function here will bring a typec -> displayport dependency
>>>>> between drivers (which you didn't document). It means it won't be
>>>>> possible to build typec core into the kernel, having the DP AltMode
>>>>> driver in the module (which also doesn't sound like a good idea).
>>>> It make sense. Perhaps moving it into class.c would be a good idea.
>>>>
>>>>
>>>>>> +
>>>>>>     int dp_altmode_probe(struct typec_altmode *alt)
>>>>>>     {
>>>>>>     	const struct typec_altmode *port = typec_altmode_get_partner(alt);
>>>> -- 
>>>> Best,
>>>> Chaoyi
>>>>
>>>>
>>>> -- 
>>>> linux-phy mailing list
>>>> linux-phy@lists.infradead.org
>>>> https://lists.infradead.org/mailman/listinfo/linux-phy
>> -- 
>> Best,
>> Chaoyi
>>
-- 
Best,
Chaoyi

Re: [PATCH v4 1/7] usb: typec: Add default HPD device when register DisplayPort altmode

[Dmitry Baryshkov]

On Wed, Sep 24, 2025 at 05:55:39PM +0800, Chaoyi Chen wrote:
> On 9/23/2025 6:40 PM, Dmitry Baryshkov wrote:
> 
> > On Tue, Sep 23, 2025 at 05:07:25PM +0800, Chaoyi Chen wrote:
> > > On 9/23/2025 11:11 AM, Dmitry Baryshkov wrote:
> > > 
> > > > On Tue, Sep 23, 2025 at 09:34:39AM +0800, Chaoyi Chen wrote:
> > > > > On 9/23/2025 9:10 AM, Dmitry Baryshkov wrote:
> > > > > 
> > > > > > On Mon, Sep 22, 2025 at 09:20:33AM +0800, Chaoyi Chen wrote:
> > > > > > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > > 
> > > > > > > Add default DRM AUX HPD bridge device when register DisplayPort
> > > > > > > altmode. That makes it redundant for each Type-C driver to implement
> > > > > > > a similar registration process in embedded scenarios.
> > > > > > > 
> > > > > > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > > ---
> > > > > > >     drivers/usb/typec/altmodes/displayport.c | 27 ++++++++++++++++++++++++
> > > > > > >     drivers/usb/typec/altmodes/displayport.h |  2 ++
> > > > > > >     drivers/usb/typec/class.c                |  8 +++++++
> > > > > > >     include/linux/usb/typec_altmode.h        |  2 ++
> > > > > > >     4 files changed, 39 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> > > > > > > index 1dcb77faf85d..e026dc6e5430 100644
> > > > > > > --- a/drivers/usb/typec/altmodes/displayport.c
> > > > > > > +++ b/drivers/usb/typec/altmodes/displayport.c
> > > > > > > @@ -14,6 +14,7 @@
> > > > > > >     #include <linux/property.h>
> > > > > > >     #include <linux/usb/pd_vdo.h>
> > > > > > >     #include <linux/usb/typec_dp.h>
> > > > > > > +#include <drm/bridge/aux-bridge.h>
> > > > > > >     #include <drm/drm_connector.h>
> > > > > > >     #include "displayport.h"
> > > > > > > @@ -182,6 +183,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
> > > > > > >     				dp->pending_irq_hpd = true;
> > > > > > >     		}
> > > > > > >     	} else {
> > > > > > > +		if (dp->port->hpd_dev)
> > > > > > > +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> > > > > > > +						  hpd ? connector_status_connected :
> > > > > > > +							connector_status_disconnected);
> > > > > > There should be no need for these calls. Once the HPD bridge is added to
> > > > > > a correct fwnode, the drm_connector_oob_hotplug_event() calls should
> > > > > > deliver the signal as expected.
> > > > > It seems that only drm_bridge_connector can do this. I'm not sure if I remember correctly. I'll give it a try.
> > > > Other connectors can implement the .oob_hotplug_event call. Calling
> > > > drm_bridge_hpd_notify() also depends on the connector setting the
> > > > callbacks via drm_bridge_hpd_enable(), a step which is done by only a
> > > > few drivers.
> > > Hmm, let's go over this again. First, drm_connector_oob_hotplug_event() requires a connector fwnode.
> > > 
> > > On the Qualcomm platforms, the fwnode corresponds to the USB-C controller device node, so
> > > 
> > > drm_connector_oob_hotplug_event(dp->connector_fwnode, ..) can handle them directly.
> > > 
> > > But our platform doesn't use the USB-C controller device node as drm connector fwnode :(
> > This sounds like an issue to be fixed. Alternative option would be make
> > the AltMode code find your fwnode and report OOB events against it.
> > But... I reallly think that using connector's fwnode is the cleanest
> > solution. In the end, your final 'display' connector is the USB-C
> > connector present on the board. If your display has a USB-C connector,
> > that will be the socket that gets the cable from the display, etc.
> > 
> > > So I use drm_dp_hpd_bridge_register() and drm_aux_hpd_bridge_notify() here, I think it just create a simple hpd bridge to bridge_list.
> > > 
> > > But drm_connector_oob_hotplug_event() use connector_list instead of bridge_list.
> > The OOB interface was created by x86 people, but we successfully reused
> > it. I think that addign drm_bridge_hpd_notify() calls just duplicates
> > the effort unnecessarily.
> 
> Yes, that commit comment said,  "It was proposed to add the displayport OF property to the DT bindings, but it was rejected in favor of properly describing the electrical signal path using of_graph."
> 
> But in the embedded case, we don't seem to have the opportunity to describe this kind of of_graph relationship between drm connector and usb connector in usb-connector.yaml. On the Qualcomm platform, the DRM connector fwnode to correspond to the USB-C controller, which is a clean solution.
> 
> However, on our platform we are using external USB-C controllers. In v4 and the previous versions, I focused on directly linking the USB-C controller with the DP controller. Referring to your suggest in [0], I think maybe this can be achieved with the help of the drm bridge chain. Assuming the bridge chain is like this:
> 
> 
> Other birdges ... ->PHY drm aux hpd bridge -> CDN-DP bridge -> DP to HDMI bridge or other bridge or nothing...

Looks good to me.

> We can use drm_bridge_chain_get_first_bridge() to get first bridge. In this case, that is drm aux hpd bridge from USB-C controller device. Next, we can obtain the fwnode corresponding to this bridge, and once we have it, we can set the connector's fwnode to it. In this way, drm_connector_oob_hotplug_event() can take effect.

drm_bridge_chain_get_last_bridge(), yes. That's what
drm_bridge_connector is doing. You'd need to make sure that there is a
drm_aux_hpd_bridge() registered for the USB-C connector node (from your
Type-C controller driver or from the altmode driver as per your patch).

> 
> 
> Would this be a good idea? Thanks.
> 
> 
> [0] https://lore.kernel.org/all/p3kgqn3euumhysckh4yyqavqv5y6any5zcrgkrcg3j5a7z7cyw@lfpkla5p3put/
> 
> 
> > 
> > > 
> > > 
> > > > > > >     		drm_connector_oob_hotplug_event(dp->connector_fwnode,
> > > > > > >     						hpd ? connector_status_connected :
> > > > > > >     						      connector_status_disconnected);
> > > > > > > @@ -206,6 +211,9 @@ static int dp_altmode_configured(struct dp_altmode *dp)
> > > > > > >     	 * configuration is complete to signal HPD.
> > > > > > >     	 */
> > > > > > >     	if (dp->pending_hpd) {
> > > > > > > +		if (dp->port->hpd_dev)
> > > > > > > +			drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> > > > > > > +						  connector_status_connected);
> > > > > > >     		drm_connector_oob_hotplug_event(dp->connector_fwnode,
> > > > > > >     						connector_status_connected);
> > > > > > >     		sysfs_notify(&dp->alt->dev.kobj, "displayport", "hpd");
> > > > > > > @@ -391,6 +399,9 @@ static int dp_altmode_vdm(struct typec_altmode *alt,
> > > > > > >     			dp->data.status = 0;
> > > > > > >     			dp->data.conf = 0;
> > > > > > >     			if (dp->hpd) {
> > > > > > > +				if (dp->port->hpd_dev)
> > > > > > > +					drm_aux_hpd_bridge_notify(dp->port->hpd_dev,
> > > > > > > +								  connector_status_disconnected);
> > > > > > >     				drm_connector_oob_hotplug_event(dp->connector_fwnode,
> > > > > > >     								connector_status_disconnected);
> > > > > > >     				dp->hpd = false;
> > > > > > > @@ -751,6 +762,18 @@ static const struct attribute_group *displayport_groups[] = {
> > > > > > >     	NULL,
> > > > > > >     };
> > > > > > > +void dp_altmode_hpd_device_register(struct typec_altmode *alt)
> > > > > > > +{
> > > > > > > +	if (alt->svid != USB_TYPEC_DP_SID)
> > > > > > > +		return;
> > > > > > > +
> > > > > > > +	alt->hpd_dev = drm_dp_hpd_bridge_register(alt->dev.parent->parent,
> > > > > > > +						  dev_of_node(alt->dev.parent->parent));
> > > > > > This needs at least a comment, what is dev.parent->parent. Also, the
> > > > > > of_node is not correct here. It should be a node of the connector,
> > > > > > rather than the device itself. Consider USB-C controllers which handle
> > > > > > several USB-C connectors (e.g. UCSI). The DRM core won't be able to
> > > > > > identify the correct bridge.
> > > > > I think  alt.dev->parent->parent is the connector device. Am I missing something?
> > > > As I wrote, it needs a comment (in the source file). No, it's not a
> > > > connector device, it's a USB-C controller device. There is no guarantee
> > > > that there is a separate struct device for the USB-C connector. On
> > > > Qualcomm platforms, the device will point to the USB-C controller (TCPM
> > > > or UCSI), which contain usb-c-connector(s) as child node(s) in DT.
> > > Thanks for the clarification.
> > I think it should be fine to pass the fwnode of the usb-c connector that
> > is outside of the USB-C controller device (if that's what your platform
> > uses). But I think this should be:
> > - the usb-c-connector node
> > - it should be coming from the Type-C controller driver, you can't guess
> >    it here.

-- 
With best wishes
Dmitry

Re: [PATCH v4 5/7] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection

[Chaoyi Chen]

On 9/23/2025 9:50 AM, Dmitry Baryshkov wrote:

[...]


>> +	/* One endpoint may correspond to one HPD bridge. */
>> +	for_each_of_graph_port_endpoint(port, dp_ep) {
>> +		/* Try to get "port" node of correspond PHY device */
>> +		struct device_node *phy_ep __free(device_node) =
>> +			of_graph_get_remote_endpoint(dp_ep);
>> +		struct device_node *phy_port __free(device_node) =
>> +			of_get_parent(phy_ep);
>> +
>> +		if (!phy_port) {
>> +			continue;
>> +		}
>> +
>> +		/*
>> +		 * A PHY port may contain two endpoints: USB connector port or CDN-DP port.
>> +		 * Try to find the node of USB connector.
> And then there can be a retimer between PHY and the USB-C connector. Or
> some signal MUX. Or DP-to-HDMI bridge. Please, don't parse DT for other
> devices. Instead you can add drm_aux_bridge to your PHY and let DRM core
> build the bridge chain following OF graph.
>
I think building a bridge chain across multiple drm_aux_hpd_bridge may be difficult. First, drm_dp_hpd_bridge_register() cannot register the bridge immediately; instead, it is deferred until drm_aux_hpd_bridge_probe(). When it is added to the bridge_list, it may not yet be attached, and attempting to attach it at that point is too late.

But, if I only use drm_aux_bridge on the USB-C connector, and use my own custom bridge on the PHY device and managing the alloc and attach bridge process myself, then things would become much easier.

-- 
Best,
Chaoyi

Re: [PATCH v4 5/7] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection

[Dmitry Baryshkov]

On Sun, Sep 28, 2025 at 05:52:35PM +0800, Chaoyi Chen wrote:
> On 9/23/2025 9:50 AM, Dmitry Baryshkov wrote:
> 
> [...]
> 
> 
> > > +	/* One endpoint may correspond to one HPD bridge. */
> > > +	for_each_of_graph_port_endpoint(port, dp_ep) {
> > > +		/* Try to get "port" node of correspond PHY device */
> > > +		struct device_node *phy_ep __free(device_node) =
> > > +			of_graph_get_remote_endpoint(dp_ep);
> > > +		struct device_node *phy_port __free(device_node) =
> > > +			of_get_parent(phy_ep);
> > > +
> > > +		if (!phy_port) {
> > > +			continue;
> > > +		}
> > > +
> > > +		/*
> > > +		 * A PHY port may contain two endpoints: USB connector port or CDN-DP port.
> > > +		 * Try to find the node of USB connector.
> > And then there can be a retimer between PHY and the USB-C connector. Or
> > some signal MUX. Or DP-to-HDMI bridge. Please, don't parse DT for other
> > devices. Instead you can add drm_aux_bridge to your PHY and let DRM core
> > build the bridge chain following OF graph.
> > 
> I think building a bridge chain across multiple drm_aux_hpd_bridge may be difficult. First, drm_dp_hpd_bridge_register() cannot register the bridge immediately; instead, it is deferred until drm_aux_hpd_bridge_probe(). When it is added to the bridge_list, it may not yet be attached, and attempting to attach it at that point is too late.
> 
> But, if I only use drm_aux_bridge on the USB-C connector, and use my own custom bridge on the PHY device and managing the alloc and attach bridge process myself, then things would become much easier.

Well... consider a your board, but add onnn,nb7vpq904m retimer between
the CDP and usb-c connector (it's not an uncommon device nowadays). Or
add fsa4480 analog audio switch. Build all the drivers as modules. You
should not need any changes to your drivers to handle such boards and
such kernel config.

With those devices you can't handle everything inside the DP driver,
since there are two "streams" of probe events: the DRM bridge needs the
"next" bridge (in the direction from the SoC to the connector), but the
USB-C events code needs "previous" mux, switch or retirmer. After some
trial and error we have ended up with having a chain of drm_aux_bridge
devices ending up with the drm_aux_hpd_bridge inside the Type-C port
manager driver. This way the typec_* depetencies are resolved first,
going from the SoC to the Type-C controller driver then the DRM bridge
devices probe backwards, creating the chain, which is finally consumer
by the DP driver inside the SoC.

-- 
With best wishes
Dmitry

Re: [PATCH v4 5/7] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection

[Chaoyi Chen]

On 9/29/2025 5:27 AM, Dmitry Baryshkov wrote:

> On Sun, Sep 28, 2025 at 05:52:35PM +0800, Chaoyi Chen wrote:
>> On 9/23/2025 9:50 AM, Dmitry Baryshkov wrote:
>>
>> [...]
>>
>>
>>>> +	/* One endpoint may correspond to one HPD bridge. */
>>>> +	for_each_of_graph_port_endpoint(port, dp_ep) {
>>>> +		/* Try to get "port" node of correspond PHY device */
>>>> +		struct device_node *phy_ep __free(device_node) =
>>>> +			of_graph_get_remote_endpoint(dp_ep);
>>>> +		struct device_node *phy_port __free(device_node) =
>>>> +			of_get_parent(phy_ep);
>>>> +
>>>> +		if (!phy_port) {
>>>> +			continue;
>>>> +		}
>>>> +
>>>> +		/*
>>>> +		 * A PHY port may contain two endpoints: USB connector port or CDN-DP port.
>>>> +		 * Try to find the node of USB connector.
>>> And then there can be a retimer between PHY and the USB-C connector. Or
>>> some signal MUX. Or DP-to-HDMI bridge. Please, don't parse DT for other
>>> devices. Instead you can add drm_aux_bridge to your PHY and let DRM core
>>> build the bridge chain following OF graph.
>>>
>> I think building a bridge chain across multiple drm_aux_hpd_bridge may be difficult. First, drm_dp_hpd_bridge_register() cannot register the bridge immediately; instead, it is deferred until drm_aux_hpd_bridge_probe(). When it is added to the bridge_list, it may not yet be attached, and attempting to attach it at that point is too late.
>>
>> But, if I only use drm_aux_bridge on the USB-C connector, and use my own custom bridge on the PHY device and managing the alloc and attach bridge process myself, then things would become much easier.
> Well... consider a your board, but add onnn,nb7vpq904m retimer between
> the CDP and usb-c connector (it's not an uncommon device nowadays). Or
> add fsa4480 analog audio switch. Build all the drivers as modules. You
> should not need any changes to your drivers to handle such boards and
> such kernel config.
>
> With those devices you can't handle everything inside the DP driver,
> since there are two "streams" of probe events: the DRM bridge needs the
> "next" bridge (in the direction from the SoC to the connector), but the
> USB-C events code needs "previous" mux, switch or retirmer. After some
> trial and error we have ended up with having a chain of drm_aux_bridge
> devices ending up with the drm_aux_hpd_bridge inside the Type-C port
> manager driver. This way the typec_* depetencies are resolved first,
> going from the SoC to the Type-C controller driver then the DRM bridge
> devices probe backwards, creating the chain, which is finally consumer
> by the DP driver inside the SoC.

Sorry, I kept trying to look for the "next bridge" in "drm_hpd_aux_bridge", and I didn't notice that "drm_aux_bridge" already had a similar implementation about "next bridge". Thanks again for your patience.


BTW the devm_drm_of_get_bridge(&auxdev->dev, auxdev->dev.of_node, 0, 0) in drm_aux_bridge cannot be used directly with tcphy->dev. I may need to create a device for the dp-port child node, and then use drm_aux_bridge_register(). But this is no longer a big issue :)

-- 
Best,
Chaoyi