From 79a45c065c0263b5bd6795db223e6549babc7e73 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 3 May 2022 08:18:46 -0500 Subject: [PATCH 1/4] soundwire: intel: uniquify debug message The same debug message is replicated multiple time, add __func__ to figure out what link is ignored. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 2e7c27d303b42d..3f3c2c99cb8e01 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1336,8 +1336,8 @@ int intel_link_startup(struct auxiliary_device *auxdev) if (bus->prop.hw_disabled) { dev_info(dev, - "SoundWire master %d is disabled, ignoring\n", - sdw->instance); + "%s: SoundWire master %d is disabled, ignoring\n", + __func__, sdw->instance); return 0; } @@ -1497,8 +1497,8 @@ int intel_link_process_wakeen_event(struct auxiliary_device *auxdev) bus = &sdw->cdns.bus; if (bus->prop.hw_disabled || !sdw->startup_done) { - dev_dbg(dev, "SoundWire master %d is disabled or not-started, ignoring\n", - bus->link_id); + dev_dbg(dev, "%s: SoundWire master %d is disabled or not-started, ignoring\n", + __func__, bus->link_id); return 0; } @@ -1557,8 +1557,8 @@ static int __maybe_unused intel_pm_prepare(struct device *dev) int ret; if (bus->prop.hw_disabled || !sdw->startup_done) { - dev_dbg(dev, "SoundWire master %d is disabled or not-started, ignoring\n", - bus->link_id); + dev_dbg(dev, "%s: SoundWire master %d is disabled or not-started, ignoring\n", + __func__, bus->link_id); return 0; } @@ -1617,8 +1617,8 @@ static int __maybe_unused intel_suspend(struct device *dev) int ret; if (bus->prop.hw_disabled || !sdw->startup_done) { - dev_dbg(dev, "SoundWire master %d is disabled or not-started, ignoring\n", - bus->link_id); + dev_dbg(dev, "%s: SoundWire master %d is disabled or not-started, ignoring\n", + __func__, bus->link_id); return 0; } @@ -1670,8 +1670,8 @@ static int __maybe_unused intel_suspend_runtime(struct device *dev) int ret; if (bus->prop.hw_disabled || !sdw->startup_done) { - dev_dbg(dev, "SoundWire master %d is disabled or not-started, ignoring\n", - bus->link_id); + dev_dbg(dev, "%s: SoundWire master %d is disabled or not-started, ignoring\n", + __func__, bus->link_id); return 0; } @@ -1735,8 +1735,8 @@ static int __maybe_unused intel_resume(struct device *dev) int ret; if (bus->prop.hw_disabled || !sdw->startup_done) { - dev_dbg(dev, "SoundWire master %d is disabled or not-started, ignoring\n", - bus->link_id); + dev_dbg(dev, "%s: SoundWire master %d is disabled or not-started, ignoring\n", + __func__, bus->link_id); return 0; } @@ -1833,8 +1833,8 @@ static int __maybe_unused intel_resume_runtime(struct device *dev) int ret; if (bus->prop.hw_disabled || !sdw->startup_done) { - dev_dbg(dev, "SoundWire master %d is disabled or not-started, ignoring\n", - bus->link_id); + dev_dbg(dev, "%s: SoundWire master %d is disabled or not-started, ignoring\n", + __func__, bus->link_id); return 0; } From e1b181e1ed3ee92bf1ceca3d3eed6c513efd3e64 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 3 May 2022 15:32:43 -0500 Subject: [PATCH 2/4] ASoC: SOF: Intel: hda-dai: enhance debug messages The same message was added twice for dai and link_dma, remove the latter one and add dai name and direction to better understand problematic sequences. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index c5c0c2d8e5cca3..784cd77eaa8032 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -270,7 +270,6 @@ static int hda_link_dma_trigger(struct snd_pcm_substream *substream, int cmd) struct hdac_ext_stream *hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream); int ret; - dev_dbg(cpu_dai->dev, "%s: cmd=%d\n", __func__, cmd); if (!hext_stream) return 0; @@ -420,13 +419,15 @@ static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream, struct snd_soc_dapm_widget *w; int ret; + dev_dbg(dai->dev, "%s: cmd=%d dai %s direction %d\n", __func__, cmd, + dai->name, substream->stream); + ret = hda_link_dma_trigger(substream, cmd); if (ret < 0) return ret; w = snd_soc_dai_get_widget(dai, substream->stream); - dev_dbg(dai->dev, "%s: cmd=%d\n", __func__, cmd); switch (cmd) { case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: From bd7d30266423e09634cc17e39d7bd599eef43b4a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 3 May 2022 15:37:36 -0500 Subject: [PATCH 3/4] fixup! ASoC: SOF: IPC4: set the BE DAI ops Add the same debug message for IPC4 and IPC3 for consistency. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 784cd77eaa8032..30eb2a605ad88f 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -464,6 +464,9 @@ static int ipc4_hda_link_dma_trigger(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai; int ret; + dev_dbg(dai->dev, "%s: cmd=%d dai %s direction %d\n", __func__, cmd, + dai->name, substream->stream); + hstream = substream->runtime->private_data; rtd = asoc_substream_to_rtd(substream); cpu_dai = asoc_rtd_to_cpu(rtd, 0); From c162e514827b5539a298441e05bddbff9ca5be4a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 2 May 2022 09:38:45 -0500 Subject: [PATCH 4/4] ASoC: SOF: reduce default verbosity of IPC logs We currently log the initiation of an IPC as well at its success. [ 3906.106987] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x80010000: GLB_DAI_MSG: CONFIG [ 3906.107189] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx succeeded: 0x80010000: GLB_DAI_MSG: CONFIG This is overkill in most cases, we already have a message thrown in case of errors and have tracepoints enabled to check for IPC duration. The only case where this might be useful is to check if there is an interleaved IPC RX. Add a flag and only print those logs if enabled. In addition, the DMA_POSITION_UPDATE for traces brings limited information in most cases and pollutes the logs for no good reason. [ 3906.322256] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc rx: 0x90020000: GLB_TRACE_MSG: DMA_POSITION [ 3906.322308] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc rx done: 0x90020000: GLB_TRACE_MSG: DMA_POSITION [ 3906.822261] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc rx: 0x90020000: GLB_TRACE_MSG: DMA_POSITION [ 3906.822319] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc rx done: 0x90020000: GLB_TRACE_MSG: DMA_POSITION [ 3907.822261] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc rx: 0x90020000: GLB_TRACE_MSG: DMA_POSITION [ 3907.822319] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc rx done: 0x90020000: GLB_TRACE_MSG: DMA_POSITION [ 3908.822251] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc rx: 0x90020000: GLB_TRACE_MSG: DMA_POSITION [ 3908.822309] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc rx done: 0x90020000: GLB_TRACE_MSG: DMA_POSITION This information is only helpful when debugging the trace support, not when using the trace. Add a flag to only print DMA position update logs if enabled. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/ipc3.c | 5 ++++- sound/soc/sof/sof-priv.h | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/ipc3.c b/sound/soc/sof/ipc3.c index a7bd55519b1931..08aa31a001df7b 100644 --- a/sound/soc/sof/ipc3.c +++ b/sound/soc/sof/ipc3.c @@ -148,6 +148,8 @@ static void ipc3_log_header(struct device *dev, u8 *text, u32 cmd) case SOF_IPC_TRACE_DMA_PARAMS: str2 = "DMA_PARAMS"; break; case SOF_IPC_TRACE_DMA_POSITION: + if (!sof_debug_check_flag(SOF_DBG_PRINT_DMA_POSITION_UPDATE_LOGS)) + return; str2 = "DMA_POSITION"; break; case SOF_IPC_TRACE_DMA_PARAMS_EXT: str2 = "DMA_PARAMS_EXT"; break; @@ -300,7 +302,8 @@ static int ipc3_wait_tx_done(struct snd_sof_ipc *ipc, void *reply_data) "ipc tx error for %#x (msg/reply size: %d/%zu): %d\n", hdr->cmd, hdr->size, msg->reply_size, ret); } else { - ipc3_log_header(sdev->dev, "ipc tx succeeded", hdr->cmd); + if (sof_debug_check_flag(SOF_DBG_PRINT_IPC_SUCCESS_LOGS)) + ipc3_log_header(sdev->dev, "ipc tx succeeded", hdr->cmd); if (msg->reply_size) /* copy the data returned from DSP */ memcpy(reply_data, msg->reply_data, diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 1d0aeb4a7adc33..106f5d3f9e69b1 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -37,6 +37,12 @@ #define SOF_DBG_IGNORE_D3_PERSISTENT BIT(7) /* ignore the DSP D3 persistent capability * and always download firmware upon D3 exit */ +#define SOF_DBG_PRINT_DMA_POSITION_UPDATE_LOGS BIT(8) /* print DMA position updates + * in dmesg logs + */ +#define SOF_DBG_PRINT_IPC_SUCCESS_LOGS BIT(9) /* print IPC success + * in dmesg logs + */ /* Flag definitions used for controlling the DSP dump behavior */ #define SOF_DBG_DUMP_REGS BIT(0)