[PATCH 0/4] ASoC: SOF: Add 'memory_info' file to debugfs
Series that adds a new debugfs entry to query status of SOF FW memory allocation at runtime. Information on used and free memory is shown separately for each zone. A new IPC message is added for this purpose and ABI minor revision is bumped accordingly to 3.18.
To implement this, additional FW configuration data needs to be parsed from the firmware file and this is done in the first patch.
A few cleanups to surrounding code is done in separate patches.
Karol Trzcinski (4): ASoC: SOF: ext_manifest: Parse firmware config dictionary ASoC: SOF: Improve code alignment in header.h ASoC: SOF: Change section comment for SOF_IPC_TEST_ ASoC: SOF: Add `memory_info` file to debugfs
include/sound/sof/debug.h | 41 +++++++++++ include/sound/sof/ext_manifest.h | 20 ++++++ include/sound/sof/header.h | 10 ++- include/uapi/sound/sof/abi.h | 2 +- sound/soc/sof/debug.c | 117 +++++++++++++++++++++++++++++++ sound/soc/sof/ipc.c | 9 +++ sound/soc/sof/loader.c | 51 ++++++++++++++ sound/soc/sof/sof-priv.h | 2 + 8 files changed, 248 insertions(+), 4 deletions(-) create mode 100644 include/sound/sof/debug.h
From: Karol Trzcinski karolx.trzcinski@linux.intel.com
Values given in this dictionary describes used firmware configuration, like feature availability, buffer size limits and similar properties.
Signed-off-by: Karol Trzcinski karolx.trzcinski@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- include/sound/sof/ext_manifest.h | 19 +++++++++++++++ sound/soc/sof/loader.c | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+)
diff --git a/include/sound/sof/ext_manifest.h b/include/sound/sof/ext_manifest.h index 342e86e54db5..31da6e611c6e 100644 --- a/include/sound/sof/ext_manifest.h +++ b/include/sound/sof/ext_manifest.h @@ -61,6 +61,7 @@ enum sof_ext_man_elem_type { SOF_EXT_MAN_ELEM_WINDOW = SOF_IPC_EXT_WINDOW, SOF_EXT_MAN_ELEM_CC_VERSION = SOF_IPC_EXT_CC_INFO, SOF_EXT_MAN_ELEM_DBG_ABI = SOF_IPC_EXT_USER_ABI_INFO, + SOF_EXT_MAN_ELEM_CONFIG_DATA = 5, /**< ABI3.17 */ };
/* extended manifest element header */ @@ -99,4 +100,22 @@ struct ext_man_dbg_abi { struct sof_ipc_user_abi_version dbg_abi; } __packed;
+/* EXT_MAN_ELEM_CONFIG_DATA elements identificators, ABI3.17 */ +enum config_elem_type { + SOF_EXT_MAN_CONFIG_EMPTY = 0, + SOF_EXT_MAN_CONFIG_IPC_MSG_SIZE = 1, +}; + +struct sof_config_elem { + uint32_t token; + uint32_t value; +} __packed; + +/* firmware configuration information */ +struct sof_ext_man_config_data { + struct sof_ext_man_elem_header hdr; + + struct sof_config_elem elems[]; +} __packed; + #endif /* __SOF_FIRMWARE_EXT_MANIFEST_H__ */ diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c index ce68d708fc6f..33d3be774380 100644 --- a/sound/soc/sof/loader.c +++ b/sound/soc/sof/loader.c @@ -197,6 +197,44 @@ static int ext_man_get_dbg_abi_info(struct snd_sof_dev *sdev, return 0; }
+static int ext_man_get_config_data(struct snd_sof_dev *sdev, + const struct sof_ext_man_elem_header *hdr) +{ + const struct sof_ext_man_config_data *config = + container_of(hdr, struct sof_ext_man_config_data, hdr); + const struct sof_config_elem *elem; + int elems_counter; + int elems_size; + int i; + + /* calculate elements counter */ + elems_size = config->hdr.size - sizeof(struct sof_ext_man_elem_header); + elems_counter = elems_size / sizeof(struct sof_config_elem); + + dev_dbg(sdev->dev, "%s can hold up to %d config elements\n", + __func__, elems_counter); + + for (i = 0; i < elems_counter; ++i) { + elem = &config->elems[i]; + dev_dbg(sdev->dev, "%s get index %d token %d val %d\n", + __func__, i, elem->token, elem->value); + switch (elem->token) { + case SOF_EXT_MAN_CONFIG_EMPTY: + /* unused memory space is zero filled - mapped to EMPTY elements */ + break; + case SOF_EXT_MAN_CONFIG_IPC_MSG_SIZE: + /* TODO: use ipc msg size from config data */ + break; + default: + dev_info(sdev->dev, "Unknown firmware configuration token %d value %d", + elem->token, elem->value); + break; + } + } + + return 0; +} + static ssize_t snd_sof_ext_man_size(const struct firmware *fw) { const struct sof_ext_man_header *head; @@ -279,6 +317,9 @@ static int snd_sof_fw_ext_man_parse(struct snd_sof_dev *sdev, case SOF_EXT_MAN_ELEM_DBG_ABI: ret = ext_man_get_dbg_abi_info(sdev, elem_hdr); break; + case SOF_EXT_MAN_ELEM_CONFIG_DATA: + ret = ext_man_get_config_data(sdev, elem_hdr); + break; default: dev_info(sdev->dev, "unknown sof_ext_man header type %d size 0x%X\n", elem_hdr->type, elem_hdr->size);
From: Karol Trzcinski karolx.trzcinski@linux.intel.com
Each define value in series should be aligned and tabs should be used instead of spaces to follow code-style.
Signed-off-by: Karol Trzcinski karolx.trzcinski@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- include/sound/sof/header.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/sound/sof/header.h b/include/sound/sof/header.h index 571e1dd54b89..ece6d48dd007 100644 --- a/include/sound/sof/header.h +++ b/include/sound/sof/header.h @@ -49,7 +49,7 @@ #define SOF_IPC_FW_READY SOF_GLB_TYPE(0x7U) #define SOF_IPC_GLB_DAI_MSG SOF_GLB_TYPE(0x8U) #define SOF_IPC_GLB_TRACE_MSG SOF_GLB_TYPE(0x9U) -#define SOF_IPC_GLB_GDB_DEBUG SOF_GLB_TYPE(0xAU) +#define SOF_IPC_GLB_GDB_DEBUG SOF_GLB_TYPE(0xAU) #define SOF_IPC_GLB_TEST_MSG SOF_GLB_TYPE(0xBU) #define SOF_IPC_GLB_PROBE SOF_GLB_TYPE(0xCU)
@@ -109,7 +109,7 @@ #define SOF_IPC_PROBE_DMA_ADD SOF_CMD_TYPE(0x003) #define SOF_IPC_PROBE_DMA_INFO SOF_CMD_TYPE(0x004) #define SOF_IPC_PROBE_DMA_REMOVE SOF_CMD_TYPE(0x005) -#define SOF_IPC_PROBE_POINT_ADD SOF_CMD_TYPE(0x006) +#define SOF_IPC_PROBE_POINT_ADD SOF_CMD_TYPE(0x006) #define SOF_IPC_PROBE_POINT_INFO SOF_CMD_TYPE(0x007) #define SOF_IPC_PROBE_POINT_REMOVE SOF_CMD_TYPE(0x008)
@@ -119,7 +119,7 @@ #define SOF_IPC_TRACE_DMA_PARAMS_EXT SOF_CMD_TYPE(0x003)
/* debug */ -#define SOF_IPC_TEST_IPC_FLOOD SOF_CMD_TYPE(0x001) +#define SOF_IPC_TEST_IPC_FLOOD SOF_CMD_TYPE(0x001)
/* Get message component id */ #define SOF_IPC_MESSAGE_ID(x) ((x) & 0xffff)
From: Karol Trzcinski karolx.trzcinski@linux.intel.com
Section comment should be coherent with IPC prefix from define names.
Signed-off-by: Karol Trzcinski karolx.trzcinski@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- include/sound/sof/header.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/sound/sof/header.h b/include/sound/sof/header.h index ece6d48dd007..13256d4fb0dd 100644 --- a/include/sound/sof/header.h +++ b/include/sound/sof/header.h @@ -118,7 +118,7 @@ #define SOF_IPC_TRACE_DMA_POSITION SOF_CMD_TYPE(0x002) #define SOF_IPC_TRACE_DMA_PARAMS_EXT SOF_CMD_TYPE(0x003)
-/* debug */ +/* test */ #define SOF_IPC_TEST_IPC_FLOOD SOF_CMD_TYPE(0x001)
/* Get message component id */
From: Karol Trzcinski karolx.trzcinski@linux.intel.com
This file content describes memory allocation status at run-time, typically to detect memory leaks.
Signed-off-by: Karol Trzcinski karolx.trzcinski@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- include/sound/sof/debug.h | 41 +++++++++++ include/sound/sof/ext_manifest.h | 1 + include/sound/sof/header.h | 4 ++ include/uapi/sound/sof/abi.h | 2 +- sound/soc/sof/debug.c | 117 +++++++++++++++++++++++++++++++ sound/soc/sof/ipc.c | 9 +++ sound/soc/sof/loader.c | 10 +++ sound/soc/sof/sof-priv.h | 2 + 8 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 include/sound/sof/debug.h
diff --git a/include/sound/sof/debug.h b/include/sound/sof/debug.h new file mode 100644 index 000000000000..3ecb5793789d --- /dev/null +++ b/include/sound/sof/debug.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */ +/* + * This file is provided under a dual BSD/GPLv2 license. When using or + * redistributing this file, you may do so under either license. + * + * Copyright(c) 2020 Intel Corporation. All rights reserved. + * + * Author: Karol Trzcinski karolx.trzcinski@linux.intel.com + */ + +#ifndef __INCLUDE_SOUND_SOF_DEBUG_H__ +#define __INCLUDE_SOUND_SOF_DEBUG_H__ + +#include <sound/sof/header.h> + +/** ABI3.18 */ +enum sof_ipc_dbg_mem_zone { + SOF_IPC_MEM_ZONE_SYS = 0, /**< System zone */ + SOF_IPC_MEM_ZONE_SYS_RUNTIME = 1, /**< System-runtime zone */ + SOF_IPC_MEM_ZONE_RUNTIME = 2, /**< Runtime zone */ + SOF_IPC_MEM_ZONE_BUFFER = 3, /**< Buffer zone */ +}; + +/** ABI3.18 */ +struct sof_ipc_dbg_mem_usage_elem { + uint32_t zone; /**< see sof_ipc_dbg_mem_zone */ + uint32_t id; /**< heap index within zone */ + uint32_t used; /**< number of bytes used in zone */ + uint32_t free; /**< number of bytes free to use within zone */ + uint32_t reserved; /**< for future use */ +} __packed; + +/** ABI3.18 */ +struct sof_ipc_dbg_mem_usage { + struct sof_ipc_reply rhdr; /**< generic IPC reply header */ + uint32_t reserved[4]; /**< reserved for future use */ + uint32_t num_elems; /**< elems[] counter */ + struct sof_ipc_dbg_mem_usage_elem elems[]; /**< memory usage information */ +} __packed; + +#endif diff --git a/include/sound/sof/ext_manifest.h b/include/sound/sof/ext_manifest.h index 31da6e611c6e..e05cb21023e5 100644 --- a/include/sound/sof/ext_manifest.h +++ b/include/sound/sof/ext_manifest.h @@ -104,6 +104,7 @@ struct ext_man_dbg_abi { enum config_elem_type { SOF_EXT_MAN_CONFIG_EMPTY = 0, SOF_EXT_MAN_CONFIG_IPC_MSG_SIZE = 1, + SOF_EXT_MAN_CONFIG_MEMORY_USAGE_SCAN = 2, /**< ABI 3.18 */ };
struct sof_config_elem { diff --git a/include/sound/sof/header.h b/include/sound/sof/header.h index 13256d4fb0dd..c93f08334bbe 100644 --- a/include/sound/sof/header.h +++ b/include/sound/sof/header.h @@ -52,6 +52,7 @@ #define SOF_IPC_GLB_GDB_DEBUG SOF_GLB_TYPE(0xAU) #define SOF_IPC_GLB_TEST_MSG SOF_GLB_TYPE(0xBU) #define SOF_IPC_GLB_PROBE SOF_GLB_TYPE(0xCU) +#define SOF_IPC_GLB_DEBUG SOF_GLB_TYPE(0xDU)
/* * DSP Command Message Types @@ -118,6 +119,9 @@ #define SOF_IPC_TRACE_DMA_POSITION SOF_CMD_TYPE(0x002) #define SOF_IPC_TRACE_DMA_PARAMS_EXT SOF_CMD_TYPE(0x003)
+/* debug */ +#define SOF_IPC_DEBUG_MEM_USAGE SOF_CMD_TYPE(0x001) + /* test */ #define SOF_IPC_TEST_IPC_FLOOD SOF_CMD_TYPE(0x001)
diff --git a/include/uapi/sound/sof/abi.h b/include/uapi/sound/sof/abi.h index 6af32f82fb99..fe2cfae94b45 100644 --- a/include/uapi/sound/sof/abi.h +++ b/include/uapi/sound/sof/abi.h @@ -26,7 +26,7 @@
/* SOF ABI version major, minor and patch numbers */ #define SOF_ABI_MAJOR 3 -#define SOF_ABI_MINOR 17 +#define SOF_ABI_MINOR 18 #define SOF_ABI_PATCH 0
/* SOF ABI version number. Format within 32bit word is MMmmmppp */ diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c index 9419a99bab53..143117334ae5 100644 --- a/sound/soc/sof/debug.c +++ b/sound/soc/sof/debug.c @@ -14,6 +14,8 @@ #include <linux/debugfs.h> #include <linux/io.h> #include <linux/pm_runtime.h> +#include <sound/sof/ext_manifest.h> +#include <sound/sof/debug.h> #include "sof-priv.h" #include "ops.h"
@@ -626,6 +628,121 @@ int snd_sof_debugfs_buf_item(struct snd_sof_dev *sdev, } EXPORT_SYMBOL_GPL(snd_sof_debugfs_buf_item);
+static int memory_info_update(struct snd_sof_dev *sdev, char *buf, size_t buff_size) +{ + struct sof_ipc_cmd_hdr msg = { + .size = sizeof(struct sof_ipc_cmd_hdr), + .cmd = SOF_IPC_GLB_DEBUG | SOF_IPC_DEBUG_MEM_USAGE, + }; + struct sof_ipc_dbg_mem_usage *reply; + int len; + int ret; + int i; + + reply = kmalloc(SOF_IPC_MSG_MAX_SIZE, GFP_KERNEL); + if (!reply) + return -ENOMEM; + + ret = pm_runtime_get_sync(sdev->dev); + if (ret < 0 && ret != -EACCES) { + pm_runtime_put_noidle(sdev->dev); + dev_err(sdev->dev, "error: enabling device failed: %d\n", ret); + goto error; + } + + ret = sof_ipc_tx_message(sdev->ipc, msg.cmd, &msg, msg.size, reply, SOF_IPC_MSG_MAX_SIZE); + pm_runtime_mark_last_busy(sdev->dev); + pm_runtime_put_autosuspend(sdev->dev); + if (ret < 0 || reply->rhdr.error < 0) { + ret = min(ret, reply->rhdr.error); + dev_err(sdev->dev, "error: reading memory info failed, %d\n", ret); + goto error; + } + + if (struct_size(reply, elems, reply->num_elems) != reply->rhdr.hdr.size) { + dev_err(sdev->dev, "error: invalid memory info ipc struct size, %d\n", + reply->rhdr.hdr.size); + ret = -EINVAL; + goto error; + } + + for (i = 0, len = 0; i < reply->num_elems; i++) { + ret = snprintf(buf + len, buff_size - len, "zone %d.%d used %#8x free %#8x\n", + reply->elems[i].zone, reply->elems[i].id, + reply->elems[i].used, reply->elems[i].free); + if (ret < 0) + goto error; + len += ret; + } + + ret = len; +error: + kfree(reply); + return ret; +} + +static ssize_t memory_info_read(struct file *file, char __user *to, size_t count, loff_t *ppos) +{ + struct snd_sof_dfsentry *dfse = file->private_data; + struct snd_sof_dev *sdev = dfse->sdev; + int data_length; + + /* read memory info from FW only once for each file read */ + if (!*ppos) { + dfse->buf_data_size = 0; + data_length = memory_info_update(sdev, dfse->buf, dfse->size); + if (data_length < 0) + return data_length; + dfse->buf_data_size = data_length; + } + + return simple_read_from_buffer(to, count, ppos, dfse->buf, dfse->buf_data_size); +} + +static int memory_info_open(struct inode *inode, struct file *file) +{ + struct snd_sof_dfsentry *dfse = inode->i_private; + struct snd_sof_dev *sdev = dfse->sdev; + + file->private_data = dfse; + + /* allocate buffer memory only in first open run, to save memory when unused */ + if (!dfse->buf) { + dfse->buf = devm_kmalloc(sdev->dev, PAGE_SIZE, GFP_KERNEL); + if (!dfse->buf) + return -ENOMEM; + dfse->size = PAGE_SIZE; + } + + return 0; +} + +static const struct file_operations memory_info_fops = { + .open = memory_info_open, + .read = memory_info_read, + .llseek = default_llseek, +}; + +int snd_sof_dbg_memory_info_init(struct snd_sof_dev *sdev) +{ + struct snd_sof_dfsentry *dfse; + + dfse = devm_kzalloc(sdev->dev, sizeof(*dfse), GFP_KERNEL); + if (!dfse) + return -ENOMEM; + + /* don't allocate buffer before first usage, to save memory when unused */ + dfse->type = SOF_DFSENTRY_TYPE_BUF; + dfse->sdev = sdev; + + debugfs_create_file("memory_info", 0444, sdev->debugfs_root, dfse, &memory_info_fops); + + /* add to dfsentry list */ + list_add(&dfse->list, &sdev->dfsentry_list); + return 0; +} +EXPORT_SYMBOL_GPL(snd_sof_dbg_memory_info_init); + int snd_sof_dbg_init(struct snd_sof_dev *sdev) { const struct snd_sof_dsp_ops *ops = sof_ops(sdev); diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index fd2b96ae4943..fc13bb06dbf3 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -181,6 +181,15 @@ static void ipc_log_header(struct device *dev, u8 *text, u32 cmd) str2 = "unknown type"; break; } break; + case SOF_IPC_GLB_DEBUG: + str = "GLB_DEBUG"; + switch (type) { + case SOF_IPC_DEBUG_MEM_USAGE: + str2 = "MEM_USAGE"; break; + default: + str2 = "unknown type"; break; + } + break; default: str = "unknown GLB command"; break; } diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c index 33d3be774380..2a8c9bff9963 100644 --- a/sound/soc/sof/loader.c +++ b/sound/soc/sof/loader.c @@ -205,6 +205,7 @@ static int ext_man_get_config_data(struct snd_sof_dev *sdev, const struct sof_config_elem *elem; int elems_counter; int elems_size; + int ret = 0; int i;
/* calculate elements counter */ @@ -225,11 +226,20 @@ static int ext_man_get_config_data(struct snd_sof_dev *sdev, case SOF_EXT_MAN_CONFIG_IPC_MSG_SIZE: /* TODO: use ipc msg size from config data */ break; + case SOF_EXT_MAN_CONFIG_MEMORY_USAGE_SCAN: + if (sdev->first_boot && elem->value) + ret = snd_sof_dbg_memory_info_init(sdev); + break; default: dev_info(sdev->dev, "Unknown firmware configuration token %d value %d", elem->token, elem->value); break; } + if (ret < 0) { + dev_err(sdev->dev, "error: processing sof_ext_man_config_data failed for token %d value 0x%x, %d\n", + elem->token, elem->value, ret); + return ret; + } }
return 0; diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 0aed2a7ab858..d8bc0178dc89 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -290,6 +290,7 @@ enum sof_debugfs_access_type { /* FS entry for debug files that can expose DSP memories, registers */ struct snd_sof_dfsentry { size_t size; + size_t buf_data_size; /* length of buffered data for file read operation */ enum sof_dfsentry_type type; /* * access_type specifies if the @@ -523,6 +524,7 @@ void snd_sof_get_status(struct snd_sof_dev *sdev, u32 panic_code, void *stack, size_t stack_words); int snd_sof_init_trace_ipc(struct snd_sof_dev *sdev); void snd_sof_handle_fw_exception(struct snd_sof_dev *sdev); +int snd_sof_dbg_memory_info_init(struct snd_sof_dev *sdev);
/* * Platform specific ops.
On Tue, 24 Nov 2020 20:00:13 +0200, Kai Vehmanen wrote:
Series that adds a new debugfs entry to query status of SOF FW memory allocation at runtime. Information on used and free memory is shown separately for each zone. A new IPC message is added for this purpose and ABI minor revision is bumped accordingly to 3.18.
To implement this, additional FW configuration data needs to be parsed from the firmware file and this is done in the first patch.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: SOF: ext_manifest: Parse firmware config dictionary commit: 7f09f79d5cb13186b0f422cf2e1c711c88c92195 [2/4] ASoC: SOF: Improve code alignment in header.h commit: 2e4f3f9141cc626147eaf6fbb8a92f17fd069553 [3/4] ASoC: SOF: Change section comment for SOF_IPC_TEST_ commit: 6dd958955d3053a9c50353e39671622af4b8fba2 [4/4] ASoC: SOF: Add `memory_info` file to debugfs commit: 5b10b62989219aa527ee4fa555d1995a3b70981b
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)
-
Kai Vehmanen -
Mark Brown