From 90ce7538659aad1c048653c23eadaba9d1648559 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Fri, 16 Jun 2023 12:00:32 +0200 Subject: [PATCH 1/8] ASoC: SOF: sof-audio: add is_virtual_widget helper Testing virtual widget is required in many functions. No function changed in this commit. Reviewed-by: Ranjani Sridharan Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20230616100039.378150-2-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- 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 1cbda595c5183..c77d07d62517a 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: - return 0; - default: - break; - } - - switch (sink_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; - } /* find route matching source and sink widgets */ list_for_each_entry(sroute, &sdev->route_list, list) From 0557864e9dbe8f6c0f86110ad5712f81649f7288 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Fri, 16 Jun 2023 12:00:33 +0200 Subject: [PATCH 2/8] ASoC: SOF: sof-audio: test virtual widget in sof_walk_widgets_in_order 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 Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20230616100039.378150-3-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- 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 c77d07d62517a..e7ef77012c358 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 d389dcb3a48cec4f03c16434c0bf98a4c635372a Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Fri, 16 Jun 2023 12:00:34 +0200 Subject: [PATCH 3/8] ASoC: SOF: core: Free the firmware trace before calling snd_sof_shutdown() 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 Reviewed-by: Daniel Baluta Reviewed-by: Ranjani Sridharan Reviewed-by: Rander Wang Reviewed-by: Bard Liao Signed-off-by: Peter Ujfalusi Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20230616100039.378150-4-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- 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 9a9d82220fd0d..30db685cc5f4b 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 d498a3bdfe954afb4155ab2bdc3ae534c949b907 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Fri, 16 Jun 2023 12:00:35 +0200 Subject: [PATCH 4/8] ASoC: SOF: Add new sof_debug flag to request message payload dump 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 Reviewed-by: Ranjani Sridharan Reviewed-by: Bard Liao Signed-off-by: Peter Ujfalusi Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20230616100039.378150-5-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- 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 cd4f6ac126eca..d4f6702e93dcb 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 d01c7636ffa051297672c55ab6088ae539d221ee Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Fri, 16 Jun 2023 12:00:36 +0200 Subject: [PATCH 5/8] ASoC: SOF: ipc3: Dump IPC message payload 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 Signed-off-by: Peter Ujfalusi Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20230616100039.378150-6-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- 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 ec1ac0fb2d9f8..2c5aac31e8b07 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 c3d275e3a84833368c47c803043105bda095a624 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Fri, 16 Jun 2023 12:00:37 +0200 Subject: [PATCH 6/8] ASoC: SOF: ipc4: Switch to use the sof_debug:bit11 to dump message payload 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 Reviewed-by: Ranjani Sridharan Reviewed-by: Bard Liao Signed-off-by: Peter Ujfalusi Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20230616100039.378150-7-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc4.c | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/sound/soc/sof/ipc4.c b/sound/soc/sof/ipc4.c index 246b56d24a6f1..ab6eddd91bb77 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,18 +381,20 @@ 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); - mutex_unlock(&ipc->tx_mutex); + if (sof_debug_check_flag(SOF_DBG_DUMP_IPC_MESSAGE_PAYLOAD)) { + struct sof_ipc4_msg *msg = NULL; -#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; + /* 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 + if (msg) + sof_ipc4_dump_payload(sdev, msg->data_ptr, msg->data_size); + } + + mutex_unlock(&ipc->tx_mutex); 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 399961423314680c6cb14ac822600b9ede2b991f Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Fri, 16 Jun 2023 12:00:38 +0200 Subject: [PATCH 7/8] ASoC: SOF: pm: Remove duplicated code in sof_suspend 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 Reviewed-by: Paul Olaru Reviewed-by: Ranjani Sridharan Signed-off-by: Peter Ujfalusi Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20230616100039.378150-8-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- 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 2b232442e84bc..704b21413c719 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 fd4e9e9bfa0b1c63946fde2ff61440ff1e5eb75b Mon Sep 17 00:00:00 2001 From: Rander Wang Date: Fri, 16 Jun 2023 12:00:39 +0200 Subject: [PATCH 8/8] ASoC: SOF: Intel: mtl: setup primary core info on MeteorLake platform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Set primary core mask and refcount. Reviewed-by: Ranjani Sridharan Reviewed-by: Péter Ujfalusi Signed-off-by: Rander Wang Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20230616100039.378150-9-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- 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 8ae331faca4ec..30fe77fd87bf8 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)