| Message ID | 20240229-topic-sm8x50-upstream-phy-combo-typec-mux-v1-0-07e24a231840@linaro.org |
|---|---|
| Headers | show |
| Series | arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode | expand |
Bad copy-pasta, subject should be: [PATCH RFT 7/7] arm64: dts: wcom-sc8280xp-lenovo-thinkpad-x13: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch On 29/02/2024 14:07, Neil Armstrong wrote: > Allow up to 4 lanes for the DisplayPort link from the PHYs to the Controllers > and allow mode-switch events to the QMP Combo PHYs. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > 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 a0fdef55a40a..6c73e0fc001f 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts > @@ -556,7 +556,7 @@ &mdss0_dp0 { > }; > > &mdss0_dp0_out { > - data-lanes = <0 1>; > + data-lanes = <0 1 2 3>; > remote-endpoint = <&usb_0_qmpphy_dp_in>; > }; > > @@ -565,7 +565,7 @@ &mdss0_dp1 { > }; > > &mdss0_dp1_out { > - data-lanes = <0 1>; > + data-lanes = <0 1 2 3>; > remote-endpoint = <&usb_1_qmpphy_dp_in>; > }; > > @@ -1112,6 +1112,7 @@ &usb_0_qmpphy { > vdda-phy-supply = <&vreg_l9d>; > vdda-pll-supply = <&vreg_l4d>; > > + mode-switch; > orientation-switch; > > status = "okay"; > @@ -1149,6 +1150,7 @@ &usb_1_qmpphy { > vdda-phy-supply = <&vreg_l4b>; > vdda-pll-supply = <&vreg_l3b>; > > + mode-switch; > orientation-switch; > > status = "okay"; >
On 29/02/2024 16:25, Dmitry Baryshkov wrote: > On Thu, 29 Feb 2024 at 15:08, Neil Armstrong <neil.armstrong@linaro.org> 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. > > I think this is not fully correct. Please correct me if I'm wrong, but > it is possible to switch between USB / USB+DP / DP-only at runtime. > See the Status Update and Configure commands. Yes, but the current implementation is still valid because we need to have the DP powered-off before changing the PHY mode. I never encountered such setup and I have no idea how to test this. > >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 122 ++++++++++++++++++++++++++++-- >> 1 file changed, 117 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >> index ac5d528fd7a1..b5fb6cbcf867 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> >> @@ -1515,6 +1516,10 @@ struct qmp_combo { >> >> struct typec_switch_dev *sw; >> enum typec_orientation orientation; >> + >> + struct typec_mux_dev *mux; >> + unsigned long mux_mode; >> + unsigned int svid; >> }; >> >> static void qmp_v3_dp_aux_init(struct qmp_combo *qmp); >> @@ -3295,17 +3300,111 @@ 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 qphy_mode new_mode; >> + unsigned int svid; >> + >> + if (state->mode == qmp->mode) >> + return 0; >> + >> + mutex_lock(&qmp->phy_mutex); >> + >> + if (state->alt) >> + svid = state->alt->svid; >> + else >> + svid = 0; // No SVID >> + >> + if (svid) { > > We should check svid against USB_TYPEC_DP_SID. Otherwise the driver > will mishandle the USB_SID_PD states. Ack will do > >> + switch (state->mode) { >> + /* DP Only */ >> + case TYPEC_DP_STATE_C: >> + case TYPEC_DP_STATE_E: >> + new_mode = QPHY_MODE_DP_ONLY; >> + break; >> + >> + /* DP + USB */ >> + case TYPEC_DP_STATE_D: >> + case TYPEC_DP_STATE_F: >> + /* Safe fallback...*/ >> + default: >> + new_mode = QPHY_MODE_COMBO; >> + break; >> + } >> + } else { >> + /* Only switch to USB_ONLY when we know we only have USB3 */ >> + if (qmp->mux_mode == TYPEC_MODE_USB3) >> + new_mode = QPHY_MODE_USB_ONLY; >> + else >> + new_mode = QPHY_MODE_COMBO; >> + } >> + >> + if (new_mode == qmp->init_mode) { >> + dev_dbg(qmp->dev, "typec_mux_set: same phy mode, bail out\n"); >> + qmp->mode = state->mode; >> + goto out; >> + } >> + >> + if (qmp->init_mode != QPHY_MODE_USB_ONLY && qmp->dp_powered_on) { >> + dev_dbg(qmp->dev, "typec_mux_set: DP is still powered on, delaying switch\n"); >> + goto out; >> + } >> + >> + dev_dbg(qmp->dev, "typec_mux_set: switching from phy mode %d to %d\n", >> + qmp->init_mode, new_mode); >> + >> + qmp->mux_mode = state->mode; >> + qmp->init_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 (qmp->init_mode == QPHY_MODE_DP_ONLY && qmp->usb_init_count) { >> + qmp->usb_init_count--; >> + } else if (qmp->init_mode != QPHY_MODE_DP_ONLY) { >> + qmp_combo_usb_power_on(qmp->usb_phy); >> + if (!qmp->usb_init_count) >> + qmp->usb_init_count++; >> + } >> + if (qmp->init_mode != QPHY_MODE_USB_ONLY && qmp->dp_init_count) >> + cfg->dp_aux_init(qmp); >> + } >> + >> +out: >> + mutex_unlock(&qmp->phy_mutex); >> + >> + 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; >> @@ -3316,10 +3415,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; >> } >> @@ -3532,7 +3644,7 @@ static int qmp_combo_probe(struct platform_device *pdev) >> if (ret) >> return ret; >> >> - ret = qmp_combo_typec_switch_register(qmp); >> + ret = qmp_combo_typec_register(qmp); >> if (ret) >> return ret; >> >> >> -- >> 2.34.1 >> >> > >
On Thu, 29 Feb 2024 at 17:47, Neil Armstrong <neil.armstrong@linaro.org> wrote: > > On 29/02/2024 16:25, Dmitry Baryshkov wrote: > > On Thu, 29 Feb 2024 at 15:08, Neil Armstrong <neil.armstrong@linaro.org> 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. > > > > I think this is not fully correct. Please correct me if I'm wrong, but > > it is possible to switch between USB / USB+DP / DP-only at runtime. > > See the Status Update and Configure commands. > > Yes, but the current implementation is still valid because we need to > have the DP powered-off before changing the PHY mode. Even for switching between 2 lane and 4 lane modes? I'll check how my USB-A+DP dongles work with respect to the altmode configuration. > > I never encountered such setup and I have no idea how to test this. > > > > >> > >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
On Thu, Feb 29, 2024 at 02:07:07PM +0100, Neil Armstrong wrote: > Allow up to 4 lanes for the DisplayPort link from the PHYs to the Controllers > and allow mode-switch events to the QMP Combo PHYs. > Please adjust $subject and commit message to suite the x13s.dts... With this series I'm reaching 4k@60 on my X13s (with some difficulty due to current hotplug issues in the DP driver) - but 4-lane DP works, and so does 2-lane combo. I tested switching between DP-only and a USB device, this worked fine a few (3-4) times, after which the USB device stopped showing up. The DP display continued to work nicely and the debug prints from the driver indicates that we're moving back and forth between the modes... The problems I had when trying to implement this previously, with the device crashing on disconnect have not been seen, across 20+ attempts. Regards, Bjorn > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > 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 a0fdef55a40a..6c73e0fc001f 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts > @@ -556,7 +556,7 @@ &mdss0_dp0 { > }; > > &mdss0_dp0_out { > - data-lanes = <0 1>; > + data-lanes = <0 1 2 3>; > remote-endpoint = <&usb_0_qmpphy_dp_in>; > }; > > @@ -565,7 +565,7 @@ &mdss0_dp1 { > }; > > &mdss0_dp1_out { > - data-lanes = <0 1>; > + data-lanes = <0 1 2 3>; > remote-endpoint = <&usb_1_qmpphy_dp_in>; > }; > > @@ -1112,6 +1112,7 @@ &usb_0_qmpphy { > vdda-phy-supply = <&vreg_l9d>; > vdda-pll-supply = <&vreg_l4d>; > > + mode-switch; > orientation-switch; > > status = "okay"; > @@ -1149,6 +1150,7 @@ &usb_1_qmpphy { > vdda-phy-supply = <&vreg_l4b>; > vdda-pll-supply = <&vreg_l3b>; > > + mode-switch; > orientation-switch; > > status = "okay"; > > -- > 2.34.1 >
On 15/03/2024 18:19, Luca Weiss wrote: > On Thu Feb 29, 2024 at 2:07 PM CET, Neil Armstrong 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 Neil, > > I tried this on QCM6490/SC7280 which should also support 4-lane DP but I > haven't had any success so far. > > On top of your patches I added the following for my device: > > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts > index f5bd51806819..e7be17844da1 100644 > --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts > @@ -712,7 +712,7 @@ &mdss_dp { > }; > > &mdss_dp_out { > - data-lanes = <0 1>; > + data-lanes = <0 1 2 3>; > remote-endpoint = <&usb_dp_qmpphy_dp_in>; > }; > > @@ -1344,6 +1344,7 @@ &usb_1_qmpphy { > vdda-phy-supply = <&vreg_l6b>; > vdda-pll-supply = <&vreg_l1b>; > > + mode-switch; > orientation-switch; > > status = "okay"; > > > The output of the dp_debug file shows it's trying to use 4 lanes: > > name = msm_dp > drm_dp_link > rate = 540000 > num_lanes = 4 > capabilities = 1 > dp_panel_info: > active = 0x0 > back_porch = 0x0 > front_porch = 0x0 > sync_width = 0x0 > active_low = 0x0 > h_skew = 0 > refresh rate = 0 > pixel clock khz = 0 > bpp = 0 > dp_link: > test_requested = 128 > num_lanes = 4 > bw_code = 20 > lclk = 540000000 > v_level = 2 > p_level = 0 > > But the monitor stays black and the following appears in dmesg: > (starts with plugging in a dongle, ends with unplugging it again) > > [ 1773.538161] xhci-hcd xhci-hcd.2.auto: xHCI Host Controller > [ 1773.538197] xhci-hcd xhci-hcd.2.auto: new USB bus registered, assigned bus number 1 > [ 1773.540215] xhci-hcd xhci-hcd.2.auto: hcc params 0x0230fe65 hci version 0x110 quirks 0x0000008000000010 > [ 1773.540260] xhci-hcd xhci-hcd.2.auto: irq 185, io mem 0x0a600000 > [ 1773.540372] xhci-hcd xhci-hcd.2.auto: xHCI Host Controller > [ 1773.540384] xhci-hcd xhci-hcd.2.auto: new USB bus registered, assigned bus number 2 > [ 1773.540396] xhci-hcd xhci-hcd.2.auto: Host supports USB 3.0 SuperSpeed > [ 1773.540524] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002, bcdDevice= 6.08 > [ 1773.540534] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1 > [ 1773.540541] usb usb1: Product: xHCI Host Controller > [ 1773.540547] usb usb1: Manufacturer: Linux 6.8.0-00058-g113103fa3b95 xhci-hcd > [ 1773.540554] usb usb1: SerialNumber: xhci-hcd.2.auto > [ 1773.540999] hub 1-0:1.0: USB hub found > [ 1773.541028] hub 1-0:1.0: 1 port detected > [ 1773.542010] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM. > [ 1773.542146] usb usb2: New USB device found, idVendor=1d6b, idProduct=0003, bcdDevice= 6.08 > [ 1773.542162] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1 > [ 1773.542174] usb usb2: Product: xHCI Host Controller > [ 1773.542183] usb usb2: Manufacturer: Linux 6.8.0-00058-g113103fa3b95 xhci-hcd > [ 1773.542193] usb usb2: SerialNumber: xhci-hcd.2.auto > [ 1773.543241] hub 2-0:1.0: USB hub found > [ 1773.543282] hub 2-0:1.0: 1 port detected > [ 1775.563969] [drm:dp_ctrl_link_train] *ERROR* max v_level reached > [ 1775.564031] [drm:dp_ctrl_link_train] *ERROR* link training #1 failed. ret=-11 Interesting #1 means the 4 lanes are not physically connected to the other side, perhaps QCM6490/SC7280 requires a specific way to enable the 4 lanes in the PHY, or some fixups in the init tables. Abhinav, any suggestions ? Neil > [ 1775.597965] [drm:dp_display_process_hpd_high] *ERROR* failed to complete DP link training > [ 1775.598149] [drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x01000000 when not busy > [ 1776.632081] [drm:dp_ctrl_link_train] *ERROR* max v_level reached > [ 1776.632145] [drm:dp_ctrl_link_train] *ERROR* link training #1 failed. ret=-11 > [ 1776.662978] [drm:dp_display_process_hpd_high] *ERROR* failed to complete DP link training > [ 1776.663039] [drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x01000000 when not busy > [ 1777.717501] [drm:dp_ctrl_link_train] *ERROR* max v_level reached > [ 1777.717524] [drm:dp_ctrl_link_train] *ERROR* link training #1 failed. ret=-11 > [ 1777.751427] [drm:dp_display_process_hpd_high] *ERROR* failed to complete DP link training > [ 1777.751518] [drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x01000000 when not busy > [ 1778.793550] [drm:dp_ctrl_link_train] *ERROR* max v_level reached > [ 1778.793617] [drm:dp_ctrl_link_train] *ERROR* link training #1 failed. ret=-11 > [ 1778.827260] [drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x01000000 when not busy > [ 1778.827334] [drm:dp_display_process_hpd_high] *ERROR* failed to complete DP link training > [ 1779.279889] xhci-hcd xhci-hcd.2.auto: remove, state 1 > [ 1779.279942] usb usb2: USB disconnect, device number 1 > [ 1779.311920] xhci-hcd xhci-hcd.2.auto: USB bus 2 deregistered > [ 1779.311987] xhci-hcd xhci-hcd.2.auto: remove, state 4 > [ 1779.312019] usb usb1: USB disconnect, device number 1 > [ 1779.317772] xhci-hcd xhci-hcd.2.auto: USB bus 1 deregistered > > Regards > Luca > > >> ... >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> Neil Armstrong (7): >> dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Add mode-switch >> phy: qcom: qmp-combo: store DP phy power state >> phy: qcom: qmp-combo: introduce QPHY_MODE >> phy: qcom: qmp-combo: register a typec mux to change the QPHY_MODE >> arm64: dts: qcom-sm8550: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch >> arm64: dts: qcom-sm8650: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch >> arm64: dts: qcom-mode-switch: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch >> >> .../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 5 + >> .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 6 +- >> arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 3 +- >> arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 3 +- >> arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 3 +- >> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 168 +++++++++++++++++++-- >> 6 files changed, 173 insertions(+), 15 deletions(-) >> --- >> base-commit: b321c0e8ca754d8cd9f23ceba958e3ea93c6519e >> change-id: 20240229-topic-sm8x50-upstream-phy-combo-typec-mux-31b5252513c9 >> >> Best regards, >
On Fri, Mar 15, 2024 at 06:35:15PM +0100, Neil Armstrong wrote: > On 15/03/2024 18:19, Luca Weiss wrote: > > On Thu Feb 29, 2024 at 2:07 PM CET, Neil Armstrong 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 Neil, > > > > I tried this on QCM6490/SC7280 which should also support 4-lane DP but I > > haven't had any success so far. > > [..] > > [ 1775.563969] [drm:dp_ctrl_link_train] *ERROR* max v_level reached > > [ 1775.564031] [drm:dp_ctrl_link_train] *ERROR* link training #1 failed. ret=-11 > > Interesting #1 means the 4 lanes are not physically connected to the other side, > perhaps QCM6490/SC7280 requires a specific way to enable the 4 lanes in the PHY, > or some fixups in the init tables. > I tested the same on rb3gen2 (qcs6490) a couple of weeks ago, with the same outcome. Looking at the AUX reads, after switching to 4-lane the link training is failing on all 4 lanes, in contrast to succeeding only on the first 2 if you e.g. forget to mux the other two. As such, my expectation is that there's something wrong in the QMP PHY (or possibly redriver) for this platform. Regards, Bjorn
On 16.03.2024 5:01 PM, Bjorn Andersson wrote: > On Fri, Mar 15, 2024 at 06:35:15PM +0100, Neil Armstrong wrote: >> On 15/03/2024 18:19, Luca Weiss wrote: >>> On Thu Feb 29, 2024 at 2:07 PM CET, Neil Armstrong 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 Neil, >>> >>> I tried this on QCM6490/SC7280 which should also support 4-lane DP but I >>> haven't had any success so far. >>> > [..] >>> [ 1775.563969] [drm:dp_ctrl_link_train] *ERROR* max v_level reached >>> [ 1775.564031] [drm:dp_ctrl_link_train] *ERROR* link training #1 failed. ret=-11 >> >> Interesting #1 means the 4 lanes are not physically connected to the other side, >> perhaps QCM6490/SC7280 requires a specific way to enable the 4 lanes in the PHY, >> or some fixups in the init tables. >> > > I tested the same on rb3gen2 (qcs6490) a couple of weeks ago, with the > same outcome. Looking at the AUX reads, after switching to 4-lane the > link training is failing on all 4 lanes, in contrast to succeeding only > on the first 2 if you e.g. forget to mux the other two. > > As such, my expectation is that there's something wrong in the QMP PHY > (or possibly redriver) for this platform. Do we have any downstream tag where 4lane dp works? I'm willing to believe the PHY story.. Konrad
On Tue Mar 26, 2024 at 10:02 PM CET, Konrad Dybcio wrote: > On 16.03.2024 5:01 PM, Bjorn Andersson wrote: > > On Fri, Mar 15, 2024 at 06:35:15PM +0100, Neil Armstrong wrote: > >> On 15/03/2024 18:19, Luca Weiss wrote: > >>> On Thu Feb 29, 2024 at 2:07 PM CET, Neil Armstrong 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 Neil, > >>> > >>> I tried this on QCM6490/SC7280 which should also support 4-lane DP but I > >>> haven't had any success so far. > >>> > > [..] > >>> [ 1775.563969] [drm:dp_ctrl_link_train] *ERROR* max v_level reached > >>> [ 1775.564031] [drm:dp_ctrl_link_train] *ERROR* link training #1 failed. ret=-11 > >> > >> Interesting #1 means the 4 lanes are not physically connected to the other side, > >> perhaps QCM6490/SC7280 requires a specific way to enable the 4 lanes in the PHY, > >> or some fixups in the init tables. > >> > > > > I tested the same on rb3gen2 (qcs6490) a couple of weeks ago, with the > > same outcome. Looking at the AUX reads, after switching to 4-lane the > > link training is failing on all 4 lanes, in contrast to succeeding only > > on the first 2 if you e.g. forget to mux the other two. > > > > As such, my expectation is that there's something wrong in the QMP PHY > > (or possibly redriver) for this platform. > > Do we have any downstream tag where 4lane dp works? I'm willing to believe > the PHY story.. Just tested on Fairphone 5 downstream and 4 lane appears to work there. This is with an USB-C to HDMI adapter that only does HDMI. FP5:/ # cat /sys/kernel/debug/drm_dp/dp_debug state=0x20a5 link_rate=270000 num_lanes=4 resolution=2560x1440@60Hz pclock=241500KHz bpp=24 test_req=DP_LINK_STATUS_UPDATED lane_count=4 bw_code=10 v_level=0 p_level=0 Sources are here: https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-5.4/+/refs/heads/odm/rc/target/13/fp5 And probably more importantly techpack/display: https://gerrit-public.fairphone.software/plugins/gitiles/platform/vendor/opensource/display-drivers/+/refs/heads/odm/rc/target/13/fp5 Dts if useful: https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/refs/heads/kernel/13/fp5 Regards Luca > > Konrad
On Fri Apr 5, 2024 at 10:08 AM CEST, Neil Armstrong wrote: > Hi Luca, > > On 29/03/2024 10:02, Luca Weiss wrote: > > On Tue Mar 26, 2024 at 10:02 PM CET, Konrad Dybcio wrote: > >> On 16.03.2024 5:01 PM, Bjorn Andersson wrote: > >>> On Fri, Mar 15, 2024 at 06:35:15PM +0100, Neil Armstrong wrote: > >>>> On 15/03/2024 18:19, Luca Weiss wrote: > >>>>> On Thu Feb 29, 2024 at 2:07 PM CET, Neil Armstrong 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 Neil, > >>>>> > >>>>> I tried this on QCM6490/SC7280 which should also support 4-lane DP but I > >>>>> haven't had any success so far. > >>>>> > >>> [..] > >>>>> [ 1775.563969] [drm:dp_ctrl_link_train] *ERROR* max v_level reached > >>>>> [ 1775.564031] [drm:dp_ctrl_link_train] *ERROR* link training #1 failed. ret=-11 > >>>> > >>>> Interesting #1 means the 4 lanes are not physically connected to the other side, > >>>> perhaps QCM6490/SC7280 requires a specific way to enable the 4 lanes in the PHY, > >>>> or some fixups in the init tables. > >>>> > >>> > >>> I tested the same on rb3gen2 (qcs6490) a couple of weeks ago, with the > >>> same outcome. Looking at the AUX reads, after switching to 4-lane the > >>> link training is failing on all 4 lanes, in contrast to succeeding only > >>> on the first 2 if you e.g. forget to mux the other two. > >>> > >>> As such, my expectation is that there's something wrong in the QMP PHY > >>> (or possibly redriver) for this platform. > >> > >> Do we have any downstream tag where 4lane dp works? I'm willing to believe > >> the PHY story.. > > > > Just tested on Fairphone 5 downstream and 4 lane appears to work there. > > This is with an USB-C to HDMI adapter that only does HDMI. > > > > FP5:/ # cat /sys/kernel/debug/drm_dp/dp_debug > > state=0x20a5 > > link_rate=270000 > > num_lanes=4 > > resolution=2560x1440@60Hz > > pclock=241500KHz > > bpp=24 > > test_req=DP_LINK_STATUS_UPDATED > > lane_count=4 > > bw_code=10 > > v_level=0 > > p_level=0 > > > > Sources are here: > > https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-5.4/+/refs/heads/odm/rc/target/13/fp5 > > And probably more importantly techpack/display: > > https://gerrit-public.fairphone.software/plugins/gitiles/platform/vendor/opensource/display-drivers/+/refs/heads/odm/rc/target/13/fp5 > > Dts if useful: > > https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/refs/heads/kernel/13/fp5 > > Could you retry with this applied ? > > https://lore.kernel.org/all/20240405000111.1450598-1-swboyd@chromium.org/ Unfortunately I do not see any change with this on QCM6490 Fairphone 5 and 4-lane DP. Regards Luca > > Thanks, > Neil > > > > > Regards > > Luca > > > >> > >> Konrad > >
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 ... Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- Neil Armstrong (7): dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Add mode-switch phy: qcom: qmp-combo: store DP phy power state phy: qcom: qmp-combo: introduce QPHY_MODE phy: qcom: qmp-combo: register a typec mux to change the QPHY_MODE arm64: dts: qcom-sm8550: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch arm64: dts: qcom-sm8650: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch arm64: dts: qcom-mode-switch: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch .../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 5 + .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 6 +- arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 3 +- arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 3 +- arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 3 +- drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 168 +++++++++++++++++++-- 6 files changed, 173 insertions(+), 15 deletions(-) --- base-commit: b321c0e8ca754d8cd9f23ceba958e3ea93c6519e change-id: 20240229-topic-sm8x50-upstream-phy-combo-typec-mux-31b5252513c9 Best regards,