| Message ID | 1622072989-793-1-git-send-email-khsieh@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | drm/msm/dp: keep aux channel up when power of dp phy | expand |
General note, please Cc dri-devel on freedreno driver patches. Quoting Kuogee Hsieh (2021-05-26 16:49:49) > Aux channel is used to perform management function and should be > running in parallel with main link. Therefore should only power > down main link and keep aux channel running when power down phy. > > Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org> > --- Does this supersede the previous patch[1] or is it in addition to? [1] https://lore.kernel.org/r/1622052503-21158-1-git-send-email-khsieh@codeaurora.org > drivers/gpu/drm/msm/dp/dp_ctrl.c | 4 ---- > drivers/phy/qualcomm/phy-qcom-qmp.c | 11 +++++++++-- > 2 files changed, 9 insertions(+), 6 deletions(-) Given that it touches two subsystems the merge path is questionable. > > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c > index 5115c05..5f93c64 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c > @@ -1844,10 +1844,6 @@ int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl) > > phy_power_off(phy); > > - /* aux channel down, reinit phy */ > - phy_exit(phy); > - phy_init(phy); > - > DRM_DEBUG_DP("DP off link/stream done\n"); > return ret; > } > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c > index b122e63..567e32e 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c > @@ -3214,12 +3214,19 @@ static int qcom_qmp_phy_power_off(struct phy *phy) > { > struct qmp_phy *qphy = phy_get_drvdata(phy); > const struct qmp_phy_cfg *cfg = qphy->cfg; > + u32 val; > > clk_disable_unprepare(qphy->pipe_clk); > > if (cfg->type == PHY_TYPE_DP) { > - /* Assert DP PHY power down */ > - writel(DP_PHY_PD_CTL_PSR_PWRDN, qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL); > + /* > + * Assert DP PHY LANE_0_1, LANE_2_3, PSR power down > + * keep aux channel up Maybe say "keep aux channel up until phy_exit() is called" so we know how long it is supposed to stay on. And just to confirm, it is on in phy_init(), right? > + */ > + val = DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN | > + DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN | > + DP_PHY_PD_CTL_PSR_PWRDN; > + writel(val, qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL); > } else { > /* PHY reset */ > if (!cfg->no_pcs_sw_reset)
On 2021-05-26 21:29, Stephen Boyd wrote: > General note, please Cc dri-devel on freedreno driver patches. > > Quoting Kuogee Hsieh (2021-05-26 16:49:49) >> Aux channel is used to perform management function and should be >> running in parallel with main link. Therefore should only power >> down main link and keep aux channel running when power down phy. >> >> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org> >> --- > > Does this supersede the previous patch[1] or is it in addition to? this is the addition one. > > [1] > https://lore.kernel.org/r/1622052503-21158-1-git-send-email-khsieh@codeaurora.org > >> drivers/gpu/drm/msm/dp/dp_ctrl.c | 4 ---- >> drivers/phy/qualcomm/phy-qcom-qmp.c | 11 +++++++++-- >> 2 files changed, 9 insertions(+), 6 deletions(-) > > Given that it touches two subsystems the merge path is questionable. ok, will separate it out > >> >> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c >> b/drivers/gpu/drm/msm/dp/dp_ctrl.c >> index 5115c05..5f93c64 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c >> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c >> @@ -1844,10 +1844,6 @@ int dp_ctrl_off_link_stream(struct dp_ctrl >> *dp_ctrl) >> >> phy_power_off(phy); >> >> - /* aux channel down, reinit phy */ >> - phy_exit(phy); >> - phy_init(phy); >> - >> DRM_DEBUG_DP("DP off link/stream done\n"); >> return ret; >> } >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c >> b/drivers/phy/qualcomm/phy-qcom-qmp.c >> index b122e63..567e32e 100644 >> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c >> @@ -3214,12 +3214,19 @@ static int qcom_qmp_phy_power_off(struct phy >> *phy) >> { >> struct qmp_phy *qphy = phy_get_drvdata(phy); >> const struct qmp_phy_cfg *cfg = qphy->cfg; >> + u32 val; >> >> clk_disable_unprepare(qphy->pipe_clk); >> >> if (cfg->type == PHY_TYPE_DP) { >> - /* Assert DP PHY power down */ >> - writel(DP_PHY_PD_CTL_PSR_PWRDN, qphy->pcs + >> QSERDES_V3_DP_PHY_PD_CTL); >> + /* >> + * Assert DP PHY LANE_0_1, LANE_2_3, PSR power down >> + * keep aux channel up > > Maybe say "keep aux channel up until phy_exit() is called" so we know > how long it is supposed to stay on. And just to confirm, it is on in > phy_init(), right? > yes, aux channel is on at phy_init >> + */ >> + val = DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN | >> + DP_PHY_PD_CTL_PLL_PWRDN | >> DP_PHY_PD_CTL_DP_CLAMP_EN | >> + DP_PHY_PD_CTL_PSR_PWRDN; >> + writel(val, qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL); >> } else { >> /* PHY reset */ >> if (!cfg->no_pcs_sw_reset)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 5115c05..5f93c64 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1844,10 +1844,6 @@ int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl) phy_power_off(phy); - /* aux channel down, reinit phy */ - phy_exit(phy); - phy_init(phy); - DRM_DEBUG_DP("DP off link/stream done\n"); return ret; } diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c index b122e63..567e32e 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c @@ -3214,12 +3214,19 @@ static int qcom_qmp_phy_power_off(struct phy *phy) { struct qmp_phy *qphy = phy_get_drvdata(phy); const struct qmp_phy_cfg *cfg = qphy->cfg; + u32 val; clk_disable_unprepare(qphy->pipe_clk); if (cfg->type == PHY_TYPE_DP) { - /* Assert DP PHY power down */ - writel(DP_PHY_PD_CTL_PSR_PWRDN, qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL); + /* + * Assert DP PHY LANE_0_1, LANE_2_3, PSR power down + * keep aux channel up + */ + val = DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN | + DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN | + DP_PHY_PD_CTL_PSR_PWRDN; + writel(val, qphy->pcs + QSERDES_V3_DP_PHY_PD_CTL); } else { /* PHY reset */ if (!cfg->no_pcs_sw_reset)
Aux channel is used to perform management function and should be running in parallel with main link. Therefore should only power down main link and keep aux channel running when power down phy. Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org> --- drivers/gpu/drm/msm/dp/dp_ctrl.c | 4 ---- drivers/phy/qualcomm/phy-qcom-qmp.c | 11 +++++++++-- 2 files changed, 9 insertions(+), 6 deletions(-)