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 <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 4 ----
drivers/phy/qualcomm/phy-qcom-qmp.c | 11 +++++++++--
2 files changed, 9 insertions(+), 6 deletions(-)
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)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
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 <[email protected]>
> ---
Does this supersede the previous patch[1] or is it in addition to?
[1] https://lore.kernel.org/r/[email protected]
> 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 <[email protected]>
>> ---
>
> Does this supersede the previous patch[1] or is it in addition to?
this is the addition one.
>
> [1]
> https://lore.kernel.org/r/[email protected]
>
>> 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)