Messages in this thread | | | Date | Thu, 29 Feb 2024 16:47:46 +0100 | From | Neil Armstrong <> | Subject | Re: [PATCH RFT 4/7] phy: qcom: qmp-combo: register a typec mux to change the QPHY_MODE |
| |
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 >> >> > >
| |