From: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>

Each link has separate power controls, but experimental results show
we need to use an all-or-none approach to the link power management.

This change has marginal power impacts, the DSP needs to be powered
anyways before SoundWire links can be powered, and even when powered a
link can be in clock-stopped mode.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.l...@linux.intel.com>
---
 drivers/soundwire/intel.c | 70 +++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 24 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index b82d02af3c4f..154aa7c0561a 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -63,7 +63,9 @@ MODULE_PARM_DESC(sdw_md_flags, "SoundWire Intel Master device 
flags (0x0 all off
 #define SDW_SHIM_WAKESTS               0x192
 
 #define SDW_SHIM_LCTL_SPA              BIT(0)
+#define SDW_SHIM_LCTL_SPA_MASK         GENMASK(3, 0)
 #define SDW_SHIM_LCTL_CPA              BIT(8)
+#define SDW_SHIM_LCTL_CPA_MASK         GENMASK(11, 8)
 
 #define SDW_SHIM_SYNC_SYNCPRD_VAL_24   (24000 / SDW_CADENCE_GSYNC_KHZ - 1)
 #define SDW_SHIM_SYNC_SYNCPRD_VAL_38_4 (38400 / SDW_CADENCE_GSYNC_KHZ - 1)
@@ -295,8 +297,8 @@ static int intel_link_power_up(struct sdw_intel *sdw)
        u32 *shim_mask = sdw->link_res->shim_mask;
        struct sdw_bus *bus = &sdw->cdns.bus;
        struct sdw_master_prop *prop = &bus->prop;
-       int spa_mask, cpa_mask;
-       int link_control;
+       u32 spa_mask, cpa_mask;
+       u32 link_control;
        int ret = 0;
        u32 syncprd;
        u32 sync_reg;
@@ -319,6 +321,8 @@ static int intel_link_power_up(struct sdw_intel *sdw)
                syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_24;
 
        if (!*shim_mask) {
+               dev_dbg(sdw->cdns.dev, "%s: powering up all links\n", __func__);
+
                /* we first need to program the SyncPRD/CPU registers */
                dev_dbg(sdw->cdns.dev,
                        "%s: first link up, programming SYNCPRD\n", __func__);
@@ -331,21 +335,24 @@ static int intel_link_power_up(struct sdw_intel *sdw)
                /* Set SyncCPU bit */
                sync_reg |= SDW_SHIM_SYNC_SYNCCPU;
                intel_writel(shim, SDW_SHIM_SYNC, sync_reg);
-       }
 
-       /* Link power up sequence */
-       link_control = intel_readl(shim, SDW_SHIM_LCTL);
-       spa_mask = (SDW_SHIM_LCTL_SPA << link_id);
-       cpa_mask = (SDW_SHIM_LCTL_CPA << link_id);
-       link_control |=  spa_mask;
+               /* Link power up sequence */
+               link_control = intel_readl(shim, SDW_SHIM_LCTL);
 
-       ret = intel_set_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
-       if (ret < 0) {
-               dev_err(sdw->cdns.dev, "Failed to power up link: %d\n", ret);
-               goto out;
-       }
+               /* only power-up enabled links */
+               spa_mask = sdw->link_res->link_mask <<
+                       SDW_REG_SHIFT(SDW_SHIM_LCTL_SPA_MASK);
+               cpa_mask = sdw->link_res->link_mask <<
+                       SDW_REG_SHIFT(SDW_SHIM_LCTL_CPA_MASK);
+
+               link_control |=  spa_mask;
+
+               ret = intel_set_bit(shim, SDW_SHIM_LCTL, link_control, 
cpa_mask);
+               if (ret < 0) {
+                       dev_err(sdw->cdns.dev, "Failed to power up link: %d\n", 
ret);
+                       goto out;
+               }
 
-       if (!*shim_mask) {
                /* SyncCPU will change once link is active */
                ret = intel_wait_bit(shim, SDW_SHIM_SYNC,
                                     SDW_SHIM_SYNC_SYNCCPU, 0);
@@ -483,7 +490,7 @@ static void intel_shim_wake(struct sdw_intel *sdw, bool 
wake_enable)
 
 static int intel_link_power_down(struct sdw_intel *sdw)
 {
-       int link_control, spa_mask, cpa_mask;
+       u32 link_control, spa_mask, cpa_mask;
        unsigned int link_id = sdw->instance;
        void __iomem *shim = sdw->link_res->shim;
        u32 *shim_mask = sdw->link_res->shim_mask;
@@ -493,24 +500,39 @@ static int intel_link_power_down(struct sdw_intel *sdw)
 
        intel_shim_master_ip_to_glue(sdw);
 
-       /* Link power down sequence */
-       link_control = intel_readl(shim, SDW_SHIM_LCTL);
-       spa_mask = ~(SDW_SHIM_LCTL_SPA << link_id);
-       cpa_mask = (SDW_SHIM_LCTL_CPA << link_id);
-       link_control &=  spa_mask;
-
-       ret = intel_clear_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
-
        if (!(*shim_mask & BIT(link_id)))
                dev_err(sdw->cdns.dev,
                        "%s: Unbalanced power-up/down calls\n", __func__);
 
        *shim_mask &= ~BIT(link_id);
 
+       if (!*shim_mask) {
+
+               dev_dbg(sdw->cdns.dev, "%s: powering down all links\n", 
__func__);
+
+               /* Link power down sequence */
+               link_control = intel_readl(shim, SDW_SHIM_LCTL);
+
+               /* only power-down enabled links */
+               spa_mask = (~sdw->link_res->link_mask) <<
+                       SDW_REG_SHIFT(SDW_SHIM_LCTL_SPA_MASK);
+               cpa_mask = sdw->link_res->link_mask <<
+                       SDW_REG_SHIFT(SDW_SHIM_LCTL_CPA_MASK);
+
+               link_control &=  spa_mask;
+
+               ret = intel_clear_bit(shim, SDW_SHIM_LCTL, link_control, 
cpa_mask);
+       }
+
+       link_control = intel_readl(shim, SDW_SHIM_LCTL);
+
        mutex_unlock(sdw->link_res->shim_lock);
 
-       if (ret < 0)
+       if (ret < 0) {
+               dev_err(sdw->cdns.dev, "%s: could not power down link\n", 
__func__);
+
                return ret;
+       }
 
        sdw->cdns.link_up = false;
        return 0;
-- 
2.17.1

Reply via email to