linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dp: keep aux channel up when power of dp phy
@ 2021-05-26 23:49 Kuogee Hsieh
  2021-05-27  4:29 ` Stephen Boyd
  0 siblings, 1 reply; 3+ messages in thread
From: Kuogee Hsieh @ 2021-05-26 23:49 UTC (permalink / raw)
  To: robdclark, sean, swboyd, vkoul, agross, bjorn.andersson
  Cc: abhinavk, aravindh, khsieh, freedreno, linux-arm-msm, linux-kernel

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(-)

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/msm/dp: keep aux channel up when power of dp phy
  2021-05-26 23:49 [PATCH] drm/msm/dp: keep aux channel up when power of dp phy Kuogee Hsieh
@ 2021-05-27  4:29 ` Stephen Boyd
  2021-05-27 15:58   ` khsieh
  0 siblings, 1 reply; 3+ messages in thread
From: Stephen Boyd @ 2021-05-27  4:29 UTC (permalink / raw)
  To: Kuogee Hsieh, agross, bjorn.andersson, robdclark, sean, vkoul
  Cc: abhinavk, aravindh, freedreno, linux-arm-msm, linux-kernel

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)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/msm/dp: keep aux channel up when power of dp phy
  2021-05-27  4:29 ` Stephen Boyd
@ 2021-05-27 15:58   ` khsieh
  0 siblings, 0 replies; 3+ messages in thread
From: khsieh @ 2021-05-27 15:58 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: agross, bjorn.andersson, robdclark, sean, vkoul, abhinavk,
	aravindh, freedreno, linux-arm-msm, linux-kernel

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)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-27 15:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-26 23:49 [PATCH] drm/msm/dp: keep aux channel up when power of dp phy Kuogee Hsieh
2021-05-27  4:29 ` Stephen Boyd
2021-05-27 15:58   ` khsieh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).