linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
@ 2025-05-27 20:40 Konrad Dybcio
  2025-05-27 20:40 ` [PATCH v3 1/6] dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch Konrad Dybcio
                   ` (9 more replies)
  0 siblings, 10 replies; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-27 20:40 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong, Krzysztof Kozlowski,
	Dmitry Baryshkov

Register a typec mux in order to change the PHY mode on the Type-C
mux events depending on the mode and the svid when in Altmode setup.

The DisplayPort phy should be left enabled if is still powered on
by the DRM DisplayPort controller, so bail out until the DisplayPort
PHY is not powered off.

The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
will be set in between of USB-Only, Combo and DisplayPort Only so
this will leave enough time to the DRM DisplayPort controller to
turn of the DisplayPort PHY.

The patchset also includes bindings changes and DT changes.

This has been successfully tested on an SM8550 board, but the
Thinkpad X13s deserved testing between non-PD USB, non-PD DisplayPort,
PD USB Hubs and PD Altmode Dongles to make sure the switch works
as expected.

The DisplayPort 4 lanes setup can be check with:
$ cat /sys/kernel/debug/dri/ae01000.display-controller/DP-1/dp_debug
	name = msm_dp
	drm_dp_link
		rate = 540000
		num_lanes = 4
...

This patchset depends on [1] to allow broadcasting the type-c mode
to the PHY, otherwise the PHY will keep the combo state while the
retimer would setup the 4 lanes in DP mode.

[1] https://lore.kernel.org/all/20240527-topic-sm8x50-upstream-retimer-broadcast-mode-v1-0-79ec91381aba@linaro.org/
Changes in v3:
- Take the series from Neil
- Rebase
- Rename many variables
- Test on X1E & X13s
- Apply a number of small cosmetic/codestyle changes
- Remove some unused variables
- Some smaller bugfixes
- Link to v2: https://lore.kernel.org/lkml/20240527-topic-sm8x50-upstream-phy-combo-typec-mux-v2-0-a03e68d7b8fc@linaro.org/
Changes in v2:
- Reference usb-switch.yaml in bindings patch
- Fix switch/case indenting
- Check svid for USB_TYPEC_DP_SID
- Fix X13s patch subject
- Update SM8650 patch to enable 4 lanes on HDK aswell
- Link to v1: https://lore.kernel.org/r/20240229-topic-sm8x50-upstream-phy-combo-typec-mux-v1-0-07e24a231840@linaro.org

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Konrad Dybcio (1):
      phy: qcom: qmp-combo: Rename 'mode' to 'phy_mode'

Neil Armstrong (5):
      dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch
      phy: qcom: qmp-combo: store DP phy power state
      phy: qcom: qmp-combo: introduce QMPPHY_MODE
      phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
      arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: Set up 4-lane DP

 .../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml         |   7 +-
 .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |   6 +-
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c          | 182 +++++++++++++++++++--
 3 files changed, 173 insertions(+), 22 deletions(-)
---
base-commit: 460178e842c7a1e48a06df684c66eb5fd630bcf7
change-id: 20250527-topic-4ln_dp_respin-c6924a8825ce

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v3 1/6] dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch
  2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
@ 2025-05-27 20:40 ` Konrad Dybcio
  2025-05-27 20:40 ` [PATCH v3 2/6] phy: qcom: qmp-combo: Rename 'mode' to 'phy_mode' Konrad Dybcio
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-27 20:40 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong, Krzysztof Kozlowski

From: Neil Armstrong <neil.armstrong@linaro.org>

The QMP USB3/DP Combo PHY can work in 3 modes:
- DisplayPort Only
- USB3 Only
- USB3 + DisplayPort Combo mode

In order to switch between those modes, the PHY needs to receive
Type-C events, allow marking to the phy with the mode-switch
property in order to allow the PHY to Type-C events.

Reference usb-switch.yaml as a simpler way to allow the mode-switch
property instead of duplicating the property definition.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml     | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
index 358a6736a951ca5db7cff7385b3657976a667358..692dbfe26e84778fcabae45ac9fa87195f796adb 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
@@ -72,10 +72,8 @@ properties:
     description:
       See include/dt-bindings/phy/phy-qcom-qmp.h
 
-  orientation-switch:
-    description:
-      Flag the PHY as possible handler of USB Type-C orientation switching
-    type: boolean
+  mode-switch: true
+  orientation-switch: true
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -105,6 +103,7 @@ required:
   - "#phy-cells"
 
 allOf:
+  - $ref: /schemas/usb/usb-switch.yaml#
   - if:
       properties:
         compatible:

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 2/6] phy: qcom: qmp-combo: Rename 'mode' to 'phy_mode'
  2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
  2025-05-27 20:40 ` [PATCH v3 1/6] dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch Konrad Dybcio
@ 2025-05-27 20:40 ` Konrad Dybcio
  2025-05-27 21:03   ` Dmitry Baryshkov
  2025-05-27 20:40 ` [PATCH v3 3/6] phy: qcom: qmp-combo: store DP phy power state Konrad Dybcio
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-27 20:40 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

There are a numbers of ""modes"" involved: USB mode, Type-C mode (with
its altmodes), phy_mode and QMP_PHY mode (DP/combo/USB/off).

Rename the generic sounding 'mode' to 'phy_mode' to hopefully make
the code easier to follow.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index b09fa00e9fe7db8d97b7179ee15d3f07fe578b0c..d0396817f3a05c7e4baeef0de1332c3a83942a51 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -1687,7 +1687,7 @@ struct qmp_combo {
 	int init_count;
 
 	struct phy *usb_phy;
-	enum phy_mode mode;
+	enum phy_mode phy_mode;
 	unsigned int usb_init_count;
 
 	struct phy *dp_phy;
@@ -3062,7 +3062,7 @@ static int qmp_combo_usb_set_mode(struct phy *phy, enum phy_mode mode, int submo
 {
 	struct qmp_combo *qmp = phy_get_drvdata(phy);
 
-	qmp->mode = mode;
+	qmp->phy_mode = mode;
 
 	return 0;
 }
@@ -3091,8 +3091,8 @@ static void qmp_combo_enable_autonomous_mode(struct qmp_combo *qmp)
 	void __iomem *pcs_misc = qmp->pcs_misc;
 	u32 intr_mask;
 
-	if (qmp->mode == PHY_MODE_USB_HOST_SS ||
-	    qmp->mode == PHY_MODE_USB_DEVICE_SS)
+	if (qmp->phy_mode == PHY_MODE_USB_HOST_SS ||
+	    qmp->phy_mode == PHY_MODE_USB_DEVICE_SS)
 		intr_mask = ARCVR_DTCT_EN | ALFPS_DTCT_EN;
 	else
 		intr_mask = ARCVR_DTCT_EN | ARCVR_DTCT_EVENT_SEL;
@@ -3135,7 +3135,7 @@ static int __maybe_unused qmp_combo_runtime_suspend(struct device *dev)
 {
 	struct qmp_combo *qmp = dev_get_drvdata(dev);
 
-	dev_vdbg(dev, "Suspending QMP phy, mode:%d\n", qmp->mode);
+	dev_vdbg(dev, "Suspending QMP phy, mode:%d\n", qmp->phy_mode);
 
 	if (!qmp->init_count) {
 		dev_vdbg(dev, "PHY not initialized, bailing out\n");
@@ -3155,7 +3155,7 @@ static int __maybe_unused qmp_combo_runtime_resume(struct device *dev)
 	struct qmp_combo *qmp = dev_get_drvdata(dev);
 	int ret = 0;
 
-	dev_vdbg(dev, "Resuming QMP phy, mode:%d\n", qmp->mode);
+	dev_vdbg(dev, "Resuming QMP phy, mode:%d\n", qmp->phy_mode);
 
 	if (!qmp->init_count) {
 		dev_vdbg(dev, "PHY not initialized, bailing out\n");

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 3/6] phy: qcom: qmp-combo: store DP phy power state
  2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
  2025-05-27 20:40 ` [PATCH v3 1/6] dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch Konrad Dybcio
  2025-05-27 20:40 ` [PATCH v3 2/6] phy: qcom: qmp-combo: Rename 'mode' to 'phy_mode' Konrad Dybcio
@ 2025-05-27 20:40 ` Konrad Dybcio
  2025-05-27 20:40 ` [PATCH v3 4/6] phy: qcom: qmp-combo: introduce QMPPHY_MODE Konrad Dybcio
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-27 20:40 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong, Dmitry Baryshkov

From: Neil Armstrong <neil.armstrong@linaro.org>

Switching the PHY Mode requires the DisplayPort PHY to be powered off,
keep track of the DisplayPort phy power state.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index d0396817f3a05c7e4baeef0de1332c3a83942a51..26480d63322b22eca9637392f888bfbe440c2a13 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -1694,6 +1694,7 @@ struct qmp_combo {
 	unsigned int dp_aux_cfg;
 	struct phy_configure_opts_dp dp_opts;
 	unsigned int dp_init_count;
+	bool dp_powered_on;
 
 	struct clk_fixed_rate pipe_clk_fixed;
 	struct clk_hw dp_link_hw;
@@ -2913,6 +2914,8 @@ static int qmp_combo_dp_power_on(struct phy *phy)
 	/* Configure link rate, swing, etc. */
 	cfg->configure_dp_phy(qmp);
 
+	qmp->dp_powered_on = true;
+
 	mutex_unlock(&qmp->phy_mutex);
 
 	return 0;
@@ -2927,6 +2930,8 @@ static int qmp_combo_dp_power_off(struct phy *phy)
 	/* Assert DP PHY power down */
 	writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
 
+	qmp->dp_powered_on = false;
+
 	mutex_unlock(&qmp->phy_mutex);
 
 	return 0;

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 4/6] phy: qcom: qmp-combo: introduce QMPPHY_MODE
  2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
                   ` (2 preceding siblings ...)
  2025-05-27 20:40 ` [PATCH v3 3/6] phy: qcom: qmp-combo: store DP phy power state Konrad Dybcio
@ 2025-05-27 20:40 ` Konrad Dybcio
  2025-05-27 20:40 ` [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE Konrad Dybcio
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-27 20:40 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong

From: Neil Armstrong <neil.armstrong@linaro.org>

Introduce an enum for the QMP Combo PHY modes, use it in the
QMP commmon phy init function and default to COMBO mode.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
[konrad: some renaming and rewording]
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 44 +++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 26480d63322b22eca9637392f888bfbe440c2a13..b34ad92021a656b39562e2685a1e7a0a93660a35 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -61,6 +61,12 @@
 
 #define PHY_INIT_COMPLETE_TIMEOUT		10000
 
+enum qmpphy_mode {
+	QMPPHY_MODE_USB3DP = 0,
+	QMPPHY_MODE_DP_ONLY,
+	QMPPHY_MODE_USB3_ONLY,
+};
+
 /* set of registers with offsets different per-PHY */
 enum qphy_reg_layout {
 	/* PCS registers */
@@ -1685,6 +1691,7 @@ struct qmp_combo {
 
 	struct mutex phy_mutex;
 	int init_count;
+	enum qmpphy_mode qmpphy_mode;
 
 	struct phy *usb_phy;
 	enum phy_mode phy_mode;
@@ -2817,12 +2824,33 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
 	if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
 		val |= SW_PORTSELECT_VAL;
 	writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
-	writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
 
-	/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
-	qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
-			SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
-			SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
+	switch (qmp->qmpphy_mode) {
+	case QMPPHY_MODE_USB3DP:
+		writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
+
+		/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
+		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
+				SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
+				SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
+		break;
+
+	case QMPPHY_MODE_DP_ONLY:
+		writel(DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
+
+		/* bring QMP DP PHY PCS block out of reset */
+		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
+				SW_DPPHY_RESET_MUX | SW_DPPHY_RESET);
+		break;
+
+	case QMPPHY_MODE_USB3_ONLY:
+		writel(USB3_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
+
+		/* bring QMP USB PHY PCS block out of reset */
+		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
+				SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
+		break;
+	}
 
 	qphy_clrbits(com, QPHY_V3_DP_COM_SWI_CTRL, 0x03);
 	qphy_clrbits(com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
@@ -3833,6 +3861,12 @@ static int qmp_combo_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
+	/*
+	 * The hw default is USB3_ONLY, but USB3+DP mode lets us more easily
+	 * check both sub-blocks' init tables for blunders at probe time.
+	 */
+	qmp->qmpphy_mode = QMPPHY_MODE_USB3DP;
+
 	qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
 	if (IS_ERR(qmp->usb_phy)) {
 		ret = PTR_ERR(qmp->usb_phy);

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
  2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
                   ` (3 preceding siblings ...)
  2025-05-27 20:40 ` [PATCH v3 4/6] phy: qcom: qmp-combo: introduce QMPPHY_MODE Konrad Dybcio
@ 2025-05-27 20:40 ` Konrad Dybcio
  2025-05-27 21:55   ` Dmitry Baryshkov
  2025-05-27 20:40 ` [PATCH v3 6/6] arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: Set up 4-lane DP Konrad Dybcio
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-27 20:40 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong

From: Neil Armstrong <neil.armstrong@linaro.org>

Register a typec mux in order to change the PHY mode on the Type-C
mux events depending on the mode and the svid when in Altmode setup.

The DisplayPort phy should be left enabled if is still powered on
by the DRM DisplayPort controller, so bail out until the DisplayPort
PHY is not powered off.

The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
will be set in between of USB-Only, Combo and DisplayPort Only so
this will leave enough time to the DRM DisplayPort controller to
turn of the DisplayPort PHY.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
[konrad: renaming, rewording, bug fixes]
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 121 ++++++++++++++++++++++++++++--
 1 file changed, 116 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index b34ad92021a656b39562e2685a1e7a0a93660a35..4c9d92d6e0b8508191d052bd85dd135e4b8d7cc7 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -19,6 +19,7 @@
 #include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/usb/typec.h>
+#include <linux/usb/typec_dp.h>
 #include <linux/usb/typec_mux.h>
 
 #include <drm/bridge/aux-bridge.h>
@@ -1709,6 +1710,8 @@ struct qmp_combo {
 
 	struct typec_switch_dev *sw;
 	enum typec_orientation orientation;
+
+	struct typec_mux_dev *mux;
 };
 
 static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
@@ -3582,17 +3585,112 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
 	return 0;
 }
 
-static void qmp_combo_typec_unregister(void *data)
+static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *state)
+{
+	struct qmp_combo *qmp = typec_mux_get_drvdata(mux);
+	const struct qmp_phy_cfg *cfg = qmp->cfg;
+	enum qmpphy_mode new_mode;
+	unsigned int svid;
+
+	guard(mutex)(&qmp->phy_mutex);
+
+	if (state->alt)
+		svid = state->alt->svid;
+	else
+		svid = 0;
+
+	if (svid == USB_TYPEC_DP_SID) {
+		switch (state->mode) {
+		/* DP Only */
+		case TYPEC_DP_STATE_C:
+		case TYPEC_DP_STATE_E:
+			new_mode = QMPPHY_MODE_DP_ONLY;
+			break;
+
+		/* DP + USB */
+		case TYPEC_DP_STATE_D:
+		case TYPEC_DP_STATE_F:
+
+		/* Safe fallback...*/
+		default:
+			new_mode = QMPPHY_MODE_USB3DP;
+			break;
+		}
+	} else {
+		/* Fall back to USB3+DP mode if we're not sure it's strictly USB3-only */
+		if (state->mode == TYPEC_MODE_USB3 || state->mode == TYPEC_STATE_USB)
+			new_mode = QMPPHY_MODE_USB3_ONLY;
+		else
+			new_mode = QMPPHY_MODE_USB3DP;
+	}
+
+	if (new_mode == qmp->qmpphy_mode) {
+		dev_dbg(qmp->dev, "typec_mux_set: same qmpphy mode, bail out\n");
+		return 0;
+	}
+
+	if (qmp->qmpphy_mode != QMPPHY_MODE_USB3_ONLY && qmp->dp_powered_on) {
+		dev_dbg(qmp->dev, "typec_mux_set: DP PHY is still in use, delaying switch\n");
+		return 0;
+	}
+
+	dev_dbg(qmp->dev, "typec_mux_set: switching from qmpphy mode %d to %d\n",
+		qmp->qmpphy_mode, new_mode);
+
+	qmp->qmpphy_mode = new_mode;
+
+	if (qmp->init_count) {
+		if (qmp->usb_init_count)
+			qmp_combo_usb_power_off(qmp->usb_phy);
+
+		if (qmp->dp_init_count)
+			writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
+
+		qmp_combo_com_exit(qmp, true);
+
+		/* Now everything's powered down, power up the right PHYs */
+		qmp_combo_com_init(qmp, true);
+
+		if (new_mode == QMPPHY_MODE_DP_ONLY) {
+			if (qmp->usb_init_count)
+				qmp->usb_init_count--;
+		}
+
+		if (new_mode == QMPPHY_MODE_USB3DP || new_mode == QMPPHY_MODE_USB3_ONLY) {
+			qmp_combo_usb_power_on(qmp->usb_phy);
+			if (!qmp->usb_init_count)
+				qmp->usb_init_count++;
+		}
+
+		if (new_mode == QMPPHY_MODE_DP_ONLY || new_mode == QMPPHY_MODE_USB3DP) {
+			if (qmp->dp_init_count)
+				cfg->dp_aux_init(qmp);
+		}
+	}
+
+	return 0;
+}
+
+static void qmp_combo_typec_switch_unregister(void *data)
 {
 	struct qmp_combo *qmp = data;
 
 	typec_switch_unregister(qmp->sw);
 }
 
-static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
+static void qmp_combo_typec_mux_unregister(void *data)
+{
+	struct qmp_combo *qmp = data;
+
+	typec_mux_unregister(qmp->mux);
+}
+
+static int qmp_combo_typec_register(struct qmp_combo *qmp)
 {
 	struct typec_switch_desc sw_desc = {};
+	struct typec_mux_desc mux_desc = { };
 	struct device *dev = qmp->dev;
+	int ret;
 
 	sw_desc.drvdata = qmp;
 	sw_desc.fwnode = dev->fwnode;
@@ -3603,10 +3701,23 @@ static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
 		return PTR_ERR(qmp->sw);
 	}
 
-	return devm_add_action_or_reset(dev, qmp_combo_typec_unregister, qmp);
+	ret = devm_add_action_or_reset(dev, qmp_combo_typec_switch_unregister, qmp);
+	if (ret)
+		return ret;
+
+	mux_desc.drvdata = qmp;
+	mux_desc.fwnode = dev->fwnode;
+	mux_desc.set = qmp_combo_typec_mux_set;
+	qmp->mux = typec_mux_register(dev, &mux_desc);
+	if (IS_ERR(qmp->mux)) {
+		dev_err(dev, "Unable to register typec mux: %pe\n", qmp->mux);
+		return PTR_ERR(qmp->mux);
+	}
+
+	return devm_add_action_or_reset(dev, qmp_combo_typec_mux_unregister, qmp);
 }
 #else
-static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
+static int qmp_combo_typec_register(struct qmp_combo *qmp)
 {
 	return 0;
 }
@@ -3839,7 +3950,7 @@ static int qmp_combo_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
-	ret = qmp_combo_typec_switch_register(qmp);
+	ret = qmp_combo_typec_register(qmp);
 	if (ret)
 		goto err_node_put;
 

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 6/6] arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: Set up 4-lane DP
  2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
                   ` (4 preceding siblings ...)
  2025-05-27 20:40 ` [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE Konrad Dybcio
@ 2025-05-27 20:40 ` Konrad Dybcio
  2025-05-27 21:10 ` [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Dmitry Baryshkov
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-27 20:40 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong, Dmitry Baryshkov

From: Neil Armstrong <neil.armstrong@linaro.org>

Allow up to 4 lanes for the DisplayPort link from the PHYs to the
controllers and allow mode-switch events to reach the QMP Combo PHYs.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
[konrad: reword]
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index ae7a275fd2236a2c71808b003fbcb66687e6e45e..336704e2a806e74c98a8dd483d8da0311df95389 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -726,7 +726,7 @@ &mdss0_dp0 {
 };
 
 &mdss0_dp0_out {
-	data-lanes = <0 1>;
+	data-lanes = <0 1 2 3>;
 	remote-endpoint = <&usb_0_qmpphy_dp_in>;
 };
 
@@ -735,7 +735,7 @@ &mdss0_dp1 {
 };
 
 &mdss0_dp1_out {
-	data-lanes = <0 1>;
+	data-lanes = <0 1 2 3>;
 	remote-endpoint = <&usb_1_qmpphy_dp_in>;
 };
 
@@ -1358,6 +1358,7 @@ &usb_0_qmpphy {
 	vdda-phy-supply = <&vreg_l9d>;
 	vdda-pll-supply = <&vreg_l4d>;
 
+	mode-switch;
 	orientation-switch;
 
 	status = "okay";
@@ -1395,6 +1396,7 @@ &usb_1_qmpphy {
 	vdda-phy-supply = <&vreg_l4b>;
 	vdda-pll-supply = <&vreg_l3b>;
 
+	mode-switch;
 	orientation-switch;
 
 	status = "okay";

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 2/6] phy: qcom: qmp-combo: Rename 'mode' to 'phy_mode'
  2025-05-27 20:40 ` [PATCH v3 2/6] phy: qcom: qmp-combo: Rename 'mode' to 'phy_mode' Konrad Dybcio
@ 2025-05-27 21:03   ` Dmitry Baryshkov
  0 siblings, 0 replies; 29+ messages in thread
From: Dmitry Baryshkov @ 2025-05-27 21:03 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio

On Tue, May 27, 2025 at 10:40:04PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> There are a numbers of ""modes"" involved: USB mode, Type-C mode (with
> its altmodes), phy_mode and QMP_PHY mode (DP/combo/USB/off).
> 
> Rename the generic sounding 'mode' to 'phy_mode' to hopefully make
> the code easier to follow.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
                   ` (5 preceding siblings ...)
  2025-05-27 20:40 ` [PATCH v3 6/6] arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: Set up 4-lane DP Konrad Dybcio
@ 2025-05-27 21:10 ` Dmitry Baryshkov
  2025-05-27 22:24   ` Konrad Dybcio
  2025-05-28 12:37 ` Rob Herring (Arm)
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Dmitry Baryshkov @ 2025-05-27 21:10 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong, Krzysztof Kozlowski

On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
> Register a typec mux in order to change the PHY mode on the Type-C
> mux events depending on the mode and the svid when in Altmode setup.
> 
> The DisplayPort phy should be left enabled if is still powered on
> by the DRM DisplayPort controller, so bail out until the DisplayPort
> PHY is not powered off.

This series doesn't seem to solve the USB side of a problem. When the
PHY is being switch to the 4-lane mode, USB controller looses PIPE
clock, so it needs to be switched to the USB-2 mode.

> 
> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
> will be set in between of USB-Only, Combo and DisplayPort Only so
> this will leave enough time to the DRM DisplayPort controller to
> turn of the DisplayPort PHY.
> 
> The patchset also includes bindings changes and DT changes.
> 
> This has been successfully tested on an SM8550 board, but the
> Thinkpad X13s deserved testing between non-PD USB, non-PD DisplayPort,
> PD USB Hubs and PD Altmode Dongles to make sure the switch works
> as expected.
> 
> The DisplayPort 4 lanes setup can be check with:
> $ cat /sys/kernel/debug/dri/ae01000.display-controller/DP-1/dp_debug
> 	name = msm_dp
> 	drm_dp_link
> 		rate = 540000
> 		num_lanes = 4
> ...
> 
> This patchset depends on [1] to allow broadcasting the type-c mode
> to the PHY, otherwise the PHY will keep the combo state while the
> retimer would setup the 4 lanes in DP mode.
> 
> [1] https://lore.kernel.org/all/20240527-topic-sm8x50-upstream-retimer-broadcast-mode-v1-0-79ec91381aba@linaro.org/
> Changes in v3:
> - Take the series from Neil
> - Rebase
> - Rename many variables
> - Test on X1E & X13s
> - Apply a number of small cosmetic/codestyle changes
> - Remove some unused variables
> - Some smaller bugfixes
> - Link to v2: https://lore.kernel.org/lkml/20240527-topic-sm8x50-upstream-phy-combo-typec-mux-v2-0-a03e68d7b8fc@linaro.org/
> Changes in v2:
> - Reference usb-switch.yaml in bindings patch
> - Fix switch/case indenting
> - Check svid for USB_TYPEC_DP_SID
> - Fix X13s patch subject
> - Update SM8650 patch to enable 4 lanes on HDK aswell
> - Link to v1: https://lore.kernel.org/r/20240229-topic-sm8x50-upstream-phy-combo-typec-mux-v1-0-07e24a231840@linaro.org
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> Konrad Dybcio (1):
>       phy: qcom: qmp-combo: Rename 'mode' to 'phy_mode'
> 
> Neil Armstrong (5):
>       dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch
>       phy: qcom: qmp-combo: store DP phy power state
>       phy: qcom: qmp-combo: introduce QMPPHY_MODE
>       phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
>       arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: Set up 4-lane DP
> 
>  .../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml         |   7 +-
>  .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |   6 +-
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c          | 182 +++++++++++++++++++--
>  3 files changed, 173 insertions(+), 22 deletions(-)
> ---
> base-commit: 460178e842c7a1e48a06df684c66eb5fd630bcf7
> change-id: 20250527-topic-4ln_dp_respin-c6924a8825ce
> 
> Best regards,
> -- 
> Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
  2025-05-27 20:40 ` [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE Konrad Dybcio
@ 2025-05-27 21:55   ` Dmitry Baryshkov
  2025-05-27 22:22     ` Konrad Dybcio
  0 siblings, 1 reply; 29+ messages in thread
From: Dmitry Baryshkov @ 2025-05-27 21:55 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong

On Tue, May 27, 2025 at 10:40:07PM +0200, Konrad Dybcio wrote:
> From: Neil Armstrong <neil.armstrong@linaro.org>
> 
> Register a typec mux in order to change the PHY mode on the Type-C
> mux events depending on the mode and the svid when in Altmode setup.
> 
> The DisplayPort phy should be left enabled if is still powered on
> by the DRM DisplayPort controller, so bail out until the DisplayPort
> PHY is not powered off.
> 
> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
> will be set in between of USB-Only, Combo and DisplayPort Only so
> this will leave enough time to the DRM DisplayPort controller to
> turn of the DisplayPort PHY.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> [konrad: renaming, rewording, bug fixes]
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 121 ++++++++++++++++++++++++++++--
>  1 file changed, 116 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index b34ad92021a656b39562e2685a1e7a0a93660a35..4c9d92d6e0b8508191d052bd85dd135e4b8d7cc7 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -19,6 +19,7 @@
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/usb/typec.h>
> +#include <linux/usb/typec_dp.h>
>  #include <linux/usb/typec_mux.h>
>  
>  #include <drm/bridge/aux-bridge.h>
> @@ -1709,6 +1710,8 @@ struct qmp_combo {
>  
>  	struct typec_switch_dev *sw;
>  	enum typec_orientation orientation;
> +
> +	struct typec_mux_dev *mux;
>  };
>  
>  static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
> @@ -3582,17 +3585,112 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
>  	return 0;
>  }
>  
> -static void qmp_combo_typec_unregister(void *data)
> +static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *state)
> +{
> +	struct qmp_combo *qmp = typec_mux_get_drvdata(mux);
> +	const struct qmp_phy_cfg *cfg = qmp->cfg;
> +	enum qmpphy_mode new_mode;
> +	unsigned int svid;
> +
> +	guard(mutex)(&qmp->phy_mutex);
> +
> +	if (state->alt)
> +		svid = state->alt->svid;
> +	else
> +		svid = 0;
> +
> +	if (svid == USB_TYPEC_DP_SID) {
> +		switch (state->mode) {
> +		/* DP Only */
> +		case TYPEC_DP_STATE_C:
> +		case TYPEC_DP_STATE_E:
> +			new_mode = QMPPHY_MODE_DP_ONLY;
> +			break;
> +
> +		/* DP + USB */
> +		case TYPEC_DP_STATE_D:
> +		case TYPEC_DP_STATE_F:
> +
> +		/* Safe fallback...*/
> +		default:
> +			new_mode = QMPPHY_MODE_USB3DP;
> +			break;
> +		}
> +	} else {
> +		/* Fall back to USB3+DP mode if we're not sure it's strictly USB3-only */

Why? if the SID is not DP, then there can't be a DP stream.

> +		if (state->mode == TYPEC_MODE_USB3 || state->mode == TYPEC_STATE_USB)
> +			new_mode = QMPPHY_MODE_USB3_ONLY;
> +		else
> +			new_mode = QMPPHY_MODE_USB3DP;
> +	}
> +
> +	if (new_mode == qmp->qmpphy_mode) {
> +		dev_dbg(qmp->dev, "typec_mux_set: same qmpphy mode, bail out\n");
> +		return 0;
> +	}
> +
> +	if (qmp->qmpphy_mode != QMPPHY_MODE_USB3_ONLY && qmp->dp_powered_on) {
> +		dev_dbg(qmp->dev, "typec_mux_set: DP PHY is still in use, delaying switch\n");
> +		return 0;
> +	}

Consider the following scenario: connect DP dongle, use modetest to
setup DP stream, disconnect dongle, connect USB3 device. What would be
the actual state of the PHY? Modetest is still running, so DP stream is
not going to be shut down from the driver.

I think we might need a generic notifier from the PHY to the user,
telling that the PHY is going away (or just that the PHY is changing the
state). Then it would be usable by both the DP and USB drivers to let
them know that they should toggle the state.

> +
> +	dev_dbg(qmp->dev, "typec_mux_set: switching from qmpphy mode %d to %d\n",
> +		qmp->qmpphy_mode, new_mode);
> +
> +	qmp->qmpphy_mode = new_mode;
> +
> +	if (qmp->init_count) {
> +		if (qmp->usb_init_count)
> +			qmp_combo_usb_power_off(qmp->usb_phy);
> +
> +		if (qmp->dp_init_count)
> +			writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
> +
> +		qmp_combo_com_exit(qmp, true);
> +
> +		/* Now everything's powered down, power up the right PHYs */
> +		qmp_combo_com_init(qmp, true);
> +
> +		if (new_mode == QMPPHY_MODE_DP_ONLY) {
> +			if (qmp->usb_init_count)
> +				qmp->usb_init_count--;
> +		}
> +
> +		if (new_mode == QMPPHY_MODE_USB3DP || new_mode == QMPPHY_MODE_USB3_ONLY) {
> +			qmp_combo_usb_power_on(qmp->usb_phy);
> +			if (!qmp->usb_init_count)
> +				qmp->usb_init_count++;
> +		}
> +
> +		if (new_mode == QMPPHY_MODE_DP_ONLY || new_mode == QMPPHY_MODE_USB3DP) {
> +			if (qmp->dp_init_count)
> +				cfg->dp_aux_init(qmp);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void qmp_combo_typec_switch_unregister(void *data)
>  {
>  	struct qmp_combo *qmp = data;
>  
>  	typec_switch_unregister(qmp->sw);
>  }
>  
> -static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
> +static void qmp_combo_typec_mux_unregister(void *data)
> +{
> +	struct qmp_combo *qmp = data;
> +
> +	typec_mux_unregister(qmp->mux);
> +}
> +
> +static int qmp_combo_typec_register(struct qmp_combo *qmp)
>  {
>  	struct typec_switch_desc sw_desc = {};
> +	struct typec_mux_desc mux_desc = { };
>  	struct device *dev = qmp->dev;
> +	int ret;
>  
>  	sw_desc.drvdata = qmp;
>  	sw_desc.fwnode = dev->fwnode;
> @@ -3603,10 +3701,23 @@ static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
>  		return PTR_ERR(qmp->sw);
>  	}
>  
> -	return devm_add_action_or_reset(dev, qmp_combo_typec_unregister, qmp);
> +	ret = devm_add_action_or_reset(dev, qmp_combo_typec_switch_unregister, qmp);
> +	if (ret)
> +		return ret;
> +
> +	mux_desc.drvdata = qmp;
> +	mux_desc.fwnode = dev->fwnode;
> +	mux_desc.set = qmp_combo_typec_mux_set;
> +	qmp->mux = typec_mux_register(dev, &mux_desc);
> +	if (IS_ERR(qmp->mux)) {
> +		dev_err(dev, "Unable to register typec mux: %pe\n", qmp->mux);
> +		return PTR_ERR(qmp->mux);
> +	}
> +
> +	return devm_add_action_or_reset(dev, qmp_combo_typec_mux_unregister, qmp);
>  }
>  #else
> -static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
> +static int qmp_combo_typec_register(struct qmp_combo *qmp)
>  {
>  	return 0;
>  }
> @@ -3839,7 +3950,7 @@ static int qmp_combo_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_node_put;
>  
> -	ret = qmp_combo_typec_switch_register(qmp);
> +	ret = qmp_combo_typec_register(qmp);
>  	if (ret)
>  		goto err_node_put;
>  
> 
> -- 
> 2.49.0
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
  2025-05-27 21:55   ` Dmitry Baryshkov
@ 2025-05-27 22:22     ` Konrad Dybcio
  2025-05-28  8:58       ` Dmitry Baryshkov
  0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-27 22:22 UTC (permalink / raw)
  To: Dmitry Baryshkov, Konrad Dybcio
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong

On 5/27/25 11:55 PM, Dmitry Baryshkov wrote:
> On Tue, May 27, 2025 at 10:40:07PM +0200, Konrad Dybcio wrote:
>> From: Neil Armstrong <neil.armstrong@linaro.org>
>>
>> Register a typec mux in order to change the PHY mode on the Type-C
>> mux events depending on the mode and the svid when in Altmode setup.
>>
>> The DisplayPort phy should be left enabled if is still powered on
>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>> PHY is not powered off.
>>
>> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
>> will be set in between of USB-Only, Combo and DisplayPort Only so
>> this will leave enough time to the DRM DisplayPort controller to
>> turn of the DisplayPort PHY.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> [konrad: renaming, rewording, bug fixes]
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> ---

[...]

>> +	} else {
>> +		/* Fall back to USB3+DP mode if we're not sure it's strictly USB3-only */
> 
> Why? if the SID is not DP, then there can't be a DP stream.
> 
>> +		if (state->mode == TYPEC_MODE_USB3 || state->mode == TYPEC_STATE_USB)
>> +			new_mode = QMPPHY_MODE_USB3_ONLY;
>> +		else
>> +			new_mode = QMPPHY_MODE_USB3DP;

To be honest I don't really know.. Neil chose to do that, but I don't
think there's a strict requirement.. Should we default to 4ln-USB3?

[...]

> Consider the following scenario: connect DP dongle, use modetest to
> setup DP stream, disconnect dongle, connect USB3 device. What would be
> the actual state of the PHY? Modetest is still running, so DP stream is
> not going to be shut down from the driver.
> 
> I think we might need a generic notifier from the PHY to the user,
> telling that the PHY is going away (or just that the PHY is changing the
> state). Then it would be usable by both the DP and USB drivers to let
> them know that they should toggle the state.


If modetest won't stop running even though there was a DP unplug
(and therefore presumably a destruction of the display), I don't
think things are designed very well

Konrad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-05-27 21:10 ` [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Dmitry Baryshkov
@ 2025-05-27 22:24   ` Konrad Dybcio
  2025-05-28  9:00     ` Dmitry Baryshkov
  0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-27 22:24 UTC (permalink / raw)
  To: Dmitry Baryshkov, Konrad Dybcio
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong, Krzysztof Kozlowski

On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
> On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
>> Register a typec mux in order to change the PHY mode on the Type-C
>> mux events depending on the mode and the svid when in Altmode setup.
>>
>> The DisplayPort phy should be left enabled if is still powered on
>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>> PHY is not powered off.
> 
> This series doesn't seem to solve the USB side of a problem. When the
> PHY is being switch to the 4-lane mode, USB controller looses PIPE
> clock, so it needs to be switched to the USB-2 mode.

I didn't find issues with that on X13s.. Not sure if it's related, but
on the SL7, after plugging in a 4ln DP connection, I need to plug in
the USB thumb drive twice for the first time (only in that sequence)

But there's nothing interesting in dmesg and the phy seems to be
programmed with the same values on both the initial and the subsequent
working plug-in

Konrad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
  2025-05-27 22:22     ` Konrad Dybcio
@ 2025-05-28  8:58       ` Dmitry Baryshkov
  2025-05-28 11:21         ` Konrad Dybcio
  2025-06-02  8:51         ` Neil Armstrong
  0 siblings, 2 replies; 29+ messages in thread
From: Dmitry Baryshkov @ 2025-05-28  8:58 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Neil Armstrong

On Wed, May 28, 2025 at 12:22:01AM +0200, Konrad Dybcio wrote:
> On 5/27/25 11:55 PM, Dmitry Baryshkov wrote:
> > On Tue, May 27, 2025 at 10:40:07PM +0200, Konrad Dybcio wrote:
> >> From: Neil Armstrong <neil.armstrong@linaro.org>
> >>
> >> Register a typec mux in order to change the PHY mode on the Type-C
> >> mux events depending on the mode and the svid when in Altmode setup.
> >>
> >> The DisplayPort phy should be left enabled if is still powered on
> >> by the DRM DisplayPort controller, so bail out until the DisplayPort
> >> PHY is not powered off.
> >>
> >> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
> >> will be set in between of USB-Only, Combo and DisplayPort Only so
> >> this will leave enough time to the DRM DisplayPort controller to
> >> turn of the DisplayPort PHY.
> >>
> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> >> [konrad: renaming, rewording, bug fixes]
> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >> ---
> 
> [...]
> 
> >> +	} else {
> >> +		/* Fall back to USB3+DP mode if we're not sure it's strictly USB3-only */
> > 
> > Why? if the SID is not DP, then there can't be a DP stream.
> > 
> >> +		if (state->mode == TYPEC_MODE_USB3 || state->mode == TYPEC_STATE_USB)
> >> +			new_mode = QMPPHY_MODE_USB3_ONLY;
> >> +		else
> >> +			new_mode = QMPPHY_MODE_USB3DP;
> 
> To be honest I don't really know.. Neil chose to do that, but I don't
> think there's a strict requirement.. Should we default to 4ln-USB3?

Yes, QMPPHY_MODE_USB3_ONLY. Nit: there is no 4ln-USB3 (it is a special
mode). We handle 2ln-USB3 only.

> 
> [...]
> 
> > Consider the following scenario: connect DP dongle, use modetest to
> > setup DP stream, disconnect dongle, connect USB3 device. What would be
> > the actual state of the PHY? Modetest is still running, so DP stream is
> > not going to be shut down from the driver.
> > 
> > I think we might need a generic notifier from the PHY to the user,
> > telling that the PHY is going away (or just that the PHY is changing the
> > state). Then it would be usable by both the DP and USB drivers to let
> > them know that they should toggle the state.
> 
> 
> If modetest won't stop running even though there was a DP unplug
> (and therefore presumably a destruction of the display), I don't
> think things are designed very well

They are, but differently. Display settings are always controlled by
DRM clients (typically, a userspace compositor). They can decide to
send data to unconnected display, they can decide to ignore HPD events,
etc. Even if userspace responds to the event, there always will be some
delay. I choose modetest, because it's a particularly good example of a
delay going to the infinity.

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-05-27 22:24   ` Konrad Dybcio
@ 2025-05-28  9:00     ` Dmitry Baryshkov
  2025-05-28 11:13       ` Konrad Dybcio
  0 siblings, 1 reply; 29+ messages in thread
From: Dmitry Baryshkov @ 2025-05-28  9:00 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Neil Armstrong, Krzysztof Kozlowski

On Wed, May 28, 2025 at 12:24:02AM +0200, Konrad Dybcio wrote:
> On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
> > On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
> >> Register a typec mux in order to change the PHY mode on the Type-C
> >> mux events depending on the mode and the svid when in Altmode setup.
> >>
> >> The DisplayPort phy should be left enabled if is still powered on
> >> by the DRM DisplayPort controller, so bail out until the DisplayPort
> >> PHY is not powered off.
> > 
> > This series doesn't seem to solve the USB side of a problem. When the
> > PHY is being switch to the 4-lane mode, USB controller looses PIPE
> > clock, so it needs to be switched to the USB-2 mode.
> 
> I didn't find issues with that on X13s.. Not sure if it's related, but
> on the SL7, after plugging in a 4ln DP connection, I need to plug in
> the USB thumb drive twice for the first time (only in that sequence)

Might be.

> But there's nothing interesting in dmesg and the phy seems to be
> programmed with the same values on both the initial and the subsequent
> working plug-in

Please try using a DP dongle with USB 2 passthrough (there are some).
Or just emulate this by enabling DP PHY / DP chain for plugged in USB3
devices. Would you be able to see the USB device on a bus?

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-05-28  9:00     ` Dmitry Baryshkov
@ 2025-05-28 11:13       ` Konrad Dybcio
  2025-05-28 11:22         ` Dmitry Baryshkov
  0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-28 11:13 UTC (permalink / raw)
  To: Dmitry Baryshkov, Konrad Dybcio
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Neil Armstrong, Krzysztof Kozlowski

On 5/28/25 11:00 AM, Dmitry Baryshkov wrote:
> On Wed, May 28, 2025 at 12:24:02AM +0200, Konrad Dybcio wrote:
>> On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
>>> On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>
>>>> The DisplayPort phy should be left enabled if is still powered on
>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>> PHY is not powered off.
>>>
>>> This series doesn't seem to solve the USB side of a problem. When the
>>> PHY is being switch to the 4-lane mode, USB controller looses PIPE
>>> clock, so it needs to be switched to the USB-2 mode.
>>
>> I didn't find issues with that on X13s.. Not sure if it's related, but
>> on the SL7, after plugging in a 4ln DP connection, I need to plug in
>> the USB thumb drive twice for the first time (only in that sequence)
> 
> Might be.
> 
>> But there's nothing interesting in dmesg and the phy seems to be
>> programmed with the same values on both the initial and the subsequent
>> working plug-in
> 
> Please try using a DP dongle with USB 2 passthrough (there are some).
> Or just emulate this by enabling DP PHY / DP chain for plugged in USB3
> devices. Would you be able to see the USB device on a bus?

I only have a dongle with both display and usb that does 2ln dp
(I tested 4ln dp on a type-c display that I don't think has a hub)

USB3 - yes, USB2 - no (but it works after a replug)

Are you talking about essentially doing qcom,select-utmi-as-pipe-clk
at runtime?

Konrad


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
  2025-05-28  8:58       ` Dmitry Baryshkov
@ 2025-05-28 11:21         ` Konrad Dybcio
  2025-05-28 11:22           ` Konrad Dybcio
  2025-06-02  8:51         ` Neil Armstrong
  1 sibling, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-28 11:21 UTC (permalink / raw)
  To: Dmitry Baryshkov, Konrad Dybcio
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Neil Armstrong

On 5/28/25 10:58 AM, Dmitry Baryshkov wrote:
> On Wed, May 28, 2025 at 12:22:01AM +0200, Konrad Dybcio wrote:
>> On 5/27/25 11:55 PM, Dmitry Baryshkov wrote:
>>> On Tue, May 27, 2025 at 10:40:07PM +0200, Konrad Dybcio wrote:
>>>> From: Neil Armstrong <neil.armstrong@linaro.org>
>>>>
>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>
>>>> The DisplayPort phy should be left enabled if is still powered on
>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>> PHY is not powered off.
>>>>
>>>> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
>>>> will be set in between of USB-Only, Combo and DisplayPort Only so
>>>> this will leave enough time to the DRM DisplayPort controller to
>>>> turn of the DisplayPort PHY.
>>>>
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> [konrad: renaming, rewording, bug fixes]
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>> ---
>>
>> [...]
>>
>>>> +	} else {
>>>> +		/* Fall back to USB3+DP mode if we're not sure it's strictly USB3-only */
>>>
>>> Why? if the SID is not DP, then there can't be a DP stream.
>>>
>>>> +		if (state->mode == TYPEC_MODE_USB3 || state->mode == TYPEC_STATE_USB)
>>>> +			new_mode = QMPPHY_MODE_USB3_ONLY;
>>>> +		else
>>>> +			new_mode = QMPPHY_MODE_USB3DP;
>>
>> To be honest I don't really know.. Neil chose to do that, but I don't
>> think there's a strict requirement.. Should we default to 4ln-USB3?
> 
> Yes, QMPPHY_MODE_USB3_ONLY. Nit: there is no 4ln-USB3 (it is a special
> mode). We handle 2ln-USB3 only.

Right, I double checked and we support SS

> 
>>
>> [...]
>>
>>> Consider the following scenario: connect DP dongle, use modetest to
>>> setup DP stream, disconnect dongle, connect USB3 device. What would be
>>> the actual state of the PHY? Modetest is still running, so DP stream is
>>> not going to be shut down from the driver.
>>>
>>> I think we might need a generic notifier from the PHY to the user,
>>> telling that the PHY is going away (or just that the PHY is changing the
>>> state). Then it would be usable by both the DP and USB drivers to let
>>> them know that they should toggle the state.
>>
>>
>> If modetest won't stop running even though there was a DP unplug
>> (and therefore presumably a destruction of the display), I don't
>> think things are designed very well
> 
> They are, but differently. Display settings are always controlled by
> DRM clients (typically, a userspace compositor). They can decide to
> send data to unconnected display, they can decide to ignore HPD events,
> etc. Even if userspace responds to the event, there always will be some
> delay. I choose modetest, because it's a particularly good example of a
> delay going to the infinity.

0_o

Konrad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-05-28 11:13       ` Konrad Dybcio
@ 2025-05-28 11:22         ` Dmitry Baryshkov
  2025-05-28 16:56           ` Konrad Dybcio
  0 siblings, 1 reply; 29+ messages in thread
From: Dmitry Baryshkov @ 2025-05-28 11:22 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Neil Armstrong, Krzysztof Kozlowski

On Wed, May 28, 2025 at 01:13:26PM +0200, Konrad Dybcio wrote:
> On 5/28/25 11:00 AM, Dmitry Baryshkov wrote:
> > On Wed, May 28, 2025 at 12:24:02AM +0200, Konrad Dybcio wrote:
> >> On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
> >>> On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
> >>>> Register a typec mux in order to change the PHY mode on the Type-C
> >>>> mux events depending on the mode and the svid when in Altmode setup.
> >>>>
> >>>> The DisplayPort phy should be left enabled if is still powered on
> >>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
> >>>> PHY is not powered off.
> >>>
> >>> This series doesn't seem to solve the USB side of a problem. When the
> >>> PHY is being switch to the 4-lane mode, USB controller looses PIPE
> >>> clock, so it needs to be switched to the USB-2 mode.
> >>
> >> I didn't find issues with that on X13s.. Not sure if it's related, but
> >> on the SL7, after plugging in a 4ln DP connection, I need to plug in
> >> the USB thumb drive twice for the first time (only in that sequence)
> > 
> > Might be.
> > 
> >> But there's nothing interesting in dmesg and the phy seems to be
> >> programmed with the same values on both the initial and the subsequent
> >> working plug-in
> > 
> > Please try using a DP dongle with USB 2 passthrough (there are some).
> > Or just emulate this by enabling DP PHY / DP chain for plugged in USB3
> > devices. Would you be able to see the USB device on a bus?
> 
> I only have a dongle with both display and usb that does 2ln dp
> (I tested 4ln dp on a type-c display that I don't think has a hub)
> 
> USB3 - yes, USB2 - no (but it works after a replug)
> 
> Are you talking about essentially doing qcom,select-utmi-as-pipe-clk
> at runtime?

I think so.

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
  2025-05-28 11:21         ` Konrad Dybcio
@ 2025-05-28 11:22           ` Konrad Dybcio
  0 siblings, 0 replies; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-28 11:22 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Neil Armstrong

On 5/28/25 1:21 PM, Konrad Dybcio wrote:
> On 5/28/25 10:58 AM, Dmitry Baryshkov wrote:
>> On Wed, May 28, 2025 at 12:22:01AM +0200, Konrad Dybcio wrote:
>>> On 5/27/25 11:55 PM, Dmitry Baryshkov wrote:
>>>> On Tue, May 27, 2025 at 10:40:07PM +0200, Konrad Dybcio wrote:
>>>>> From: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>
>>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>>
>>>>> The DisplayPort phy should be left enabled if is still powered on
>>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>>> PHY is not powered off.
>>>>>
>>>>> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
>>>>> will be set in between of USB-Only, Combo and DisplayPort Only so
>>>>> this will leave enough time to the DRM DisplayPort controller to
>>>>> turn of the DisplayPort PHY.
>>>>>
>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>> [konrad: renaming, rewording, bug fixes]
>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>> ---
>>>
>>> [...]
>>>
>>>>> +	} else {
>>>>> +		/* Fall back to USB3+DP mode if we're not sure it's strictly USB3-only */
>>>>
>>>> Why? if the SID is not DP, then there can't be a DP stream.
>>>>
>>>>> +		if (state->mode == TYPEC_MODE_USB3 || state->mode == TYPEC_STATE_USB)
>>>>> +			new_mode = QMPPHY_MODE_USB3_ONLY;
>>>>> +		else
>>>>> +			new_mode = QMPPHY_MODE_USB3DP;
>>>
>>> To be honest I don't really know.. Neil chose to do that, but I don't
>>> think there's a strict requirement.. Should we default to 4ln-USB3?
>>
>> Yes, QMPPHY_MODE_USB3_ONLY. Nit: there is no 4ln-USB3 (it is a special
>> mode). We handle 2ln-USB3 only.
> 
> Right, I double checked and we support SS

I clicked ctrl-z one too many times - I meant SS+ 10Gbps

Konrad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
                   ` (6 preceding siblings ...)
  2025-05-27 21:10 ` [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Dmitry Baryshkov
@ 2025-05-28 12:37 ` Rob Herring (Arm)
  2025-05-29  8:43 ` Jens Glathe
  2025-05-29 11:13 ` Alex
  9 siblings, 0 replies; 29+ messages in thread
From: Rob Herring (Arm) @ 2025-05-28 12:37 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: linux-phy, Kishon Vijay Abraham I, Neil Armstrong, Konrad Dybcio,
	Bjorn Andersson, linux-arm-msm, Krzysztof Kozlowski,
	Dmitry Baryshkov, devicetree, Vinod Koul, linux-kernel,
	Krzysztof Kozlowski, Marijn Suijten, Conor Dooley


On Tue, 27 May 2025 22:40:02 +0200, Konrad Dybcio wrote:
> Register a typec mux in order to change the PHY mode on the Type-C
> mux events depending on the mode and the svid when in Altmode setup.
> 
> The DisplayPort phy should be left enabled if is still powered on
> by the DRM DisplayPort controller, so bail out until the DisplayPort
> PHY is not powered off.
> 
> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
> will be set in between of USB-Only, Combo and DisplayPort Only so
> this will leave enough time to the DRM DisplayPort controller to
> turn of the DisplayPort PHY.
> 
> The patchset also includes bindings changes and DT changes.
> 
> This has been successfully tested on an SM8550 board, but the
> Thinkpad X13s deserved testing between non-PD USB, non-PD DisplayPort,
> PD USB Hubs and PD Altmode Dongles to make sure the switch works
> as expected.
> 
> The DisplayPort 4 lanes setup can be check with:
> $ cat /sys/kernel/debug/dri/ae01000.display-controller/DP-1/dp_debug
> 	name = msm_dp
> 	drm_dp_link
> 		rate = 540000
> 		num_lanes = 4
> ...
> 
> This patchset depends on [1] to allow broadcasting the type-c mode
> to the PHY, otherwise the PHY will keep the combo state while the
> retimer would setup the 4 lanes in DP mode.
> 
> [1] https://lore.kernel.org/all/20240527-topic-sm8x50-upstream-retimer-broadcast-mode-v1-0-79ec91381aba@linaro.org/
> Changes in v3:
> - Take the series from Neil
> - Rebase
> - Rename many variables
> - Test on X1E & X13s
> - Apply a number of small cosmetic/codestyle changes
> - Remove some unused variables
> - Some smaller bugfixes
> - Link to v2: https://lore.kernel.org/lkml/20240527-topic-sm8x50-upstream-phy-combo-typec-mux-v2-0-a03e68d7b8fc@linaro.org/
> Changes in v2:
> - Reference usb-switch.yaml in bindings patch
> - Fix switch/case indenting
> - Check svid for USB_TYPEC_DP_SID
> - Fix X13s patch subject
> - Update SM8650 patch to enable 4 lanes on HDK aswell
> - Link to v1: https://lore.kernel.org/r/20240229-topic-sm8x50-upstream-phy-combo-typec-mux-v1-0-07e24a231840@linaro.org
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> Konrad Dybcio (1):
>       phy: qcom: qmp-combo: Rename 'mode' to 'phy_mode'
> 
> Neil Armstrong (5):
>       dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch
>       phy: qcom: qmp-combo: store DP phy power state
>       phy: qcom: qmp-combo: introduce QMPPHY_MODE
>       phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
>       arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: Set up 4-lane DP
> 
>  .../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml         |   7 +-
>  .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |   6 +-
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c          | 182 +++++++++++++++++++--
>  3 files changed, 173 insertions(+), 22 deletions(-)
> ---
> base-commit: 460178e842c7a1e48a06df684c66eb5fd630bcf7
> change-id: 20250527-topic-4ln_dp_respin-c6924a8825ce
> 
> Best regards,
> --
> Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> 
> 


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: using specified base-commit 460178e842c7a1e48a06df684c66eb5fd630bcf7

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/qcom/' for 20250527-topic-4ln_dp_respin-v3-0-f9a0763ec289@oss.qualcomm.com:

arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sm7125-xiaomi-curtana.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r4.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360-wifi.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r1-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick-r0.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler-rev1-inx.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r4.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r1.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar-r4.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-acer-aspire1.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r9.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1-kb.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar-r3.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r10.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-idp.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-parade.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler-rev1-boe-rt5682s.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler-rev1-boe.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r3-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-ti.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r3.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-r1-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-lte-ti.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler-rev1-inx-rt5682s.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel-lte-parade.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar-r2.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r5.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r10-kb.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sm7125-xiaomi-joyeuse.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r10.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#
arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick-r0-lte.dtb: phy@88e8000 (qcom,sc7180-qmp-usb3-dp-phy): 'oneOf' conditional failed, one must be fixed:
	'port' is a required property
	'ports' is a required property
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml#






^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-05-28 11:22         ` Dmitry Baryshkov
@ 2025-05-28 16:56           ` Konrad Dybcio
  2025-06-02  8:55             ` Neil Armstrong
  0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2025-05-28 16:56 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Neil Armstrong, Krzysztof Kozlowski

On 5/28/25 1:22 PM, Dmitry Baryshkov wrote:
> On Wed, May 28, 2025 at 01:13:26PM +0200, Konrad Dybcio wrote:
>> On 5/28/25 11:00 AM, Dmitry Baryshkov wrote:
>>> On Wed, May 28, 2025 at 12:24:02AM +0200, Konrad Dybcio wrote:
>>>> On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
>>>>> On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
>>>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>>>
>>>>>> The DisplayPort phy should be left enabled if is still powered on
>>>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>>>> PHY is not powered off.
>>>>>
>>>>> This series doesn't seem to solve the USB side of a problem. When the
>>>>> PHY is being switch to the 4-lane mode, USB controller looses PIPE
>>>>> clock, so it needs to be switched to the USB-2 mode.
>>>>
>>>> I didn't find issues with that on X13s.. Not sure if it's related, but
>>>> on the SL7, after plugging in a 4ln DP connection, I need to plug in
>>>> the USB thumb drive twice for the first time (only in that sequence)
>>>
>>> Might be.
>>>
>>>> But there's nothing interesting in dmesg and the phy seems to be
>>>> programmed with the same values on both the initial and the subsequent
>>>> working plug-in
>>>
>>> Please try using a DP dongle with USB 2 passthrough (there are some).
>>> Or just emulate this by enabling DP PHY / DP chain for plugged in USB3
>>> devices. Would you be able to see the USB device on a bus?
>>
>> I only have a dongle with both display and usb that does 2ln dp
>> (I tested 4ln dp on a type-c display that I don't think has a hub)
>>
>> USB3 - yes, USB2 - no (but it works after a replug)
>>
>> Are you talking about essentially doing qcom,select-utmi-as-pipe-clk
>> at runtime?
> 
> I think so.

So after quite some time playing with that, I noticed that the USB2
device was never actually kicked off the bus.. and works totally fine
after connecting the display output (2ln DP)

I was looking at dmesg, checking for discovery/disconnect messages,
but the device was simply never disconnected (which makes sense given
repurposing USB3 TX/RX lanes doesn't affect the D+/D- of USB2)

I also read some docs and learnt that what we call
qcom,select-utmi-as-pipe-clk is suppossed to be a debug feature
and is unnecessary to set on USB2.0-only controllers

The USB controller programming guide though doesn't talk about DP,
but I'd expect that we may need to set that override for 4lnDP+USB2
use cases (which I don't have a dongle for)

Konrad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
                   ` (7 preceding siblings ...)
  2025-05-28 12:37 ` Rob Herring (Arm)
@ 2025-05-29  8:43 ` Jens Glathe
  2025-05-29 11:13 ` Alex
  9 siblings, 0 replies; 29+ messages in thread
From: Jens Glathe @ 2025-05-29  8:43 UTC (permalink / raw)
  To: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson
  Cc: Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong, Krzysztof Kozlowski,
	Dmitry Baryshkov

On 27.05.25 22:40, Konrad Dybcio wrote:
> The DisplayPort 4 lanes setup can be check with:
> $ cat /sys/kernel/debug/dri/ae01000.display-controller/DP-1/dp_debug
> 	name = msm_dp
> 	drm_dp_link
> 		rate = 540000
> 		num_lanes = 4
> ...

Hi Konrad,

thank you for the patch. I applied it, added `mode-switch;` to all combo 
qmpphys in sc8280xp.dtsi, enabled 4 lanes on the mdss_dp's, and it works 
on the windows Dev Kit 2023 (Blackrock) on usb1, with both orientations. 
Getting 4k@60 with 4 lanes. DPMS suspend/wakeup works, too. usb0 does 
only data, as before (need to investigate).

My suggestion would be to put the `mode-switch;` into the SoC dtsi 
(sc8280xp and x1e80100, maybe more?). Shouldn't hurt IMO.

Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>

with best regards

Jens


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
                   ` (8 preceding siblings ...)
  2025-05-29  8:43 ` Jens Glathe
@ 2025-05-29 11:13 ` Alex
  9 siblings, 0 replies; 29+ messages in thread
From: Alex @ 2025-05-29 11:13 UTC (permalink / raw)
  To: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson
  Cc: Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Konrad Dybcio, Neil Armstrong, Krzysztof Kozlowski,
	Dmitry Baryshkov


On 5/27/25 22:40, Konrad Dybcio wrote:
> Register a typec mux in order to change the PHY mode on the Type-C
> mux events depending on the mode and the svid when in Altmode setup.
>
> The DisplayPort phy should be left enabled if is still powered on
> by the DRM DisplayPort controller, so bail out until the DisplayPort
> PHY is not powered off.
>
> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
> will be set in between of USB-Only, Combo and DisplayPort Only so
> this will leave enough time to the DRM DisplayPort controller to
> turn of the DisplayPort PHY.
>
> The patchset also includes bindings changes and DT changes.
>
> This has been successfully tested on an SM8550 board, but the
> Thinkpad X13s deserved testing between non-PD USB, non-PD DisplayPort,
> PD USB Hubs and PD Altmode Dongles to make sure the switch works
> as expected.
>
> The DisplayPort 4 lanes setup can be check with:
> $ cat /sys/kernel/debug/dri/ae01000.display-controller/DP-1/dp_debug
> 	name = msm_dp
> 	drm_dp_link
> 		rate = 540000
> 		num_lanes = 4
> ...


Hi,


Thanks for the respin. Together with `mode-switch;` for x1e80100.dtsi 
and 4 lane DP change in respective .dts, successfully tested on Asus 
Zenbook A14 (Parade PS8833 on both Type-C ports).

Tested with:

- Type-C USB3.0 pendrive

- Type-C to DP cable (x4 DP lanes)

- Type-C to HDMI/USB/... dongle (x2 USB3, x2 DP)

All three variants work, in both orientations, on both ports. When 
switching from x4 DP lanes cable to USB3 pendrive no re-plug was needed, 
it works right away. Suspend and resume (to my surprise) also works.


Tested-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com> # x1e80100, 
ps8833


Regards,

Alex


>
> This patchset depends on [1] to allow broadcasting the type-c mode
> to the PHY, otherwise the PHY will keep the combo state while the
> retimer would setup the 4 lanes in DP mode.
>
> [1] https://lore.kernel.org/all/20240527-topic-sm8x50-upstream-retimer-broadcast-mode-v1-0-79ec91381aba@linaro.org/
> Changes in v3:
> - Take the series from Neil
> - Rebase
> - Rename many variables
> - Test on X1E & X13s
> - Apply a number of small cosmetic/codestyle changes
> - Remove some unused variables
> - Some smaller bugfixes
> - Link to v2: https://lore.kernel.org/lkml/20240527-topic-sm8x50-upstream-phy-combo-typec-mux-v2-0-a03e68d7b8fc@linaro.org/
> Changes in v2:
> - Reference usb-switch.yaml in bindings patch
> - Fix switch/case indenting
> - Check svid for USB_TYPEC_DP_SID
> - Fix X13s patch subject
> - Update SM8650 patch to enable 4 lanes on HDK aswell
> - Link to v1: https://lore.kernel.org/r/20240229-topic-sm8x50-upstream-phy-combo-typec-mux-v1-0-07e24a231840@linaro.org
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> Konrad Dybcio (1):
>        phy: qcom: qmp-combo: Rename 'mode' to 'phy_mode'
>
> Neil Armstrong (5):
>        dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch
>        phy: qcom: qmp-combo: store DP phy power state
>        phy: qcom: qmp-combo: introduce QMPPHY_MODE
>        phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
>        arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: Set up 4-lane DP
>
>   .../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml         |   7 +-
>   .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |   6 +-
>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c          | 182 +++++++++++++++++++--
>   3 files changed, 173 insertions(+), 22 deletions(-)
> ---
> base-commit: 460178e842c7a1e48a06df684c66eb5fd630bcf7
> change-id: 20250527-topic-4ln_dp_respin-c6924a8825ce
>
> Best regards,

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
  2025-05-28  8:58       ` Dmitry Baryshkov
  2025-05-28 11:21         ` Konrad Dybcio
@ 2025-06-02  8:51         ` Neil Armstrong
  2025-06-03 10:32           ` Dmitry Baryshkov
  1 sibling, 1 reply; 29+ messages in thread
From: Neil Armstrong @ 2025-06-02  8:51 UTC (permalink / raw)
  To: Dmitry Baryshkov, Konrad Dybcio
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel

On 28/05/2025 10:58, Dmitry Baryshkov wrote:
> On Wed, May 28, 2025 at 12:22:01AM +0200, Konrad Dybcio wrote:
>> On 5/27/25 11:55 PM, Dmitry Baryshkov wrote:
>>> On Tue, May 27, 2025 at 10:40:07PM +0200, Konrad Dybcio wrote:
>>>> From: Neil Armstrong <neil.armstrong@linaro.org>
>>>>
>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>
>>>> The DisplayPort phy should be left enabled if is still powered on
>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>> PHY is not powered off.
>>>>
>>>> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
>>>> will be set in between of USB-Only, Combo and DisplayPort Only so
>>>> this will leave enough time to the DRM DisplayPort controller to
>>>> turn of the DisplayPort PHY.
>>>>
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> [konrad: renaming, rewording, bug fixes]
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>> ---
>>
>> [...]
>>
>>>> +	} else {
>>>> +		/* Fall back to USB3+DP mode if we're not sure it's strictly USB3-only */
>>>
>>> Why? if the SID is not DP, then there can't be a DP stream.
>>>
>>>> +		if (state->mode == TYPEC_MODE_USB3 || state->mode == TYPEC_STATE_USB)
>>>> +			new_mode = QMPPHY_MODE_USB3_ONLY;
>>>> +		else
>>>> +			new_mode = QMPPHY_MODE_USB3DP;
>>
>> To be honest I don't really know.. Neil chose to do that, but I don't
>> think there's a strict requirement.. Should we default to 4ln-USB3?
> 
> Yes, QMPPHY_MODE_USB3_ONLY. Nit: there is no 4ln-USB3 (it is a special
> mode). We handle 2ln-USB3 only.
> 
>>
>> [...]
>>
>>> Consider the following scenario: connect DP dongle, use modetest to
>>> setup DP stream, disconnect dongle, connect USB3 device. What would be
>>> the actual state of the PHY? Modetest is still running, so DP stream is
>>> not going to be shut down from the driver.
>>>
>>> I think we might need a generic notifier from the PHY to the user,
>>> telling that the PHY is going away (or just that the PHY is changing the
>>> state). Then it would be usable by both the DP and USB drivers to let
>>> them know that they should toggle the state.
>>
>>
>> If modetest won't stop running even though there was a DP unplug
>> (and therefore presumably a destruction of the display), I don't
>> think things are designed very well
> 
> They are, but differently. Display settings are always controlled by
> DRM clients (typically, a userspace compositor). They can decide to
> send data to unconnected display, they can decide to ignore HPD events,
> etc. Even if userspace responds to the event, there always will be some
> delay. I choose modetest, because it's a particularly good example of a
> delay going to the infinity.
> 

DP link state is handled separately from the DRM state, if you look at the
MSM/DP, you get the following calls on an hdp event:
dp_bridge_hpd_notify
hpd_event_thread
dp_hpd_unplug_handle
dp_ctrl_off_link
phy_power_off
dp_display_host_phy_exit
phy_exit

independently of any DRM state change, DRM will be notified at the end of
a disconnect with dp_display_notify_disconnect().

So even with a modeset running, phy will disabled following an UCSI disconnect
event.

Neil

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-05-28 16:56           ` Konrad Dybcio
@ 2025-06-02  8:55             ` Neil Armstrong
  2025-06-03 11:03               ` Konrad Dybcio
  0 siblings, 1 reply; 29+ messages in thread
From: Neil Armstrong @ 2025-06-02  8:55 UTC (permalink / raw)
  To: Konrad Dybcio, Dmitry Baryshkov
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Krzysztof Kozlowski

On 28/05/2025 18:56, Konrad Dybcio wrote:
> On 5/28/25 1:22 PM, Dmitry Baryshkov wrote:
>> On Wed, May 28, 2025 at 01:13:26PM +0200, Konrad Dybcio wrote:
>>> On 5/28/25 11:00 AM, Dmitry Baryshkov wrote:
>>>> On Wed, May 28, 2025 at 12:24:02AM +0200, Konrad Dybcio wrote:
>>>>> On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
>>>>>> On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
>>>>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>>>>
>>>>>>> The DisplayPort phy should be left enabled if is still powered on
>>>>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>>>>> PHY is not powered off.
>>>>>>
>>>>>> This series doesn't seem to solve the USB side of a problem. When the
>>>>>> PHY is being switch to the 4-lane mode, USB controller looses PIPE
>>>>>> clock, so it needs to be switched to the USB-2 mode.
>>>>>
>>>>> I didn't find issues with that on X13s.. Not sure if it's related, but
>>>>> on the SL7, after plugging in a 4ln DP connection, I need to plug in
>>>>> the USB thumb drive twice for the first time (only in that sequence)
>>>>
>>>> Might be.
>>>>
>>>>> But there's nothing interesting in dmesg and the phy seems to be
>>>>> programmed with the same values on both the initial and the subsequent
>>>>> working plug-in
>>>>
>>>> Please try using a DP dongle with USB 2 passthrough (there are some).
>>>> Or just emulate this by enabling DP PHY / DP chain for plugged in USB3
>>>> devices. Would you be able to see the USB device on a bus?
>>>
>>> I only have a dongle with both display and usb that does 2ln dp
>>> (I tested 4ln dp on a type-c display that I don't think has a hub)
>>>
>>> USB3 - yes, USB2 - no (but it works after a replug)
>>>
>>> Are you talking about essentially doing qcom,select-utmi-as-pipe-clk
>>> at runtime?
>>
>> I think so.
> 
> So after quite some time playing with that, I noticed that the USB2
> device was never actually kicked off the bus.. and works totally fine
> after connecting the display output (2ln DP)
> 
> I was looking at dmesg, checking for discovery/disconnect messages,
> but the device was simply never disconnected (which makes sense given
> repurposing USB3 TX/RX lanes doesn't affect the D+/D- of USB2)
> 
> I also read some docs and learnt that what we call
> qcom,select-utmi-as-pipe-clk is suppossed to be a debug feature
> and is unnecessary to set on USB2.0-only controllers
> 
> The USB controller programming guide though doesn't talk about DP,
> but I'd expect that we may need to set that override for 4lnDP+USB2
> use cases (which I don't have a dongle for)

Yeah basically we need to:
1) power-off the USB3 PHY
2) switch to UTMI clock

In the following states:
- USB safe mode (USB2 lanes may still be connected)
- 4lanes DP mode
- DP-only mode

But for this, the dwc3 should also get USB-C events with an addition mode-switch property.
The flatten DWC3 node now allows that !

Neil

> 
> Konrad


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE
  2025-06-02  8:51         ` Neil Armstrong
@ 2025-06-03 10:32           ` Dmitry Baryshkov
  0 siblings, 0 replies; 29+ messages in thread
From: Dmitry Baryshkov @ 2025-06-03 10:32 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Konrad Dybcio, Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel

On Mon, Jun 02, 2025 at 10:51:42AM +0200, Neil Armstrong wrote:
> On 28/05/2025 10:58, Dmitry Baryshkov wrote:
> > On Wed, May 28, 2025 at 12:22:01AM +0200, Konrad Dybcio wrote:
> > > On 5/27/25 11:55 PM, Dmitry Baryshkov wrote:
> > > > On Tue, May 27, 2025 at 10:40:07PM +0200, Konrad Dybcio wrote:
> > > > > From: Neil Armstrong <neil.armstrong@linaro.org>
> > > > > 
> > > > > Register a typec mux in order to change the PHY mode on the Type-C
> > > > > mux events depending on the mode and the svid when in Altmode setup.
> > > > > 
> > > > > The DisplayPort phy should be left enabled if is still powered on
> > > > > by the DRM DisplayPort controller, so bail out until the DisplayPort
> > > > > PHY is not powered off.
> > > > > 
> > > > > The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
> > > > > will be set in between of USB-Only, Combo and DisplayPort Only so
> > > > > this will leave enough time to the DRM DisplayPort controller to
> > > > > turn of the DisplayPort PHY.
> > > > > 
> > > > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > > > [konrad: renaming, rewording, bug fixes]
> > > > > Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > > > > ---
> > > 
> > > [...]
> > > 
> > > > > +	} else {
> > > > > +		/* Fall back to USB3+DP mode if we're not sure it's strictly USB3-only */
> > > > 
> > > > Why? if the SID is not DP, then there can't be a DP stream.
> > > > 
> > > > > +		if (state->mode == TYPEC_MODE_USB3 || state->mode == TYPEC_STATE_USB)
> > > > > +			new_mode = QMPPHY_MODE_USB3_ONLY;
> > > > > +		else
> > > > > +			new_mode = QMPPHY_MODE_USB3DP;
> > > 
> > > To be honest I don't really know.. Neil chose to do that, but I don't
> > > think there's a strict requirement.. Should we default to 4ln-USB3?
> > 
> > Yes, QMPPHY_MODE_USB3_ONLY. Nit: there is no 4ln-USB3 (it is a special
> > mode). We handle 2ln-USB3 only.
> > 
> > > 
> > > [...]
> > > 
> > > > Consider the following scenario: connect DP dongle, use modetest to
> > > > setup DP stream, disconnect dongle, connect USB3 device. What would be
> > > > the actual state of the PHY? Modetest is still running, so DP stream is
> > > > not going to be shut down from the driver.
> > > > 
> > > > I think we might need a generic notifier from the PHY to the user,
> > > > telling that the PHY is going away (or just that the PHY is changing the
> > > > state). Then it would be usable by both the DP and USB drivers to let
> > > > them know that they should toggle the state.
> > > 
> > > 
> > > If modetest won't stop running even though there was a DP unplug
> > > (and therefore presumably a destruction of the display), I don't
> > > think things are designed very well
> > 
> > They are, but differently. Display settings are always controlled by
> > DRM clients (typically, a userspace compositor). They can decide to
> > send data to unconnected display, they can decide to ignore HPD events,
> > etc. Even if userspace responds to the event, there always will be some
> > delay. I choose modetest, because it's a particularly good example of a
> > delay going to the infinity.
> > 
> 
> DP link state is handled separately from the DRM state, if you look at the
> MSM/DP, you get the following calls on an hdp event:
> dp_bridge_hpd_notify
> hpd_event_thread

And this part will hopefully go away soon... Which means that DP link
will stay on until atomic_disable().

> dp_hpd_unplug_handle
> dp_ctrl_off_link
> phy_power_off
> dp_display_host_phy_exit
> phy_exit
> 
> independently of any DRM state change, DRM will be notified at the end of
> a disconnect with dp_display_notify_disconnect().
> 
> So even with a modeset running, phy will disabled following an UCSI disconnect
> event.
> 
> Neil

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-06-02  8:55             ` Neil Armstrong
@ 2025-06-03 11:03               ` Konrad Dybcio
  2025-06-03 13:17                 ` Dmitry Baryshkov
  0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2025-06-03 11:03 UTC (permalink / raw)
  To: Neil Armstrong, Dmitry Baryshkov
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Krzysztof Kozlowski

On 6/2/25 10:55 AM, Neil Armstrong wrote:
> On 28/05/2025 18:56, Konrad Dybcio wrote:
>> On 5/28/25 1:22 PM, Dmitry Baryshkov wrote:
>>> On Wed, May 28, 2025 at 01:13:26PM +0200, Konrad Dybcio wrote:
>>>> On 5/28/25 11:00 AM, Dmitry Baryshkov wrote:
>>>>> On Wed, May 28, 2025 at 12:24:02AM +0200, Konrad Dybcio wrote:
>>>>>> On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
>>>>>>> On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
>>>>>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>>>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>>>>>
>>>>>>>> The DisplayPort phy should be left enabled if is still powered on
>>>>>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>>>>>> PHY is not powered off.
>>>>>>>
>>>>>>> This series doesn't seem to solve the USB side of a problem. When the
>>>>>>> PHY is being switch to the 4-lane mode, USB controller looses PIPE
>>>>>>> clock, so it needs to be switched to the USB-2 mode.
>>>>>>
>>>>>> I didn't find issues with that on X13s.. Not sure if it's related, but
>>>>>> on the SL7, after plugging in a 4ln DP connection, I need to plug in
>>>>>> the USB thumb drive twice for the first time (only in that sequence)
>>>>>
>>>>> Might be.
>>>>>
>>>>>> But there's nothing interesting in dmesg and the phy seems to be
>>>>>> programmed with the same values on both the initial and the subsequent
>>>>>> working plug-in
>>>>>
>>>>> Please try using a DP dongle with USB 2 passthrough (there are some).
>>>>> Or just emulate this by enabling DP PHY / DP chain for plugged in USB3
>>>>> devices. Would you be able to see the USB device on a bus?
>>>>
>>>> I only have a dongle with both display and usb that does 2ln dp
>>>> (I tested 4ln dp on a type-c display that I don't think has a hub)
>>>>
>>>> USB3 - yes, USB2 - no (but it works after a replug)
>>>>
>>>> Are you talking about essentially doing qcom,select-utmi-as-pipe-clk
>>>> at runtime?
>>>
>>> I think so.
>>
>> So after quite some time playing with that, I noticed that the USB2
>> device was never actually kicked off the bus.. and works totally fine
>> after connecting the display output (2ln DP)
>>
>> I was looking at dmesg, checking for discovery/disconnect messages,
>> but the device was simply never disconnected (which makes sense given
>> repurposing USB3 TX/RX lanes doesn't affect the D+/D- of USB2)
>>
>> I also read some docs and learnt that what we call
>> qcom,select-utmi-as-pipe-clk is suppossed to be a debug feature
>> and is unnecessary to set on USB2.0-only controllers
>>
>> The USB controller programming guide though doesn't talk about DP,
>> but I'd expect that we may need to set that override for 4lnDP+USB2
>> use cases (which I don't have a dongle for)
> 
> Yeah basically we need to:
> 1) power-off the USB3 PHY
> 2) switch to UTMI clock
> 
> In the following states:
> - USB safe mode (USB2 lanes may still be connected)
> - 4lanes DP mode
> - DP-only mode
> 
> But for this, the dwc3 should also get USB-C events with an addition mode-switch property.
> The flatten DWC3 node now allows that !

Yeah, I got even more confirmation this is intended..

I hacked up something that boils down to:

diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 7977860932b1..e5a0a8ec624d 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -464,6 +464,11 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
 		break;
 	}
 
+	if (dwc->mode_fn)
+		dwc->mode_fn(dwc, mode);

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 7334de85ad10..ea56f5087ecb 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
+static void mode_fn(struct dwc3 *dwc, enum usb_role role)
+{
+	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
+
+	if (dwc->usb3_generic_phy[0])
+		dwc3_qcom_select_utmi_clk(qcom, role == USB_ROLE_NONE);
 }


It was easy to tap into because there's already mode switch handling..
But obviously it's a hack - should I register a typec_mux in there?
Should it go to dwc3-common or dwc3-qcom?

Konrad

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-06-03 11:03               ` Konrad Dybcio
@ 2025-06-03 13:17                 ` Dmitry Baryshkov
  2025-08-06 15:48                   ` Konrad Dybcio
  0 siblings, 1 reply; 29+ messages in thread
From: Dmitry Baryshkov @ 2025-06-03 13:17 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Neil Armstrong, Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Krzysztof Kozlowski

On Tue, Jun 03, 2025 at 01:03:11PM +0200, Konrad Dybcio wrote:
> On 6/2/25 10:55 AM, Neil Armstrong wrote:
> > On 28/05/2025 18:56, Konrad Dybcio wrote:
> >> On 5/28/25 1:22 PM, Dmitry Baryshkov wrote:
> >>> On Wed, May 28, 2025 at 01:13:26PM +0200, Konrad Dybcio wrote:
> >>>> On 5/28/25 11:00 AM, Dmitry Baryshkov wrote:
> >>>>> On Wed, May 28, 2025 at 12:24:02AM +0200, Konrad Dybcio wrote:
> >>>>>> On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
> >>>>>>> On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
> >>>>>>>> Register a typec mux in order to change the PHY mode on the Type-C
> >>>>>>>> mux events depending on the mode and the svid when in Altmode setup.
> >>>>>>>>
> >>>>>>>> The DisplayPort phy should be left enabled if is still powered on
> >>>>>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
> >>>>>>>> PHY is not powered off.
> >>>>>>>
> >>>>>>> This series doesn't seem to solve the USB side of a problem. When the
> >>>>>>> PHY is being switch to the 4-lane mode, USB controller looses PIPE
> >>>>>>> clock, so it needs to be switched to the USB-2 mode.
> >>>>>>
> >>>>>> I didn't find issues with that on X13s.. Not sure if it's related, but
> >>>>>> on the SL7, after plugging in a 4ln DP connection, I need to plug in
> >>>>>> the USB thumb drive twice for the first time (only in that sequence)
> >>>>>
> >>>>> Might be.
> >>>>>
> >>>>>> But there's nothing interesting in dmesg and the phy seems to be
> >>>>>> programmed with the same values on both the initial and the subsequent
> >>>>>> working plug-in
> >>>>>
> >>>>> Please try using a DP dongle with USB 2 passthrough (there are some).
> >>>>> Or just emulate this by enabling DP PHY / DP chain for plugged in USB3
> >>>>> devices. Would you be able to see the USB device on a bus?
> >>>>
> >>>> I only have a dongle with both display and usb that does 2ln dp
> >>>> (I tested 4ln dp on a type-c display that I don't think has a hub)
> >>>>
> >>>> USB3 - yes, USB2 - no (but it works after a replug)
> >>>>
> >>>> Are you talking about essentially doing qcom,select-utmi-as-pipe-clk
> >>>> at runtime?
> >>>
> >>> I think so.
> >>
> >> So after quite some time playing with that, I noticed that the USB2
> >> device was never actually kicked off the bus.. and works totally fine
> >> after connecting the display output (2ln DP)
> >>
> >> I was looking at dmesg, checking for discovery/disconnect messages,
> >> but the device was simply never disconnected (which makes sense given
> >> repurposing USB3 TX/RX lanes doesn't affect the D+/D- of USB2)
> >>
> >> I also read some docs and learnt that what we call
> >> qcom,select-utmi-as-pipe-clk is suppossed to be a debug feature
> >> and is unnecessary to set on USB2.0-only controllers
> >>
> >> The USB controller programming guide though doesn't talk about DP,
> >> but I'd expect that we may need to set that override for 4lnDP+USB2
> >> use cases (which I don't have a dongle for)
> > 
> > Yeah basically we need to:
> > 1) power-off the USB3 PHY
> > 2) switch to UTMI clock
> > 
> > In the following states:
> > - USB safe mode (USB2 lanes may still be connected)
> > - 4lanes DP mode
> > - DP-only mode
> > 
> > But for this, the dwc3 should also get USB-C events with an addition mode-switch property.
> > The flatten DWC3 node now allows that !
> 
> Yeah, I got even more confirmation this is intended..
> 
> I hacked up something that boils down to:
> 
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index 7977860932b1..e5a0a8ec624d 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -464,6 +464,11 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
>  		break;
>  	}
>  
> +	if (dwc->mode_fn)
> +		dwc->mode_fn(dwc, mode);
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 7334de85ad10..ea56f5087ecb 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> +static void mode_fn(struct dwc3 *dwc, enum usb_role role)
> +{
> +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
> +
> +	if (dwc->usb3_generic_phy[0])
> +		dwc3_qcom_select_utmi_clk(qcom, role == USB_ROLE_NONE);

This part is a hack for devices with no USB-2 passthrough, isn't it?

>  }
> 
> 
> It was easy to tap into because there's already mode switch handling..
> But obviously it's a hack - should I register a typec_mux in there?

I think so, we should listen to mode changes, instead of host/device
changes.

> Should it go to dwc3-common or dwc3-qcom?

I'd say, dwc3-qcom. Not all dwc3 controllers are USB 3 capable, not
talking about the USB-C.

> 
> Konrad

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-06-03 13:17                 ` Dmitry Baryshkov
@ 2025-08-06 15:48                   ` Konrad Dybcio
  2025-08-07  8:16                     ` Neil Armstrong
  0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2025-08-06 15:48 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Neil Armstrong, Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Krzysztof Kozlowski

On 6/3/25 3:17 PM, Dmitry Baryshkov wrote:
> On Tue, Jun 03, 2025 at 01:03:11PM +0200, Konrad Dybcio wrote:
>> On 6/2/25 10:55 AM, Neil Armstrong wrote:
>>> On 28/05/2025 18:56, Konrad Dybcio wrote:
>>>> On 5/28/25 1:22 PM, Dmitry Baryshkov wrote:
>>>>> On Wed, May 28, 2025 at 01:13:26PM +0200, Konrad Dybcio wrote:
>>>>>> On 5/28/25 11:00 AM, Dmitry Baryshkov wrote:
>>>>>>> On Wed, May 28, 2025 at 12:24:02AM +0200, Konrad Dybcio wrote:
>>>>>>>> On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
>>>>>>>>> On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
>>>>>>>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>>>>>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>>>>>>>
>>>>>>>>>> The DisplayPort phy should be left enabled if is still powered on
>>>>>>>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>>>>>>>> PHY is not powered off.
>>>>>>>>>
>>>>>>>>> This series doesn't seem to solve the USB side of a problem. When the
>>>>>>>>> PHY is being switch to the 4-lane mode, USB controller looses PIPE
>>>>>>>>> clock, so it needs to be switched to the USB-2 mode.
>>>>>>>>
>>>>>>>> I didn't find issues with that on X13s.. Not sure if it's related, but
>>>>>>>> on the SL7, after plugging in a 4ln DP connection, I need to plug in
>>>>>>>> the USB thumb drive twice for the first time (only in that sequence)
>>>>>>>
>>>>>>> Might be.
>>>>>>>
>>>>>>>> But there's nothing interesting in dmesg and the phy seems to be
>>>>>>>> programmed with the same values on both the initial and the subsequent
>>>>>>>> working plug-in
>>>>>>>
>>>>>>> Please try using a DP dongle with USB 2 passthrough (there are some).
>>>>>>> Or just emulate this by enabling DP PHY / DP chain for plugged in USB3
>>>>>>> devices. Would you be able to see the USB device on a bus?
>>>>>>
>>>>>> I only have a dongle with both display and usb that does 2ln dp
>>>>>> (I tested 4ln dp on a type-c display that I don't think has a hub)
>>>>>>
>>>>>> USB3 - yes, USB2 - no (but it works after a replug)
>>>>>>
>>>>>> Are you talking about essentially doing qcom,select-utmi-as-pipe-clk
>>>>>> at runtime?
>>>>>
>>>>> I think so.
>>>>
>>>> So after quite some time playing with that, I noticed that the USB2
>>>> device was never actually kicked off the bus.. and works totally fine
>>>> after connecting the display output (2ln DP)
>>>>
>>>> I was looking at dmesg, checking for discovery/disconnect messages,
>>>> but the device was simply never disconnected (which makes sense given
>>>> repurposing USB3 TX/RX lanes doesn't affect the D+/D- of USB2)
>>>>
>>>> I also read some docs and learnt that what we call
>>>> qcom,select-utmi-as-pipe-clk is suppossed to be a debug feature
>>>> and is unnecessary to set on USB2.0-only controllers
>>>>
>>>> The USB controller programming guide though doesn't talk about DP,
>>>> but I'd expect that we may need to set that override for 4lnDP+USB2
>>>> use cases (which I don't have a dongle for)
>>>
>>> Yeah basically we need to:
>>> 1) power-off the USB3 PHY
>>> 2) switch to UTMI clock
>>>
>>> In the following states:
>>> - USB safe mode (USB2 lanes may still be connected)
>>> - 4lanes DP mode
>>> - DP-only mode
>>>
>>> But for this, the dwc3 should also get USB-C events with an addition mode-switch property.
>>> The flatten DWC3 node now allows that !
>>
>> Yeah, I got even more confirmation this is intended..
>>
>> I hacked up something that boils down to:
>>
>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>> index 7977860932b1..e5a0a8ec624d 100644
>> --- a/drivers/usb/dwc3/drd.c
>> +++ b/drivers/usb/dwc3/drd.c
>> @@ -464,6 +464,11 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
>>  		break;
>>  	}
>>  
>> +	if (dwc->mode_fn)
>> +		dwc->mode_fn(dwc, mode);
>>
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> index 7334de85ad10..ea56f5087ecb 100644
>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> +static void mode_fn(struct dwc3 *dwc, enum usb_role role)
>> +{
>> +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>> +
>> +	if (dwc->usb3_generic_phy[0])
>> +		dwc3_qcom_select_utmi_clk(qcom, role == USB_ROLE_NONE);
> 
> This part is a hack for devices with no USB-2 passthrough, isn't it?
> 
>>  }
>>
>>
>> It was easy to tap into because there's already mode switch handling..
>> But obviously it's a hack - should I register a typec_mux in there?
> 
> I think so, we should listen to mode changes, instead of host/device
> changes.
> 
>> Should it go to dwc3-common or dwc3-qcom?
> 
> I'd say, dwc3-qcom. Not all dwc3 controllers are USB 3 capable, not
> talking about the USB-C.

I did some coding and we can't switch clock sources at runtime (not a
huge shocker), but the bigger issue is that, paraphrasing the HPG, the
DWC3 controller must be programmed as if it was not SS-capable (probably
skipping starting some subcores), which is not trivial

I also came up with a sneaky workaround of simply keeping the USB PLL
always-on, but the hardware disagrees to do so if the PHY is configured
in the DP_ONLY mode (which I suppose makes sense)

All in all, I was not able to find people with a device that actually
does 4ln DP + USB2 and IIUC the only drawback would be that USB2 would
not work (and not stall the system). Not sure if/how it recovers after
you'd plug something else into that port later on, but again, I don't
have anyone that could test it.

With that in mind, would you be okay with me resubmitting this series
with just a rebase & taking care of the comment to patch 5 (pertaining
to the default mode setting if svid != DP)?

Konrad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode
  2025-08-06 15:48                   ` Konrad Dybcio
@ 2025-08-07  8:16                     ` Neil Armstrong
  0 siblings, 0 replies; 29+ messages in thread
From: Neil Armstrong @ 2025-08-07  8:16 UTC (permalink / raw)
  To: Konrad Dybcio, Dmitry Baryshkov
  Cc: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Marijn Suijten, linux-arm-msm, linux-phy, devicetree,
	linux-kernel, Krzysztof Kozlowski

On 06/08/2025 17:48, Konrad Dybcio wrote:
> On 6/3/25 3:17 PM, Dmitry Baryshkov wrote:
>> On Tue, Jun 03, 2025 at 01:03:11PM +0200, Konrad Dybcio wrote:
>>> On 6/2/25 10:55 AM, Neil Armstrong wrote:
>>>> On 28/05/2025 18:56, Konrad Dybcio wrote:
>>>>> On 5/28/25 1:22 PM, Dmitry Baryshkov wrote:
>>>>>> On Wed, May 28, 2025 at 01:13:26PM +0200, Konrad Dybcio wrote:
>>>>>>> On 5/28/25 11:00 AM, Dmitry Baryshkov wrote:
>>>>>>>> On Wed, May 28, 2025 at 12:24:02AM +0200, Konrad Dybcio wrote:
>>>>>>>>> On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
>>>>>>>>>> On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
>>>>>>>>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>>>>>>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>>>>>>>>
>>>>>>>>>>> The DisplayPort phy should be left enabled if is still powered on
>>>>>>>>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>>>>>>>>> PHY is not powered off.
>>>>>>>>>>
>>>>>>>>>> This series doesn't seem to solve the USB side of a problem. When the
>>>>>>>>>> PHY is being switch to the 4-lane mode, USB controller looses PIPE
>>>>>>>>>> clock, so it needs to be switched to the USB-2 mode.
>>>>>>>>>
>>>>>>>>> I didn't find issues with that on X13s.. Not sure if it's related, but
>>>>>>>>> on the SL7, after plugging in a 4ln DP connection, I need to plug in
>>>>>>>>> the USB thumb drive twice for the first time (only in that sequence)
>>>>>>>>
>>>>>>>> Might be.
>>>>>>>>
>>>>>>>>> But there's nothing interesting in dmesg and the phy seems to be
>>>>>>>>> programmed with the same values on both the initial and the subsequent
>>>>>>>>> working plug-in
>>>>>>>>
>>>>>>>> Please try using a DP dongle with USB 2 passthrough (there are some).
>>>>>>>> Or just emulate this by enabling DP PHY / DP chain for plugged in USB3
>>>>>>>> devices. Would you be able to see the USB device on a bus?
>>>>>>>
>>>>>>> I only have a dongle with both display and usb that does 2ln dp
>>>>>>> (I tested 4ln dp on a type-c display that I don't think has a hub)
>>>>>>>
>>>>>>> USB3 - yes, USB2 - no (but it works after a replug)
>>>>>>>
>>>>>>> Are you talking about essentially doing qcom,select-utmi-as-pipe-clk
>>>>>>> at runtime?
>>>>>>
>>>>>> I think so.
>>>>>
>>>>> So after quite some time playing with that, I noticed that the USB2
>>>>> device was never actually kicked off the bus.. and works totally fine
>>>>> after connecting the display output (2ln DP)
>>>>>
>>>>> I was looking at dmesg, checking for discovery/disconnect messages,
>>>>> but the device was simply never disconnected (which makes sense given
>>>>> repurposing USB3 TX/RX lanes doesn't affect the D+/D- of USB2)
>>>>>
>>>>> I also read some docs and learnt that what we call
>>>>> qcom,select-utmi-as-pipe-clk is suppossed to be a debug feature
>>>>> and is unnecessary to set on USB2.0-only controllers
>>>>>
>>>>> The USB controller programming guide though doesn't talk about DP,
>>>>> but I'd expect that we may need to set that override for 4lnDP+USB2
>>>>> use cases (which I don't have a dongle for)
>>>>
>>>> Yeah basically we need to:
>>>> 1) power-off the USB3 PHY
>>>> 2) switch to UTMI clock
>>>>
>>>> In the following states:
>>>> - USB safe mode (USB2 lanes may still be connected)
>>>> - 4lanes DP mode
>>>> - DP-only mode
>>>>
>>>> But for this, the dwc3 should also get USB-C events with an addition mode-switch property.
>>>> The flatten DWC3 node now allows that !
>>>
>>> Yeah, I got even more confirmation this is intended..
>>>
>>> I hacked up something that boils down to:
>>>
>>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>>> index 7977860932b1..e5a0a8ec624d 100644
>>> --- a/drivers/usb/dwc3/drd.c
>>> +++ b/drivers/usb/dwc3/drd.c
>>> @@ -464,6 +464,11 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
>>>   		break;
>>>   	}
>>>   
>>> +	if (dwc->mode_fn)
>>> +		dwc->mode_fn(dwc, mode);
>>>
>>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>>> index 7334de85ad10..ea56f5087ecb 100644
>>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>>> +static void mode_fn(struct dwc3 *dwc, enum usb_role role)
>>> +{
>>> +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>>> +
>>> +	if (dwc->usb3_generic_phy[0])
>>> +		dwc3_qcom_select_utmi_clk(qcom, role == USB_ROLE_NONE);
>>
>> This part is a hack for devices with no USB-2 passthrough, isn't it?
>>
>>>   }
>>>
>>>
>>> It was easy to tap into because there's already mode switch handling..
>>> But obviously it's a hack - should I register a typec_mux in there?
>>
>> I think so, we should listen to mode changes, instead of host/device
>> changes.
>>
>>> Should it go to dwc3-common or dwc3-qcom?
>>
>> I'd say, dwc3-qcom. Not all dwc3 controllers are USB 3 capable, not
>> talking about the USB-C.
> 
> I did some coding and we can't switch clock sources at runtime (not a
> huge shocker), but the bigger issue is that, paraphrasing the HPG, the
> DWC3 controller must be programmed as if it was not SS-capable (probably
> skipping starting some subcores), which is not trivial
> 
> I also came up with a sneaky workaround of simply keeping the USB PLL
> always-on, but the hardware disagrees to do so if the PHY is configured
> in the DP_ONLY mode (which I suppose makes sense)
> 
> All in all, I was not able to find people with a device that actually
> does 4ln DP + USB2 and IIUC the only drawback would be that USB2 would
> not work (and not stall the system). Not sure if/how it recovers after
> you'd plug something else into that port later on, but again, I don't
> have anyone that could test it.
> 
> With that in mind, would you be okay with me resubmitting this series
> with just a rebase & taking care of the comment to patch 5 (pertaining
> to the default mode setting if svid != DP)?

Sure please do so, and I'll retest on the SM8550 & SM8650 platforms.

Thanks for looking !
Neil

> 
> Konrad



^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2025-08-07  8:16 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-27 20:40 [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Konrad Dybcio
2025-05-27 20:40 ` [PATCH v3 1/6] dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch Konrad Dybcio
2025-05-27 20:40 ` [PATCH v3 2/6] phy: qcom: qmp-combo: Rename 'mode' to 'phy_mode' Konrad Dybcio
2025-05-27 21:03   ` Dmitry Baryshkov
2025-05-27 20:40 ` [PATCH v3 3/6] phy: qcom: qmp-combo: store DP phy power state Konrad Dybcio
2025-05-27 20:40 ` [PATCH v3 4/6] phy: qcom: qmp-combo: introduce QMPPHY_MODE Konrad Dybcio
2025-05-27 20:40 ` [PATCH v3 5/6] phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE Konrad Dybcio
2025-05-27 21:55   ` Dmitry Baryshkov
2025-05-27 22:22     ` Konrad Dybcio
2025-05-28  8:58       ` Dmitry Baryshkov
2025-05-28 11:21         ` Konrad Dybcio
2025-05-28 11:22           ` Konrad Dybcio
2025-06-02  8:51         ` Neil Armstrong
2025-06-03 10:32           ` Dmitry Baryshkov
2025-05-27 20:40 ` [PATCH v3 6/6] arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: Set up 4-lane DP Konrad Dybcio
2025-05-27 21:10 ` [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode Dmitry Baryshkov
2025-05-27 22:24   ` Konrad Dybcio
2025-05-28  9:00     ` Dmitry Baryshkov
2025-05-28 11:13       ` Konrad Dybcio
2025-05-28 11:22         ` Dmitry Baryshkov
2025-05-28 16:56           ` Konrad Dybcio
2025-06-02  8:55             ` Neil Armstrong
2025-06-03 11:03               ` Konrad Dybcio
2025-06-03 13:17                 ` Dmitry Baryshkov
2025-08-06 15:48                   ` Konrad Dybcio
2025-08-07  8:16                     ` Neil Armstrong
2025-05-28 12:37 ` Rob Herring (Arm)
2025-05-29  8:43 ` Jens Glathe
2025-05-29 11:13 ` Alex

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).