[PATCH 0/8] ASoC: SOF: misc updates for 6.5
Couple of improvements on virtual_widget support, firmware trace free, IPC payload dump, duplicated code in suspend and MeteorLake primary code support.
Bard Liao (2): ASoC: SOF: sof-audio: add is_virtual_widget helper ASoC: SOF: sof-audio: test virtual widget in sof_walk_widgets_in_order
Peter Ujfalusi (5): ASoC: SOF: core: Free the firmware trace before calling snd_sof_shutdown() ASoC: SOF: Add new sof_debug flag to request message payload dump ASoC: SOF: ipc3: Dump IPC message payload ASoC: SOF: ipc4: Switch to use the sof_debug:bit11 to dump message payload ASoC: SOF: pm: Remove duplicated code in sof_suspend
Rander Wang (1): ASoC: SOF: Intel: mtl: setup primary core info on MeteorLake platform
sound/soc/sof/core.c | 4 +++- sound/soc/sof/intel/mtl.c | 19 ++++++++++++---- sound/soc/sof/ipc3.c | 39 ++++++++++++++++++++++++++++++++ sound/soc/sof/ipc4.c | 46 ++++++++++++++++++-------------------- sound/soc/sof/pm.c | 12 ++++------ sound/soc/sof/sof-audio.c | 47 ++++++++++++++++++++++++++------------- sound/soc/sof/sof-priv.h | 3 +++ 7 files changed, 117 insertions(+), 53 deletions(-)
From: Bard Liao yung-chuan.liao@linux.intel.com
Testing virtual widget is required in many functions. No function changed in this commit.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/sof-audio.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 1cbda595c518..c77d07d62517 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -14,6 +14,20 @@ #include "sof-of-dev.h" #include "ops.h"
+static bool is_virtual_widget(struct snd_sof_dev *sdev, struct snd_soc_dapm_widget *widget, + const char *func) +{ + switch (widget->id) { + case snd_soc_dapm_out_drv: + case snd_soc_dapm_output: + case snd_soc_dapm_input: + dev_dbg(sdev->dev, "%s: %s is a virtual widget\n", func, widget->name); + return true; + default: + return false; + } +} + static void sof_reset_route_setup_status(struct snd_sof_dev *sdev, struct snd_sof_widget *widget) { const struct sof_ipc_tplg_ops *tplg_ops = sof_ipc_get_ops(sdev, tplg); @@ -231,23 +245,9 @@ int sof_route_setup(struct snd_sof_dev *sdev, struct snd_soc_dapm_widget *wsourc bool route_found = false;
/* ignore routes involving virtual widgets in topology */ - switch (src_widget->id) { - case snd_soc_dapm_out_drv: - case snd_soc_dapm_output: - case snd_soc_dapm_input: + if (is_virtual_widget(sdev, src_widget->widget, __func__) || + is_virtual_widget(sdev, sink_widget->widget, __func__)) return 0; - default: - break; - } - - switch (sink_widget->id) { - case snd_soc_dapm_out_drv: - case snd_soc_dapm_output: - case snd_soc_dapm_input: - return 0; - default: - break; - }
/* find route matching source and sink widgets */ list_for_each_entry(sroute, &sdev->route_list, list)
From: Bard Liao yung-chuan.liao@linux.intel.com
Virtual widgets are added for the purpose of showing connections between aggregated DAIs in SDW topologies. However, we shouldn't touch them in SOF.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/sof-audio.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index c77d07d62517..e7ef77012c35 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -396,6 +396,9 @@ sof_unprepare_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_dapm_widg const struct sof_ipc_tplg_widget_ops *widget_ops; struct snd_soc_dapm_path *p;
+ if (is_virtual_widget(sdev, widget, __func__)) + return; + /* skip if the widget is in use or if it is already unprepared */ if (!swidget || !swidget->prepared || swidget->use_count > 0) goto sink_unprepare; @@ -433,6 +436,9 @@ sof_prepare_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_dapm_widget struct snd_soc_dapm_path *p; int ret;
+ if (is_virtual_widget(sdev, widget, __func__)) + return 0; + widget_ops = tplg_ops ? tplg_ops->widget : NULL; if (!widget_ops) return 0; @@ -488,6 +494,9 @@ static int sof_free_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_dap int err; int ret = 0;
+ if (is_virtual_widget(sdev, widget, __func__)) + return 0; + if (widget->dobj.private) { err = sof_widget_free(sdev, widget->dobj.private); if (err < 0) @@ -527,6 +536,9 @@ static int sof_set_up_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_d struct snd_soc_dapm_path *p; int ret;
+ if (is_virtual_widget(sdev, widget, __func__)) + return 0; + if (swidget) { int i;
@@ -592,6 +604,9 @@ sof_walk_widgets_in_order(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm, return 0;
for_each_dapm_widgets(list, i, widget) { + if (is_virtual_widget(sdev, widget, __func__)) + continue; + /* starting widget for playback is AIF type */ if (dir == SNDRV_PCM_STREAM_PLAYBACK && widget->id != snd_soc_dapm_aif_in) continue;
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
The shutdown is called on reboot/shutdown of the machine. At this point the firmware tracing cannot be used anymore but in case of IPC3 it is using and keeping a DMA channel active (dtrace).
For Tiger Lake platforms we have a quirk in place to fix rare reboot issues when a DMA was active before rebooting the system. If the tracing is enabled this quirk will be always used and a print appears on the kernel log which might be misleading or not even correct.
Release the fw tracing before executing the shutdown to make sure that this known DMA user is cleared away.
Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Daniel Baluta daniel.baluta@nxp.com Reviewed-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 Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 9a9d82220fd0..30db685cc5f4 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -504,8 +504,10 @@ int snd_sof_device_shutdown(struct device *dev) if (IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)) cancel_work_sync(&sdev->probe_work);
- if (sdev->fw_state == SOF_FW_BOOT_COMPLETE) + if (sdev->fw_state == SOF_FW_BOOT_COMPLETE) { + sof_fw_trace_free(sdev); return snd_sof_shutdown(sdev); + }
return 0; }
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
We only print out the header information of an IPC message in debug level, either in verbose or non verbose way (Kconfig option).
On top of the header information the message itself can help reproducing and identifying issues.
BIT(11) can be used to request a message payload dump if it is supported by the IPC implementation.
Since IPC message payload printing is only implemented for IPC4, the flag will not have any effect to IPC3 for now.
Reviewed-by: Kai Vehmanen kai.vehmanen@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 Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/sof-priv.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index cd4f6ac126ec..d4f6702e93dc 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -48,6 +48,9 @@ struct snd_sof_pcm_stream; #define SOF_DBG_FORCE_NOCODEC BIT(10) /* ignore all codec-related * configurations */ +#define SOF_DBG_DUMP_IPC_MESSAGE_PAYLOAD BIT(11) /* On top of the IPC message header + * dump the message payload also + */ #define SOF_DBG_DSPLESS_MODE BIT(15) /* Do not initialize and use the DSP */
/* Flag definitions used for controlling the DSP dump behavior */
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
Dump the IPC message payload if BIT(11) of sof_debug is set and the message contains more data than just a header.
The header size differs between TX and RX and in case of set_get_data, the header is always the reply header for the message regardless if it is TX or RX.
The use of printk(KERN_DEBUG "..."); is on purpose to keep the dmesg output tidy.
Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/ipc3.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/sound/soc/sof/ipc3.c b/sound/soc/sof/ipc3.c index ec1ac0fb2d9f..2c5aac31e8b0 100644 --- a/sound/soc/sof/ipc3.c +++ b/sound/soc/sof/ipc3.c @@ -223,6 +223,14 @@ static inline void ipc3_log_header(struct device *dev, u8 *text, u32 cmd) } #endif
+static void sof_ipc3_dump_payload(struct snd_sof_dev *sdev, + void *ipc_data, size_t size) +{ + printk(KERN_DEBUG "Size of payload following the header: %zu\n", size); + print_hex_dump_debug("Message payload: ", DUMP_PREFIX_OFFSET, + 16, 4, ipc_data, size, false); +} + static int sof_ipc3_get_reply(struct snd_sof_dev *sdev) { struct snd_sof_ipc_msg *msg = sdev->msg; @@ -374,6 +382,29 @@ static int sof_ipc3_tx_msg(struct snd_sof_dev *sdev, void *msg_data, size_t msg_
ret = ipc3_tx_msg_unlocked(ipc, msg_data, msg_bytes, reply_data, reply_bytes);
+ if (sof_debug_check_flag(SOF_DBG_DUMP_IPC_MESSAGE_PAYLOAD)) { + size_t payload_bytes, header_bytes; + char *payload = NULL; + + /* payload is indicated by non zero msg/reply_bytes */ + if (msg_bytes > sizeof(struct sof_ipc_cmd_hdr)) { + payload = msg_data; + + header_bytes = sizeof(struct sof_ipc_cmd_hdr); + payload_bytes = msg_bytes - header_bytes; + } else if (reply_bytes > sizeof(struct sof_ipc_reply)) { + payload = reply_data; + + header_bytes = sizeof(struct sof_ipc_reply); + payload_bytes = reply_bytes - header_bytes; + } + + if (payload) { + payload += header_bytes; + sof_ipc3_dump_payload(sdev, payload, payload_bytes); + } + } + mutex_unlock(&ipc->tx_mutex);
return ret; @@ -472,6 +503,14 @@ static int sof_ipc3_set_get_data(struct snd_sof_dev *sdev, void *data, size_t da offset += payload_size; }
+ if (sof_debug_check_flag(SOF_DBG_DUMP_IPC_MESSAGE_PAYLOAD)) { + size_t header_bytes = sizeof(struct sof_ipc_reply); + char *payload = (char *)cdata; + + payload += header_bytes; + sof_ipc3_dump_payload(sdev, payload, data_bytes - header_bytes); + } + mutex_unlock(&sdev->ipc->tx_mutex);
kfree(cdata_chunk);
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
Use the SOF_DBG_DUMP_IPC_MESSAGE_PAYLOAD flag to print the message payload instead of the DEBUG_VERBOSE, which would need code modification and kernel re-compilation.
Reviewed-by: Kai Vehmanen kai.vehmanen@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 Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/ipc4.c | 46 +++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/sound/soc/sof/ipc4.c b/sound/soc/sof/ipc4.c index 246b56d24a6f..ab6eddd91bb7 100644 --- a/sound/soc/sof/ipc4.c +++ b/sound/soc/sof/ipc4.c @@ -17,15 +17,6 @@ #include "ipc4-priv.h" #include "ops.h"
-#ifdef DEBUG_VERBOSE -#define sof_ipc4_dump_payload(sdev, ipc_data, size) \ - print_hex_dump_debug("Message payload: ", \ - DUMP_PREFIX_OFFSET, \ - 16, 4, ipc_data, size, false) -#else -#define sof_ipc4_dump_payload(sdev, ipc_data, size) do { } while (0) -#endif - static const struct sof_ipc4_fw_status { int status; char *msg; @@ -256,6 +247,13 @@ static void sof_ipc4_log_header(struct device *dev, u8 *text, struct sof_ipc4_ms } #endif
+static void sof_ipc4_dump_payload(struct snd_sof_dev *sdev, + void *ipc_data, size_t size) +{ + print_hex_dump_debug("Message payload: ", DUMP_PREFIX_OFFSET, + 16, 4, ipc_data, size, false); +} + static int sof_ipc4_get_reply(struct snd_sof_dev *sdev) { struct snd_sof_ipc_msg *msg = sdev->msg; @@ -362,9 +360,6 @@ static int sof_ipc4_tx_msg(struct snd_sof_dev *sdev, void *msg_data, size_t msg_ void *reply_data, size_t reply_bytes, bool no_pm) { struct snd_sof_ipc *ipc = sdev->ipc; -#ifdef DEBUG_VERBOSE - struct sof_ipc4_msg *msg = NULL; -#endif int ret;
if (!msg_data) @@ -386,19 +381,21 @@ static int sof_ipc4_tx_msg(struct snd_sof_dev *sdev, void *msg_data, size_t msg_
ret = ipc4_tx_msg_unlocked(ipc, msg_data, msg_bytes, reply_data, reply_bytes);
+ if (sof_debug_check_flag(SOF_DBG_DUMP_IPC_MESSAGE_PAYLOAD)) { + struct sof_ipc4_msg *msg = NULL; + + /* payload is indicated by non zero msg/reply_bytes */ + if (msg_bytes) + msg = msg_data; + else if (reply_bytes) + msg = reply_data; + + if (msg) + sof_ipc4_dump_payload(sdev, msg->data_ptr, msg->data_size); + } + mutex_unlock(&ipc->tx_mutex);
-#ifdef DEBUG_VERBOSE - /* payload is indicated by non zero msg/reply_bytes */ - if (msg_bytes) - msg = msg_data; - else if (reply_bytes) - msg = reply_data; - - if (msg) - sof_ipc4_dump_payload(sdev, msg->data_ptr, msg->data_size); -#endif - return ret; }
@@ -516,7 +513,8 @@ static int sof_ipc4_set_get_data(struct snd_sof_dev *sdev, void *data, if (!set && payload_bytes != offset) ipc4_msg->data_size = offset;
- sof_ipc4_dump_payload(sdev, ipc4_msg->data_ptr, ipc4_msg->data_size); + if (sof_debug_check_flag(SOF_DBG_DUMP_IPC_MESSAGE_PAYLOAD)) + sof_ipc4_dump_payload(sdev, ipc4_msg->data_ptr, ipc4_msg->data_size);
out: mutex_unlock(&sdev->ipc->tx_mutex);
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
Over time the function has changed and now there is no need to have the duplicated sof_fw_trace_suspend() and sof_suspend_clients() in the if (target_state == SOF_DSP_PM_D0) branch.
Remove it and add a simple check with a single goto statement.
Reviewed-by: Daniel Baluta daniel.baluta@nxp.com Reviewed-by: Paul Olaru olarupaulstelian97@gmail.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/pm.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 2b232442e84b..704b21413c71 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -234,20 +234,16 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
pm_state.event = target_state;
- /* Skip to platform-specific suspend if DSP is entering D0 */ - if (target_state == SOF_DSP_PM_D0) { - sof_fw_trace_suspend(sdev, pm_state); - /* Notify clients not managed by pm framework about core suspend */ - sof_suspend_clients(sdev, pm_state); - goto suspend; - } - /* suspend DMA trace */ sof_fw_trace_suspend(sdev, pm_state);
/* Notify clients not managed by pm framework about core suspend */ sof_suspend_clients(sdev, pm_state);
+ /* Skip to platform-specific suspend if DSP is entering D0 */ + if (target_state == SOF_DSP_PM_D0) + goto suspend; + #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE) /* cache debugfs contents during runtime suspend */ if (runtime_suspend)
From: Rander Wang rander.wang@intel.com
Set primary core mask and refcount.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com 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 --- sound/soc/sof/intel/mtl.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c index 8ae331faca4e..30fe77fd87bf 100644 --- a/sound/soc/sof/intel/mtl.c +++ b/sound/soc/sof/intel/mtl.c @@ -361,11 +361,17 @@ static int mtl_dsp_core_power_up(struct snd_sof_dev *sdev, int core) ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR, MTL_DSP2CXCTL_PRIMARY_CORE, dspcxctl, (dspcxctl & cpa) == cpa, HDA_DSP_REG_POLL_INTERVAL_US, HDA_DSP_RESET_TIMEOUT_US); - if (ret < 0) + if (ret < 0) { dev_err(sdev->dev, "%s: timeout on MTL_DSP2CXCTL_PRIMARY_CORE read\n", __func__); + return ret; + }
- return ret; + /* set primary core mask and refcount to 1 */ + sdev->enabled_cores_mask = BIT(SOF_DSP_PRIMARY_CORE); + sdev->dsp_core_ref_count[SOF_DSP_PRIMARY_CORE] = 1; + + return 0; }
static int mtl_dsp_core_power_down(struct snd_sof_dev *sdev, int core) @@ -388,10 +394,15 @@ static int mtl_dsp_core_power_down(struct snd_sof_dev *sdev, int core) !(dspcxctl & MTL_DSP2CXCTL_PRIMARY_CORE_CPA_MASK), HDA_DSP_REG_POLL_INTERVAL_US, HDA_DSP_PD_TIMEOUT * USEC_PER_MSEC); - if (ret < 0) + if (ret < 0) { dev_err(sdev->dev, "failed to power down primary core\n"); + return ret; + }
- return ret; + sdev->enabled_cores_mask = 0; + sdev->dsp_core_ref_count[SOF_DSP_PRIMARY_CORE] = 0; + + return 0; }
int mtl_power_down_dsp(struct snd_sof_dev *sdev)
On Fri, 16 Jun 2023 12:00:31 +0200, Pierre-Louis Bossart wrote:
Couple of improvements on virtual_widget support, firmware trace free, IPC payload dump, duplicated code in suspend and MeteorLake primary code support.
Bard Liao (2): ASoC: SOF: sof-audio: add is_virtual_widget helper ASoC: SOF: sof-audio: test virtual widget in sof_walk_widgets_in_order
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/8] ASoC: SOF: sof-audio: add is_virtual_widget helper commit: 90ce7538659aad1c048653c23eadaba9d1648559 [2/8] ASoC: SOF: sof-audio: test virtual widget in sof_walk_widgets_in_order commit: 0557864e9dbe8f6c0f86110ad5712f81649f7288 [3/8] ASoC: SOF: core: Free the firmware trace before calling snd_sof_shutdown() commit: d389dcb3a48cec4f03c16434c0bf98a4c635372a [4/8] ASoC: SOF: Add new sof_debug flag to request message payload dump commit: d498a3bdfe954afb4155ab2bdc3ae534c949b907 [5/8] ASoC: SOF: ipc3: Dump IPC message payload commit: d01c7636ffa051297672c55ab6088ae539d221ee [6/8] ASoC: SOF: ipc4: Switch to use the sof_debug:bit11 to dump message payload commit: c3d275e3a84833368c47c803043105bda095a624 [7/8] ASoC: SOF: pm: Remove duplicated code in sof_suspend commit: 399961423314680c6cb14ac822600b9ede2b991f [8/8] ASoC: SOF: Intel: mtl: setup primary core info on MeteorLake platform commit: fd4e9e9bfa0b1c63946fde2ff61440ff1e5eb75b
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)
-
Mark Brown -
Pierre-Louis Bossart