| Message ID | 20240527-topic-sm8x50-upstream-phy-combo-typec-mux-v2-2-a03e68d7b8fc@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode | expand |
On Mon, May 27, 2024 at 10:42:34AM +0200, Neil Armstrong wrote: > Switching the PHY Mode requires the DisplayPort PHY to be powered off, > keep track of the DisplayPort phy power state. How is this different from dp_init_count? > > Signed-off-by: Neil Armstrong <neil.armstrong@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 7f999e8a433d..183cd9cd1884 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -1512,6 +1512,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; > @@ -2685,6 +2686,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; > @@ -2699,6 +2702,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.34.1 >
On 27/05/2024 10:59, Dmitry Baryshkov wrote: > On Mon, May 27, 2024 at 10:42:34AM +0200, Neil Armstrong wrote: >> Switching the PHY Mode requires the DisplayPort PHY to be powered off, >> keep track of the DisplayPort phy power state. > > How is this different from dp_init_count? dp_init_count tracks the DP PHY init, while dp_powered_on tracks the DP PHY beeing powered on by the DRM DP driver, those are not the same state at all. While testing, I figured that de-initializing the DP PHY while is was powered-on by the DRM DP, caused the system to freeze and crash. SO I've added this to track this state and try to de-init the DP phy if still in use. Neil > >> >> Signed-off-by: Neil Armstrong <neil.armstrong@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 7f999e8a433d..183cd9cd1884 100644 >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c >> @@ -1512,6 +1512,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; >> @@ -2685,6 +2686,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; >> @@ -2699,6 +2702,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.34.1 >> >
On Thu, Jun 06, 2024 at 03:29:52PM +0200, Neil Armstrong wrote: > On 27/05/2024 10:59, Dmitry Baryshkov wrote: > > On Mon, May 27, 2024 at 10:42:34AM +0200, Neil Armstrong wrote: > > > Switching the PHY Mode requires the DisplayPort PHY to be powered off, > > > keep track of the DisplayPort phy power state. > > > > How is this different from dp_init_count? > > dp_init_count tracks the DP PHY init, while dp_powered_on tracks > the DP PHY beeing powered on by the DRM DP driver, those are > not the same state at all. > > While testing, I figured that de-initializing the DP PHY while > is was powered-on by the DRM DP, caused the system to freeze and crash. > > SO I've added this to track this state and try to de-init the DP phy > if still in use. If you are to send next iteration, please add these bits to the commit message. Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c index 7f999e8a433d..183cd9cd1884 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c @@ -1512,6 +1512,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; @@ -2685,6 +2686,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; @@ -2699,6 +2702,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;
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> --- drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 5 +++++ 1 file changed, 5 insertions(+)