From: Kai Vehmanen kai.vehmanen@linux.intel.com

Move duplicated code in sof_wait_trace_avail() to a helper function.

Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/trace.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/sound/soc/sof/trace.c b/sound/soc/sof/trace.c index d588e4b70fad..a2d89d295f0f 100644 --- a/sound/soc/sof/trace.c +++ b/sound/soc/sof/trace.c @@ -13,10 +13,9 @@ #include "sof-priv.h" #include "ops.h"

-static size_t sof_wait_trace_avail(struct snd_sof_dev *sdev, - loff_t pos, size_t buffer_size) +static size_t sof_trace_avail(struct snd_sof_dev *sdev, + loff_t pos, size_t buffer_size) { - wait_queue_entry_t wait; loff_t host_offset = READ_ONCE(sdev->host_offset);

/* @@ -31,6 +30,19 @@ static size_t sof_wait_trace_avail(struct snd_sof_dev *sdev, if (host_offset > pos) return host_offset - pos;

+ return 0; +} + +static size_t sof_wait_trace_avail(struct snd_sof_dev *sdev, + loff_t pos, size_t buffer_size) +{ + wait_queue_entry_t wait; + size_t ret = sof_trace_avail(sdev, pos, buffer_size); + + /* data immediately available */ + if (ret) + return ret; + /* wait for available trace data from FW */ init_waitqueue_entry(&wait, current); set_current_state(TASK_INTERRUPTIBLE); @@ -42,12 +54,7 @@ static size_t sof_wait_trace_avail(struct snd_sof_dev *sdev, } remove_wait_queue(&sdev->trace_sleep, &wait);

- /* return bytes available for copy */ - host_offset = READ_ONCE(sdev->host_offset); - if (host_offset < pos) - return buffer_size - pos; - - return host_offset - pos; + return sof_trace_avail(sdev, pos, buffer_size); }

static ssize_t sof_dfsentry_trace_read(struct file *file, char __user *buffer,

From: Kai Vehmanen kai.vehmanen@linux.intel.com

Current trace implementation gets out of sync when sof device is put to suspend. The debugfs file handle is kept open, but firmware will reset its state. After resume, debugfs client's read offset will not be synchronized to firmware and this may result in traces read in incorrect order and/or stale data being read after resume.

Add logic to signal end-of-file to read() when firmware tracing has ended, and all trace data has been read. This allows debugfs client to capture all trace data, and reopen the trace file to ensure proper synchronization with firmware after reopening the node.

Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/sof-priv.h | 2 ++ sound/soc/sof/trace.c | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+)

diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 1e85d6f9c5c3..01a6219c326b 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -417,6 +417,8 @@ struct snd_sof_dev { u32 host_offset; u32 dtrace_is_enabled; u32 dtrace_error; + u32 dtrace_draining; + u32 msi_enabled;

void *private; /* core does not touch this */ diff --git a/sound/soc/sof/trace.c b/sound/soc/sof/trace.c index a2d89d295f0f..b02520f8e595 100644 --- a/sound/soc/sof/trace.c +++ b/sound/soc/sof/trace.c @@ -43,6 +43,15 @@ static size_t sof_wait_trace_avail(struct snd_sof_dev *sdev, if (ret) return ret;

+ if (!sdev->dtrace_is_enabled && sdev->dtrace_draining) { + /* + * tracing has ended and all traces have been + * read by client, return EOF + */ + sdev->dtrace_draining = false; + return 0; + } + /* wait for available trace data from FW */ init_waitqueue_entry(&wait, current); set_current_state(TASK_INTERRUPTIBLE); @@ -104,10 +113,23 @@ static ssize_t sof_dfsentry_trace_read(struct file *file, char __user *buffer, return count; }

+static int sof_dfsentry_trace_release(struct inode *inode, struct file *file) +{ + struct snd_sof_dfsentry *dfse = inode->i_private; + struct snd_sof_dev *sdev = dfse->sdev; + + /* avoid duplicate traces at next open */ + if (!sdev->dtrace_is_enabled) + sdev->host_offset = 0; + + return 0; +} + static const struct file_operations sof_dfs_trace_fops = { .open = simple_open, .read = sof_dfsentry_trace_read, .llseek = default_llseek, + .release = sof_dfsentry_trace_release, };

static int trace_debugfs_create(struct snd_sof_dev *sdev) @@ -155,6 +177,7 @@ int snd_sof_init_trace_ipc(struct snd_sof_dev *sdev) params.stream_tag = 0;

sdev->host_offset = 0; + sdev->dtrace_draining = false;

ret = snd_sof_dma_trace_init(sdev, &params.stream_tag); if (ret < 0) { @@ -291,6 +314,8 @@ void snd_sof_release_trace(struct snd_sof_dev *sdev) "error: fail in snd_sof_dma_trace_release %d\n", ret);

sdev->dtrace_is_enabled = false; + sdev->dtrace_draining = true; + wake_up(&sdev->trace_sleep); } EXPORT_SYMBOL(snd_sof_release_trace);

In a number of debug cases, the DMA-based trace can add problems (e.g. with HDaudio channel allocation). It also generates additional traffic on the bus and if the DMA handling is unreliable will prevent audio use-cases from working normally. Using the trace also requires tools to be installed on the target.

The trace can be instead handled as dynamic debug. We can use a Kconfig to force the trace to be enabled in all cases, or use a module parameter to enable it on a need-basis, e.g. by setting "options snd_sof sof_debug=0x1" in a /etc/modprobe.d file.

Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/Kconfig | 8 ++++++++ sound/soc/sof/core.c | 26 ++++++++++++++++++++------ sound/soc/sof/sof-priv.h | 1 + sound/soc/sof/trace.c | 17 ++++++++++++++++- 4 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index a9a1d502daae..b56b0a4982cd 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -131,6 +131,14 @@ config SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE Say Y if you want to enable caching the memory windows. If unsure, select "N".

+config SND_SOC_SOF_DEBUG_ENABLE_FIRMWARE_TRACE + bool "SOF enable firmware trace" + help + The firmware trace can be enabled either at build-time with + this option, or dynamically by setting flags in the SOF core + module parameter (similar to dynamic debug) + If unsure, select "N". + endif ## SND_SOC_SOF_DEBUG

endif ## SND_SOC_SOF_OPTIONS diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 5beda47cdf9f..70b471be07b2 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -16,6 +16,12 @@ #include "sof-priv.h" #include "ops.h"

+static int sof_core_debug; +module_param_named(sof_debug, sof_core_debug, int, 0444); +MODULE_PARM_DESC(sof_debug, "SOF core debug options (0x0 all off)"); + +#define SOF_CORE_ENABLE_TRACE BIT(0) + /* SOF defaults if not provided by the platform in ms */ #define TIMEOUT_DEFAULT_IPC_MS 5 #define TIMEOUT_DEFAULT_BOOT_MS 100 @@ -350,12 +356,20 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) goto fw_run_err; }

- /* init DMA trace */ - ret = snd_sof_init_trace(sdev); - if (ret < 0) { - /* non fatal */ - dev_warn(sdev->dev, - "warning: failed to initialize trace %d\n", ret); + if (IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_FIRMWARE_TRACE) || + (sof_core_debug & SOF_CORE_ENABLE_TRACE)) { + sdev->dtrace_is_supported = true; + + /* init DMA trace */ + ret = snd_sof_init_trace(sdev); + if (ret < 0) { + /* non fatal */ + dev_warn(sdev->dev, + "warning: failed to initialize trace %d\n", + ret); + } + } else { + dev_dbg(sdev->dev, "SOF firmware trace disabled\n"); }

/* hereafter all FW boot flows are for PM reasons */ diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 01a6219c326b..03d852293d27 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -415,6 +415,7 @@ struct snd_sof_dev { int dma_trace_pages; wait_queue_head_t trace_sleep; u32 host_offset; + u32 dtrace_is_supported; /* set with Kconfig or module parameter */ u32 dtrace_is_enabled; u32 dtrace_error; u32 dtrace_draining; diff --git a/sound/soc/sof/trace.c b/sound/soc/sof/trace.c index b02520f8e595..2613dd3e0dfc 100644 --- a/sound/soc/sof/trace.c +++ b/sound/soc/sof/trace.c @@ -165,6 +165,9 @@ int snd_sof_init_trace_ipc(struct snd_sof_dev *sdev) struct sof_ipc_reply ipc_reply; int ret;

+ if (!sdev->dtrace_is_supported) + return 0; + if (sdev->dtrace_is_enabled || !sdev->dma_trace_pages) return -EINVAL;

@@ -217,6 +220,9 @@ int snd_sof_init_trace(struct snd_sof_dev *sdev) { int ret;

+ if (!sdev->dtrace_is_supported) + return 0; + /* set false before start initialization */ sdev->dtrace_is_enabled = false;

@@ -272,6 +278,9 @@ EXPORT_SYMBOL(snd_sof_init_trace); int snd_sof_trace_update_pos(struct snd_sof_dev *sdev, struct sof_ipc_dma_trace_posn *posn) { + if (!sdev->dtrace_is_supported) + return 0; + if (sdev->dtrace_is_enabled && sdev->host_offset != posn->host_offset) { sdev->host_offset = posn->host_offset; wake_up(&sdev->trace_sleep); @@ -288,6 +297,9 @@ int snd_sof_trace_update_pos(struct snd_sof_dev *sdev, /* an error has occurred within the DSP that prevents further trace */ void snd_sof_trace_notify_for_error(struct snd_sof_dev *sdev) { + if (!sdev->dtrace_is_supported) + return; + if (sdev->dtrace_is_enabled) { dev_err(sdev->dev, "error: waking up any trace sleepers\n"); sdev->dtrace_error = true; @@ -300,7 +312,7 @@ void snd_sof_release_trace(struct snd_sof_dev *sdev) { int ret;

- if (!sdev->dtrace_is_enabled) + if (!sdev->dtrace_is_supported || !sdev->dtrace_is_enabled) return;

ret = snd_sof_dma_trace_trigger(sdev, SNDRV_PCM_TRIGGER_STOP); @@ -321,6 +333,9 @@ EXPORT_SYMBOL(snd_sof_release_trace);

void snd_sof_free_trace(struct snd_sof_dev *sdev) { + if (!sdev->dtrace_is_supported) + return; + snd_sof_release_trace(sdev);

snd_dma_free_pages(&sdev->dmatb);

Add debug option to disable pm_runtime. This is not intended for production devices but is very useful for platform bringup.

Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/sof-pci-dev.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c index e2b19782f01a..7c0ae5aee600 100644 --- a/sound/soc/sof/sof-pci-dev.c +++ b/sound/soc/sof/sof-pci-dev.c @@ -29,6 +29,12 @@ static char *tplg_path; module_param(tplg_path, charp, 0444); MODULE_PARM_DESC(tplg_path, "alternate path for SOF topology.");

+static int sof_pci_debug; +module_param_named(sof_debug, sof_pci_debug, int, 0444); +MODULE_PARM_DESC(sof_debug, "SOF PCI debug options (0x0 all off)"); + +#define SOF_PCI_DISABLE_PM_RUNTIME BIT(0) + #if IS_ENABLED(CONFIG_SND_SOC_SOF_APOLLOLAKE) static const struct sof_dev_desc bxt_desc = { .machines = snd_soc_acpi_intel_bxt_machines, @@ -213,6 +219,9 @@ static void sof_pci_probe_complete(struct device *dev) { dev_dbg(dev, "Completing SOF PCI probe");

+ if (sof_pci_debug & SOF_PCI_DISABLE_PM_RUNTIME) + return; + /* allow runtime_pm */ pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS); pm_runtime_use_autosuspend(dev); @@ -331,7 +340,8 @@ static void sof_pci_remove(struct pci_dev *pci) snd_sof_device_remove(&pci->dev);

/* follow recommendation in pci-driver.c to increment usage counter */ - pm_runtime_get_noresume(&pci->dev); + if (!(sof_pci_debug & SOF_PCI_DISABLE_PM_RUNTIME)) + pm_runtime_get_noresume(&pci->dev);

/* release pci regions and disable device */ pci_release_regions(pci);