From fb9f8125ed9d9b8e11f309a7dbfbe7b40de48fba Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:07:58 +0200 Subject: ASoC: SOF: Add dsp_max_burst_size_in_ms member to snd_sof_pcm_stream The dsp_max_burst_size_in_ms can be used to save the length of the maximum burst size in ms the host DMA will use. Platform code can place constraint using this to avoid user space requesting too small ALSA buffer which will result xruns. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-2-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/sof-audio.h | 1 + 1 file changed, 1 insertion(+) (limited to 'sound') diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 9ea2ac5adac7..04e5cb2c70a7 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -322,6 +322,7 @@ struct snd_sof_pcm_stream { struct work_struct period_elapsed_work; struct snd_soc_dapm_widget_list *list; /* list of connected DAPM widgets */ bool d0i3_compatible; /* DSP can be in D0I3 when this pcm is opened */ + unsigned int dsp_max_burst_size_in_ms; /* The maximum size of the host DMA burst in ms */ /* * flag to indicate that the DSP pipelines should be kept * active or not while suspending the stream -- cgit v1.2.3 From 842bb8b62cc6f3546d61eb63115b32ebc6dd4a87 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:07:59 +0200 Subject: ASoC: SOF: ipc4-topology: Save the DMA maximum burst size for PCMs When setting up the pcm widget, save the DSP buffer size (in ms) for platform code to place a constraint on playback. On playback the DMA will fill the buffer on start and if the period size is smaller it will immediately overrun. On capture the DMA will move data in 1ms bursts. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-3-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4-topology.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'sound') diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index da4a83afb87a..bb4cf6dd1e18 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -412,8 +412,9 @@ static int sof_ipc4_widget_setup_pcm(struct snd_sof_widget *swidget) struct sof_ipc4_available_audio_format *available_fmt; struct snd_soc_component *scomp = swidget->scomp; struct sof_ipc4_copier *ipc4_copier; + struct snd_sof_pcm *spcm; int node_type = 0; - int ret; + int ret, dir; ipc4_copier = kzalloc(sizeof(*ipc4_copier), GFP_KERNEL); if (!ipc4_copier) @@ -447,6 +448,25 @@ static int sof_ipc4_widget_setup_pcm(struct snd_sof_widget *swidget) } dev_dbg(scomp->dev, "host copier '%s' node_type %u\n", swidget->widget->name, node_type); + spcm = snd_sof_find_spcm_comp(scomp, swidget->comp_id, &dir); + if (!spcm) + goto skip_gtw_cfg; + + if (dir == SNDRV_PCM_STREAM_PLAYBACK) { + struct snd_sof_pcm_stream *sps = &spcm->stream[dir]; + + sof_update_ipc_object(scomp, &sps->dsp_max_burst_size_in_ms, + SOF_COPIER_DEEP_BUFFER_TOKENS, + swidget->tuples, + swidget->num_tuples, sizeof(u32), 1); + /* Set default DMA buffer size if it is not specified in topology */ + if (!sps->dsp_max_burst_size_in_ms) + sps->dsp_max_burst_size_in_ms = SOF_IPC4_MIN_DMA_BUFFER_SIZE; + } else { + /* Capture data is copied from DSP to host in 1ms bursts */ + spcm->stream[dir].dsp_max_burst_size_in_ms = 1; + } + skip_gtw_cfg: ipc4_copier->gtw_attr = kzalloc(sizeof(*ipc4_copier->gtw_attr), GFP_KERNEL); if (!ipc4_copier->gtw_attr) { -- cgit v1.2.3 From fe76d2e75a6da97edd2b4ec5cfb9efd541be087a Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:00 +0200 Subject: ASoC: SOF: Intel: hda-pcm: Use dsp_max_burst_size_in_ms to place constraint If the PCM have the dsp_max_burst_size_in_ms set then place a constraint to limit the minimum buffer time to avoid xruns caused by DMA bursts spinning on the ALSA buffer. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-4-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-pcm.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'sound') diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c index 18f07364d219..69fefcd1abc5 100644 --- a/sound/soc/sof/intel/hda-pcm.c +++ b/sound/soc/sof/intel/hda-pcm.c @@ -259,6 +259,27 @@ int hda_dsp_pcm_open(struct snd_sof_dev *sdev, snd_pcm_hw_constraint_mask64(substream->runtime, SNDRV_PCM_HW_PARAM_FORMAT, SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_S32); + /* + * The dsp_max_burst_size_in_ms is the length of the maximum burst size + * of the host DMA in the ALSA buffer. + * + * On playback start the DMA will transfer dsp_max_burst_size_in_ms + * amount of data in one initial burst to fill up the host DMA buffer. + * Consequent DMA burst sizes are shorter and their length can vary. + * To make sure that userspace allocate large enough ALSA buffer we need + * to place a constraint on the buffer time. + * + * On capture the DMA will transfer 1ms chunks. + * + * Exact dsp_max_burst_size_in_ms constraint is racy, so set the + * constraint to a minimum of 2x dsp_max_burst_size_in_ms. + */ + if (spcm->stream[direction].dsp_max_burst_size_in_ms) + snd_pcm_hw_constraint_minmax(substream->runtime, + SNDRV_PCM_HW_PARAM_BUFFER_TIME, + spcm->stream[direction].dsp_max_burst_size_in_ms * USEC_PER_MSEC * 2, + UINT_MAX); + /* binding pcm substream to hda stream */ substream->runtime->private_data = &dsp_stream->hstream; return 0; -- cgit v1.2.3 From 67b182bea08a8d1092b91b57aefdfe420fce1634 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:01 +0200 Subject: ASoC: SOF: Intel: hda: Implement get_stream_position (Linear Link Position) When the Linear Link Position is not available in firmware SRAM window we use the host accessible position registers to read it. The address of the PPLCLLPL/U registers depend on the number of streams (playback+capture). At probe time the pplc_addr is calculated for each stream and we can use it to read the LLP without the need of address re-calculation. Set the get_stream_position callback in sof_hda_common_ops for all platforms: The callback is used for IPC4 delay calculations only but the register is a generic HDA register, not tied to any specific IPC version. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Rander Wang Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-5-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-common-ops.c | 2 ++ sound/soc/sof/intel/hda-stream.c | 32 ++++++++++++++++++++++++++++++++ sound/soc/sof/intel/hda.h | 3 +++ 3 files changed, 37 insertions(+) (limited to 'sound') diff --git a/sound/soc/sof/intel/hda-common-ops.c b/sound/soc/sof/intel/hda-common-ops.c index 2b385cddc385..80a69599a8c3 100644 --- a/sound/soc/sof/intel/hda-common-ops.c +++ b/sound/soc/sof/intel/hda-common-ops.c @@ -57,6 +57,8 @@ struct snd_sof_dsp_ops sof_hda_common_ops = { .pcm_pointer = hda_dsp_pcm_pointer, .pcm_ack = hda_dsp_pcm_ack, + .get_stream_position = hda_dsp_get_stream_llp, + /* firmware loading */ .load_firmware = snd_sof_load_firmware_raw, diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index b387b1a69d7e..48ea187f7230 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -1063,3 +1063,35 @@ snd_pcm_uframes_t hda_dsp_stream_get_position(struct hdac_stream *hstream, return pos; } + +/** + * hda_dsp_get_stream_llp - Retrieve the LLP (Linear Link Position) of the stream + * @sdev: SOF device + * @component: ASoC component + * @substream: PCM substream + * + * Returns the raw Linear Link Position value + */ +u64 hda_dsp_get_stream_llp(struct snd_sof_dev *sdev, + struct snd_soc_component *component, + struct snd_pcm_substream *substream) +{ + struct hdac_stream *hstream = substream->runtime->private_data; + struct hdac_ext_stream *hext_stream = stream_to_hdac_ext_stream(hstream); + u32 llp_l, llp_u; + + /* + * The pplc_addr have been calculated during probe in + * hda_dsp_stream_init(): + * pplc_addr = sdev->bar[HDA_DSP_PP_BAR] + + * SOF_HDA_PPLC_BASE + + * SOF_HDA_PPLC_MULTI * total_stream + + * SOF_HDA_PPLC_INTERVAL * stream_index + * + * Use this pre-calculated address to avoid repeated re-calculation. + */ + llp_l = readl(hext_stream->pplc_addr + AZX_REG_PPLCLLPL); + llp_u = readl(hext_stream->pplc_addr + AZX_REG_PPLCLLPU); + + return ((u64)llp_u << 32) | llp_l; +} diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index b36eb7c78913..9d26cad785fe 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -662,6 +662,9 @@ bool hda_dsp_check_stream_irq(struct snd_sof_dev *sdev); snd_pcm_uframes_t hda_dsp_stream_get_position(struct hdac_stream *hstream, int direction, bool can_sleep); +u64 hda_dsp_get_stream_llp(struct snd_sof_dev *sdev, + struct snd_soc_component *component, + struct snd_pcm_substream *substream); struct hdac_ext_stream * hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction, u32 flags); -- cgit v1.2.3 From 4374f698d7d9f849b66f3fa8f7a64f0bc1a53d7f Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:02 +0200 Subject: ASoC: SOF: Intel: mtl/lnl: Use the generic get_stream_position callback Drop the MTL mtl_dsp_get_stream_hda_link_position() function and related defines since it can only work on platforms which have 19 streams because of the use of 0x948 as base offset for the LLP registers. The generic hda_dsp_get_stream_hda_link_position() takes the number of streams into consideration when reading the LLP registers for the stream and can handle different HDA configurations. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Rander Wang Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-6-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/lnl.c | 2 -- sound/soc/sof/intel/mtl.c | 14 -------------- sound/soc/sof/intel/mtl.h | 10 ---------- 3 files changed, 26 deletions(-) (limited to 'sound') diff --git a/sound/soc/sof/intel/lnl.c b/sound/soc/sof/intel/lnl.c index 7ae017a00184..d1c73d407e68 100644 --- a/sound/soc/sof/intel/lnl.c +++ b/sound/soc/sof/intel/lnl.c @@ -134,8 +134,6 @@ int sof_lnl_ops_init(struct snd_sof_dev *sdev) sof_lnl_ops.runtime_resume = lnl_hda_dsp_runtime_resume; } - sof_lnl_ops.get_stream_position = mtl_dsp_get_stream_hda_link_position; - /* dsp core get/put */ sof_lnl_ops.core_get = mtl_dsp_core_get; sof_lnl_ops.core_put = mtl_dsp_core_put; diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c index df05dc77b8d5..060c34988e90 100644 --- a/sound/soc/sof/intel/mtl.c +++ b/sound/soc/sof/intel/mtl.c @@ -626,18 +626,6 @@ static int mtl_dsp_disable_interrupts(struct snd_sof_dev *sdev) return mtl_enable_interrupts(sdev, false); } -u64 mtl_dsp_get_stream_hda_link_position(struct snd_sof_dev *sdev, - struct snd_soc_component *component, - struct snd_pcm_substream *substream) -{ - struct hdac_stream *hstream = substream->runtime->private_data; - u32 llp_l, llp_u; - - llp_l = snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR, MTL_PPLCLLPL(hstream->index)); - llp_u = snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR, MTL_PPLCLLPU(hstream->index)); - return ((u64)llp_u << 32) | llp_l; -} - int mtl_dsp_core_get(struct snd_sof_dev *sdev, int core) { const struct sof_ipc_pm_ops *pm_ops = sdev->ipc->ops->pm; @@ -707,8 +695,6 @@ int sof_mtl_ops_init(struct snd_sof_dev *sdev) sof_mtl_ops.core_get = mtl_dsp_core_get; sof_mtl_ops.core_put = mtl_dsp_core_put; - sof_mtl_ops.get_stream_position = mtl_dsp_get_stream_hda_link_position; - sdev->private = kzalloc(sizeof(struct sof_ipc4_fw_data), GFP_KERNEL); if (!sdev->private) return -ENOMEM; diff --git a/sound/soc/sof/intel/mtl.h b/sound/soc/sof/intel/mtl.h index cc5a1f46fd09..ea8c1b83f712 100644 --- a/sound/soc/sof/intel/mtl.h +++ b/sound/soc/sof/intel/mtl.h @@ -6,12 +6,6 @@ * Copyright(c) 2020-2022 Intel Corporation. All rights reserved. */ -/* HDA Registers */ -#define MTL_PPLCLLPL_BASE 0x948 -#define MTL_PPLCLLPU_STRIDE 0x10 -#define MTL_PPLCLLPL(x) (MTL_PPLCLLPL_BASE + (x) * MTL_PPLCLLPU_STRIDE) -#define MTL_PPLCLLPU(x) (MTL_PPLCLLPL_BASE + 0x4 + (x) * MTL_PPLCLLPU_STRIDE) - /* DSP Registers */ #define MTL_HFDSSCS 0x1000 #define MTL_HFDSSCS_SPA_MASK BIT(16) @@ -103,9 +97,5 @@ int mtl_dsp_ipc_get_window_offset(struct snd_sof_dev *sdev, u32 id); void mtl_ipc_dump(struct snd_sof_dev *sdev); -u64 mtl_dsp_get_stream_hda_link_position(struct snd_sof_dev *sdev, - struct snd_soc_component *component, - struct snd_pcm_substream *substream); - int mtl_dsp_core_get(struct snd_sof_dev *sdev, int core); int mtl_dsp_core_put(struct snd_sof_dev *sdev, int core); -- cgit v1.2.3 From ce2faa9a180c1984225689b6b1cb26045f8b7470 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:03 +0200 Subject: ASoC: SOF: Introduce a new callback pair to be used for PCM delay reporting For delay calculation we need two information: Number of bytes transferred between the DSP and host memory (ALSA buffer) Number of frames transferred between the DSP and external device (link/codec/DMIC/etc). The reason for the different units (bytes vs frames) on host and dai side is that the format on the dai side is decided by the firmware and might not be the same as on the host side, thus the expectation is that the counter reflects the number of frames. The kernel know the host side format and in there we have access to the DMA position which is in bytes. In a simplified way, the DSP caused delay is the difference between the two counters. The existing get_stream_position callback is defined to retrieve the frame counter on the DAI side but it's name is too generic to be intuitive and makes it hard to define a callback for the host side. This patch introduces a new set of callbacks to replace the get_stream_position and define the host side equivalent: get_dai_frame_counter get_host_byte_counter Subsequent patches will remove the old callback. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-7-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ops.h | 24 ++++++++++++++++++++++++ sound/soc/sof/sof-priv.h | 21 +++++++++++++++++++++ 2 files changed, 45 insertions(+) (limited to 'sound') diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h index 6cf21e829e07..d83cd771015c 100644 --- a/sound/soc/sof/ops.h +++ b/sound/soc/sof/ops.h @@ -533,6 +533,30 @@ static inline u64 snd_sof_pcm_get_stream_position(struct snd_sof_dev *sdev, return 0; } +static inline u64 +snd_sof_pcm_get_dai_frame_counter(struct snd_sof_dev *sdev, + struct snd_soc_component *component, + struct snd_pcm_substream *substream) +{ + if (sof_ops(sdev) && sof_ops(sdev)->get_dai_frame_counter) + return sof_ops(sdev)->get_dai_frame_counter(sdev, component, + substream); + + return 0; +} + +static inline u64 +snd_sof_pcm_get_host_byte_counter(struct snd_sof_dev *sdev, + struct snd_soc_component *component, + struct snd_pcm_substream *substream) +{ + if (sof_ops(sdev) && sof_ops(sdev)->get_host_byte_counter) + return sof_ops(sdev)->get_host_byte_counter(sdev, component, + substream); + + return 0; +} + /* machine driver */ static inline int snd_sof_machine_register(struct snd_sof_dev *sdev, void *pdata) diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index d453a4ce3b21..91043f177dfa 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -270,6 +270,27 @@ struct snd_sof_dsp_ops { struct snd_soc_component *component, struct snd_pcm_substream *substream); /* optional */ + /* + * optional callback to retrieve the number of frames left/arrived from/to + * the DSP on the DAI side (link/codec/DMIC/etc). + * + * The callback is used when the firmware does not provide this information + * via the shared SRAM window and it can be retrieved by host. + */ + u64 (*get_dai_frame_counter)(struct snd_sof_dev *sdev, + struct snd_soc_component *component, + struct snd_pcm_substream *substream); /* optional */ + + /* + * Optional callback to retrieve the number of bytes left/arrived from/to + * the DSP on the host side (bytes between host ALSA buffer and DSP). + * + * The callback is needed for ALSA delay reporting. + */ + u64 (*get_host_byte_counter)(struct snd_sof_dev *sdev, + struct snd_soc_component *component, + struct snd_pcm_substream *substream); /* optional */ + /* host read DSP stream data */ int (*ipc_msg_data)(struct snd_sof_dev *sdev, struct snd_sof_pcm_stream *sps, -- cgit v1.2.3 From fd6f6a0632bc891673490bf4a92304172251825c Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:04 +0200 Subject: ASoC: SOF: Intel: Set the dai/host get frame/byte counter callbacks Add implementation for reading the LDP (Linear DMA Position) to be used as get_host_byte_counter(). The LDP is counting the number of bytes moved between the DSP and host memory. Set the get_dai_frame_counter to hda_dsp_get_stream_llp, which is counting the frames on the link side of the DSP. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-8-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-common-ops.c | 2 ++ sound/soc/sof/intel/hda-stream.c | 31 +++++++++++++++++++++++++++++++ sound/soc/sof/intel/hda.h | 3 +++ 3 files changed, 36 insertions(+) (limited to 'sound') diff --git a/sound/soc/sof/intel/hda-common-ops.c b/sound/soc/sof/intel/hda-common-ops.c index 80a69599a8c3..4d7ea18604ee 100644 --- a/sound/soc/sof/intel/hda-common-ops.c +++ b/sound/soc/sof/intel/hda-common-ops.c @@ -58,6 +58,8 @@ struct snd_sof_dsp_ops sof_hda_common_ops = { .pcm_ack = hda_dsp_pcm_ack, .get_stream_position = hda_dsp_get_stream_llp, + .get_dai_frame_counter = hda_dsp_get_stream_llp, + .get_host_byte_counter = hda_dsp_get_stream_ldp, /* firmware loading */ .load_firmware = snd_sof_load_firmware_raw, diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 48ea187f7230..8504a4f27b60 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -1095,3 +1095,34 @@ u64 hda_dsp_get_stream_llp(struct snd_sof_dev *sdev, return ((u64)llp_u << 32) | llp_l; } + +/** + * hda_dsp_get_stream_ldp - Retrieve the LDP (Linear DMA Position) of the stream + * @sdev: SOF device + * @component: ASoC component + * @substream: PCM substream + * + * Returns the raw Linear Link Position value + */ +u64 hda_dsp_get_stream_ldp(struct snd_sof_dev *sdev, + struct snd_soc_component *component, + struct snd_pcm_substream *substream) +{ + struct hdac_stream *hstream = substream->runtime->private_data; + struct hdac_ext_stream *hext_stream = stream_to_hdac_ext_stream(hstream); + u32 ldp_l, ldp_u; + + /* + * The pphc_addr have been calculated during probe in + * hda_dsp_stream_init(): + * pphc_addr = sdev->bar[HDA_DSP_PP_BAR] + + * SOF_HDA_PPHC_BASE + + * SOF_HDA_PPHC_INTERVAL * stream_index + * + * Use this pre-calculated address to avoid repeated re-calculation. + */ + ldp_l = readl(hext_stream->pphc_addr + AZX_REG_PPHCLDPL); + ldp_u = readl(hext_stream->pphc_addr + AZX_REG_PPHCLDPU); + + return ((u64)ldp_u << 32) | ldp_l; +} diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 9d26cad785fe..81a1d4606d3c 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -665,6 +665,9 @@ snd_pcm_uframes_t hda_dsp_stream_get_position(struct hdac_stream *hstream, u64 hda_dsp_get_stream_llp(struct snd_sof_dev *sdev, struct snd_soc_component *component, struct snd_pcm_substream *substream); +u64 hda_dsp_get_stream_ldp(struct snd_sof_dev *sdev, + struct snd_soc_component *component, + struct snd_pcm_substream *substream); struct hdac_ext_stream * hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction, u32 flags); -- cgit v1.2.3 From 37679a1bd372c8308a3faccf3438c9df642565b3 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:05 +0200 Subject: ASoC: SOF: ipc4-pcm: Use the snd_sof_pcm_get_dai_frame_counter() for pcm_delay Switch to the new callback to retrieve the DAI (link) frame counter. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-9-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4-pcm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'sound') diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index 0f332c8cdbe6..d0795f77cc15 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -897,11 +897,12 @@ static snd_pcm_sframes_t sof_ipc4_pcm_delay(struct snd_soc_component *component, } /* - * HDaudio links don't support the LLP counter reported by firmware - * the link position is read directly from hardware registers. + * If the LLP counter is not reported by firmware in the SRAM window + * then read the dai (link) position via host accessible means if + * available. */ if (!time_info->llp_offset) { - tmp_ptr = snd_sof_pcm_get_stream_position(sdev, component, substream); + tmp_ptr = snd_sof_pcm_get_dai_frame_counter(sdev, component, substream); if (!tmp_ptr) return 0; } else { -- cgit v1.2.3 From 4ab6c38c664442c1fc9911eb3c5c6953d3dbcca5 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:06 +0200 Subject: ASoC: SOF: Intel: hda-common-ops: Do not set the get_stream_position callback The get_stream_position has been replaced by get_dai_frame_counter, it should not be set to allow it to be dropped from core code. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-10-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-common-ops.c | 1 - 1 file changed, 1 deletion(-) (limited to 'sound') diff --git a/sound/soc/sof/intel/hda-common-ops.c b/sound/soc/sof/intel/hda-common-ops.c index 4d7ea18604ee..d71bb66b9991 100644 --- a/sound/soc/sof/intel/hda-common-ops.c +++ b/sound/soc/sof/intel/hda-common-ops.c @@ -57,7 +57,6 @@ struct snd_sof_dsp_ops sof_hda_common_ops = { .pcm_pointer = hda_dsp_pcm_pointer, .pcm_ack = hda_dsp_pcm_ack, - .get_stream_position = hda_dsp_get_stream_llp, .get_dai_frame_counter = hda_dsp_get_stream_llp, .get_host_byte_counter = hda_dsp_get_stream_ldp, -- cgit v1.2.3 From 07007b8ac42cffc23043d00e56b0f67a75dc4b22 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:07 +0200 Subject: ASoC: SOF: Remove the get_stream_position callback The get_stream_position has been replaced by get_dai_frame_counter and all related code can be dropped form the core. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-11-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ops.h | 10 ---------- sound/soc/sof/sof-priv.h | 9 --------- 2 files changed, 19 deletions(-) (limited to 'sound') diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h index d83cd771015c..3cd748e13460 100644 --- a/sound/soc/sof/ops.h +++ b/sound/soc/sof/ops.h @@ -523,16 +523,6 @@ static inline int snd_sof_pcm_platform_ack(struct snd_sof_dev *sdev, return 0; } -static inline u64 snd_sof_pcm_get_stream_position(struct snd_sof_dev *sdev, - struct snd_soc_component *component, - struct snd_pcm_substream *substream) -{ - if (sof_ops(sdev) && sof_ops(sdev)->get_stream_position) - return sof_ops(sdev)->get_stream_position(sdev, component, substream); - - return 0; -} - static inline u64 snd_sof_pcm_get_dai_frame_counter(struct snd_sof_dev *sdev, struct snd_soc_component *component, diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 91043f177dfa..d3c436f82604 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -261,15 +261,6 @@ struct snd_sof_dsp_ops { /* pcm ack */ int (*pcm_ack)(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream); /* optional */ - /* - * optional callback to retrieve the link DMA position for the substream - * when the position is not reported in the shared SRAM windows but - * instead from a host-accessible hardware counter. - */ - u64 (*get_stream_position)(struct snd_sof_dev *sdev, - struct snd_soc_component *component, - struct snd_pcm_substream *substream); /* optional */ - /* * optional callback to retrieve the number of frames left/arrived from/to * the DSP on the DAI side (link/codec/DMIC/etc). -- cgit v1.2.3 From 31d2874d083ba6cc2a4f4b26dab73c3be1c92658 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:08 +0200 Subject: ASoC: SOF: ipc4-pcm: Move struct sof_ipc4_timestamp_info definition locally 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 Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-12-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4-pcm.c | 14 ++++++++++++++ sound/soc/sof/ipc4-priv.h | 14 -------------- 2 files changed, 14 insertions(+), 14 deletions(-) (limited to 'sound') diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index d0795f77cc15..2d7295221884 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -15,6 +15,20 @@ #include "ipc4-topology.h" #include "ipc4-fw-reg.h" +/** + * struct sof_ipc4_timestamp_info - IPC4 timestamp info + * @host_copier: the host copier of the pcm stream + * @dai_copier: the dai copier of the pcm stream + * @stream_start_offset: reported by fw in memory window + * @llp_offset: llp offset in memory window + */ +struct sof_ipc4_timestamp_info { + struct sof_ipc4_copier *host_copier; + struct sof_ipc4_copier *dai_copier; + u64 stream_start_offset; + u32 llp_offset; +}; + static int sof_ipc4_set_multi_pipeline_state(struct snd_sof_dev *sdev, u32 state, struct ipc4_pipeline_set_state_data *trigger_list) { diff --git a/sound/soc/sof/ipc4-priv.h b/sound/soc/sof/ipc4-priv.h index f3b908b093f9..afed618a15f0 100644 --- a/sound/soc/sof/ipc4-priv.h +++ b/sound/soc/sof/ipc4-priv.h @@ -92,20 +92,6 @@ struct sof_ipc4_fw_data { struct mutex pipeline_state_mutex; /* protect pipeline triggers, ref counts and states */ }; -/** - * struct sof_ipc4_timestamp_info - IPC4 timestamp info - * @host_copier: the host copier of the pcm stream - * @dai_copier: the dai copier of the pcm stream - * @stream_start_offset: reported by fw in memory window - * @llp_offset: llp offset in memory window - */ -struct sof_ipc4_timestamp_info { - struct sof_ipc4_copier *host_copier; - struct sof_ipc4_copier *dai_copier; - u64 stream_start_offset; - u32 llp_offset; -}; - extern const struct sof_ipc_fw_loader_ops ipc4_loader_ops; extern const struct sof_ipc_tplg_ops ipc4_tplg_ops; extern const struct sof_ipc_tplg_control_ops tplg_ipc4_control_ops; -- cgit v1.2.3 From 55ca6ca227bfc5a8d0a0c2c5d6e239777226a604 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:09 +0200 Subject: ASoC: SOF: ipc4-pcm: Combine the SOF_IPC4_PIPE_PAUSED cases in pcm_trigger 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 Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-13-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4-pcm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'sound') diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index 2d7295221884..4e41b16d3205 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -478,14 +478,12 @@ static int sof_ipc4_pcm_trigger(struct snd_soc_component *component, /* determine the pipeline state */ switch (cmd) { - case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - state = SOF_IPC4_PIPE_PAUSED; - break; case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_START: state = SOF_IPC4_PIPE_RUNNING; break; + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: state = SOF_IPC4_PIPE_PAUSED; -- cgit v1.2.3 From 3ce3bc36d91510389955b47e36ea4c4e94fcbdd3 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:10 +0200 Subject: ASoC: SOF: ipc4-pcm: Invalidate the stream_start_offset in PAUSED state 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 Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-14-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4-pcm.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'sound') diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index 4e41b16d3205..905dbc4852b1 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -437,8 +437,19 @@ static int sof_ipc4_trigger_pipelines(struct snd_soc_component *component, } /* return if this is the final state */ - if (state == SOF_IPC4_PIPE_PAUSED) + if (state == SOF_IPC4_PIPE_PAUSED) { + struct sof_ipc4_timestamp_info *time_info; + + /* + * Invalidate the stream_start_offset to make sure that it is + * going to be updated if the stream resumes + */ + time_info = spcm->stream[substream->stream].private; + if (time_info) + time_info->stream_start_offset = SOF_IPC4_INVALID_STREAM_POSITION; + goto free; + } skip_pause_transition: /* else set the RUNNING/RESET state in the DSP */ ret = sof_ipc4_set_multi_pipeline_state(sdev, state, trigger_list); -- cgit v1.2.3 From 77165bd955d55114028b06787a530b8f9220e4b0 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:11 +0200 Subject: ASoC: SOF: sof-pcm: Add pointer callback to sof_ipc_pcm_ops The IPC specific pointer callback can be used when additional or custom handling is needed during the pointer calculation, like executing a delay calculation at the same time to minimize drift between the reported pointer and the calculated delay. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-15-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/pcm.c | 8 ++++++++ sound/soc/sof/sof-audio.h | 8 +++++++- 2 files changed, 15 insertions(+), 1 deletion(-) (limited to 'sound') diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 33d576b17647..f03cee94bce6 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -388,13 +388,21 @@ static snd_pcm_uframes_t sof_pcm_pointer(struct snd_soc_component *component, { struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component); + const struct sof_ipc_pcm_ops *pcm_ops = sof_ipc_get_ops(sdev, pcm); struct snd_sof_pcm *spcm; snd_pcm_uframes_t host, dai; + int ret = -EOPNOTSUPP; /* nothing to do for BE */ if (rtd->dai_link->no_pcm) return 0; + if (pcm_ops && pcm_ops->pointer) + ret = pcm_ops->pointer(component, substream, &host); + + if (ret != -EOPNOTSUPP) + return ret ? ret : host; + /* use dsp ops pointer callback directly if set */ if (sof_ops(sdev)->pcm_pointer) return sof_ops(sdev)->pcm_pointer(sdev, substream); diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 04e5cb2c70a7..86bbb531e142 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -103,7 +103,10 @@ struct snd_sof_dai_config_data { * additional memory in the SOF PCM stream structure * @pcm_free: Function pointer for PCM free that can be used for freeing any * additional memory in the SOF PCM stream structure - * @delay: Function pointer for pcm delay calculation + * @pointer: Function pointer for pcm pointer + * Note: the @pointer callback may return -EOPNOTSUPP which should be + * handled in a same way as if the callback is not provided + * @delay: Function pointer for pcm delay reporting * @reset_hw_params_during_stop: Flag indicating whether the hw_params should be reset during the * STOP pcm trigger * @ipc_first_on_start: Send IPC before invoking platform trigger during @@ -124,6 +127,9 @@ struct sof_ipc_pcm_ops { int (*dai_link_fixup)(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params); int (*pcm_setup)(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm); void (*pcm_free)(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm); + int (*pointer)(struct snd_soc_component *component, + struct snd_pcm_substream *substream, + snd_pcm_uframes_t *pointer); snd_pcm_sframes_t (*delay)(struct snd_soc_component *component, struct snd_pcm_substream *substream); bool reset_hw_params_during_stop; -- cgit v1.2.3 From 0ea06680dfcb4464ac6c05968433d060efb44345 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:12 +0200 Subject: ASoC: SOF: ipc4-pcm: Correct the delay calculation 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 Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-16-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4-pcm.c | 159 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 127 insertions(+), 32 deletions(-) (limited to 'sound') diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index 905dbc4852b1..e915f9f87a6c 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -19,14 +19,22 @@ * struct sof_ipc4_timestamp_info - IPC4 timestamp info * @host_copier: the host copier of the pcm stream * @dai_copier: the dai copier of the pcm stream - * @stream_start_offset: reported by fw in memory window + * @stream_start_offset: reported by fw in memory window (converted to frames) + * @stream_end_offset: reported by fw in memory window (converted to frames) * @llp_offset: llp offset in memory window + * @boundary: wrap boundary should be used for the LLP frame counter + * @delay: Calculated and stored in pointer callback. The stored value is + * returned in the delay callback. */ struct sof_ipc4_timestamp_info { struct sof_ipc4_copier *host_copier; struct sof_ipc4_copier *dai_copier; u64 stream_start_offset; + u64 stream_end_offset; u32 llp_offset; + + u64 boundary; + snd_pcm_sframes_t delay; }; static int sof_ipc4_set_multi_pipeline_state(struct snd_sof_dev *sdev, u32 state, @@ -726,6 +734,10 @@ static int sof_ipc4_pcm_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm if (abi_version < SOF_IPC4_FW_REGS_ABI_VER) support_info = false; + /* For delay reporting the get_host_byte_counter callback is needed */ + if (!sof_ops(sdev) || !sof_ops(sdev)->get_host_byte_counter) + support_info = false; + for_each_pcm_streams(stream) { pipeline_list = &spcm->stream[stream].pipeline_list; @@ -858,7 +870,6 @@ static int sof_ipc4_get_stream_start_offset(struct snd_sof_dev *sdev, struct sof_ipc4_copier *host_copier = time_info->host_copier; struct sof_ipc4_copier *dai_copier = time_info->dai_copier; struct sof_ipc4_pipeline_registers ppl_reg; - u64 stream_start_position; u32 dai_sample_size; u32 ch, node_index; u32 offset; @@ -875,38 +886,51 @@ static int sof_ipc4_get_stream_start_offset(struct snd_sof_dev *sdev, if (ppl_reg.stream_start_offset == SOF_IPC4_INVALID_STREAM_POSITION) return -EINVAL; - stream_start_position = ppl_reg.stream_start_offset; ch = dai_copier->data.out_format.fmt_cfg; ch = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(ch); dai_sample_size = (dai_copier->data.out_format.bit_depth >> 3) * ch; - /* convert offset to sample count */ - do_div(stream_start_position, dai_sample_size); - time_info->stream_start_offset = stream_start_position; + + /* convert offsets to frame count */ + time_info->stream_start_offset = ppl_reg.stream_start_offset; + do_div(time_info->stream_start_offset, dai_sample_size); + time_info->stream_end_offset = ppl_reg.stream_end_offset; + do_div(time_info->stream_end_offset, dai_sample_size); + + /* + * Calculate the wrap boundary need to be used for delay calculation + * The host counter is in bytes, it will wrap earlier than the frames + * based link counter. + */ + time_info->boundary = div64_u64(~((u64)0), + frames_to_bytes(substream->runtime, 1)); + /* Initialize the delay value to 0 (no delay) */ + time_info->delay = 0; return 0; } -static snd_pcm_sframes_t sof_ipc4_pcm_delay(struct snd_soc_component *component, - struct snd_pcm_substream *substream) +static int sof_ipc4_pcm_pointer(struct snd_soc_component *component, + struct snd_pcm_substream *substream, + snd_pcm_uframes_t *pointer) { struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component); struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct sof_ipc4_timestamp_info *time_info; struct sof_ipc4_llp_reading_slot llp; - snd_pcm_uframes_t head_ptr, tail_ptr; + snd_pcm_uframes_t head_cnt, tail_cnt; struct snd_sof_pcm_stream *stream; + u64 dai_cnt, host_cnt, host_ptr; struct snd_sof_pcm *spcm; - u64 tmp_ptr; int ret; spcm = snd_sof_find_spcm_dai(component, rtd); if (!spcm) - return 0; + return -EOPNOTSUPP; stream = &spcm->stream[substream->stream]; time_info = stream->private; if (!time_info) - return 0; + return -EOPNOTSUPP; /* * stream_start_offset is updated to memory window by FW based on @@ -916,46 +940,116 @@ static snd_pcm_sframes_t sof_ipc4_pcm_delay(struct snd_soc_component *component, if (time_info->stream_start_offset == SOF_IPC4_INVALID_STREAM_POSITION) { ret = sof_ipc4_get_stream_start_offset(sdev, substream, stream, time_info); if (ret < 0) - return 0; + return -EOPNOTSUPP; } + /* For delay calculation we need the host counter */ + host_cnt = snd_sof_pcm_get_host_byte_counter(sdev, component, substream); + host_ptr = host_cnt; + + /* convert the host_cnt to frames */ + host_cnt = div64_u64(host_cnt, frames_to_bytes(substream->runtime, 1)); + /* * If the LLP counter is not reported by firmware in the SRAM window - * then read the dai (link) position via host accessible means if + * then read the dai (link) counter via host accessible means if * available. */ if (!time_info->llp_offset) { - tmp_ptr = snd_sof_pcm_get_dai_frame_counter(sdev, component, substream); - if (!tmp_ptr) - return 0; + dai_cnt = snd_sof_pcm_get_dai_frame_counter(sdev, component, substream); + if (!dai_cnt) + return -EOPNOTSUPP; } else { sof_mailbox_read(sdev, time_info->llp_offset, &llp, sizeof(llp)); - tmp_ptr = ((u64)llp.reading.llp_u << 32) | llp.reading.llp_l; + dai_cnt = ((u64)llp.reading.llp_u << 32) | llp.reading.llp_l; } + dai_cnt += time_info->stream_end_offset; - /* In two cases dai dma position is not accurate + /* In two cases dai dma counter is not accurate * (1) dai pipeline is started before host pipeline - * (2) multiple streams mixed into one. Each stream has the same dai dma position + * (2) multiple streams mixed into one. Each stream has the same dai dma + * counter + * + * Firmware calculates correct stream_start_offset for all cases + * including above two. + * Driver subtracts stream_start_offset from dai dma counter to get + * accurate one + */ + + /* + * On stream start the dai counter might not yet have reached the + * stream_start_offset value which means that no frames have left the + * DSP yet from the audio stream (on playback, capture streams have + * offset of 0 as we start capturing right away). + * In this case we need to adjust the distance between the counters by + * increasing the host counter by (offset - dai_counter). + * Otherwise the dai_counter needs to be adjusted to reflect the number + * of valid frames passed on the DAI side. * - * 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 + * The delay is the difference between the counters on the two + * sides of the DSP. */ - tmp_ptr -= time_info->stream_start_offset; + if (dai_cnt < time_info->stream_start_offset) { + host_cnt += time_info->stream_start_offset - dai_cnt; + dai_cnt = 0; + } else { + dai_cnt -= time_info->stream_start_offset; + } + + /* Wrap the dai counter at the boundary where the host counter wraps */ + div64_u64_rem(dai_cnt, time_info->boundary, &dai_cnt); - /* Calculate the delay taking into account that both pointer can wrap */ - div64_u64_rem(tmp_ptr, substream->runtime->boundary, &tmp_ptr); if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - head_ptr = substream->runtime->status->hw_ptr; - tail_ptr = tmp_ptr; + head_cnt = host_cnt; + tail_cnt = dai_cnt; } else { - head_ptr = tmp_ptr; - tail_ptr = substream->runtime->status->hw_ptr; + head_cnt = dai_cnt; + tail_cnt = host_cnt; + } + + if (head_cnt < tail_cnt) { + time_info->delay = time_info->boundary - tail_cnt + head_cnt; + goto out; } - if (head_ptr < tail_ptr) - return substream->runtime->boundary - tail_ptr + head_ptr; + time_info->delay = head_cnt - tail_cnt; + +out: + /* + * Convert the host byte counter to PCM pointer which wraps in buffer + * and it is in frames + */ + div64_u64_rem(host_ptr, snd_pcm_lib_buffer_bytes(substream), &host_ptr); + *pointer = bytes_to_frames(substream->runtime, host_ptr); + + return 0; +} + +static snd_pcm_sframes_t sof_ipc4_pcm_delay(struct snd_soc_component *component, + struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); + struct sof_ipc4_timestamp_info *time_info; + struct snd_sof_pcm_stream *stream; + struct snd_sof_pcm *spcm; + + spcm = snd_sof_find_spcm_dai(component, rtd); + if (!spcm) + return 0; + + stream = &spcm->stream[substream->stream]; + time_info = stream->private; + /* + * Report the stored delay value calculated in the pointer callback. + * In the unlikely event that the calculation was skipped/aborted, the + * default 0 delay returned. + */ + if (time_info) + return time_info->delay; + + /* No delay information available, report 0 as delay */ + return 0; - return head_ptr - tail_ptr; } const struct sof_ipc_pcm_ops ipc4_pcm_ops = { @@ -965,6 +1059,7 @@ const struct sof_ipc_pcm_ops ipc4_pcm_ops = { .dai_link_fixup = sof_ipc4_pcm_dai_link_fixup, .pcm_setup = sof_ipc4_pcm_setup, .pcm_free = sof_ipc4_pcm_free, + .pointer = sof_ipc4_pcm_pointer, .delay = sof_ipc4_pcm_delay, .ipc_first_on_start = true, .platform_stop_during_hw_free = true, -- cgit v1.2.3 From 1abc2642588e06f6180b3fbb21968cf5d0ba9e5f Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 21 Mar 2024 15:08:14 +0200 Subject: ASoC: SOF: Intel: hda: Compensate LLP in case it is not reset During pause/reset or stop/start the LLP counter is not reset, which will result broken delay reporting. Read the LLP value on STOP/PAUSE trigger and use it in LLP reading to normalize the LLP from the register. Cc: stable@vger.kernel.org # 6.8 Signed-off-by: Peter Ujfalusi Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Link: https://msgid.link/r/20240321130814.4412-18-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-dai-ops.c | 11 +++++++++++ sound/soc/sof/intel/hda-pcm.c | 8 ++++++++ sound/soc/sof/intel/hda-stream.c | 9 ++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) (limited to 'sound') diff --git a/sound/soc/sof/intel/hda-dai-ops.c b/sound/soc/sof/intel/hda-dai-ops.c index c50ca9e72d37..b073720b4cf4 100644 --- a/sound/soc/sof/intel/hda-dai-ops.c +++ b/sound/soc/sof/intel/hda-dai-ops.c @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -362,6 +363,16 @@ static int hda_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *cpu_dai, case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: snd_hdac_ext_stream_clear(hext_stream); + + /* + * Save the LLP registers in case the stream is + * restarting due PAUSE_RELEASE, or START without a pcm + * close/open since in this case the LLP register is not reset + * to 0 and the delay calculation will return with invalid + * results. + */ + hext_stream->pplcllpl = readl(hext_stream->pplc_addr + AZX_REG_PPLCLLPL); + hext_stream->pplcllpu = readl(hext_stream->pplc_addr + AZX_REG_PPLCLLPU); break; default: dev_err(sdev->dev, "unknown trigger command %d\n", cmd); diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c index 69fefcd1abc5..d7b446f3f973 100644 --- a/sound/soc/sof/intel/hda-pcm.c +++ b/sound/soc/sof/intel/hda-pcm.c @@ -282,6 +282,14 @@ int hda_dsp_pcm_open(struct snd_sof_dev *sdev, /* binding pcm substream to hda stream */ substream->runtime->private_data = &dsp_stream->hstream; + + /* + * Reset the llp cache values (they are used for LLP compensation in + * case the counter is not reset) + */ + dsp_stream->pplcllpl = 0; + dsp_stream->pplcllpu = 0; + return 0; } diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 8504a4f27b60..0c189d3b19c1 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -1064,6 +1064,8 @@ snd_pcm_uframes_t hda_dsp_stream_get_position(struct hdac_stream *hstream, return pos; } +#define merge_u64(u32_u, u32_l) (((u64)(u32_u) << 32) | (u32_l)) + /** * hda_dsp_get_stream_llp - Retrieve the LLP (Linear Link Position) of the stream * @sdev: SOF device @@ -1093,7 +1095,12 @@ u64 hda_dsp_get_stream_llp(struct snd_sof_dev *sdev, llp_l = readl(hext_stream->pplc_addr + AZX_REG_PPLCLLPL); llp_u = readl(hext_stream->pplc_addr + AZX_REG_PPLCLLPU); - return ((u64)llp_u << 32) | llp_l; + /* Compensate the LLP counter with the saved offset */ + if (hext_stream->pplcllpl || hext_stream->pplcllpu) + return merge_u64(llp_u, llp_l) - + merge_u64(hext_stream->pplcllpu, hext_stream->pplcllpl); + + return merge_u64(llp_u, llp_l); } /** -- cgit v1.2.3