[PATCH 0/4] Extend ipc stream parameters sent to DSP
From: Daniel Baluta daniel.baluta@nxp.com
MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
We need a way to send extra parameters to DSP firmware. In order to do this, we introduce ext_data array at the end of ipc_stream_params.
With this new addition we can send compress parameters.
This requires SOF ABI bump.
Daniel Baluta (4): ASoC: SOF: compress: Dynamically allocate pcm params struct ASoC: SOF: Copy compress parameters into extended data ASoC: SOF: compress: Prevent current kernel running with older FW uapi: sof: abi: Bump SOF ABI for ext_data_length
include/sound/sof/stream.h | 6 ++- include/uapi/sound/sof/abi.h | 2 +- sound/soc/sof/compress.c | 73 ++++++++++++++++++++++++------------ 3 files changed, 54 insertions(+), 27 deletions(-)
From: Daniel Baluta daniel.baluta@nxp.com
We need to extend sof_ipc_pcm_parmas with additional data in order to send compress_params to SOF FW.
The extensions will be done at runtime so we need to dynamically allocate pcm object of type struct sof_ipc_pcm_params.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/compress.c | 53 ++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 24 deletions(-)
diff --git a/sound/soc/sof/compress.c b/sound/soc/sof/compress.c index 47639b6344c8..45c2ff61ee4d 100644 --- a/sound/soc/sof/compress.c +++ b/sound/soc/sof/compress.c @@ -168,7 +168,7 @@ static int sof_compr_set_params(struct snd_soc_component *component, struct snd_compr_runtime *crtd = cstream->runtime; struct sof_ipc_pcm_params_reply ipc_params_reply; struct snd_compr_tstamp *tstamp; - struct sof_ipc_pcm_params pcm; + struct sof_ipc_pcm_params *pcm; struct snd_sof_pcm *spcm; int ret;
@@ -179,40 +179,42 @@ static int sof_compr_set_params(struct snd_soc_component *component, if (!spcm) return -EINVAL;
+ pcm = kzalloc(sizeof(*pcm), GFP_KERNEL); + if (!pcm) + return -ENOMEM; + cstream->dma_buffer.dev.type = SNDRV_DMA_TYPE_DEV_SG; cstream->dma_buffer.dev.dev = sdev->dev; ret = snd_compr_malloc_pages(cstream, crtd->buffer_size); if (ret < 0) - return ret; + goto out;
ret = create_page_table(component, cstream, crtd->dma_area, crtd->dma_bytes); if (ret < 0) - return ret; - - memset(&pcm, 0, sizeof(pcm)); - - pcm.params.buffer.pages = PFN_UP(crtd->dma_bytes); - pcm.hdr.size = sizeof(pcm); - pcm.hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_PCM_PARAMS; - - pcm.comp_id = spcm->stream[cstream->direction].comp_id; - pcm.params.hdr.size = sizeof(pcm.params); - pcm.params.buffer.phy_addr = spcm->stream[cstream->direction].page_table.addr; - pcm.params.buffer.size = crtd->dma_bytes; - pcm.params.direction = cstream->direction; - pcm.params.channels = params->codec.ch_out; - pcm.params.rate = params->codec.sample_rate; - pcm.params.buffer_fmt = SOF_IPC_BUFFER_INTERLEAVED; - pcm.params.frame_fmt = SOF_IPC_FRAME_S32_LE; - pcm.params.sample_container_bytes = + goto out; + + pcm->params.buffer.pages = PFN_UP(crtd->dma_bytes); + pcm->hdr.size = sizeof(*pcm); + pcm->hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_PCM_PARAMS; + + pcm->comp_id = spcm->stream[cstream->direction].comp_id; + pcm->params.hdr.size = sizeof(pcm->params); + pcm->params.buffer.phy_addr = spcm->stream[cstream->direction].page_table.addr; + pcm->params.buffer.size = crtd->dma_bytes; + pcm->params.direction = cstream->direction; + pcm->params.channels = params->codec.ch_out; + pcm->params.rate = params->codec.sample_rate; + pcm->params.buffer_fmt = SOF_IPC_BUFFER_INTERLEAVED; + pcm->params.frame_fmt = SOF_IPC_FRAME_S32_LE; + pcm->params.sample_container_bytes = snd_pcm_format_physical_width(SNDRV_PCM_FORMAT_S32) >> 3; - pcm.params.host_period_bytes = params->buffer.fragment_size; + pcm->params.host_period_bytes = params->buffer.fragment_size;
- ret = sof_ipc_tx_message(sdev->ipc, &pcm, sizeof(pcm), + ret = sof_ipc_tx_message(sdev->ipc, pcm, sizeof(*pcm), &ipc_params_reply, sizeof(ipc_params_reply)); if (ret < 0) { dev_err(component->dev, "error ipc failed\n"); - return ret; + goto out; }
tstamp->byte_offset = sdev->stream_box.offset + ipc_params_reply.posn_offset; @@ -220,7 +222,10 @@ static int sof_compr_set_params(struct snd_soc_component *component,
spcm->prepared[cstream->direction] = true;
- return 0; +out: + kfree(pcm); + + return ret; }
static int sof_compr_get_params(struct snd_soc_component *component,
From: Daniel Baluta daniel.baluta@nxp.com
Allocate memory at the end of sof_ipc_stream_params to store snd_compr_params in order to be sent them to SOF firmware.
This will help firmware correctly configure codecs parameters.
Notice, that we use 2 bytes from the reserved pool in order to store the extended data length. This is compatible with older FWs where there was no extended data.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- include/sound/sof/stream.h | 6 ++++-- sound/soc/sof/compress.c | 17 +++++++++++++---- 2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/include/sound/sof/stream.h b/include/sound/sof/stream.h index 1db3bbc3e65d..9377113f13e4 100644 --- a/include/sound/sof/stream.h +++ b/include/sound/sof/stream.h @@ -86,9 +86,11 @@ struct sof_ipc_stream_params { uint32_t host_period_bytes; uint16_t no_stream_position; /**< 1 means don't send stream position */ uint8_t cont_update_posn; /**< 1 means continuous update stream position */ - - uint8_t reserved[5]; + uint8_t reserved0; + int16_t ext_data_length; /**< 0, means no extended data */ + uint8_t reserved[2]; uint16_t chmap[SOF_IPC_MAX_CHANNELS]; /**< channel map - SOF_CHMAP_ */ + uint8_t ext_data[]; /**< extended data */ } __packed;
/* PCM params info - SOF_IPC_STREAM_PCM_PARAMS */ diff --git a/sound/soc/sof/compress.c b/sound/soc/sof/compress.c index 45c2ff61ee4d..1204dce29ef9 100644 --- a/sound/soc/sof/compress.c +++ b/sound/soc/sof/compress.c @@ -170,6 +170,7 @@ static int sof_compr_set_params(struct snd_soc_component *component, struct snd_compr_tstamp *tstamp; struct sof_ipc_pcm_params *pcm; struct snd_sof_pcm *spcm; + size_t ext_data_size; int ret;
tstamp = crtd->private_data; @@ -179,7 +180,12 @@ static int sof_compr_set_params(struct snd_soc_component *component, if (!spcm) return -EINVAL;
- pcm = kzalloc(sizeof(*pcm), GFP_KERNEL); + ext_data_size = sizeof(params->codec); + + if (sizeof(*pcm) + ext_data_size > sdev->ipc->max_payload_size) + return -EINVAL; + + pcm = kzalloc(sizeof(*pcm) + ext_data_size, GFP_KERNEL); if (!pcm) return -ENOMEM;
@@ -194,11 +200,11 @@ static int sof_compr_set_params(struct snd_soc_component *component, goto out;
pcm->params.buffer.pages = PFN_UP(crtd->dma_bytes); - pcm->hdr.size = sizeof(*pcm); + pcm->hdr.size = sizeof(*pcm) + ext_data_size; pcm->hdr.cmd = SOF_IPC_GLB_STREAM_MSG | SOF_IPC_STREAM_PCM_PARAMS;
pcm->comp_id = spcm->stream[cstream->direction].comp_id; - pcm->params.hdr.size = sizeof(pcm->params); + pcm->params.hdr.size = sizeof(pcm->params) + ext_data_size; pcm->params.buffer.phy_addr = spcm->stream[cstream->direction].page_table.addr; pcm->params.buffer.size = crtd->dma_bytes; pcm->params.direction = cstream->direction; @@ -209,8 +215,11 @@ static int sof_compr_set_params(struct snd_soc_component *component, pcm->params.sample_container_bytes = snd_pcm_format_physical_width(SNDRV_PCM_FORMAT_S32) >> 3; pcm->params.host_period_bytes = params->buffer.fragment_size; + pcm->params.ext_data_length = ext_data_size; + + memcpy((u8 *)pcm->params.ext_data, ¶ms->codec, ext_data_size);
- ret = sof_ipc_tx_message(sdev->ipc, pcm, sizeof(*pcm), + ret = sof_ipc_tx_message(sdev->ipc, pcm, sizeof(*pcm) + ext_data_size, &ipc_params_reply, sizeof(ipc_params_reply)); if (ret < 0) { dev_err(component->dev, "error ipc failed\n");
From: Daniel Baluta daniel.baluta@nxp.com
After introducing extended parameters we need to forbid older firmware versions to run with the current and future kernel versions.
Although in theory the communication protocol will still work the semantics at application level are undefined. So, prevent this by disallowing older firmwares to run with newer kernels.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/compress.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/sound/soc/sof/compress.c b/sound/soc/sof/compress.c index 1204dce29ef9..67139e15f862 100644 --- a/sound/soc/sof/compress.c +++ b/sound/soc/sof/compress.c @@ -167,12 +167,23 @@ static int sof_compr_set_params(struct snd_soc_component *component, struct snd_soc_pcm_runtime *rtd = cstream->private_data; struct snd_compr_runtime *crtd = cstream->runtime; struct sof_ipc_pcm_params_reply ipc_params_reply; + struct sof_ipc_fw_ready *ready = &sdev->fw_ready; + struct sof_ipc_fw_version *v = &ready->version; struct snd_compr_tstamp *tstamp; struct sof_ipc_pcm_params *pcm; struct snd_sof_pcm *spcm; size_t ext_data_size; int ret;
+ if (v->abi_version < SOF_ABI_VER(3, 22, 0)) { + dev_err(component->dev, + "Compress params not supported with FW ABI version %d:%d:%d\n", + SOF_ABI_VERSION_MAJOR(v->abi_version), + SOF_ABI_VERSION_MINOR(v->abi_version), + SOF_ABI_VERSION_PATCH(v->abi_version)); + return -EINVAL; + } + tstamp = crtd->private_data;
spcm = snd_sof_find_spcm_dai(component, rtd);
From: Daniel Baluta daniel.baluta@nxp.com
Add new field to sof_ipc_stream_params in order to extend stream params struct with extended data to store compress parameters.
Older kernel will still work this as they ext_data_length will always be zero.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- include/uapi/sound/sof/abi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/sound/sof/abi.h b/include/uapi/sound/sof/abi.h index c88f467374ae..b7dce4df7ecd 100644 --- a/include/uapi/sound/sof/abi.h +++ b/include/uapi/sound/sof/abi.h @@ -28,7 +28,7 @@
/* SOF ABI version major, minor and patch numbers */ #define SOF_ABI_MAJOR 3 -#define SOF_ABI_MINOR 21 +#define SOF_ABI_MINOR 22 #define SOF_ABI_PATCH 0
/* SOF ABI version number. Format within 32bit word is MMmmmppp */
On Tue, 12 Jul 2022 17:15:27 +0300, Daniel Baluta wrote:
From: Daniel Baluta daniel.baluta@nxp.com
MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
We need a way to send extra parameters to DSP firmware. In order to do this, we introduce ext_data array at the end of ipc_stream_params.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: SOF: compress: Dynamically allocate pcm params struct commit: d5770daef62d2e4d33015089bab392ef867fd35a [2/4] ASoC: SOF: Copy compress parameters into extended data commit: 3f70c360d484466da7420f395d4675ca02436e32 [3/4] ASoC: SOF: compress: Prevent current kernel running with older FW commit: 246b135fcdba57a4e77a702580391ae1942c1e3b [4/4] uapi: sof: abi: Bump SOF ABI for ext_data_length commit: 75b5b7a1ccf606281c4afe365a57ccca486641a2
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Daniel Baluta -
Mark Brown