From 4f3ce1cb0b7ca6143d25c6ad446ab03ab0e1e306 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 13:30:15 -0600 Subject: [PATCH 01/27] soundwire: cadence: merge routines to clear/set bits Use a single loop to wait for hardware to set/clear fields. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 0048ace63daae3..8d23fd67a9c61d 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -212,26 +212,6 @@ static inline void cdns_updatel(struct sdw_cdns *cdns, cdns_writel(cdns, offset, tmp); } -static int cdns_clear_bit(struct sdw_cdns *cdns, int offset, u32 value) -{ - int timeout = 10; - u32 reg_read; - - writel(value, cdns->registers + offset); - - /* Wait for bit to be self cleared */ - do { - reg_read = readl(cdns->registers + offset); - if ((reg_read & value) == 0) - return 0; - - timeout--; - usleep_range(50, 100); - } while (timeout != 0); - - return -EAGAIN; -} - static int cdns_set_wait(struct sdw_cdns *cdns, int offset, u32 mask, u32 value) { int timeout = 10; @@ -250,6 +230,14 @@ static int cdns_set_wait(struct sdw_cdns *cdns, int offset, u32 mask, u32 value) return -ETIMEDOUT; } +static int cdns_clear_bit(struct sdw_cdns *cdns, int offset, u32 value) +{ + writel(value, cdns->registers + offset); + + /* Wait for bit to be self cleared */ + return cdns_set_wait(cdns, offset, value, 0); +} + /* * all changes to the MCP_CONFIG, MCP_CONTROL, MCP_CMDCTRL and MCP_PHYCTRL * need to be confirmed with a write to MCP_CONFIG_UPDATE From 363123a3851c789a9edd24ce18bbe0d5e62ffd9d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 13:43:09 -0600 Subject: [PATCH 02/27] soundwire: cadence: s/update_config/config_update Somehow we inverted the two, align with register definition to avoid further confusion. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 8d23fd67a9c61d..44c65dd88f3636 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -242,7 +242,7 @@ static int cdns_clear_bit(struct sdw_cdns *cdns, int offset, u32 value) * all changes to the MCP_CONFIG, MCP_CONTROL, MCP_CMDCTRL and MCP_PHYCTRL * need to be confirmed with a write to MCP_CONFIG_UPDATE */ -static int cdns_update_config(struct sdw_cdns *cdns) +static int cdns_config_update(struct sdw_cdns *cdns) { int ret; @@ -845,7 +845,7 @@ int sdw_cdns_exit_reset(struct sdw_cdns *cdns) CDNS_MCP_CONFIG_OP_NORMAL); /* commit changes */ - return cdns_update_config(cdns); + return cdns_config_update(cdns); } EXPORT_SYMBOL(sdw_cdns_exit_reset); @@ -1102,7 +1102,7 @@ int sdw_cdns_init(struct sdw_cdns *cdns) cdns_writel(cdns, CDNS_MCP_CONFIG, val); /* commit changes */ - return cdns_update_config(cdns); + return cdns_config_update(cdns); } EXPORT_SYMBOL(sdw_cdns_init); @@ -1314,7 +1314,7 @@ int sdw_cdns_clock_stop(struct sdw_cdns *cdns, bool block_wake) CDNS_MCP_CONTROL_CMD_ACCEPT, 0); /* commit changes */ - cdns_update_config(cdns); + cdns_config_update(cdns); /* Prepare slaves for clock stop */ ret = sdw_bus_prep_clk_stop(&cdns->bus); @@ -1396,7 +1396,7 @@ int sdw_cdns_clock_restart(struct sdw_cdns *cdns, bool bus_reset) CDNS_MCP_CONFIG_OP, CDNS_MCP_CONFIG_OP_NORMAL); - cdns_update_config(cdns); + cdns_config_update(cdns); if (!bus_reset) { ret = sdw_bus_exit_clk_stop(&cdns->bus); From d59c1290d0149dba2919cfff713c4082d67d6bb2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 13:56:10 -0600 Subject: [PATCH 03/27] soundwire: cadence: handle error cases with CONFIG_UPDATE config_update() may time out or cannot be use in ClockStopMode, cover all cases and bail as needed. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 44c65dd88f3636..b36462940813ca 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -246,6 +246,11 @@ static int cdns_config_update(struct sdw_cdns *cdns) { int ret; + if (sdw_cdns_is_clock_stop(cdns)) { + dev_err(cdns->dev, "Cannot program MCP_CONFIG_UPDATE in ClockStopMode\n"); + return -EINVAL; + } + ret = cdns_clear_bit(cdns, CDNS_MCP_CONFIG_UPDATE, CDNS_MCP_CONFIG_UPDATE_BIT); if (ret < 0) @@ -1314,7 +1319,11 @@ int sdw_cdns_clock_stop(struct sdw_cdns *cdns, bool block_wake) CDNS_MCP_CONTROL_CMD_ACCEPT, 0); /* commit changes */ - cdns_config_update(cdns); + ret = cdns_config_update(cdns); + if (ret < 0) { + dev_err(cdns->dev, "%s: config_update failed\n", __func__); + return ret; + } /* Prepare slaves for clock stop */ ret = sdw_bus_prep_clk_stop(&cdns->bus); @@ -1396,7 +1405,11 @@ int sdw_cdns_clock_restart(struct sdw_cdns *cdns, bool bus_reset) CDNS_MCP_CONFIG_OP, CDNS_MCP_CONFIG_OP_NORMAL); - cdns_config_update(cdns); + ret = cdns_config_update(cdns); + if (ret < 0) { + dev_err(cdns->dev, "%s: config_update failed\n", __func__); + return ret; + } if (!bus_reset) { ret = sdw_bus_exit_clk_stop(&cdns->bus); From 56c70f05babf4ea5e5827240367c289fe42a54ae Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 14:20:19 -0600 Subject: [PATCH 04/27] soundwire: cadence: move clock/SSP related inits to dedicated function This helps isolate code and align with recommended programming flows Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index b36462940813ca..aa08736f548ffa 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1044,11 +1044,7 @@ static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols) return val; } -/** - * sdw_cdns_init() - Cadence initialization - * @cdns: Cadence instance - */ -int sdw_cdns_init(struct sdw_cdns *cdns) +static void cdns_init_clock_ctrl(struct sdw_cdns *cdns) { struct sdw_bus *bus = &cdns->bus; struct sdw_master_prop *prop = &bus->prop; @@ -1074,6 +1070,18 @@ int sdw_cdns_init(struct sdw_cdns *cdns) /* Set SSP interval to default value */ cdns_writel(cdns, CDNS_MCP_SSP_CTRL0, CDNS_DEFAULT_SSP_INTERVAL); cdns_writel(cdns, CDNS_MCP_SSP_CTRL1, CDNS_DEFAULT_SSP_INTERVAL); +} + +/** + * sdw_cdns_init() - Cadence initialization + * @cdns: Cadence instance + */ +int sdw_cdns_init(struct sdw_cdns *cdns) +{ + u32 val; + int ret; + + cdns_init_clock_ctrl(cdns); /* reset msg_count to default value of FIFOLEVEL */ cdns->msg_count = cdns_readl(cdns, CDNS_MCP_FIFOLEVEL); From d172da1f0e9b8f0f29d2ff8e04a90bc526f60c28 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 11 Feb 2020 17:00:16 -0600 Subject: [PATCH 05/27] soundwire: cadence: make SSP interval programmable In multi-master mode, the IP will only accept SSP intervals with integer relationships between the frame rate and the gsync frequency. E.g for a 48kHz frame rate and 4 kHz gsync signal, the SSP interval can only be 1, 2, 3, 4, 6, 12. To simplify we only allow one SSP per gsync interval. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 7 ++++--- drivers/soundwire/cadence_master.h | 3 +++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index aa08736f548ffa..c41541a5f71167 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -184,7 +184,6 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask"); #define CDNS_PDI_CONFIG_PORT GENMASK(4, 0) /* Driver defaults */ -#define CDNS_DEFAULT_SSP_INTERVAL 0x18 #define CDNS_TX_TIMEOUT 2000 #define CDNS_SCP_RX_FIFOLEVEL 0x2 @@ -1049,6 +1048,7 @@ static void cdns_init_clock_ctrl(struct sdw_cdns *cdns) struct sdw_bus *bus = &cdns->bus; struct sdw_master_prop *prop = &bus->prop; u32 val; + u32 ssp_interval; int divider; /* Set clock divider */ @@ -1068,8 +1068,9 @@ static void cdns_init_clock_ctrl(struct sdw_cdns *cdns) cdns_writel(cdns, CDNS_MCP_FRAME_SHAPE_INIT, val); /* Set SSP interval to default value */ - cdns_writel(cdns, CDNS_MCP_SSP_CTRL0, CDNS_DEFAULT_SSP_INTERVAL); - cdns_writel(cdns, CDNS_MCP_SSP_CTRL1, CDNS_DEFAULT_SSP_INTERVAL); + ssp_interval = prop->default_frame_rate / SDW_CADENCE_GSYNC_HZ; + cdns_writel(cdns, CDNS_MCP_SSP_CTRL0, ssp_interval); + cdns_writel(cdns, CDNS_MCP_SSP_CTRL1, ssp_interval); } /** diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index 516ed26c40e3ae..6be92badcb5af8 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -5,6 +5,9 @@ #ifndef __SDW_CADENCE_H #define __SDW_CADENCE_H +#define SDW_CADENCE_GSYNC_KHZ 4 /* 4 kHz */ +#define SDW_CADENCE_GSYNC_HZ (SDW_CADENCE_GSYNC_KHZ * 1000) + /** * struct sdw_cdns_pdi: PDI (Physical Data Interface) instance * From 9b36491df03ecf8652a3586e2203492054091019 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 14:35:18 -0600 Subject: [PATCH 06/27] soundwire: cadence: reorder MCP_CONFIG settings Follow hardware programming flows and add placeholder comment for multi-master mode. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index c41541a5f71167..ac457183b2f82e 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1098,20 +1098,22 @@ int sdw_cdns_init(struct sdw_cdns *cdns) /* Configure mcp config */ val = cdns_readl(cdns, CDNS_MCP_CONFIG); - /* Set Max cmd retry to 15 */ - val |= CDNS_MCP_CONFIG_MCMD_RETRY; + /* Set cmd mode for Tx and Rx cmds */ + val &= ~CDNS_MCP_CONFIG_CMD; - /* Set frame delay between PREQ and ping frame to 15 frames */ - val |= 0xF << SDW_REG_SHIFT(CDNS_MCP_CONFIG_MPREQ_DELAY); + /* Disable sniffer mode */ + val &= ~CDNS_MCP_CONFIG_SNIFFER; /* Disable auto bus release */ val &= ~CDNS_MCP_CONFIG_BUS_REL; - /* Disable sniffer mode */ - val &= ~CDNS_MCP_CONFIG_SNIFFER; + /* Multi-master support to be added here */ - /* Set cmd mode for Tx and Rx cmds */ - val &= ~CDNS_MCP_CONFIG_CMD; + /* Set frame delay between PREQ and ping frame to 15 frames */ + val |= 0xF << SDW_REG_SHIFT(CDNS_MCP_CONFIG_MPREQ_DELAY); + + /* Set Max cmd retry to 15 */ + val |= CDNS_MCP_CONFIG_MCMD_RETRY; cdns_writel(cdns, CDNS_MCP_CONFIG, val); From 10684f419a6e5471d7eca19846f35c9c8315c1e3 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 16:58:49 -0600 Subject: [PATCH 07/27] soundwire: cadence: enable NORMAL operation in cdns_init() Follow recommended programming sequences, this needs to be enabled before the reset sequence. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index ac457183b2f82e..807a5e8e5a3523 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -843,11 +843,6 @@ int sdw_cdns_exit_reset(struct sdw_cdns *cdns) CDNS_MCP_CONTROL_HW_RST, CDNS_MCP_CONTROL_HW_RST); - /* enable bus operations with clock and data */ - cdns_updatel(cdns, CDNS_MCP_CONFIG, - CDNS_MCP_CONFIG_OP, - CDNS_MCP_CONFIG_OP_NORMAL); - /* commit changes */ return cdns_config_update(cdns); } @@ -1098,6 +1093,10 @@ int sdw_cdns_init(struct sdw_cdns *cdns) /* Configure mcp config */ val = cdns_readl(cdns, CDNS_MCP_CONFIG); + /* enable bus operations with clock and data */ + val &= ~CDNS_MCP_CONFIG_OP; + val |= CDNS_MCP_CONFIG_OP_NORMAL; + /* Set cmd mode for Tx and Rx cmds */ val &= ~CDNS_MCP_CONFIG_CMD; From 90f693e59584758c5593cc0964ed5d39c614fd64 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 22 Jan 2020 18:26:02 -0600 Subject: [PATCH 08/27] soundwire: cadence: remove PREQ_DELAY assignment The hardware default is 0x1F, and the existing code does an OR with 0xF. This is a no-op, remove. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 807a5e8e5a3523..9b88b85deaf269 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1108,8 +1108,7 @@ int sdw_cdns_init(struct sdw_cdns *cdns) /* Multi-master support to be added here */ - /* Set frame delay between PREQ and ping frame to 15 frames */ - val |= 0xF << SDW_REG_SHIFT(CDNS_MCP_CONFIG_MPREQ_DELAY); + /* leave frame delay to hardware default of 0x1F */ /* Set Max cmd retry to 15 */ val |= CDNS_MCP_CONFIG_MCMD_RETRY; From 3ccf47f9e7670125124f6a24dd3ee720ce97bec8 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 22 Jan 2020 18:27:59 -0600 Subject: [PATCH 09/27] soundwire: cadence: remove automatic command retries This is a good idea on paper, but it's not recommended at all when operating in multi-master mode. It's also not recommended when doing bank switches, since the retransmission would happen at the next SSP, and the command protocol is stuck in the mean time. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 9b88b85deaf269..99bffcd7cc18cd 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1110,8 +1110,7 @@ int sdw_cdns_init(struct sdw_cdns *cdns) /* leave frame delay to hardware default of 0x1F */ - /* Set Max cmd retry to 15 */ - val |= CDNS_MCP_CONFIG_MCMD_RETRY; + /* leave command retry to hardware default of 0 */ cdns_writel(cdns, CDNS_MCP_CONFIG, val); From d195f904b95bfb974650eda74aa29dacb69bfce0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 14 Feb 2020 11:44:51 -0600 Subject: [PATCH 10/27] soundwire: cadence: remove reset support from clock_restart The exit_reset part can be dealt with in the caller, along with the required syncArm/syncGo sequence. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 31 ++++++++++-------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 99bffcd7cc18cd..ddd42412cf0cd5 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1396,30 +1396,19 @@ int sdw_cdns_clock_restart(struct sdw_cdns *cdns, bool bus_reset) */ cdns_updatel(cdns, CDNS_MCP_CONTROL, CDNS_MCP_CONTROL_CMD_ACCEPT, 0); - if (bus_reset) { - /* program maximum length reset to be safe */ - cdns_updatel(cdns, CDNS_MCP_CONTROL, - CDNS_MCP_CONTROL_RST_DELAY, - CDNS_MCP_CONTROL_RST_DELAY); - - /* use hardware generated reset */ - cdns_updatel(cdns, CDNS_MCP_CONTROL, - CDNS_MCP_CONTROL_HW_RST, - CDNS_MCP_CONTROL_HW_RST); - } + if (!bus_reset) { - /* enable bus operations with clock and data */ - cdns_updatel(cdns, CDNS_MCP_CONFIG, - CDNS_MCP_CONFIG_OP, - CDNS_MCP_CONFIG_OP_NORMAL); + /* enable bus operations with clock and data */ + cdns_updatel(cdns, CDNS_MCP_CONFIG, + CDNS_MCP_CONFIG_OP, + CDNS_MCP_CONFIG_OP_NORMAL); - ret = cdns_config_update(cdns); - if (ret < 0) { - dev_err(cdns->dev, "%s: config_update failed\n", __func__); - return ret; - } + ret = cdns_config_update(cdns); + if (ret < 0) { + dev_err(cdns->dev, "%s: config_update failed\n", __func__); + return ret; + } - if (!bus_reset) { ret = sdw_bus_exit_clk_stop(&cdns->bus); if (ret < 0) dev_err(cdns->dev, "bus failed to exit clock stop %d\n", ret); From ad8a7e7d94e2bff1ead34940f551a624672cf6c0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 17:31:16 -0600 Subject: [PATCH 11/27] soundwire: cadence: commit changes in the exit_reset() sequence Follow recommended flows, the BUS_RESET must be programmed before the UPDATE_CONFIG. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index ddd42412cf0cd5..c55ba1671f16f8 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1075,7 +1075,6 @@ static void cdns_init_clock_ctrl(struct sdw_cdns *cdns) int sdw_cdns_init(struct sdw_cdns *cdns) { u32 val; - int ret; cdns_init_clock_ctrl(cdns); @@ -1114,8 +1113,8 @@ int sdw_cdns_init(struct sdw_cdns *cdns) cdns_writel(cdns, CDNS_MCP_CONFIG, val); - /* commit changes */ - return cdns_config_update(cdns); + /* changes will be committed later */ + return 0; } EXPORT_SYMBOL(sdw_cdns_init); From 59bad4f1efdb04c2dac7595d69dc57bba233f32a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 11 Feb 2020 16:33:26 -0600 Subject: [PATCH 12/27] soundwire: cadence: multi-link support Enable multi-link (aka multi-master configuration). In this configuration, updatres and commands with the 'ssp_sync' tag will be deferred and controlled by the gsync hardware signal. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index c55ba1671f16f8..e88e2e19c64057 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1105,7 +1105,9 @@ int sdw_cdns_init(struct sdw_cdns *cdns) /* Disable auto bus release */ val &= ~CDNS_MCP_CONFIG_BUS_REL; - /* Multi-master support to be added here */ + if (cdns->bus.multi_link) + /* Set Multi-master mode to take gsync into account */ + val |= CDNS_MCP_CONFIG_MMASTER; /* leave frame delay to hardware default of 0x1F */ From 253b5a6fc5b5b76ef4e1756dc639bf060b60efce Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 22 Jan 2020 18:15:50 -0600 Subject: [PATCH 13/27] soundwire: intel: ignore software command retries with multiple links synchronized in hardware, retrying commands in software is not recommended. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index d35491c013db20..de449e2a642edc 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1346,6 +1346,12 @@ static int intel_master_probe(struct platform_device *pdev) complete(&parent_res->probe_complete); parent_res->cdns = &sdw->cdns; + /* + * Ignore BIOS err_threshold, it's a really bad idea when dealing + * with multiple hardware synchronized links + */ + sdw->cdns.bus.prop.err_threshold = 0; + return 0; } From bd8d0a115083a178f9299cf5c9bc17a56982e0d5 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 09:02:46 -0600 Subject: [PATCH 14/27] soundwire: intel: reuse code for wait loops to set/clear bits Refactor code and use same routines on set/clear Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index de449e2a642edc..a6905ad57d9147 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -149,15 +149,14 @@ static inline void intel_writew(void __iomem *base, int offset, u16 value) writew(value, base + offset); } -static int intel_clear_bit(void __iomem *base, int offset, u32 value, u32 mask) +static int intel_wait_bit(void __iomem *base, int offset, u32 mask, u32 target) { int timeout = 10; u32 reg_read; - writel(value, base + offset); do { reg_read = readl(base + offset); - if (!(reg_read & mask)) + if ((reg_read & mask) == target) return 0; timeout--; @@ -167,22 +166,16 @@ static int intel_clear_bit(void __iomem *base, int offset, u32 value, u32 mask) return -EAGAIN; } -static int intel_set_bit(void __iomem *base, int offset, u32 value, u32 mask) +static int intel_clear_bit(void __iomem *base, int offset, u32 value, u32 mask) { - int timeout = 10; - u32 reg_read; - writel(value, base + offset); - do { - reg_read = readl(base + offset); - if (reg_read & mask) - return 0; - - timeout--; - udelay(50); - } while (timeout != 0); + return intel_wait_bit(base, offset, mask, 0); +} - return -EAGAIN; +static int intel_set_bit(void __iomem *base, int offset, u32 value, u32 mask) +{ + writel(value, base + offset); + return intel_wait_bit(base, offset, mask, mask); } /* From 8f50029a310576518b4b23eb9047ae6aa0218a97 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 6 Feb 2020 15:15:24 -0600 Subject: [PATCH 15/27] soundwire: intel: add definitions for shim_mask We want to make sure SHIM register fields such as SYNCPRD are only programmed once. Since we don't have a controller-level driver, we need master-level drivers to collaborate: the registers will only be programmed when the first link is powered-up. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.h | 2 ++ drivers/soundwire/intel_init.c | 1 + include/linux/soundwire/sdw_intel.h | 2 ++ 3 files changed, 5 insertions(+) diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h index 39c78213eda39e..04e8ca4e14f9fa 100644 --- a/drivers/soundwire/intel.h +++ b/drivers/soundwire/intel.h @@ -23,6 +23,7 @@ * @ops: Shim callback ops * @dev: device implementing hw_params and free callbacks * @shim_lock: mutex to handle access to shared SHIM registers + * @shim_mask: global pointer to check SHIM register initialization * @clock_stop_quirks: mask defining requested behavior on pm_suspend * @cdns: Cadence master descriptor * @list: used to walk-through all masters exposed by the same controller @@ -40,6 +41,7 @@ struct sdw_intel_link_res { const struct sdw_intel_ops *ops; struct device *dev; struct mutex *shim_lock; /* protect shared registers */ + u32 *shim_mask; u32 clock_stop_quirks; struct sdw_cdns *cdns; struct list_head list; diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 71472f53e88b40..7bcf2c314a598c 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -248,6 +248,7 @@ static struct sdw_intel_ctx link->dev = res->dev; link->clock_stop_quirks = res->clock_stop_quirks; link->shim_lock = &ctx->shim_lock; + link->shim_mask = &ctx->shim_mask; init_completion(&link->probe_complete); diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 979b41b5dcb44b..120ffddc03d2cd 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -115,6 +115,7 @@ struct sdw_intel_slave_id { * links * @link_list: list to handle interrupts across all links * @shim_lock: mutex to handle concurrent rmw access to shared SHIM registers. + * @shim_mask: flags to track initialization of SHIM shared registers */ struct sdw_intel_ctx { int count; @@ -126,6 +127,7 @@ struct sdw_intel_ctx { struct sdw_intel_slave_id *ids; struct list_head link_list; struct mutex shim_lock; /* lock for access to shared SHIM registers */ + u32 shim_mask; }; /** From b243eddd705826660a88be3cb2d889d66d1cfb75 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 15 Jan 2020 21:12:35 -0600 Subject: [PATCH 16/27] soundwire: intel: follow the recommended hardware flow to set SHIM_SYNC The current hard-coded value only works for CNL/WHL/CML platforms, it needs to be updated for IceLake and TigerLake (change from 24 MHz to 38.4 MHz oscillator). In addition, the values needs to be programmed before powering-up the links. Note that the SYNCPRD value is tied only to the XTAL value and not the current bus frequency or the frame rate. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 89 ++++++++++++++++++++++++++++++--------- 1 file changed, 68 insertions(+), 21 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index a6905ad57d9147..eb325a1157eb60 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -63,7 +63,8 @@ MODULE_PARM_DESC(sdw_md_flags, "SoundWire Intel Master device flags (0x0 all off #define SDW_SHIM_LCTL_SPA BIT(0) #define SDW_SHIM_LCTL_CPA BIT(8) -#define SDW_SHIM_SYNC_SYNCPRD_VAL 0x176F +#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) #define SDW_SHIM_SYNC_SYNCPRD GENMASK(14, 0) #define SDW_SHIM_SYNC_SYNCCPU BIT(15) #define SDW_SHIM_SYNC_CMDSYNC_MASK GENMASK(19, 16) @@ -298,11 +299,48 @@ static int intel_link_power_up(struct sdw_intel *sdw) { unsigned int link_id = sdw->instance; void __iomem *shim = sdw->link_res->shim; + 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, ret; + int link_control; + int ret = 0; + u32 syncprd; + u32 sync_reg; mutex_lock(sdw->link_res->shim_lock); + /* + * The hardware relies on an internal counter, + * typically 4kHz, to generate the SoundWire SSP - + * which defines a 'safe' synchronization point + * between commands and audio transport and allows for + * multi link synchronization. The SYNCPRD value is + * only dependent on the oscillator clock provided to + * the IP, so adjust based on _DSD properties reported + * in DSDT tables. The values reported are based on + * either 24MHz (CNL/CML) or 38.4 MHz (ICL/TGL+). + */ + if (prop->mclk_freq % 6000000) + syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_38_4; + else + syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_24; + + if (!*shim_mask) { + /* we first need to program the SyncPRD/CPU registers */ + dev_dbg(sdw->cdns.dev, + "%s: first link up, programming SYNCPRD\n", __func__); + + /* set SyncPRD period */ + sync_reg = intel_readl(shim, SDW_SHIM_SYNC); + sync_reg |= (syncprd << + SDW_REG_SHIFT(SDW_SHIM_SYNC_SYNCPRD)); + + /* 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); @@ -310,13 +348,29 @@ static int intel_link_power_up(struct sdw_intel *sdw) link_control |= spa_mask; ret = intel_set_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask); - mutex_unlock(sdw->link_res->shim_lock); + if (ret < 0) { + dev_err(sdw->cdns.dev, "Failed to power up link: %d\n", ret); + goto out; + } - if (ret < 0) - return ret; + if (!*shim_mask) { + /* SyncCPU will change once link is active */ + ret = intel_wait_bit(shim, SDW_SHIM_SYNC, + SDW_SHIM_SYNC_SYNCCPU, 0); + if (ret < 0) { + dev_err(sdw->cdns.dev, + "Failed to set SHIM_SYNC: %d\n", ret); + goto out; + } + } + + *shim_mask |= BIT(link_id); sdw->cdns.link_up = true; - return 0; +out: + mutex_unlock(sdw->link_res->shim_lock); + + return ret; } /* this needs to be called with shim_lock */ @@ -374,7 +428,7 @@ static int intel_shim_init(struct sdw_intel *sdw, bool clock_stop) { void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; - int sync_reg, ret = 0; + int ret = 0; u16 ioctl = 0, act = 0; mutex_lock(sdw->link_res->shim_lock); @@ -404,20 +458,6 @@ static int intel_shim_init(struct sdw_intel *sdw, bool clock_stop) intel_writew(shim, SDW_SHIM_CTMCTL(link_id), act); usleep_range(10, 15); - if (!clock_stop) { - /* Now set SyncPRD period */ - sync_reg = intel_readl(shim, SDW_SHIM_SYNC); - sync_reg |= (SDW_SHIM_SYNC_SYNCPRD_VAL << - SDW_REG_SHIFT(SDW_SHIM_SYNC_SYNCPRD)); - - /* Set SyncCPU bit */ - sync_reg |= SDW_SHIM_SYNC_SYNCCPU; - ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg, - SDW_SHIM_SYNC_SYNCCPU); - if (ret < 0) - dev_err(sdw->cdns.dev, "Failed to set sync period: %d\n", ret); - - } mutex_unlock(sdw->link_res->shim_lock); return ret; @@ -454,6 +494,7 @@ static int intel_link_power_down(struct sdw_intel *sdw) int 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; int ret = 0; mutex_lock(sdw->link_res->shim_lock); @@ -468,6 +509,12 @@ static int intel_link_power_down(struct sdw_intel *sdw) 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); + mutex_unlock(sdw->link_res->shim_lock); if (ret < 0) From 1241b8e24bab14f8634fffde9bcab67b28252395 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 15:03:11 -0600 Subject: [PATCH 17/27] soundwire: intel: introduce a helper to arm link synchronization Move code from pre_bank_switch to dedicated helper, will be used in follow-up patches as recommended by programming flows. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index eb325a1157eb60..e78466850e5fe5 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -524,6 +524,21 @@ static int intel_link_power_down(struct sdw_intel *sdw) return 0; } +static void intel_shim_sync_arm(struct sdw_intel *sdw) +{ + void __iomem *shim = sdw->link_res->shim; + u32 sync_reg; + + mutex_lock(sdw->link_res->shim_lock); + + /* update SYNC register */ + sync_reg = intel_readl(shim, SDW_SHIM_SYNC); + sync_reg |= (SDW_SHIM_SYNC_CMDSYNC << sdw->instance); + intel_writel(shim, SDW_SHIM_SYNC, sync_reg); + + mutex_unlock(sdw->link_res->shim_lock); +} + /* * PDI routines */ @@ -737,21 +752,12 @@ static int intel_pre_bank_switch(struct sdw_bus *bus) { struct sdw_cdns *cdns = bus_to_cdns(bus); struct sdw_intel *sdw = cdns_to_intel(cdns); - void __iomem *shim = sdw->link_res->shim; - int sync_reg; /* Write to register only for multi-link */ if (!bus->multi_link) return 0; - mutex_lock(sdw->link_res->shim_lock); - - /* Read SYNC register */ - sync_reg = intel_readl(shim, SDW_SHIM_SYNC); - sync_reg |= SDW_SHIM_SYNC_CMDSYNC << sdw->instance; - intel_writel(shim, SDW_SHIM_SYNC, sync_reg); - - mutex_unlock(sdw->link_res->shim_lock); + intel_shim_sync_arm(sdw); return 0; } From 7dad12798f7c0b2ac6b5b00dad2a0ce29458d7b7 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 15:19:25 -0600 Subject: [PATCH 18/27] soundwire: intel: introduce helper for link synchronization After arming the synchronization, the SYNCGO field controls the hardware-based synchronization between links. Move the programming and wait for clear of SYNCGO to dedicated helper. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 47 ++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index e78466850e5fe5..e0e6cf5e2a4531 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -539,6 +539,44 @@ static void intel_shim_sync_arm(struct sdw_intel *sdw) mutex_unlock(sdw->link_res->shim_lock); } +static int intel_shim_sync_go_unlocked(struct sdw_intel *sdw) +{ + void __iomem *shim = sdw->link_res->shim; + u32 sync_reg; + int ret; + + /* Read SYNC register */ + sync_reg = intel_readl(shim, SDW_SHIM_SYNC); + + /* + * Set SyncGO bit to synchronously trigger a bank switch for + * all the masters. A write to SYNCGO bit clears CMDSYNC bit for all + * the Masters. + */ + sync_reg |= SDW_SHIM_SYNC_SYNCGO; + + ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg, + SDW_SHIM_SYNC_SYNCGO); + + if (ret < 0) + dev_err(sdw->cdns.dev, "SyncGO clear failed: %d\n", ret); + + return ret; +} + +static int intel_shim_sync_go(struct sdw_intel *sdw) +{ + int ret; + + mutex_lock(sdw->link_res->shim_lock); + + ret = intel_shim_sync_go_unlocked(sdw); + + mutex_unlock(sdw->link_res->shim_lock); + + return ret; +} + /* * PDI routines */ @@ -790,15 +828,8 @@ static int intel_post_bank_switch(struct sdw_bus *bus) ret = 0; goto unlock; } - /* - * Set SyncGO bit to synchronously trigger a bank switch for - * all the masters. A write to SYNCGO bit clears CMDSYNC bit for all - * the Masters. - */ - sync_reg |= SDW_SHIM_SYNC_SYNCGO; - ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg, - SDW_SHIM_SYNC_SYNCGO); + ret = intel_shim_sync_go_unlocked(sdw); unlock: mutex_unlock(sdw->link_res->shim_lock); From 226a5ab93ef0f0cfb993597e0df1c517c183ee8f Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 17 Jan 2020 17:54:23 -0600 Subject: [PATCH 19/27] soundwire: intel: add multi-link support The multi-link support is enabled with a hardware gsync signal connecting all links. All commands and operations which typically are handled on an SSP boundary will be deferred further and enabled across all links with the 'syncGo' sequence. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 8 ++- drivers/soundwire/intel.c | 111 ++++++++++++++++++++++++++--- 2 files changed, 107 insertions(+), 12 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index e88e2e19c64057..6592327469b344 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -844,7 +844,13 @@ int sdw_cdns_exit_reset(struct sdw_cdns *cdns) CDNS_MCP_CONTROL_HW_RST); /* commit changes */ - return cdns_config_update(cdns); + cdns_updatel(cdns, CDNS_MCP_CONFIG_UPDATE, + CDNS_MCP_CONFIG_UPDATE_BIT, + CDNS_MCP_CONFIG_UPDATE_BIT); + + /* don't wait here */ + return 0; + } EXPORT_SYMBOL(sdw_cdns_exit_reset); diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index e0e6cf5e2a4531..ec11a11fe39c02 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -33,6 +33,7 @@ #define SDW_INTEL_MASTER_DISABLE_PM_RUNTIME BIT(0) #define SDW_INTEL_MASTER_DISABLE_CLOCK_STOP BIT(1) #define SDW_INTEL_MASTER_DISABLE_PM_RUNTIME_IDLE BIT(2) +#define SDW_INTEL_MASTER_DISABLE_MULTI_LINK BIT(3) static int md_flags; module_param_named(sdw_md_flags, md_flags, int, 0444); @@ -1361,17 +1362,18 @@ static int intel_init(struct sdw_intel *sdw) { bool clock_stop; + dev_err(sdw->cdns.dev, "%s: start\n", __func__); + /* Initialize shim and controller */ intel_link_power_up(sdw); clock_stop = sdw_cdns_is_clock_stop(&sdw->cdns); - intel_shim_init(sdw, clock_stop); + dev_err(sdw->cdns.dev, "%s: shim_init clock stop %d\n", __func__, clock_stop); - if (clock_stop) - return 0; + intel_shim_init(sdw, clock_stop); - return sdw_cdns_init(&sdw->cdns); + return 0; } /* @@ -1438,6 +1440,7 @@ int intel_master_startup(struct platform_device *pdev) struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_cdns_stream_config config; int link_flags; + bool multi_link; u32 clock_stop_quirks; int ret; @@ -1447,7 +1450,16 @@ int intel_master_startup(struct platform_device *pdev) return 0; } - /* Initialize shim, controller and Cadence IP */ + link_flags = md_flags >> (sdw->cdns.bus.link_id * 8); + multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK); + if (!multi_link) { + dev_dbg(&pdev->dev, "Multi-link is disabled\n"); + bus->multi_link = false; + } else { + bus->multi_link = true; + } + + /* Initialize shim, controller */ ret = intel_init(sdw); if (ret) goto err_init; @@ -1466,12 +1478,33 @@ int intel_master_startup(struct platform_device *pdev) goto err_init; } + /* + * follow recommended programming flows to avoid timeouts when + * gsync is enabled + */ + if (multi_link) + intel_shim_sync_arm(sdw); + + ret = sdw_cdns_init(&sdw->cdns); + if (ret < 0) { + dev_err(sdw->cdns.dev, "unable to initialize Cadence IP\n"); + goto err_interrupt; + } + ret = sdw_cdns_exit_reset(&sdw->cdns); if (ret < 0) { dev_err(sdw->cdns.dev, "unable to exit bus reset sequence\n"); goto err_interrupt; } + if (multi_link) { + ret = intel_shim_sync_go(sdw); + if (ret < 0) { + dev_err(sdw->cdns.dev, "sync go failed: %d\n", ret); + goto err_interrupt; + } + } + /* Register DAIs */ ret = intel_register_dai(sdw); if (ret) { @@ -1483,7 +1516,6 @@ int intel_master_startup(struct platform_device *pdev) intel_debugfs_init(sdw); /* Enable runtime PM */ - link_flags = md_flags >> (sdw->cdns.bus.link_id * 8); if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME)) { pm_runtime_set_autosuspend_delay(&pdev->dev, 3000); pm_runtime_use_autosuspend(&pdev->dev); @@ -1734,6 +1766,7 @@ static int intel_resume(struct device *dev) struct sdw_cdns *cdns = dev_get_drvdata(dev); struct sdw_intel *sdw = cdns_to_intel(cdns); int link_flags; + bool multi_link; int ret; if (cdns->bus.prop.hw_disabled) { @@ -1742,6 +1775,9 @@ static int intel_resume(struct device *dev) return 0; } + link_flags = md_flags >> (sdw->cdns.bus.link_id * 8); + multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK); + if (sdw->link_res->pm_runtime_suspended) { dev_dbg(dev, "%s: pm_runtime status was suspended, forcing active\n", @@ -1755,7 +1791,6 @@ static int intel_resume(struct device *dev) sdw->link_res->pm_runtime_suspended = false; - link_flags = md_flags >> (sdw->cdns.bus.link_id * 8); if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME_IDLE)) pm_runtime_idle(dev); } @@ -1779,12 +1814,33 @@ static int intel_resume(struct device *dev) return ret; } + /* + * follow recommended programming flows to avoid timeouts when + * gsync is enabled + */ + if (multi_link) + intel_shim_sync_arm(sdw); + + ret = sdw_cdns_init(&sdw->cdns); + if (ret < 0) { + dev_err(sdw->cdns.dev, "unable to initialize Cadence IP during resume\n"); + return ret; + } + ret = sdw_cdns_exit_reset(cdns); if (ret < 0) { dev_err(dev, "unable to exit bus reset sequence during resume\n"); return ret; } + if (multi_link) { + ret = intel_shim_sync_go(sdw); + if (ret < 0) { + dev_err(sdw->cdns.dev, "sync go failed during resume\n"); + return ret; + } + } + /* * after system resume, the pm_runtime suspend() may kick in * during the enumeration, before any children device force the @@ -1806,6 +1862,8 @@ static int intel_resume_runtime(struct device *dev) struct sdw_intel *sdw = cdns_to_intel(cdns); u32 clock_stop_quirks; bool clock_stop0; + int link_flags; + bool multi_link; int status; int ret; @@ -1815,6 +1873,9 @@ static int intel_resume_runtime(struct device *dev) return 0; } + link_flags = md_flags >> (sdw->cdns.bus.link_id * 8); + multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK); + clock_stop_quirks = sdw->link_res->clock_stop_quirks; if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) { @@ -1837,11 +1898,32 @@ static int intel_resume_runtime(struct device *dev) return ret; } + /* + * follow recommended programming flows to avoid + * timeouts when gsync is enabled + */ + if (multi_link) + intel_shim_sync_arm(sdw); + + ret = sdw_cdns_init(&sdw->cdns); + if (ret < 0) { + dev_err(sdw->cdns.dev, "unable to initialize Cadence IP during resume\n"); + return ret; + } + ret = sdw_cdns_exit_reset(cdns); if (ret < 0) { dev_err(dev, "unable to exit bus reset sequence during resume\n"); return ret; } + + if (multi_link) { + ret = intel_shim_sync_go(sdw); + if (ret < 0) { + dev_err(sdw->cdns.dev, "sync go failed during resume\n"); + return ret; + } + } } else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) { ret = intel_init(sdw); if (ret) { @@ -1858,11 +1940,18 @@ static int intel_resume_runtime(struct device *dev) */ clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns); - /* - * make sure all Slaves are tagged as UNATTACHED and - * provide reason for reinitialization - */ if (!clock_stop0) { + + /* + * Re-initialize the IP since it was powered-off + */ + sdw_cdns_init(&sdw->cdns); + + /* + * make sure all Slaves are tagged as UNATTACHED and + * provide reason for reinitialization + */ + status = SDW_UNATTACH_REQUEST_MASTER_RESET; sdw_clear_slave_status(&sdw->cdns.bus, status); } From 2a3b54f1201853434c7d1eb8d0af58a10cc91c64 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 14 Feb 2020 11:46:23 -0600 Subject: [PATCH 20/27] soundwire: intel: add missing support for all clock stop modes Deal with the BUS_RESET case, which is the default. The only change is to add support for the exit sequence using the syncArm/syncGo mode for the exit reset sequence. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 49 +++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index ec11a11fe39c02..ee7e80711bef98 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1942,11 +1942,6 @@ static int intel_resume_runtime(struct device *dev) if (!clock_stop0) { - /* - * Re-initialize the IP since it was powered-off - */ - sdw_cdns_init(&sdw->cdns); - /* * make sure all Slaves are tagged as UNATTACHED and * provide reason for reinitialization @@ -1954,12 +1949,31 @@ static int intel_resume_runtime(struct device *dev) status = SDW_UNATTACH_REQUEST_MASTER_RESET; sdw_clear_slave_status(&sdw->cdns.bus, status); - } - ret = sdw_cdns_enable_interrupt(cdns, true); - if (ret < 0) { - dev_err(dev, "cannot enable interrupts during resume\n"); - return ret; + ret = sdw_cdns_enable_interrupt(cdns, true); + if (ret < 0) { + dev_err(dev, "cannot enable interrupts during resume\n"); + return ret; + } + + /* + * follow recommended programming flows to avoid + * timeouts when gsync is enabled + */ + if (multi_link) + intel_shim_sync_arm(sdw); + + /* + * Re-initialize the IP since it was powered-off + */ + sdw_cdns_init(&sdw->cdns); + + } else { + ret = sdw_cdns_enable_interrupt(cdns, true); + if (ret < 0) { + dev_err(dev, "cannot enable interrupts during resume\n"); + return ret; + } } ret = sdw_cdns_clock_restart(cdns, !clock_stop0); @@ -1968,6 +1982,21 @@ static int intel_resume_runtime(struct device *dev) return ret; } + if (!clock_stop0) { + ret = sdw_cdns_exit_reset(cdns); + if (ret < 0) { + dev_err(dev, "unable to exit bus reset sequence during resume\n"); + return ret; + } + + if (multi_link) { + ret = intel_shim_sync_go(sdw); + if (ret < 0) { + dev_err(sdw->cdns.dev, "sync go failed during resume\n"); + return ret; + } + } + } } else if (!clock_stop_quirks) { ret = intel_init(sdw); if (ret) { From 511335faa46e6ed29a2a5a0f6829ace1d546f472 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 23 Jan 2020 17:31:13 -0600 Subject: [PATCH 21/27] soundwire: bus: update multi-link definition with hw sync details Hardware-based synchronization is typically required when the bus->multi_link flag is set. On Intel platforms, when the Cadence IP is configured in 'Multi Master Mode', the hardware synchronization is required even when a stream only uses a single segment. The existing code only deal with hardware synchronization when a stream uses more than one segment so to remain backwards compatible we add a configuration threshold. For Intel cases this threshold will be set to one, other platforms may be able to use the SSP-based sync in those cases. Signed-off-by: Pierre-Louis Bossart --- include/linux/soundwire/sdw.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 021ced5a066e36..ead36ef08c4cf1 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -807,6 +807,11 @@ struct sdw_master_ops { * @multi_link: Store bus property that indicates if multi links * are supported. This flag is populated by drivers after reading * appropriate firmware (ACPI/DT). + * @hw_sync_min_links: Number of links used by a stream above which + * hardware-based synchronization is required. This value is only + * meaningful if multi_link is set. If set to 1, hardware-based + * synchronization will be used even if a stream only uses a single + * SoundWire segment. */ struct sdw_bus { struct device *dev; @@ -828,6 +833,7 @@ struct sdw_bus { unsigned int clk_stop_timeout; u32 bank_switch_timeout; bool multi_link; + int hw_sync_min_links; }; int sdw_add_bus_master(struct sdw_bus *bus); From e2dee59d8bb9edf802daef61a4e1f4a72a10b061 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 23 Jan 2020 18:33:06 -0600 Subject: [PATCH 22/27] soundwire: intel: add multi-link hw_synchronization information set the flags as required by hardware implementation Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index ee7e80711bef98..aa60ba0f6d71d0 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1439,6 +1439,7 @@ int intel_master_startup(struct platform_device *pdev) struct sdw_cdns *cdns = platform_get_drvdata(pdev); struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_cdns_stream_config config; + struct sdw_bus *bus; int link_flags; bool multi_link; u32 clock_stop_quirks; @@ -1450,13 +1451,22 @@ int intel_master_startup(struct platform_device *pdev) return 0; } + bus = &sdw->cdns.bus; + link_flags = md_flags >> (sdw->cdns.bus.link_id * 8); multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK); if (!multi_link) { dev_dbg(&pdev->dev, "Multi-link is disabled\n"); bus->multi_link = false; } else { + /* + * hardware-based synchronization is required regardless + * of the number of segments used by a stream: SSP-based + * synchronization is gated by gsync when the multi-master + * mode is set. + */ bus->multi_link = true; + bus->hw_sync_min_links = 1; } /* Initialize shim, controller */ From 927586d8d737fea23819929f5864c598941e39ec Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 23 Jan 2020 18:34:22 -0600 Subject: [PATCH 23/27] soundwire: stream: enable hw_sync as needed by hardware Use platform-specific information to decide when to use hw_sync, not only a number of links > 1. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index c55b1845b1a55b..e0e8e2985d7192 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -689,9 +689,9 @@ static int sdw_bank_switch(struct sdw_bus *bus, int m_rt_count) /* * Set the multi_link flag only when both the hardware supports - * and there is a stream handled by multiple masters + * and hardware-based sync is required */ - multi_link = bus->multi_link && (m_rt_count > 1); + multi_link = bus->multi_link && (m_rt_count >= bus->hw_sync_min_links); if (multi_link) ret = sdw_transfer_defer(bus, wr_msg, &bus->defer_msg); @@ -760,13 +760,16 @@ static int do_bank_switch(struct sdw_stream_runtime *stream) const struct sdw_master_ops *ops; struct sdw_bus *bus; bool multi_link = false; + int m_rt_count; int ret = 0; + m_rt_count = stream->m_rt_count; + list_for_each_entry(m_rt, &stream->master_list, stream_node) { bus = m_rt->bus; ops = bus->ops; - if (bus->multi_link) { + if (bus->multi_link && m_rt_count >= bus->hw_sync_min_links) { multi_link = true; mutex_lock(&bus->msg_lock); } @@ -787,7 +790,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream) * synchronized across all Masters and happens later as a * part of post_bank_switch ops. */ - ret = sdw_bank_switch(bus, stream->m_rt_count); + ret = sdw_bank_switch(bus, m_rt_count); if (ret < 0) { dev_err(bus->dev, "Bank switch failed: %d\n", ret); goto error; @@ -813,7 +816,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream) ret); goto error; } - } else if (bus->multi_link && stream->m_rt_count > 1) { + } else if (multi_link) { dev_err(bus->dev, "Post bank switch ops not implemented\n"); goto error; @@ -831,7 +834,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream) goto error; } - if (bus->multi_link) + if (multi_link) mutex_unlock(&bus->msg_lock); } From 38a10aab5dcbf73b965754bf142f966b5759fa1f Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 7 Feb 2020 14:17:37 -0600 Subject: [PATCH 24/27] soundwire: intel: add dynamic debug trace for clock-stop invalid configs Detect cases where the clock is assumed to be stopped but the IP is not in the relevant state, and add a dynamic debug trace. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index aa60ba0f6d71d0..b67cda7c713891 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -2008,6 +2008,13 @@ static int intel_resume_runtime(struct device *dev) } } } else if (!clock_stop_quirks) { + + clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns); + if (!clock_stop0) + dev_err(dev, + "%s invalid configuration, clock was not stopped", + __func__); + ret = intel_init(sdw); if (ret) { dev_err(dev, "%s failed: %d", __func__, ret); From cbf6fea4e08fbde9c888d3ee6e88db956d9bdaa0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 7 Feb 2020 17:08:27 -0600 Subject: [PATCH 25/27] [WORKAROUND] soundwire: intel: disable pm_runtime before unregistering platform device The SoundWire master device pm_runtime resume causes race conditions and errors before the .remove is called, so disable at the parent level. It's not clear why we cannot power up the links on pm_runtime_resume though, this should work. At any rate this will be removed before upstream since we are no longer going to use platform devices. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel_init.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 7bcf2c314a598c..a01c876ab7ea80 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -69,6 +69,8 @@ static int sdw_intel_cleanup(struct sdw_intel_ctx *ctx) if (link_mask && !(link_mask & BIT(i))) continue; + pm_runtime_disable(&link->pdev->dev); + platform_device_unregister(link->pdev); if (!link->clock_stop_quirks) From 65c0f7fa91b7fe25c0f647740a90f3e1c3e5ede1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 13 Feb 2020 11:58:33 -0600 Subject: [PATCH 26/27] soundwire: intel: pass link_mask information to each master While the hardware exposes independent bits to power-up each master, the recommended sequence is to power all links or none. Idle links can still use the clock stop mode while the master is powered. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.h | 2 ++ drivers/soundwire/intel_init.c | 1 + 2 files changed, 3 insertions(+) diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h index 04e8ca4e14f9fa..b66dbcfa5cc6af 100644 --- a/drivers/soundwire/intel.h +++ b/drivers/soundwire/intel.h @@ -28,6 +28,7 @@ * @cdns: Cadence master descriptor * @list: used to walk-through all masters exposed by the same controller * @pm_runtime_suspended: flag to avoid resuming before system suspend + * @link_mask: global mask needed for power-up/down sequences */ struct sdw_intel_link_res { void *parent_res; @@ -46,6 +47,7 @@ struct sdw_intel_link_res { struct sdw_cdns *cdns; struct list_head list; bool pm_runtime_suspended; + u32 link_mask; }; #define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1) diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index a01c876ab7ea80..c4527eb00f9a6d 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -251,6 +251,7 @@ static struct sdw_intel_ctx link->clock_stop_quirks = res->clock_stop_quirks; link->shim_lock = &ctx->shim_lock; link->shim_mask = &ctx->shim_mask; + link->link_mask = link_mask; init_completion(&link->probe_complete); From 3144357c4ac0f423690f7fa0e72de96917116c41 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 7 Feb 2020 17:20:43 -0600 Subject: [PATCH 27/27] 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 --- drivers/soundwire/intel.c | 73 ++++++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 24 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index b67cda7c713891..648894f22e18df 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -62,7 +62,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) @@ -303,8 +305,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; @@ -328,6 +330,9 @@ 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__); @@ -340,21 +345,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); @@ -492,7 +500,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; @@ -502,24 +510,41 @@ 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;