Merge series from Peter Ujfalusi <peter.ujfalusi@linux.intel.com>:
The series harmonizes the debug prints for pipeline state changes.
Currently we only print readable state change for single pipeline
changes but when multiple pipeline's state is changed, it is omitted.
Use human readable information in both cases in a harmonized way to aid
debugging.
All streams (currently) which is configured to use ChainDMA can only work
on Link/host DMA pairs where the link side position can be access via host
registers (like HDA on CAVS 2.5 platforms).
Since the firmware does not provide time_info for ChainDMA, unlike for HDA
stream, the kernel should calculate the start and end offsets that is
needed for the delay calculation.
With this small change we can report accurate delays when the stream is
configured to use ChainDMA.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://patch.msgid.link/20250619102848.12389-1-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Instead of just looking for a hw_config with matching rate only it sounds
better to try to find the best matching configuration.
If we have multiple hw_configurations with the same rate, but each with
different format for example then we have been picking the first config
with the matching rate, which can be a problem and it wil depend on how
the configs are ordered.
Instead we should be trying to find the best match out of the configs
1. rate + format + channels are matching
2. rate + format are matching
3. rate matching
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://patch.msgid.link/20250619102657.12109-1-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Harmonize sof_ipc4_set_pipeline_state() dbg print with the new print
in sof_ipc4_set_multi_pipeline_state().
Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://patch.msgid.link/20250619121121.25241-4-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Add a dev_dbg to sof_ipc4_set_multi_pipeline_state(). The debug print
lists the pipeline instance numbers that are included in the
SOF_IPC4_GLB_SET_PIPELINE_STATE message. Without this log its very
hard to tell what pipelines are affected. This print is very helpful
when analyzing SOF logs automatically.
Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://patch.msgid.link/20250619121121.25241-3-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The firmware does not provide any information for capture streams via the
shared pipeline registers.
To avoid reporting invalid delay value for capture streams to user space
we need to disable it.
Fixes: af74dbd0db ("ASoC: SOF: ipc4-pcm: allocate time info for pcm delay feature")
Cc: stable@vger.kernel.org
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Link: https://patch.msgid.link/20250509085951.15696-1-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
In preparation for making the kmalloc family of allocators type aware,
we need to make sure that the returned type from the allocation matches
the type of the variable being assigned. (Before, the allocator would
always return "void *", which can be implicitly cast to any pointer type.)
The assigned type is "struct snd_sof_pipeline **", but the returned type
will be "struct snd_sof_widget **". These are the same size allocation
(pointer size) but the types don't match. Adjust the allocation type to
match the assignment.
Signed-off-by: Kees Cook <kees@kernel.org>
Fixes: 9c04363d22 ("ASoC: SOF: Introduce struct snd_sof_pipeline")
Acked-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://patch.msgid.link/20250426062511.work.859-kees@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
Merge series from Peter Ujfalusi <peter.ujfalusi@linux.intel.com>:
Introduce new wrapper to present spcm related debug and error prints in
a unified way and provide additional details to help to understand the
reasons and configuration used when the log was captured.
Change the way we print information about the ipc4 copier module to
use type specific prints, again to provide better information for
debugging.
Instead of initializing the be_rate within the loop by checking i == 0 at
each iteration, move the be_rate reference initialization from the loop.
For BE single rate check we will have single comparison done at each
iteration compared to two in case the num_input_formats were higher than 1.
We still need to run the loop from index 0 to check for FE-BE rate match.
The patch also fixes bogus reports from gcc static analyzer thinking that
be_rate is used uninitialized later in the function (which was not true).
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://patch.msgid.link/20250206094914.21135-1-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Introduce spcm_dbg() and spcm_err() macros to provide consistent printing
for debug and error messages which includes usable information in the
print's prefix.
Update the prints in pcm.c, ipc3-pcm.c and ipc4-pcm.c to take advantage of
the features provided by the macros.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://patch.msgid.link/20250206092828.7569-4-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Fix an issue detected by the Smatch tool:
sound/soc/sof/ipc4-pcm.c: sof_ipc4_pcm_dai_link_fixup_rate()
error: uninitialized symbol 'be_rate'.
The warning highlights a case where `be_rate` could remain uninitialized
if `num_input_formats` is zero, which would cause undefined behavior
when setting `rate->min` and `rate->max` based on `be_rate`.
To address this issue, a `WARN_ON_ONCE(!num_input_formats)` check was
added to ensure `num_input_formats` is greater than zero. If this
condition fails, the function returns `-EINVAL`, preventing the use of
an uninitialized `be_rate`.
This change improves the robustness of the function by catching an
invalid state early and providing better feedback during development.
Signed-off-by: Suraj Sonawane <surajsonawane0215@gmail.com>
Link: https://patch.msgid.link/20241107063609.11627-1-surajsonawane0215@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Rename the sof_ipc4_copier_is_single_format() to
sof_ipc4_copier_is_single_bitdepth() to clear the confusion of the use of
'format' when we are querying information on the bit depth.
Format is used to describe a combination of parameters (rate, channels,
sample format / bit depth).
Rename the flags used to store the result at the same time.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://msgid.link/r/20240530111918.21974-4-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
For some reason a number of files included the "All rights reserved"
statement. Good old copy-paste made sure this mistake proliferated.
Remove the "All rights reserved" in all Intel-copyright to align with
internal guidance.
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20240503140359.259762-2-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The ChainDMA operation differs from normal pipelines that it is only
created when the stream started, in fact a PCM using ChainDMA has no
pipelines, modules.
To reset a ChainDMA, it needs to be first allocated in firmware. When
PulseAudio/PipeWire starts, they will probe the PCMs by opening them, check
hw_params and then close the PCM without starting audio.
Unconditionally resetting the ChainDMA can result the following error:
ipc tx : 0xe040000|0x0: GLB_CHAIN_DMA
ipc tx reply: 0x2e000007|0x0: GLB_CHAIN_DMA
FW reported error: 7 - Unsupported operation requested
ipc error for msg 0xe040000|0x0
sof_pcm_stream_free: pcm_ops hw_free failed -22
Add a new chain_dma_allocated flag to sof_ipc4_pcm_stream_priv to store the
ChainDMA allocation state and use this flag to skip sending the reset if
the ChainDMA is not allocated.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://msgid.link/r/20240409110036.9411-5-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Using the sof_ipc4_timestamp_info struct directly as sps->private data
is too restrictive, add a new generic sof_ipc4_pcm_stream_priv struct
containing the time_info to allow new information to be stored in a
generic way.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://msgid.link/r/20240409110036.9411-4-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The pointer to sof_ipc4_timestamp_info named most of the time as
'time_info' only to be named as 'stream_info' or 'info' in two function.
Use the consistent name of 'time_info' throughout the file.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://msgid.link/r/20240409110036.9411-3-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Throughout the file the pointer for snd_sof_pcm_stream is named either
'stream' or (wrongly) 'spcm' which confuses the reader.
Use 'sps' for the pointer name as it is the most common name used in SOF
codebase.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://msgid.link/r/20240409110036.9411-2-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
This patch improves the delay calculation by relying on the
LLP (Linear Link Position) on the DAI side and the
LDP (Linear Data Pointer) on the host side. The LDP provides the same DMA
position as LPIB, but with a linear count instead of a position in the
ALSA ring buffer. The LDP values are provided in bytes and must be
converted to frames. The difference in units means that the host counter
will wrap earlier than the LLP. We need to wrap the LLP at the same
boundary as the host counter.
The ASoC framework relies on separate pointer and delay callback.
Measurement errors can be reduced by processing all the counter values in
the pointer callback. The delay value is stored, and will be reported to
higher levels in the delay callback.
For playback, the firmware provides a stream_start offset to handle
mixing/pause usages, where the DAI might have started earlier than the
PCM device. The delay calculation must be special-cased when the link
counter has not reached the start offset value, i.e. no valid audio has
left the DSP.
Cc: stable@vger.kernel.org # 6.8
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://msgid.link/r/20240321130814.4412-16-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
When the final state is SOF_IPC4_PIPE_PAUSED, it is possible that the
stream will be restarted (resume or start) in which case we need to update
the offset from the firmware.
Cc: stable@vger.kernel.org # 6.8
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://msgid.link/r/20240321130814.4412-14-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The SNDRV_PCM_TRIGGER_PAUSE_PUSH does not need to be a separate case, it
can be handled along with STOP and SUSPEND
Cc: stable@vger.kernel.org # 6.8
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://msgid.link/r/20240321130814.4412-13-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The sof_ipc4_timestamp_info is only used by ipc4-pcm.c internally, it
should not be in a generic header implying that it might be used elsewhere.
Cc: stable@vger.kernel.org # 6.8
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://msgid.link/r/20240321130814.4412-12-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Switch to the new callback to retrieve the DAI (link) frame counter.
Cc: stable@vger.kernel.org # 6.8
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://msgid.link/r/20240321130814.4412-9-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Merge series from Cezary Rojewski <cezary.rojewski@intel.com>:
The avs-driver continues to be utilized on more recent Intel machines.
As TGL-based (cAVS 2.5) e.g.: RPL, inherit most of the functionality
from previous platforms:
SKL <- APL <- CNL <- ICL <- TGL
rather than putting everything into a single file, the platform-specific
bits are split into cnl/icl/tgl.c files instead. Makes the division clear
and code easier to maintain.
Layout of the patchset:
First are two changes combined together address the sound-clipping
problem, present when only one stream is running - specifically one
CAPTURE stream.
Follow up is naming-scheme adjustment for some of the existing functions
what improves code incohesiveness. As existing IPC/IRQ code operates
solely on cAVS 1.5 architecture, it needs no abstraction. The situation
changes when newer platforms come into the picture. Thus the next two
patches abstract the existing IPC/IRQ handlers so that majority of the
common code can be re-used.
The ICCMAX change stands out a bit - the AudioDSP firmware loading
procedure differs on ICL-based platforms (and onwards) and having a
separate commit makes the situation clear to the developers who are
going to support the solution from LTS perspective. For that reason
I decided not to merge it into the commit introducing the icl.c file.
The existing code uses (stream_tag - 1) for the host and link dma id.
This is correct for playback, but for capture this results in an
invalid dma_type being used. The firmware assumes that the dma_id for
capture is always larger than DAI_NUM_HDA_OUT
This patch adds the offset for num_playback_streams, filled on Intel
platforms with the value extracted from the hardware capabilities.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://msgid.link/r/20240213101247.28887-7-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
When the system is suspended while audio is active, the
sof_ipc4_pcm_hw_free() is invoked to reset the pipelines since during
suspend the DSP is turned off, streams will be re-started after resume.
If the firmware crashes during while audio is running (or when we reset
the stream before suspend) then the sof_ipc4_set_multi_pipeline_state()
will fail with IPC error and the state change is interrupted.
This will cause misalignment between the kernel and firmware state on next
DSP boot resulting errors returned by firmware for IPC messages, eventually
failing the audio resume.
On stream close the errors are ignored so the kernel state will be
corrected on the next DSP boot, so the second boot after the DSP panic.
If sof_ipc4_trigger_pipelines() is called from sof_ipc4_pcm_hw_free() then
state parameter is SOF_IPC4_PIPE_RESET and only in this case.
Treat a forced pipeline reset similarly to how we treat a pcm_free by
ignoring error on state sending to allow the kernel's state to be
consistent with the state the firmware will have after the next boot.
Link: https://github.com/thesofproject/sof/issues/8721
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://msgid.link/r/20240213115233.15716-1-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
LLP is supported by I2S and SDW platforms but not supported by HDA
platforms. This log is used to notify developer current LLP status
for current device. Since above case is known to developers, the log
is unnecessary and should be removed.
Fixes: 7cb19007ba ("ASoC: SOF: ipc4-pcm: add hw_params")
Signed-off-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://msgid.link/r/20240115092209.7184-2-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The pipeline priority is set in topology and driver should sort pipeline
based on priority for trigger order.
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20231006084454.19170-3-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
ASoC is now unified asoc_xxx() into snd_soc_xxx().
This patch convert asoc_xxx() to snd_soc_xxx().
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87sf71fo32.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
When a copier exposes a single format, we can fixup the BE dailink with
that format. This is helpful when some codec have format restrictions and
e.g. don't support a 32-bit format. In that case, the copier output
formats mirror that restriction in the topology file.
An alternate solution was suggested earlier using a dedicated topology
token. When specified, the token would be used to fix-up the dailink. The
main reason why this solution was chosen is that there is a risk of a
disconnect between token definition and copier format. With a single piece
of information as suggested in this patch, there are fewer risks of a bad
configuration.
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230915093507.7242-3-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The call to snd_sof_find_spcm_dai() could return NULL,
add nullable check for the return value to avoid null
pointer defenrece.
Fixes: 7cb19007ba ("ASoC: SOF: ipc4-pcm: add hw_params")
Signed-off-by: Chao Song <chao.song@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230816133311.7523-1-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
A pipeline is identified by two indices: 'instance_id' and 'pipeline_id'
This is clearly seen in kernel logs when creating a pipeline
"Create widget pipeline.20 instance 0 - pipe 20 - core 0"
but other logs are less clear
"ipc4 set pipeline 1 state 4"
Change definitions and logs to make sure the logs clearly identify
which of the two indices are used in state transitions.
No functional change.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/20230807210959.506849-20-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Do not reset pipelines during the stop/suspend triggers in the BE DAI
ops as the BE DAI pipeline needs to be left in the PAUSED state. It should
only be reset during hw_free. This simplification is already done for
the FE pipelines and the DAI trigger only toggles the states between
PAUSED and RUNNING.
But because the FE DAI hw_free is invoked first and all the pipelines are
freed during this op, we need to make sure that the BE DAI pipeline also
gets reset before it is freed. So do not skip the pipelines that have the
skip_during_fe_trigger flag set when resetting pipelines.
Also, because the pipeline state changes are split between the FE and BE
DAI ops now, protect the BE DAI pipeline state changes with the
pipeline_state_mutex as well.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com
Link: https://lore.kernel.org/r/20230515112022.30297-3-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org
Merge series from Peter Ujfalusi <peter.ujfalusi@linux.intel.com>:
Last minute patch for correct the pasue/resume operation with IPC4. The
issues are hardto reproduce and needs extended stress testing to be hit,
in which case the audio breaks due to DMA errors.
In the case of IPC4, a pipeline is only paused during STOP/PAUSE/SUSPEND
triggers and the FW keeps the host DMA running when a pipeline is
paused. The start/stop tests iterate through STOP/START triggers without
involving a hw_free. This means that the pipeline state will only toggle
between PAUSED (during the STOP trigger) and RUNNING (during the START
trigger). So this test should be treated in the same way as a
PAUSE_PUSH/PAUSE_RELEASE test and the DMA should be kept running when
toggling the pipeline states between PAUSED and RUNNING.
Since there is no way to tell if a STOP trigger will be followed by hw_free
or not, this patch proposes to always skip DMA stop during the STOP trigger
and handle it later during hw_free. Introduce a new flag in struct
sof_ipc_pcm_ops, delayed_platform_trigger, that will be used to ensure that
the host DMA will not be stopped during the STOP/PAUSE/RELEASE triggers
and set it for IPC4. The platform_trigger call to stop the DMA will be
invoked during PCM hw_free instead when the pipeline is reset.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230420114137.27613-3-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Convert all existing calls that pass "NULL, 0" for reply data to the new
no_reply calls. Also convert any calls that pass in data but don't
actually parse the result.
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/20230419194057.42205-3-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The recommended sequence for triggering the host DMA is to first program
the DMA in the FW before setting the RUN bit to start the stream in the
host. With IPC3, this sequence is honored because the FW programs the
DMA when the HW_PARAMS IPC is sent during PCM hw_params and then the host
sets the RUN bit during sof_pcm_trigger(). But with IPC4,
sof_pcm_trigger() sends the SET_PIPELINE_STATE IPC to program the DMA in
the FW after the DMA RUN bit is set.
In order to minimize the impact for IPC3, introduce a new flag as part
of struct sof_ipc_pcm_ops, ipc_first_on_start, which will be set for IPC4
only. With this flag set, the SET_PIPELINE_STATE IPC will be sent before
the DMA RUN bit is set by the host during the START/PAUSE_RELEASE
triggers.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230322094346.6019-4-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Add logic for setting up and tearing down chained DMA connections.
Since pipelines are not used, all the logic to set the pipeline states
can be bypassed, with only the DMA programming sequences remaining. In
addition the same format needs to be used for host- and link-DMA,
without the usual fixup to use the S32_LE format on the link.
Note however that for convenience and compatibility with existing
definitions, the topology relies on the concept of pipelines with a
'USE_CHAIN_DMA' token indicating that all the logic shall be bypassed.
Unlike 'normal' ALSA sequences, the chain DMA is not programmed in
hw_params/hw_free. The IPC message to set-up and tear-down chained DMA
are sent in sof_ipc4_trigger_pipelines(), but the contents prepared
earlier.
Chained DMA is only supported by the Intel HDA DAI for now, and only
S16_LE and S32_LE formats are supported for now.
Signed-off-by: Jyri Sarha <jyri.sarha@intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230321092654.7292-4-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Backend DAIs may support multiple audio formats. Modify pipeline
setup to select a suitable configuration based on topology and
frontend DAI runtime configuration.
For sampling rate, if one of the BE DAI configurations has
a sampling rate matching that of FE DAI, configure BE DAI to
this rate.
For sample format, the current code hardcodes DAI copier sample format
to 32bit for both playback and capture pipelines. This is not always
desired, so lift the limitation and set the sample format based on
topology definitions for the copiers. For capture pipelines, we want to
set the BE DAI pipeline format based on topology instead of using the FE
DAI format. This covers the common use-case where BE DAI outputs data at
a higher sample precision and sample width is reduced later in the
pipeline. Instead of hardcoding to 32bit, use the BE DAI copier output
format defined in topology.
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230316141458.13940-2-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Introduce a new struct sof_ipc4_pin_format which contains the pin index
and the buffer size. Replace the type of available input/output audio
formats in struct sof_ipc4_available_audio_format with this new struct
type and rename them to input_pin_fmts and output_pin_fmts.
Also, add a new token, SOF_TKN_CAVS_AUDIO_FORMAT_PIN_INDEX that will be
used to parse the pin index for the audio format from topology.
Currently we only set the audio format for Pin 0 in topology, so the
default value will be 0 for all audio formats.
Finally, parse the pin_index and the input/output buffer sizes
along with audio formats into the pin_format arrays in struct
sof_ipc4_available_audio_format. This makes the base_config array in struct
sof_ipc4_available_audio_format redundant. So remove it. This change
will allow the addition of audio formats for the non-zero pins in
topology transparent to the topology parser in the kernel.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230313124856.8140-8-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Add a new field, input_audio_fmts, in struct
sof_ipc4_available_audio_format and parse all the available input audio
formats into this new field and not into the base_config field. This is
preparation to remove the base_config array from the struct
sof_ipc4_available_audio_format.
This simplifies the sof_ipc4_init_audio_fmt()
function as the reference audio format for matching with input params
has the same size.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230313124856.8140-4-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The delay function is used to calculate the difference
between hw_ptr and dai dma position. I2S, DMIC and SDW will
use dai dma position in shared SRAM window to calculate the
delay. HDaudio will retrieve dai dma position from host mmio memory
space since it doesn't support LLP counter reported by firmware.
In two cases dai dma position is inaccurate for delay calculation
(1) dai pipeline is started before host pipeline
(2) multiple streams mixed into one. Each stream has the same dai
dma position
Firmware calculates correct stream_start_offset for all cases including
above two. Driver subtracts stream_start_offset from dai dma position to
get accurate one.
Signed-off-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230202132954.26773-10-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Use the hw_params to init time info for ipc4 delay calculation.
Signed-off-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230202132954.26773-6-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Allocate time info when pcm is loaded by topology
and free it when pcm is unloaded by topology.
Signed-off-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230202132954.26773-5-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
If the pipeline setup fails at the first widget/pipeline then we will have
no spipe stored under the pipeline_list->pipelines, the
pipeline_list->count is 0.
If this is the case we would have a NULL pointer dereference if the
execution is allowed to proceed.
Check for this condition along with the pipeline_list->pipelines check
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://lore.kernel.org/r/20230127120031.10709-19-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
When starting/stopping multiple streams in parallel, pipeline triggers
and pipeline frees can get interleaved. So use the same mutex used for
pipeline trigger to protect the pipeline frees as well. Rename the
trigger_mutex to pipeline_state_mutex for more clarity.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230127120031.10709-16-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Use the started_count and paused_count to implement reference counting
when making decisions to start/stop/pause pipelines during the FE DAI
trigger. This is necessary to trigger the shared pipelines in the FE DAI
trigger properly.
With IPC4, the FE trigger will issue multiple pipeline state changes,
and the triggers are propagated downstream to connected pipelines by
the SOF driver - not the firmware. This creates a window for race
conditions where an FE trigger preempts another one, which results
in inconsistent pipeline states and refcounts.
This patch introduces a mutex lock for the pcm trigger that guarantees
that IPC4 state and resources are accessed in a serialized manner.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230127120031.10709-15-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
For more clarity, rename the struct ipc4_pipeline_set_state_data
variable to trigger_list instead of data. No functionality change.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230127120031.10709-14-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Introduce struct snd_sof_pipeline to save the information about
pipelines including the pipeline widget, their status wrt how many PCM's
are using them and whether they are complete or not.
In struct snd_sof_widget, replace pipe_widget with spipe and remove
complete. In struct snd_sof_pcm_stream_pipeline_list, replace
pipe_widgets with pipelines.
Update all users accordingly.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20230127120031.10709-13-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
