2023-03-14 01:50:32

by Bard Liao

[permalink] [raw]
Subject: [PATCH 00/16] soundwire: updates before LunarLake introduction

This series provides a set of cleanups and new abstractions needed for the
introduction of LunarLake support.

For now this is an iso-functionality change, with changes on the Intel and
Cadence sides. The low-level support for LunarLake will be introduced in a
follow-up series that depends on HDaudio multi-link extensions.

Pierre-Louis Bossart (16):
soundwire: intel: move common definitions to header file
soundwire: intel: remove stale/misleading comment
soundwire: intel: remove PDI-level restrictions on rates and formats
soundwire: intel: remove useless abstraction
soundwire: intel: simplify sync_go sequence
soundwire: intel: add sync_arm/sync_go to ops
soundwire: intel: use indirection before moving bus start/stop
sequences
soundwire: intel: move bus common sequences to different file
soundwire: intel: add abstraction for cmdsync check
soundwire: intel: move bank switch routine to common
intel_bus_common.c
soundwire: cadence: remove CDNS_MCP_CONFIG_SSPMOD
soundwire: cadence: add helpers to access IP_MCP registers
soundwire: cadence: split access to IP_MCP_CONFIG fields
soundwire: cadence: split access to IP_MCP_CONTROL fields
soundwire: cadence: split access to IP_MCP_CMDCTRL fields
soundwire: cadence: change access to IP_MCP_CMD_BASE

drivers/soundwire/Makefile | 3 +-
drivers/soundwire/cadence_master.c | 139 +++++++-----
drivers/soundwire/cadence_master.h | 3 +
drivers/soundwire/intel.c | 325 ++-------------------------
drivers/soundwire/intel.h | 67 ++++++
drivers/soundwire/intel_bus_common.c | 259 +++++++++++++++++++++
include/linux/soundwire/sdw_intel.h | 11 +
7 files changed, 444 insertions(+), 363 deletions(-)
create mode 100644 drivers/soundwire/intel_bus_common.c

--
2.25.1



2023-03-14 01:50:33

by Bard Liao

[permalink] [raw]
Subject: [PATCH 15/16] soundwire: cadence: split access to IP_MCP_CMDCTRL fields

From: Pierre-Louis Bossart <[email protected]>

The latest Cadence IP splits the MCP_CMDCTRL fields in two registers:
MCP_CMDCTRL and IP_MCP_CMDCTRL. Rename the relevant fields and change
the access methods used for those fields.

In practice we only use the Parity error insertion in IP_CMD_CTRL.

For existing solutions, this is an iso-functionality change.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/cadence_master.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 4c82712944b9..5128923f051e 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -54,9 +54,9 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask");
#define CDNS_IP_MCP_CONTROL_CMD_ACCEPT BIT(1)
#define CDNS_IP_MCP_CONTROL_BLOCK_WAKEUP BIT(0)

-#define CDNS_MCP_CMDCTRL 0x8
+#define CDNS_IP_MCP_CMDCTRL 0x8 /* IP offset added at run-time */

-#define CDNS_MCP_CMDCTRL_INSERT_PARITY_ERR BIT(2)
+#define CDNS_IP_MCP_CMDCTRL_INSERT_PARITY_ERR BIT(2)

#define CDNS_MCP_SSPSTAT 0xC
#define CDNS_MCP_FRAME_SHAPE 0x10
@@ -428,9 +428,9 @@ static int cdns_parity_error_injection(void *data, u64 value)
mutex_lock(&bus->bus_lock);

/* program hardware to inject parity error */
- cdns_updatel(cdns, CDNS_MCP_CMDCTRL,
- CDNS_MCP_CMDCTRL_INSERT_PARITY_ERR,
- CDNS_MCP_CMDCTRL_INSERT_PARITY_ERR);
+ cdns_ip_updatel(cdns, CDNS_IP_MCP_CMDCTRL,
+ CDNS_IP_MCP_CMDCTRL_INSERT_PARITY_ERR,
+ CDNS_IP_MCP_CMDCTRL_INSERT_PARITY_ERR);

/* commit changes */
cdns_updatel(cdns, CDNS_MCP_CONFIG_UPDATE,
@@ -442,9 +442,9 @@ static int cdns_parity_error_injection(void *data, u64 value)
dev_info(cdns->dev, "parity error injection, read: %d\n", ret);

/* program hardware to disable parity error */
- cdns_updatel(cdns, CDNS_MCP_CMDCTRL,
- CDNS_MCP_CMDCTRL_INSERT_PARITY_ERR,
- 0);
+ cdns_ip_updatel(cdns, CDNS_IP_MCP_CMDCTRL,
+ CDNS_IP_MCP_CMDCTRL_INSERT_PARITY_ERR,
+ 0);

/* commit changes */
cdns_updatel(cdns, CDNS_MCP_CONFIG_UPDATE,
--
2.25.1


2023-03-14 01:50:33

by Bard Liao

[permalink] [raw]
Subject: [PATCH 09/16] soundwire: intel: add abstraction for cmdsync check

From: Pierre-Louis Bossart <[email protected]>

If we add one more callback, we can have common bank switch sequences
between old and new hardware: the only difference is where the CMDSYNC
register is located.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/intel.c | 24 +++++++++++++-----------
drivers/soundwire/intel.h | 7 +++++++
include/linux/soundwire/sdw_intel.h | 3 +++
3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 77d698908595..1131ecb4b5e7 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -325,6 +325,15 @@ static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
mutex_unlock(sdw->link_res->shim_lock);
}

+static bool intel_check_cmdsync_unlocked(struct sdw_intel *sdw)
+{
+ void __iomem *shim = sdw->link_res->shim;
+ int sync_reg;
+
+ sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
+ return !!(sync_reg & SDW_SHIM_SYNC_CMDSYNC_MASK);
+}
+
static int intel_link_power_up(struct sdw_intel *sdw)
{
unsigned int link_id = sdw->instance;
@@ -695,8 +704,7 @@ static int intel_post_bank_switch(struct sdw_intel *sdw)
{
struct sdw_cdns *cdns = &sdw->cdns;
struct sdw_bus *bus = &cdns->bus;
- void __iomem *shim = sdw->link_res->shim;
- int sync_reg, ret;
+ int ret = 0;

/* Write to register only for multi-link */
if (!bus->multi_link)
@@ -704,9 +712,6 @@ static int intel_post_bank_switch(struct sdw_intel *sdw)

mutex_lock(sdw->link_res->shim_lock);

- /* Read SYNC register */
- sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
-
/*
* post_bank_switch() ops is called from the bus in loop for
* all the Masters in the steam with the expectation that
@@ -715,13 +720,9 @@ static int intel_post_bank_switch(struct sdw_intel *sdw)
*
* So, set the SYNCGO bit only if CMDSYNC bit is set for any Master.
*/
- if (!(sync_reg & SDW_SHIM_SYNC_CMDSYNC_MASK)) {
- ret = 0;
- goto unlock;
- }
+ if (sdw_intel_sync_check_cmdsync_unlocked(sdw))
+ ret = sdw_intel_sync_go_unlocked(sdw);

- ret = sdw_intel_sync_go_unlocked(sdw);
-unlock:
mutex_unlock(sdw->link_res->shim_lock);

if (ret < 0)
@@ -1147,6 +1148,7 @@ const struct sdw_intel_hw_ops sdw_intel_cnl_hw_ops = {
.sync_arm = intel_shim_sync_arm,
.sync_go_unlocked = intel_shim_sync_go_unlocked,
.sync_go = intel_shim_sync_go,
+ .sync_check_cmdsync_unlocked = intel_check_cmdsync_unlocked,
};
EXPORT_SYMBOL_NS(sdw_intel_cnl_hw_ops, SOUNDWIRE_INTEL);

diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index abd1a500defa..7a69cf755954 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -187,6 +187,13 @@ static inline int sdw_intel_sync_go(struct sdw_intel *sdw)
return -ENOTSUPP;
}

+static inline bool sdw_intel_sync_check_cmdsync_unlocked(struct sdw_intel *sdw)
+{
+ if (SDW_INTEL_CHECK_OPS(sdw, sync_check_cmdsync_unlocked))
+ return SDW_INTEL_OPS(sdw, sync_check_cmdsync_unlocked)(sdw);
+ return false;
+}
+
/* common bus management */
int intel_start_bus(struct sdw_intel *sdw);
int intel_start_bus_after_reset(struct sdw_intel *sdw);
diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
index 06fa30929ebd..207701aeeb47 100644
--- a/include/linux/soundwire/sdw_intel.h
+++ b/include/linux/soundwire/sdw_intel.h
@@ -313,6 +313,8 @@ struct sdw_intel;
* @sync_go_unlocked: helper for multi-link synchronization -
* shim_lock is assumed to be locked at higher level
* @sync_go: helper for multi-link synchronization
+ * @sync_check_cmdsync_unlocked: helper for multi-link synchronization
+ * and bank switch - shim_lock is assumed to be locked at higher level
*/
struct sdw_intel_hw_ops {
void (*debugfs_init)(struct sdw_intel *sdw);
@@ -338,6 +340,7 @@ struct sdw_intel_hw_ops {
void (*sync_arm)(struct sdw_intel *sdw);
int (*sync_go_unlocked)(struct sdw_intel *sdw);
int (*sync_go)(struct sdw_intel *sdw);
+ bool (*sync_check_cmdsync_unlocked)(struct sdw_intel *sdw);
};

extern const struct sdw_intel_hw_ops sdw_intel_cnl_hw_ops;
--
2.25.1


2023-03-14 01:50:33

by Bard Liao

[permalink] [raw]
Subject: [PATCH 07/16] soundwire: intel: use indirection before moving bus start/stop sequences

From: Pierre-Louis Bossart <[email protected]>

There was no benefit to using the existing abstraction, but since we
are going to move the code make sure we do use the ops.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/intel.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 902934cbb27b..8395a20e5739 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1311,13 +1311,13 @@ static int intel_stop_bus(struct sdw_intel *sdw, bool clock_stop)
return ret;
}

- ret = intel_link_power_down(sdw);
+ ret = sdw_intel_link_power_down(sdw);
if (ret) {
dev_err(dev, "%s: Link power down failed: %d\n", __func__, ret);
return ret;
}

- intel_shim_wake(sdw, wake_enable);
+ sdw_intel_shim_wake(sdw, wake_enable);

return 0;
}
--
2.25.1


2023-03-14 01:50:33

by Bard Liao

[permalink] [raw]
Subject: [PATCH 11/16] soundwire: cadence: remove CDNS_MCP_CONFIG_SSPMOD

From: Pierre-Louis Bossart <[email protected]>

This field is not used, and its definition is not aligned with the
hardware specification.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/cadence_master.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index e835dabb516c..4f34fc72dbd5 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -33,7 +33,6 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask");
#define CDNS_MCP_CONFIG_MMASTER BIT(7)
#define CDNS_MCP_CONFIG_BUS_REL BIT(6)
#define CDNS_MCP_CONFIG_SNIFFER BIT(5)
-#define CDNS_MCP_CONFIG_SSPMOD BIT(4)
#define CDNS_MCP_CONFIG_CMD BIT(3)
#define CDNS_MCP_CONFIG_OP GENMASK(2, 0)
#define CDNS_MCP_CONFIG_OP_NORMAL 0
--
2.25.1


2023-03-14 01:50:33

by Bard Liao

[permalink] [raw]
Subject: [PATCH 16/16] soundwire: cadence: change access to IP_MCP_CMD_BASE

From: Pierre-Louis Bossart <[email protected]>

The latest Cadence IP moves MCP_CMD_BASE and MCP_CMD_RESP to the
IP_MCP_CMD_BASE and IP_MCP_CMD_RESP registers located in different
area and accessed with a fixed offset.

Unlike other patches, the fields are not renamed to avoid a very
invasive and low-value set of changes.

For existing solutions, this is an iso-functionality change.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/cadence_master.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 5128923f051e..39502bc75712 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -129,8 +129,8 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask");
#define CDNS_MCP_FIFOSTAT 0x7C
#define CDNS_MCP_RX_FIFO_AVAIL GENMASK(5, 0)

-#define CDNS_MCP_CMD_BASE 0x80
-#define CDNS_MCP_RESP_BASE 0x80
+#define CDNS_IP_MCP_CMD_BASE 0x80 /* IP offset added at run-time */
+#define CDNS_IP_MCP_RESP_BASE 0x80 /* IP offset added at run-time */
/* FIFO can hold 8 commands */
#define CDNS_MCP_CMD_LEN 8
#define CDNS_MCP_CMD_WORD_LEN 0x4
@@ -590,10 +590,10 @@ static void cdns_read_response(struct sdw_cdns *cdns)
num_resp = ARRAY_SIZE(cdns->response_buf);
}

- cmd_base = CDNS_MCP_CMD_BASE;
+ cmd_base = CDNS_IP_MCP_CMD_BASE;

for (i = 0; i < num_resp; i++) {
- cdns->response_buf[i] = cdns_readl(cdns, cmd_base);
+ cdns->response_buf[i] = cdns_ip_readl(cdns, cmd_base);
cmd_base += CDNS_MCP_CMD_WORD_LEN;
}
}
@@ -612,7 +612,7 @@ _cdns_xfer_msg(struct sdw_cdns *cdns, struct sdw_msg *msg, int cmd,
cdns->msg_count = count;
}

- base = CDNS_MCP_CMD_BASE;
+ base = CDNS_IP_MCP_CMD_BASE;
addr = msg->addr + offset;

for (i = 0; i < count; i++) {
@@ -625,7 +625,7 @@ _cdns_xfer_msg(struct sdw_cdns *cdns, struct sdw_msg *msg, int cmd,
data |= msg->buf[i + offset];

data |= FIELD_PREP(CDNS_MCP_CMD_SSP_TAG, msg->ssp_sync);
- cdns_writel(cdns, base, data);
+ cdns_ip_writel(cdns, base, data);
base += CDNS_MCP_CMD_WORD_LEN;
}

@@ -673,10 +673,10 @@ cdns_program_scp_addr(struct sdw_cdns *cdns, struct sdw_msg *msg)
data[0] |= msg->addr_page1;
data[1] |= msg->addr_page2;

- base = CDNS_MCP_CMD_BASE;
- cdns_writel(cdns, base, data[0]);
+ base = CDNS_IP_MCP_CMD_BASE;
+ cdns_ip_writel(cdns, base, data[0]);
base += CDNS_MCP_CMD_WORD_LEN;
- cdns_writel(cdns, base, data[1]);
+ cdns_ip_writel(cdns, base, data[1]);

time = wait_for_completion_timeout(&cdns->tx_complete,
msecs_to_jiffies(CDNS_TX_TIMEOUT));
--
2.25.1


2023-03-14 01:50:33

by Bard Liao

[permalink] [raw]
Subject: [PATCH 02/16] soundwire: intel: remove stale/misleading comment

From: Pierre-Louis Bossart <[email protected]>

The PDIs don't really have a notion of rates and formats, only
channels are relevant.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/intel.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 20067f9cd128..924dff670170 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1056,7 +1056,6 @@ static int intel_create_dai(struct sdw_cdns *cdns,
if (num == 0)
return 0;

- /* TODO: Read supported rates/formats from hardware */
for (i = off; i < (off + num); i++) {
dais[i].name = devm_kasprintf(cdns->dev, GFP_KERNEL,
"SDW%d Pin%d",
--
2.25.1


2023-03-14 01:50:33

by Bard Liao

[permalink] [raw]
Subject: [PATCH 12/16] soundwire: cadence: add helpers to access IP_MCP registers

From: Pierre-Louis Bossart <[email protected]>

The latest Cadence IP splits some of the existing registers into two,
separated by a fixed offset. The bitfields themselves remain at the
same position, so we can use new helpers to dynamically add the fixed
offset.

For example, the existing MCP_CONFIG is now split in two with
MCP_CONFIG and IP_MCP_CONFIG (the naming comes directly from the
design document).

This patch adds helpers to access registers with the IP_ prefix. The
addition of the 'ip' prefix for helpers, registers and bitfields is
intentional to help reviewers spot any mistake.

For existing solutions, the offset is exactly zero so there's no
functional change - the MCP_CONFIG and IP_MCP_CONFIG are aliased to
the same address.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/cadence_master.c | 16 ++++++++++++++++
drivers/soundwire/cadence_master.h | 3 +++
2 files changed, 19 insertions(+)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 4f34fc72dbd5..4461a7fa4124 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -205,6 +205,16 @@ static inline void cdns_writel(struct sdw_cdns *cdns, int offset, u32 value)
writel(value, cdns->registers + offset);
}

+static inline u32 cdns_ip_readl(struct sdw_cdns *cdns, int offset)
+{
+ return cdns_readl(cdns, cdns->ip_offset + offset);
+}
+
+static inline void cdns_ip_writel(struct sdw_cdns *cdns, int offset, u32 value)
+{
+ return cdns_writel(cdns, cdns->ip_offset + offset, value);
+}
+
static inline void cdns_updatel(struct sdw_cdns *cdns,
int offset, u32 mask, u32 val)
{
@@ -215,6 +225,12 @@ static inline void cdns_updatel(struct sdw_cdns *cdns,
cdns_writel(cdns, offset, tmp);
}

+static inline void cdns_ip_updatel(struct sdw_cdns *cdns,
+ int offset, u32 mask, u32 val)
+{
+ cdns_updatel(cdns, cdns->ip_offset + offset, mask, val);
+}
+
static int cdns_set_wait(struct sdw_cdns *cdns, int offset, u32 mask, u32 value)
{
int timeout = 10;
diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
index dec0b4f993c1..b653734085d9 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -107,6 +107,7 @@ struct sdw_cdns_dai_runtime {
* @dev: Linux device
* @bus: Bus handle
* @instance: instance number
+ * @ip_offset: version-dependent offset to access IP_MCP registers and fields
* @response_buf: SoundWire response buffer
* @tx_complete: Tx completion
* @ports: Data ports
@@ -122,6 +123,8 @@ struct sdw_cdns {
struct sdw_bus bus;
unsigned int instance;

+ u32 ip_offset;
+
/*
* The datasheet says the RX FIFO AVAIL can be 2 entries more
* than the FIFO capacity, so allow for this.
--
2.25.1


2023-03-14 01:50:34

by Bard Liao

[permalink] [raw]
Subject: [PATCH 01/16] soundwire: intel: move common definitions to header file

From: Pierre-Louis Bossart <[email protected]>

Prepare for reused for addition of new hardware

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/intel.c | 32 --------------------------------
drivers/soundwire/intel.h | 29 +++++++++++++++++++++++++++++
2 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 2651767272c7..20067f9cd128 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -19,38 +19,6 @@
#include "bus.h"
#include "intel.h"

-
-enum intel_pdi_type {
- INTEL_PDI_IN = 0,
- INTEL_PDI_OUT = 1,
- INTEL_PDI_BD = 2,
-};
-
-#define cdns_to_intel(_cdns) container_of(_cdns, struct sdw_intel, cdns)
-
-/*
- * Read, write helpers for HW registers
- */
-static inline int intel_readl(void __iomem *base, int offset)
-{
- return readl(base + offset);
-}
-
-static inline void intel_writel(void __iomem *base, int offset, int value)
-{
- writel(value, base + offset);
-}
-
-static inline u16 intel_readw(void __iomem *base, int offset)
-{
- return readw(base + offset);
-}
-
-static inline void intel_writew(void __iomem *base, int offset, u16 value)
-{
- writew(value, base + offset);
-}
-
static int intel_wait_bit(void __iomem *base, int offset, u32 mask, u32 target)
{
int timeout = 10;
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index de9883313c8f..089c41babfc1 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -50,6 +50,35 @@ struct sdw_intel {
#endif
};

+enum intel_pdi_type {
+ INTEL_PDI_IN = 0,
+ INTEL_PDI_OUT = 1,
+ INTEL_PDI_BD = 2,
+};
+
+/*
+ * Read, write helpers for HW registers
+ */
+static inline int intel_readl(void __iomem *base, int offset)
+{
+ return readl(base + offset);
+}
+
+static inline void intel_writel(void __iomem *base, int offset, int value)
+{
+ writel(value, base + offset);
+}
+
+static inline u16 intel_readw(void __iomem *base, int offset)
+{
+ return readw(base + offset);
+}
+
+static inline void intel_writew(void __iomem *base, int offset, u16 value)
+{
+ writew(value, base + offset);
+}
+
#define cdns_to_intel(_cdns) container_of(_cdns, struct sdw_intel, cdns)

#define INTEL_MASTER_RESET_ITERATIONS 10
--
2.25.1


2023-03-14 01:50:34

by Bard Liao

[permalink] [raw]
Subject: [PATCH 03/16] soundwire: intel: remove PDI-level restrictions on rates and formats

From: Pierre-Louis Bossart <[email protected]>

This is not relevant and not aligned with hardware definitions. In
addition, we've tested higher resolution formats so this is ignored at
a higher level.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/intel.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 924dff670170..6c17baab7923 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1066,15 +1066,11 @@ static int intel_create_dai(struct sdw_cdns *cdns,
if (type == INTEL_PDI_BD || type == INTEL_PDI_OUT) {
dais[i].playback.channels_min = 1;
dais[i].playback.channels_max = max_ch;
- dais[i].playback.rates = SNDRV_PCM_RATE_48000;
- dais[i].playback.formats = SNDRV_PCM_FMTBIT_S16_LE;
}

if (type == INTEL_PDI_BD || type == INTEL_PDI_IN) {
dais[i].capture.channels_min = 1;
dais[i].capture.channels_max = max_ch;
- dais[i].capture.rates = SNDRV_PCM_RATE_48000;
- dais[i].capture.formats = SNDRV_PCM_FMTBIT_S16_LE;
}

dais[i].ops = &intel_pcm_dai_ops;
--
2.25.1


2023-03-14 01:50:34

by Bard Liao

[permalink] [raw]
Subject: [PATCH 05/16] soundwire: intel: simplify sync_go sequence

From: Pierre-Louis Bossart <[email protected]>

In the existing code, the SHIM_SYNC::SYNC_GO bit is set, and the code
waits for it to return to zero.

That second wait part is just wrong: the SYNC_GO bit is *write-only* so
there's no way to know if it's cleared by hardware. The code works
because the value for a read-only bit is zero, but that's really just
luck.

Simplify the sequence to a plain read-modify-write.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/intel.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 2c1c905f8889..6fdb10117e59 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -475,7 +475,6 @@ 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);
@@ -487,13 +486,9 @@ static int intel_shim_sync_go_unlocked(struct sdw_intel *sdw)
*/
sync_reg |= SDW_SHIM_SYNC_SYNCGO;

- ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
- SDW_SHIM_SYNC_SYNCGO);
+ intel_writel(shim, SDW_SHIM_SYNC, sync_reg);

- if (ret < 0)
- dev_err(sdw->cdns.dev, "SyncGO clear failed: %d\n", ret);
-
- return ret;
+ return 0;
}

static int intel_shim_sync_go(struct sdw_intel *sdw)
--
2.25.1


2023-03-14 01:50:34

by Bard Liao

[permalink] [raw]
Subject: [PATCH 06/16] soundwire: intel: add sync_arm/sync_go to ops

From: Pierre-Louis Bossart <[email protected]>

The bus start/stop sequences can be reused between platforms if we add
a couple of new callbacks. In following patches the code will be moved to
a shared file.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/intel.c | 16 ++++++++++------
drivers/soundwire/intel.h | 20 ++++++++++++++++++++
include/linux/soundwire/sdw_intel.h | 8 ++++++++
3 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 6fdb10117e59..902934cbb27b 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -686,7 +686,7 @@ static int intel_pre_bank_switch(struct sdw_intel *sdw)
if (!bus->multi_link)
return 0;

- intel_shim_sync_arm(sdw);
+ sdw_intel_sync_arm(sdw);

return 0;
}
@@ -720,7 +720,7 @@ static int intel_post_bank_switch(struct sdw_intel *sdw)
goto unlock;
}

- ret = intel_shim_sync_go_unlocked(sdw);
+ ret = sdw_intel_sync_go_unlocked(sdw);
unlock:
mutex_unlock(sdw->link_res->shim_lock);

@@ -1140,7 +1140,7 @@ static int intel_start_bus(struct sdw_intel *sdw)
* gsync is enabled
*/
if (bus->multi_link)
- intel_shim_sync_arm(sdw);
+ sdw_intel_sync_arm(sdw);

ret = sdw_cdns_init(cdns);
if (ret < 0) {
@@ -1155,7 +1155,7 @@ static int intel_start_bus(struct sdw_intel *sdw)
}

if (bus->multi_link) {
- ret = intel_shim_sync_go(sdw);
+ ret = sdw_intel_sync_go(sdw);
if (ret < 0) {
dev_err(dev, "%s: sync go failed: %d\n", __func__, ret);
goto err_interrupt;
@@ -1210,7 +1210,7 @@ static int intel_start_bus_after_reset(struct sdw_intel *sdw)
* timeouts when gsync is enabled
*/
if (bus->multi_link)
- intel_shim_sync_arm(sdw);
+ sdw_intel_sync_arm(sdw);

/*
* Re-initialize the IP since it was powered-off
@@ -1239,7 +1239,7 @@ static int intel_start_bus_after_reset(struct sdw_intel *sdw)
}

if (bus->multi_link) {
- ret = intel_shim_sync_go(sdw);
+ ret = sdw_intel_sync_go(sdw);
if (ret < 0) {
dev_err(sdw->cdns.dev, "sync go failed during resume\n");
goto err_interrupt;
@@ -1342,6 +1342,10 @@ const struct sdw_intel_hw_ops sdw_intel_cnl_hw_ops = {

.pre_bank_switch = intel_pre_bank_switch,
.post_bank_switch = intel_post_bank_switch,
+
+ .sync_arm = intel_shim_sync_arm,
+ .sync_go_unlocked = intel_shim_sync_go_unlocked,
+ .sync_go = intel_shim_sync_go,
};
EXPORT_SYMBOL_NS(sdw_intel_cnl_hw_ops, SOUNDWIRE_INTEL);

diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 089c41babfc1..28b21a92e28b 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -167,4 +167,24 @@ static inline void sdw_intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
SDW_INTEL_OPS(sdw, shim_wake)(sdw, wake_enable);
}

+static inline void sdw_intel_sync_arm(struct sdw_intel *sdw)
+{
+ if (SDW_INTEL_CHECK_OPS(sdw, sync_arm))
+ SDW_INTEL_OPS(sdw, sync_arm)(sdw);
+}
+
+static inline int sdw_intel_sync_go_unlocked(struct sdw_intel *sdw)
+{
+ if (SDW_INTEL_CHECK_OPS(sdw, sync_go_unlocked))
+ return SDW_INTEL_OPS(sdw, sync_go_unlocked)(sdw);
+ return -ENOTSUPP;
+}
+
+static inline int sdw_intel_sync_go(struct sdw_intel *sdw)
+{
+ if (SDW_INTEL_CHECK_OPS(sdw, sync_go))
+ return SDW_INTEL_OPS(sdw, sync_go)(sdw);
+ return -ENOTSUPP;
+}
+
#endif /* __SDW_INTEL_LOCAL_H */
diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
index 91f0dc564fe5..06fa30929ebd 100644
--- a/include/linux/soundwire/sdw_intel.h
+++ b/include/linux/soundwire/sdw_intel.h
@@ -309,6 +309,10 @@ struct sdw_intel;
* @shim_wake: enable/disable in-band wake management
* @pre_bank_switch: helper for bus management
* @post_bank_switch: helper for bus management
+ * @sync_arm: helper for multi-link synchronization
+ * @sync_go_unlocked: helper for multi-link synchronization -
+ * shim_lock is assumed to be locked at higher level
+ * @sync_go: helper for multi-link synchronization
*/
struct sdw_intel_hw_ops {
void (*debugfs_init)(struct sdw_intel *sdw);
@@ -330,6 +334,10 @@ struct sdw_intel_hw_ops {

int (*pre_bank_switch)(struct sdw_intel *sdw);
int (*post_bank_switch)(struct sdw_intel *sdw);
+
+ void (*sync_arm)(struct sdw_intel *sdw);
+ int (*sync_go_unlocked)(struct sdw_intel *sdw);
+ int (*sync_go)(struct sdw_intel *sdw);
};

extern const struct sdw_intel_hw_ops sdw_intel_cnl_hw_ops;
--
2.25.1


2023-03-15 13:54:28

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 00/16] soundwire: updates before LunarLake introduction

On 14-03-23, 09:53, Bard Liao wrote:
> This series provides a set of cleanups and new abstractions needed for the
> introduction of LunarLake support.
>
> For now this is an iso-functionality change, with changes on the Intel and
> Cadence sides. The low-level support for LunarLake will be introduced in a
> follow-up series that depends on HDaudio multi-link extensions.

Applied, thanks

--
~Vinod