From 5ee74eb280d0aa1f27fbdba5dcbde31e2df369e2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:56 +0800 Subject: soundwire: intel: don't manage link power individually 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 Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200901150556.19432-11-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 70 +++++++++++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 24 deletions(-) (limited to 'drivers/soundwire/intel.c') diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 97c8cfc54ddd..710f5eba936b 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; -- cgit v1.2.3