[alsa-devel] [RFC PATCH 00/11] soundwire: intel: simplify DAI/PDI handling
In the initial SoundWire code released by Intel, the PDIs and ports on the Master interface were dynamically allocated. This wasn't a bad idea at the time and would have allowed for interesting routing.
Fast-forward to 2019, with the hardware available on CometLake/IceLake, that dynamic allocation makes it complicated to deal with statically-allocated ASoC dailinks and topology-defined DAIs. In this series, we suggest a drastic simplification where the SoundWire code reuses information provided by DAIs and dailinks. We also suggest removing the dynamic allocation of ports on the master since in practice there is a 1:1 mapping between ports and PDIs.
In the second part of the series, we suggest adding new callbacks to SoundWire DAIs, so that all the SoundWire stream operations are contained at the DAI level. This solution results in a very simple integration with the SOF code (which will be shared in a separate series since SOF will not apply directly on top of soundwire/next). The SOF parts only call a SoundWire init/release API, and provides 2 callbacks for hw_params and free, with all the details of the SoundWire DAIs and IP handled in drivers/soundwire.
This solution has been tested on CometLake/IceLake with simple capture/playback. When ASoC supports the multi-cpu capability needed for synchronized playback/capture across multiple links, we will have to modify slightly this solution so that the stream alloc, release and trigger operations are done once. This is future work that will take place later, likely after 5.4, and which should not impact the SOF integration.
The code in this patchset is the result of collaboration between Bard Liao, Rander Wang and Pierre Bossart, with ideas coming from all 3 sides. It's likely that there are still some parts in the code that can be improved, hence the RFC state.
Bard Liao (3): soundwire: intel: fix intel_register_dai PDI offsets and numbers soundwire: intel: remove playback/capture stream_name soundwire: cadence_master: improve PDI allocation
Pierre-Louis Bossart (3): soundwire: remove DAI_ID_RANGE definitions soundwire: cadence/intel: simplify PDI/port mapping soundwire: intel: don't filter out PDI0/1
Rander Wang (5): soundwire: intel: improve .config_stream callback, add .free_stream soundwire: intel: add prepare support in sdw dai driver soundwire: intel: add trigger support in sdw dai driver soundwire: intel: do sdw stream setup in setup function soundwire: intel: free all resources on hw_free()
drivers/soundwire/cadence_master.c | 158 ++++------------ drivers/soundwire/cadence_master.h | 34 +--- drivers/soundwire/intel.c | 278 ++++++++++++++++------------ include/linux/soundwire/sdw.h | 3 - include/linux/soundwire/sdw_intel.h | 4 +- 5 files changed, 209 insertions(+), 268 deletions(-)
From: Bard Liao yung-chuan.liao@linux.intel.com
There are two issues, likely copy/paste:
1. Use cdns->pcm.num_in instead of stream_num_in for consistency with the rest of the code. This was not detected earlier since platforms did not have input-only PDIs.
2. use the correct offset for bi-dir PDM, based on IN and OUT PDIs. Again this was not detected since PDM was not supported earlier.
Reported-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 c02b17a92ac2..c95222f18c75 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -980,7 +980,7 @@ static int intel_register_dai(struct sdw_intel *sdw) /* Create PCM DAIs */ stream = &cdns->pcm;
- ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, stream->num_in, + ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, cdns->pcm.num_in, off, stream->num_ch_in, true); if (ret) return ret; @@ -1011,7 +1011,7 @@ static int intel_register_dai(struct sdw_intel *sdw) if (ret) return ret;
- off += cdns->pdm.num_bd; + off += cdns->pdm.num_out; ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, cdns->pdm.num_bd, off, stream->num_ch_bd, false); if (ret)
There is no reason to reserve a range of DAI IDs for SoundWire. This is not scalable and it's better to let the ASoC core allocate the dai->id when registering a component.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 2 -- include/linux/soundwire/sdw.h | 3 --- 2 files changed, 5 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index c95222f18c75..d5563cfc6549 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -952,8 +952,6 @@ static int intel_create_dai(struct sdw_cdns *cdns, dais[i].capture.formats = SNDRV_PCM_FMTBIT_S16_LE; }
- dais[i].id = SDW_DAI_ID_RANGE_START + i; - if (pcm) dais[i].ops = &intel_pcm_dai_ops; else diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index be9fe08d4e9c..db974003fe13 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -40,9 +40,6 @@ struct sdw_slave;
#define SDW_VALID_PORT_RANGE(n) ((n) <= 14 && (n) >= 1)
-#define SDW_DAI_ID_RANGE_START 100 -#define SDW_DAI_ID_RANGE_END 200 - enum { SDW_PORT_DIRN_SINK = 0, SDW_PORT_DIRN_SOURCE,
From: Bard Liao yung-chuan.liao@linux.intel.com
We will create dai widget in SOF.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 17 ----------------- 1 file changed, 17 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index d5563cfc6549..883289a04c82 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -922,14 +922,6 @@ static int intel_create_dai(struct sdw_cdns *cdns, return -ENOMEM;
if (type == INTEL_PDI_BD || type == INTEL_PDI_OUT) { - dais[i].playback.stream_name = - kasprintf(GFP_KERNEL, "SDW%d Tx%d", - cdns->instance, i); - if (!dais[i].playback.stream_name) { - kfree(dais[i].name); - return -ENOMEM; - } - dais[i].playback.channels_min = 1; dais[i].playback.channels_max = max_ch; dais[i].playback.rates = SNDRV_PCM_RATE_48000; @@ -937,15 +929,6 @@ static int intel_create_dai(struct sdw_cdns *cdns, }
if (type == INTEL_PDI_BD || type == INTEL_PDI_IN) { - dais[i].capture.stream_name = - kasprintf(GFP_KERNEL, "SDW%d Rx%d", - cdns->instance, i); - if (!dais[i].capture.stream_name) { - kfree(dais[i].name); - kfree(dais[i].playback.stream_name); - return -ENOMEM; - } - dais[i].capture.channels_min = 1; dais[i].capture.channels_max = max_ch; dais[i].capture.rates = SNDRV_PCM_RATE_48000;
The existing Linux code uses a 1:1 mapping between ports and PDIs, but still has an independent allocation of ports and PDIs.
Let's simplify the code and remove the port layer by only using PDIs.
This patch does not change any functionality, just removes unnecessary code.
This will also allow for further simplifications where the PDIs are not dynamically allocated but instead described in a topology file.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/cadence_master.c | 110 ++++-------------------- drivers/soundwire/cadence_master.h | 32 ++----- drivers/soundwire/intel.c | 133 ++++++----------------------- 3 files changed, 51 insertions(+), 224 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 17ac2ecd8d5c..fcab2e2f4249 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -899,7 +899,8 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns, struct sdw_cdns_stream_config config) { struct sdw_cdns_streams *stream; - int offset, i, ret; + int offset; + int ret;
cdns->pcm.num_bd = config.pcm_bd; cdns->pcm.num_in = config.pcm_in; @@ -966,18 +967,6 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns, stream->num_pdi = stream->num_bd + stream->num_in + stream->num_out; cdns->num_ports += stream->num_pdi;
- cdns->ports = devm_kcalloc(cdns->dev, cdns->num_ports, - sizeof(*cdns->ports), GFP_KERNEL); - if (!cdns->ports) { - ret = -ENOMEM; - return ret; - } - - for (i = 0; i < cdns->num_ports; i++) { - cdns->ports[i].assigned = false; - cdns->ports[i].num = i + 1; /* Port 0 reserved for bulk */ - } - return 0; } EXPORT_SYMBOL(sdw_cdns_pdi_init); @@ -1270,13 +1259,11 @@ static struct sdw_cdns_pdi *cdns_find_pdi(struct sdw_cdns *cdns, * sdw_cdns_config_stream: Configure a stream * * @cdns: Cadence instance - * @port: Cadence data port * @ch: Channel count * @dir: Data direction * @pdi: PDI to be used */ void sdw_cdns_config_stream(struct sdw_cdns *cdns, - struct sdw_cdns_port *port, u32 ch, u32 dir, struct sdw_cdns_pdi *pdi) { u32 offset, val = 0; @@ -1284,89 +1271,26 @@ void sdw_cdns_config_stream(struct sdw_cdns *cdns, if (dir == SDW_DATA_DIR_RX) val = CDNS_PORTCTRL_DIRN;
- offset = CDNS_PORTCTRL + port->num * CDNS_PORT_OFFSET; + offset = CDNS_PORTCTRL + pdi->num * CDNS_PORT_OFFSET; cdns_updatel(cdns, offset, CDNS_PORTCTRL_DIRN, val);
- val = port->num; + val = pdi->num; val |= ((1 << ch) - 1) << SDW_REG_SHIFT(CDNS_PDI_CONFIG_CHANNEL); cdns_writel(cdns, CDNS_PDI_CONFIG(pdi->num), val); } EXPORT_SYMBOL(sdw_cdns_config_stream);
/** - * cdns_get_num_pdi() - Get number of PDIs required - * - * @cdns: Cadence instance - * @pdi: PDI to be used - * @num: Number of PDIs - * @ch_count: Channel count - */ -static int cdns_get_num_pdi(struct sdw_cdns *cdns, - struct sdw_cdns_pdi *pdi, - unsigned int num, u32 ch_count) -{ - int i, pdis = 0; - - for (i = 0; i < num; i++) { - if (pdi[i].assigned) - continue; - - if (pdi[i].ch_count < ch_count) - ch_count -= pdi[i].ch_count; - else - ch_count = 0; - - pdis++; - - if (!ch_count) - break; - } - - if (ch_count) - return 0; - - return pdis; -} - -/** - * sdw_cdns_get_stream() - Get stream information - * - * @cdns: Cadence instance - * @stream: Stream to be allocated - * @ch: Channel count - * @dir: Data direction - */ -int sdw_cdns_get_stream(struct sdw_cdns *cdns, - struct sdw_cdns_streams *stream, - u32 ch, u32 dir) -{ - int pdis = 0; - - if (dir == SDW_DATA_DIR_RX) - pdis = cdns_get_num_pdi(cdns, stream->in, stream->num_in, ch); - else - pdis = cdns_get_num_pdi(cdns, stream->out, stream->num_out, ch); - - /* check if we found PDI, else find in bi-directional */ - if (!pdis) - pdis = cdns_get_num_pdi(cdns, stream->bd, stream->num_bd, ch); - - return pdis; -} -EXPORT_SYMBOL(sdw_cdns_get_stream); - -/** - * sdw_cdns_alloc_stream() - Allocate a stream + * sdw_cdns_alloc_pdi() - Allocate a PDI * * @cdns: Cadence instance * @stream: Stream to be allocated - * @port: Cadence data port * @ch: Channel count * @dir: Data direction */ -int sdw_cdns_alloc_stream(struct sdw_cdns *cdns, - struct sdw_cdns_streams *stream, - struct sdw_cdns_port *port, u32 ch, u32 dir) +struct sdw_cdns_pdi *sdw_cdns_alloc_pdi(struct sdw_cdns *cdns, + struct sdw_cdns_streams *stream, + u32 ch, u32 dir) { struct sdw_cdns_pdi *pdi = NULL;
@@ -1379,18 +1303,16 @@ int sdw_cdns_alloc_stream(struct sdw_cdns *cdns, if (!pdi) pdi = cdns_find_pdi(cdns, stream->num_bd, stream->bd);
- if (!pdi) - return -EIO; - - port->pdi = pdi; - pdi->l_ch_num = 0; - pdi->h_ch_num = ch - 1; - pdi->dir = dir; - pdi->ch_count = ch; + if (pdi) { + pdi->l_ch_num = 0; + pdi->h_ch_num = ch - 1; + pdi->dir = dir; + pdi->ch_count = ch; + }
- return 0; + return pdi; } -EXPORT_SYMBOL(sdw_cdns_alloc_stream); +EXPORT_SYMBOL(sdw_cdns_alloc_pdi);
MODULE_LICENSE("Dual BSD/GPL"); MODULE_DESCRIPTION("Cadence Soundwire Library"); diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index 9d7a48ac6fc4..43493fc3d2ee 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -28,23 +28,6 @@ struct sdw_cdns_pdi { enum sdw_stream_type type; };
-/** - * struct sdw_cdns_port: Cadence port structure - * - * @num: port number - * @assigned: port assigned - * @ch: channel count - * @direction: data port direction - * @pdi: pdi for this port - */ -struct sdw_cdns_port { - unsigned int num; - bool assigned; - unsigned int ch; - enum sdw_data_direction direction; - struct sdw_cdns_pdi *pdi; -}; - /** * struct sdw_cdns_streams: Cadence stream data structure * @@ -95,8 +78,8 @@ struct sdw_cdns_stream_config { * struct sdw_cdns_dma_data: Cadence DMA data * * @name: SoundWire stream name - * @nr_ports: Number of ports - * @port: Ports + * @stream: stream runtime + * @pdi: PDI used for this dai * @bus: Bus handle * @stream_type: Stream type * @link_id: Master link id @@ -104,8 +87,7 @@ struct sdw_cdns_stream_config { struct sdw_cdns_dma_data { char *name; struct sdw_stream_runtime *stream; - int nr_ports; - struct sdw_cdns_port **port; + struct sdw_cdns_pdi *pdi; struct sdw_bus *bus; enum sdw_stream_type stream_type; int link_id; @@ -171,10 +153,10 @@ void sdw_cdns_debugfs_init(struct sdw_cdns *cdns, struct dentry *root); int sdw_cdns_get_stream(struct sdw_cdns *cdns, struct sdw_cdns_streams *stream, u32 ch, u32 dir); -int sdw_cdns_alloc_stream(struct sdw_cdns *cdns, - struct sdw_cdns_streams *stream, - struct sdw_cdns_port *port, u32 ch, u32 dir); -void sdw_cdns_config_stream(struct sdw_cdns *cdns, struct sdw_cdns_port *port, +struct sdw_cdns_pdi *sdw_cdns_alloc_pdi(struct sdw_cdns *cdns, + struct sdw_cdns_streams *stream, + u32 ch, u32 dir); +void sdw_cdns_config_stream(struct sdw_cdns *cdns, u32 ch, u32 dir, struct sdw_cdns_pdi *pdi);
int sdw_cdns_pcm_set_stream(struct snd_soc_dai *dai, diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 883289a04c82..e21c994d46b9 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -656,66 +656,6 @@ static int intel_post_bank_switch(struct sdw_bus *bus) * DAI routines */
-static struct sdw_cdns_port *intel_alloc_port(struct sdw_intel *sdw, - u32 ch, u32 dir, bool pcm) -{ - struct sdw_cdns *cdns = &sdw->cdns; - struct sdw_cdns_port *port = NULL; - int i, ret = 0; - - for (i = 0; i < cdns->num_ports; i++) { - if (cdns->ports[i].assigned) - continue; - - port = &cdns->ports[i]; - port->assigned = true; - port->direction = dir; - port->ch = ch; - break; - } - - if (!port) { - dev_err(cdns->dev, "Unable to find a free port\n"); - return NULL; - } - - if (pcm) { - ret = sdw_cdns_alloc_stream(cdns, &cdns->pcm, port, ch, dir); - if (ret) - goto out; - - intel_pdi_shim_configure(sdw, port->pdi); - sdw_cdns_config_stream(cdns, port, ch, dir, port->pdi); - - intel_pdi_alh_configure(sdw, port->pdi); - - } else { - ret = sdw_cdns_alloc_stream(cdns, &cdns->pdm, port, ch, dir); - } - -out: - if (ret) { - port->assigned = false; - port = NULL; - } - - return port; -} - -static void intel_port_cleanup(struct sdw_cdns_dma_data *dma) -{ - int i; - - for (i = 0; i < dma->nr_ports; i++) { - if (dma->port[i]) { - dma->port[i]->pdi->assigned = false; - dma->port[i]->pdi = NULL; - dma->port[i]->assigned = false; - dma->port[i] = NULL; - } - } -} - static int intel_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -740,9 +680,11 @@ static int intel_hw_params(struct snd_pcm_substream *substream, struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_cdns_dma_data *dma; + struct sdw_cdns_pdi *pdi; struct sdw_stream_config sconfig; struct sdw_port_config *pconfig; - int ret, i, ch, dir; + int ch, dir; + int ret; bool pcm = true;
dma = snd_soc_dai_get_dma_data(dai, substream); @@ -755,38 +697,31 @@ static int intel_hw_params(struct snd_pcm_substream *substream, else dir = SDW_DATA_DIR_TX;
- if (dma->stream_type == SDW_STREAM_PDM) { - /* TODO: Check whether PDM decimator is already in use */ - dma->nr_ports = sdw_cdns_get_stream(cdns, &cdns->pdm, ch, dir); + if (dma->stream_type == SDW_STREAM_PDM) pcm = false; - } else { - dma->nr_ports = sdw_cdns_get_stream(cdns, &cdns->pcm, ch, dir); - }
- if (!dma->nr_ports) { - dev_err(dai->dev, "ports/resources not available\n"); - return -EINVAL; + /* FIXME: We would need to get PDI info from topology */ + if (pcm) + pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pcm, ch, dir); + else + pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pdm, ch, dir); + + if (!pdi) { + ret = -EINVAL; + goto error; }
- dma->port = kcalloc(dma->nr_ports, sizeof(*dma->port), GFP_KERNEL); - if (!dma->port) - return -ENOMEM; + /* do run-time configurations for SHIM, ALH and PDI/PORT */ + intel_pdi_shim_configure(sdw, pdi); + intel_pdi_alh_configure(sdw, pdi); + sdw_cdns_config_stream(cdns, ch, dir, pdi);
- for (i = 0; i < dma->nr_ports; i++) { - dma->port[i] = intel_alloc_port(sdw, ch, dir, pcm); - if (!dma->port[i]) { - ret = -EINVAL; - goto port_error; - } - }
/* Inform DSP about PDI stream number */ - for (i = 0; i < dma->nr_ports; i++) { - ret = intel_config_stream(sdw, substream, dai, params, - dma->port[i]->pdi->intel_alh_id); - if (ret) - goto port_error; - } + ret = intel_config_stream(sdw, substream, dai, params, + pdi->intel_alh_id); + if (ret) + goto error;
sconfig.direction = dir; sconfig.ch_count = ch; @@ -801,32 +736,22 @@ static int intel_hw_params(struct snd_pcm_substream *substream, }
/* Port configuration */ - pconfig = kcalloc(dma->nr_ports, sizeof(*pconfig), GFP_KERNEL); + pconfig = kcalloc(1, sizeof(*pconfig), GFP_KERNEL); if (!pconfig) { ret = -ENOMEM; - goto port_error; + goto error; }
- for (i = 0; i < dma->nr_ports; i++) { - pconfig[i].num = dma->port[i]->num; - pconfig[i].ch_mask = (1 << ch) - 1; - } + pconfig->num = pdi->num; + pconfig->ch_mask = (1 << ch) - 1;
ret = sdw_stream_add_master(&cdns->bus, &sconfig, - pconfig, dma->nr_ports, dma->stream); - if (ret) { + pconfig, 1, dma->stream); + if (ret) dev_err(cdns->dev, "add master to stream failed:%d\n", ret); - goto stream_error; - } - - kfree(pconfig); - return ret;
-stream_error: kfree(pconfig); -port_error: - intel_port_cleanup(dma); - kfree(dma->port); +error: return ret; }
@@ -846,8 +771,6 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) dev_err(dai->dev, "remove master from stream %s failed: %d\n", dma->stream->name, ret);
- intel_port_cleanup(dma); - kfree(dma->port); return ret; }
PDI0/1 are reserved for Bulk and filtered out in the existing code. That leads to endless confusions on whether the index is the raw or corrected one. In addition we will need support for Bulk at some point so it's just simpler to expose those PDIs and not use it for now than try to be smart unless we have to remove the smarts.
This patch requires a topology change to use PDIs starting at offset 2 explicitly.
Note that there is a known discrepancy between hardware documentation and what ALH stream works in practice, future fixes are likely.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/cadence_master.c | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index fcab2e2f4249..8510d4ee8044 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -183,9 +183,6 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask"); #define CDNS_DEFAULT_SSP_INTERVAL 0x18 #define CDNS_TX_TIMEOUT 2000
-#define CDNS_PCM_PDI_OFFSET 0x2 -#define CDNS_PDM_PDI_OFFSET 0x6 - #define CDNS_SCP_RX_FIFOLEVEL 0x2
/* @@ -295,11 +292,7 @@ static int cdns_reg_show(struct seq_file *s, void *data) ret += scnprintf(buf + ret, RD_BUF - ret, "\nDPn B0 Registers\n");
- /* - * in sdw_cdns_pdi_init() we filter out the Bulk PDIs, - * so the indices need to be corrected again - */ - num_ports = cdns->num_ports + CDNS_PCM_PDI_OFFSET; + num_ports = cdns->num_ports;
for (i = 0; i < num_ports; i++) { ret += scnprintf(buf + ret, RD_BUF - ret, @@ -912,11 +905,8 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns, /* Allocate PDIs for PCMs */ stream = &cdns->pcm;
- /* First two PDIs are reserved for bulk transfers */ - if (stream->num_bd < CDNS_PCM_PDI_OFFSET) - return -EINVAL; - stream->num_bd -= CDNS_PCM_PDI_OFFSET; - offset = CDNS_PCM_PDI_OFFSET; + /* we allocate PDI0 and PDI1 which are used for Bulk */ + offset = 0;
ret = cdns_allocate_pdi(cdns, &stream->bd, stream->num_bd, offset); @@ -934,6 +924,9 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
ret = cdns_allocate_pdi(cdns, &stream->out, stream->num_out, offset); + + offset += stream->num_out; + if (ret) return ret;
@@ -943,7 +936,6 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
/* Allocate PDIs for PDMs */ stream = &cdns->pdm; - offset = CDNS_PDM_PDI_OFFSET; ret = cdns_allocate_pdi(cdns, &stream->bd, stream->num_bd, offset); if (ret) @@ -1240,12 +1232,13 @@ EXPORT_SYMBOL(cdns_set_sdw_stream); * Find and return a free PDI for a given PDI array */ static struct sdw_cdns_pdi *cdns_find_pdi(struct sdw_cdns *cdns, + unsigned int offset, unsigned int num, struct sdw_cdns_pdi *pdi) { int i;
- for (i = 0; i < num; i++) { + for (i = offset; i < num; i++) { if (pdi[i].assigned) continue; pdi[i].assigned = true; @@ -1295,13 +1288,13 @@ struct sdw_cdns_pdi *sdw_cdns_alloc_pdi(struct sdw_cdns *cdns, struct sdw_cdns_pdi *pdi = NULL;
if (dir == SDW_DATA_DIR_RX) - pdi = cdns_find_pdi(cdns, stream->num_in, stream->in); + pdi = cdns_find_pdi(cdns, 0, stream->num_in, stream->in); else - pdi = cdns_find_pdi(cdns, stream->num_out, stream->out); + pdi = cdns_find_pdi(cdns, 0, stream->num_out, stream->out);
/* check if we found a PDI, else find in bi-directional */ if (!pdi) - pdi = cdns_find_pdi(cdns, stream->num_bd, stream->bd); + pdi = cdns_find_pdi(cdns, 2, stream->num_bd, stream->bd);
if (pdi) { pdi->l_ch_num = 0;
From: Bard Liao yung-chuan.liao@linux.intel.com
PDI number should match dai->id, there is no need to track if a PDI is allocated or not.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/cadence_master.c | 27 ++++++++++++++------------- drivers/soundwire/cadence_master.h | 4 +--- drivers/soundwire/intel.c | 5 ++--- 3 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 8510d4ee8044..1e8cc14fc2ec 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -875,7 +875,6 @@ static int cdns_allocate_pdi(struct sdw_cdns *cdns,
for (i = 0; i < num; i++) { pdi[i].num = i + pdi_offset; - pdi[i].assigned = false; }
*stream = pdi; @@ -1229,21 +1228,20 @@ EXPORT_SYMBOL(cdns_set_sdw_stream); * @num: Number of PDIs * @pdi: PDI instances * - * Find and return a free PDI for a given PDI array + * Find a PDI for a given PDI array. The PDI num and dai_id are + * expected to match, return NULL otherwise. */ static struct sdw_cdns_pdi *cdns_find_pdi(struct sdw_cdns *cdns, unsigned int offset, unsigned int num, - struct sdw_cdns_pdi *pdi) + struct sdw_cdns_pdi *pdi, + int dai_id) { int i;
- for (i = offset; i < num; i++) { - if (pdi[i].assigned) - continue; - pdi[i].assigned = true; - return &pdi[i]; - } + for (i = offset; i < offset + num; i++) + if (pdi[i].num == dai_id) + return &pdi[i];
return NULL; } @@ -1283,18 +1281,21 @@ EXPORT_SYMBOL(sdw_cdns_config_stream); */ struct sdw_cdns_pdi *sdw_cdns_alloc_pdi(struct sdw_cdns *cdns, struct sdw_cdns_streams *stream, - u32 ch, u32 dir) + u32 ch, u32 dir, int dai_id) { struct sdw_cdns_pdi *pdi = NULL;
if (dir == SDW_DATA_DIR_RX) - pdi = cdns_find_pdi(cdns, 0, stream->num_in, stream->in); + pdi = cdns_find_pdi(cdns, 0, stream->num_in, stream->in, + dai_id); else - pdi = cdns_find_pdi(cdns, 0, stream->num_out, stream->out); + pdi = cdns_find_pdi(cdns, 0, stream->num_out, stream->out, + dai_id);
/* check if we found a PDI, else find in bi-directional */ if (!pdi) - pdi = cdns_find_pdi(cdns, 2, stream->num_bd, stream->bd); + pdi = cdns_find_pdi(cdns, 2, stream->num_bd, stream->bd, + dai_id);
if (pdi) { pdi->l_ch_num = 0; diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index 43493fc3d2ee..001457cbe5ad 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -8,7 +8,6 @@ /** * struct sdw_cdns_pdi: PDI (Physical Data Interface) instance * - * @assigned: pdi assigned * @num: pdi number * @intel_alh_id: link identifier * @l_ch_num: low channel for PDI @@ -18,7 +17,6 @@ * @type: stream type, PDM or PCM */ struct sdw_cdns_pdi { - bool assigned; int num; int intel_alh_id; int l_ch_num; @@ -155,7 +153,7 @@ int sdw_cdns_get_stream(struct sdw_cdns *cdns, u32 ch, u32 dir); struct sdw_cdns_pdi *sdw_cdns_alloc_pdi(struct sdw_cdns *cdns, struct sdw_cdns_streams *stream, - u32 ch, u32 dir); + u32 ch, u32 dir, int dai_id); void sdw_cdns_config_stream(struct sdw_cdns *cdns, u32 ch, u32 dir, struct sdw_cdns_pdi *pdi);
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index e21c994d46b9..0001f433e848 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -700,11 +700,10 @@ static int intel_hw_params(struct snd_pcm_substream *substream, if (dma->stream_type == SDW_STREAM_PDM) pcm = false;
- /* FIXME: We would need to get PDI info from topology */ if (pcm) - pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pcm, ch, dir); + pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pcm, ch, dir, dai->id); else - pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pdm, ch, dir); + pdi = sdw_cdns_alloc_pdi(cdns, &cdns->pdm, ch, dir, dai->id);
if (!pdi) { ret = -EINVAL;
From: Rander Wang rander.wang@linux.intel.com
We need the link_id as a parameter for the config callback, and we also need a matching free callback in case any resources need to be released.
Signed-off-by: Rander Wang rander.wang@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 19 +++++++++++++++++-- include/linux/soundwire/sdw_intel.h | 4 +++- 2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 0001f433e848..529d7bc693d1 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -578,17 +578,31 @@ intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) static int intel_config_stream(struct sdw_intel *sdw, struct snd_pcm_substream *substream, struct snd_soc_dai *dai, - struct snd_pcm_hw_params *hw_params, int link_id) + struct snd_pcm_hw_params *hw_params, + int link_id, int alh_stream_id) { struct sdw_intel_link_res *res = sdw->res;
if (res->ops && res->ops->config_stream && res->arg) return res->ops->config_stream(res->arg, - substream, dai, hw_params, link_id); + substream, dai, hw_params, + link_id, alh_stream_id);
return -EIO; }
+static int intel_free_stream(struct sdw_intel *sdw, + struct snd_pcm_substream *substream, + struct snd_soc_dai *dai, + int link_id) +{ + if (sdw->res->ops && sdw->res->ops->free_stream && sdw->res->arg) + return sdw->res->ops->free_stream(sdw->res->arg, + substream, dai, link_id); + + return 0; +} + /* * bank switch routines */ @@ -718,6 +732,7 @@ static int intel_hw_params(struct snd_pcm_substream *substream,
/* Inform DSP about PDI stream number */ ret = intel_config_stream(sdw, substream, dai, params, + sdw->instance, pdi->intel_alh_id); if (ret) goto error; diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index c9427cb6020b..c5a2caf3a9d5 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -12,7 +12,9 @@ */ struct sdw_intel_ops { int (*config_stream)(void *arg, void *substream, - void *dai, void *hw_params, int stream_num); + void *dai, void *hw_params, + int link_id, int alh_stream_id); + int (*free_stream)(void *arg, void *substream, void *dai, int link_id); };
/**
From: Rander Wang rander.wang@linux.intel.com
It gets sdw runtime information from dai to prepare stream.
Signed-off-by: Rander Wang rander.wang@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 529d7bc693d1..b9ce13d01334 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -769,6 +769,21 @@ static int intel_hw_params(struct snd_pcm_substream *substream, return ret; }
+static int intel_prepare(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct sdw_cdns_dma_data *dma; + + dma = snd_soc_dai_get_dma_data(dai, substream); + if (!dma) { + dev_err(dai->dev, "failed to get dma data in %s", + __func__); + return -EIO; + } + + return sdw_prepare_stream(dma->stream); +} + static int intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -825,6 +840,7 @@ static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai, static const struct snd_soc_dai_ops intel_pcm_dai_ops = { .startup = intel_startup, .hw_params = intel_hw_params, + .prepare = intel_prepare, .hw_free = intel_hw_free, .shutdown = intel_shutdown, .set_sdw_stream = intel_pcm_set_sdw_stream, @@ -832,6 +848,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
static const struct snd_soc_dai_ops intel_pdm_dai_ops = { .hw_params = intel_hw_params, + .prepare = intel_prepare, .hw_free = intel_hw_free, .shutdown = intel_shutdown, .set_sdw_stream = intel_pdm_set_sdw_stream,
From: Rander Wang rander.wang@linux.intel.com
Sdw stream is enabled and disabled in trigger function.
Signed-off-by: Rander Wang rander.wang@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index b9ce13d01334..f3764f67919e 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -784,6 +784,43 @@ static int intel_prepare(struct snd_pcm_substream *substream, return sdw_prepare_stream(dma->stream); }
+static int intel_trigger(struct snd_pcm_substream *substream, int cmd, + struct snd_soc_dai *dai) +{ + struct sdw_cdns_dma_data *dma; + int ret; + + dma = snd_soc_dai_get_dma_data(dai, substream); + if (!dma) { + dev_err(dai->dev, "failed to get dma data in %s", __func__); + return -EIO; + } + + switch (cmd) { + case SNDRV_PCM_TRIGGER_START: + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + case SNDRV_PCM_TRIGGER_RESUME: + ret = sdw_enable_stream(dma->stream); + break; + + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_STOP: + ret = sdw_disable_stream(dma->stream); + break; + + default: + ret = -EINVAL; + break; + } + + if (ret) + dev_err(dai->dev, + "%s trigger %d failed: %d", + __func__, cmd, ret); + return ret; +} + static int intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -841,6 +878,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = { .startup = intel_startup, .hw_params = intel_hw_params, .prepare = intel_prepare, + .trigger = intel_trigger, .hw_free = intel_hw_free, .shutdown = intel_shutdown, .set_sdw_stream = intel_pcm_set_sdw_stream, @@ -849,6 +887,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = { static const struct snd_soc_dai_ops intel_pdm_dai_ops = { .hw_params = intel_hw_params, .prepare = intel_prepare, + .trigger = intel_trigger, .hw_free = intel_hw_free, .shutdown = intel_shutdown, .set_sdw_stream = intel_pdm_set_sdw_stream,
From: Rander Wang rander.wang@linux.intel.com
The sdw stream is allocated and stored in dai to share the sdw runtime information.
Signed-off-by: Rander Wang rander.wang@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 57 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index f3764f67919e..336203303480 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -670,6 +670,58 @@ static int intel_post_bank_switch(struct sdw_bus *bus) * DAI routines */
+static int sdw_stream_setup(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct sdw_stream_runtime *sdw_stream = NULL; + char *name; + int i, ret; + + name = kzalloc(32, GFP_KERNEL); + if (!name) + return -ENOMEM; + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + snprintf(name, 32, "%s-Playback", dai->name); + else + snprintf(name, 32, "%s-Capture", dai->name); + + sdw_stream = sdw_alloc_stream(name); + if (!sdw_stream) { + dev_err(dai->dev, "alloc stream failed for DAI %s", dai->name); + ret = -ENOMEM; + goto error; + } + + /* Set stream pointer on CPU DAI */ + ret = snd_soc_dai_set_sdw_stream(dai, sdw_stream, substream->stream); + if (ret < 0) { + dev_err(dai->dev, "failed to set stream pointer on cpu dai %s", + dai->name); + goto release_stream; + } + + /* Set stream pointer on all CODEC DAIs */ + for (i = 0; i < rtd->num_codecs; i++) { + ret = snd_soc_dai_set_sdw_stream(rtd->codec_dais[i], sdw_stream, + substream->stream); + if (ret < 0) { + dev_err(dai->dev, "failed to set stream pointer on codec dai %s", + rtd->codec_dais[i]->name); + goto release_stream; + } + } + + return 0; + +release_stream: + sdw_release_stream(sdw_stream); +error: + kfree(name); + return ret; +} + static int intel_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -682,9 +734,11 @@ static int intel_startup(struct snd_pcm_substream *substream, "pm_runtime_get_sync failed in %s, ret %d\n", __func__, ret); pm_runtime_put_noidle(cdns->dev); + + return ret; }
- return ret; + return sdw_stream_setup(substream, dai); }
static int intel_hw_params(struct snd_pcm_substream *substream, @@ -885,6 +939,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = { };
static const struct snd_soc_dai_ops intel_pdm_dai_ops = { + .startup = intel_startup, .hw_params = intel_hw_params, .prepare = intel_prepare, .trigger = intel_trigger,
From: Rander Wang rander.wang@linux.intel.com
Make sure all calls to the SoundWire stream API are done and involve callback
Signed-off-by: Rander Wang rander.wang@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 336203303480..60bc5f33afbf 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -879,6 +879,7 @@ static int intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); + struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_cdns_dma_data *dma; int ret;
@@ -886,12 +887,28 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) if (!dma) return -EIO;
+ ret = sdw_deprepare_stream(dma->stream); + if (ret) { + dev_err(dai->dev, "sdw_deprepare_stream: failed %d", ret); + return ret; + } + ret = sdw_stream_remove_master(&cdns->bus, dma->stream); - if (ret < 0) + if (ret < 0) { dev_err(dai->dev, "remove master from stream %s failed: %d\n", dma->stream->name, ret); + return ret; + }
- return ret; + ret = intel_free_stream(sdw, substream, dai, sdw->instance); + if (ret < 0) { + dev_err(dai->dev, "intel_free_stream: failed %d", ret); + return ret; + } + + sdw_release_stream(dma->stream); + + return 0; }
static void intel_shutdown(struct snd_pcm_substream *substream,
participants (1)
-
Pierre-Louis Bossart