From b10ec85ebd9e4764baa52370b492b2b1576bebae Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Tue, 7 May 2024 13:34:00 +0300 Subject: [PATCH] audio: base_fw: do not use platform interface for vendor extensions In commit 14c4e8675730 ("audio: base_fw: add platform layer to IPC4 hw_config data"), the platform specific code was moved to platform layer. This commit implements a lighter weight abstraction for the moved code. Instead of using the platform layer, the Intel specific vendor code is added directly in base_fw_intel.c and guarded by a Kconfig. All other IPC4 build targets will use an empty implementation. This avoids the need to add a platform definition for all IPC4 targets. The common implementation in base_fw.c is sufficient to cover all mandatory functionality required e.g. by the upstream SOF Linux driver's IPC4 implementation. The interfaces are renamed to refer to "vendor" instead of "platform", to avoid any confusion with the platform layer with the new implementation. Signed-off-by: Kai Vehmanen --- app/boards/intel_adsp_ace15_mtpm.conf | 1 + app/boards/intel_adsp_ace20_lnl.conf | 1 + app/boards/intel_adsp_cavs25.conf | 1 + app/boards/intel_adsp_cavs25_tgph.conf | 1 + src/audio/Kconfig | 8 + src/audio/base_fw.c | 16 +- .../base_fw_intel.c} | 36 ++--- src/include/ipc4/base_fw_platform.h | 80 ---------- src/include/ipc4/base_fw_vendor.h | 138 ++++++++++++++++++ zephyr/CMakeLists.txt | 7 +- 10 files changed, 180 insertions(+), 109 deletions(-) rename src/{platform/intel/ace/lib/base_fw_platform.c => audio/base_fw_intel.c} (91%) delete mode 100644 src/include/ipc4/base_fw_platform.h create mode 100644 src/include/ipc4/base_fw_vendor.h diff --git a/app/boards/intel_adsp_ace15_mtpm.conf b/app/boards/intel_adsp_ace15_mtpm.conf index 5923272fa986..c594f806c4b4 100644 --- a/app/boards/intel_adsp_ace15_mtpm.conf +++ b/app/boards/intel_adsp_ace15_mtpm.conf @@ -1,5 +1,6 @@ CONFIG_METEORLAKE=y CONFIG_IPC_MAJOR_4=y +CONFIG_IPC4_BASE_FW_INTEL=y CONFIG_COMP_SRC=y CONFIG_COMP_SRC_IPC4_FULL_MATRIX=y diff --git a/app/boards/intel_adsp_ace20_lnl.conf b/app/boards/intel_adsp_ace20_lnl.conf index 88a8e6522cfe..81bb35b47fca 100644 --- a/app/boards/intel_adsp_ace20_lnl.conf +++ b/app/boards/intel_adsp_ace20_lnl.conf @@ -1,5 +1,6 @@ CONFIG_LUNARLAKE=y CONFIG_IPC_MAJOR_4=y +CONFIG_IPC4_BASE_FW_INTEL=y CONFIG_COMP_SRC=y CONFIG_COMP_SRC_IPC4_FULL_MATRIX=y diff --git a/app/boards/intel_adsp_cavs25.conf b/app/boards/intel_adsp_cavs25.conf index 7905e68c78b1..ddf70ba9aa85 100644 --- a/app/boards/intel_adsp_cavs25.conf +++ b/app/boards/intel_adsp_cavs25.conf @@ -16,6 +16,7 @@ CONFIG_DEBUG_COREDUMP_MEMORY_DUMP_MIN=y CONFIG_TRACE=n CONFIG_IPC_MAJOR_4=y +CONFIG_IPC4_BASE_FW_INTEL=y CONFIG_RIMAGE_SIGNING_SCHEMA="tgl-cavs" CONFIG_PCM_CONVERTER_FORMAT_S16LE=y CONFIG_PCM_CONVERTER_FORMAT_S24LE=y diff --git a/app/boards/intel_adsp_cavs25_tgph.conf b/app/boards/intel_adsp_cavs25_tgph.conf index 6d8440d28298..58855d28183a 100644 --- a/app/boards/intel_adsp_cavs25_tgph.conf +++ b/app/boards/intel_adsp_cavs25_tgph.conf @@ -15,6 +15,7 @@ CONFIG_DEBUG_COREDUMP_MEMORY_DUMP_MIN=y CONFIG_TRACE=n CONFIG_IPC_MAJOR_4=y +CONFIG_IPC4_BASE_FW_INTEL=y CONFIG_RIMAGE_SIGNING_SCHEMA="tgl-cavs" CONFIG_PCM_CONVERTER_FORMAT_S16LE=y CONFIG_PCM_CONVERTER_FORMAT_S24LE=y diff --git a/src/audio/Kconfig b/src/audio/Kconfig index 7bd05c46f239..79f969026f78 100644 --- a/src/audio/Kconfig +++ b/src/audio/Kconfig @@ -12,6 +12,14 @@ config COMP_BASEFW_IPC4 help Select for BASEFW component +config IPC4_BASE_FW_INTEL + bool "BASEFW component Intel vendor extensions" + depends on COMP_BASEFW_IPC4 + help + Enable Intel vendor extensions for base firmware module. + This implements a set of additional IPC4 properties that + extend the base spec. + rsource "copier/Kconfig" config HOST_DMA_RELOAD_DELAY_ENABLE diff --git a/src/audio/base_fw.c b/src/audio/base_fw.c index 6dd56d5cd4a8..9ba6fc087606 100644 --- a/src/audio/base_fw.c +++ b/src/audio/base_fw.c @@ -8,7 +8,7 @@ #include #include #include -#include +#include #include #include #include @@ -121,7 +121,7 @@ static int basefw_config(uint32_t *data_offset, char *data) tuple = tlv_next(tuple); /* add platform specific tuples */ - platform_basefw_fw_config(&plat_data_offset, (char *)tuple); + basefw_vendor_fw_config(&plat_data_offset, (char *)tuple); *data_offset = (int)((char *)tuple - data) + plat_data_offset; @@ -152,7 +152,7 @@ static int basefw_hw_config(uint32_t *data_offset, char *data) tuple = tlv_next(tuple); /* add platform specific tuples */ - platform_basefw_hw_config(&plat_data_offset, (char *)tuple); + basefw_vendor_hw_config(&plat_data_offset, (char *)tuple); *data_offset = (int)((char *)tuple - data) + plat_data_offset; @@ -298,7 +298,7 @@ static int basefw_libraries_info_get(uint32_t *data_offset, char *data) for (int lib_id = 0; lib_id < LIB_MANAGER_MAX_LIBS; ++lib_id) { if (lib_id == 0) { - desc = platform_base_fw_get_manifest(); + desc = basefw_vendor_get_manifest(); } else { #if CONFIG_LIBRARY_MANAGER desc = (struct sof_man_fw_desc *)lib_manager_get_library_manifest(lib_id); @@ -336,7 +336,7 @@ static int basefw_libraries_info_get(uint32_t *data_offset, char *data) static int basefw_modules_info_get(uint32_t *data_offset, char *data) { - return platform_basefw_modules_info_get(data_offset, data); + return basefw_vendor_modules_info_get(data_offset, data); } int schedulers_info_get(uint32_t *data_off_size, @@ -479,7 +479,7 @@ static int basefw_get_large_config(struct comp_dev *dev, break; } - return platform_basefw_get_large_config(dev, param_id, first_block, last_block, + return basefw_vendor_get_large_config(dev, param_id, first_block, last_block, data_offset, data); }; @@ -507,8 +507,8 @@ static int basefw_set_large_config(struct comp_dev *dev, break; } - return platform_basefw_set_large_config(dev, param_id, first_block, last_block, - data_offset, data); + return basefw_vendor_set_large_config(dev, param_id, first_block, last_block, + data_offset, data); }; static const struct comp_driver comp_basefw = { diff --git a/src/platform/intel/ace/lib/base_fw_platform.c b/src/audio/base_fw_intel.c similarity index 91% rename from src/platform/intel/ace/lib/base_fw_platform.c rename to src/audio/base_fw_intel.c index 90f39e0362c7..c878a0003834 100644 --- a/src/platform/intel/ace/lib/base_fw_platform.c +++ b/src/audio/base_fw_intel.c @@ -27,9 +27,9 @@ struct ipc4_modules_info { struct sof_man_module modules[0]; } __packed __aligned(4); -LOG_MODULE_REGISTER(basefw_platform, CONFIG_SOF_LOG_LEVEL); +LOG_MODULE_REGISTER(basefw_intel, CONFIG_SOF_LOG_LEVEL); -int platform_basefw_fw_config(uint32_t *data_offset, char *data) +int basefw_vendor_fw_config(uint32_t *data_offset, char *data) { struct sof_tlv *tuple = (struct sof_tlv *)data; @@ -47,7 +47,7 @@ int platform_basefw_fw_config(uint32_t *data_offset, char *data) return 0; } -int platform_basefw_hw_config(uint32_t *data_offset, char *data) +int basefw_vendor_hw_config(uint32_t *data_offset, char *data) { struct sof_tlv *tuple = (struct sof_tlv *)data; uint32_t value; @@ -69,7 +69,7 @@ int platform_basefw_hw_config(uint32_t *data_offset, char *data) return 0; } -struct sof_man_fw_desc *platform_base_fw_get_manifest(void) +struct sof_man_fw_desc *basefw_vendor_get_manifest(void) { struct sof_man_fw_desc *desc; @@ -78,10 +78,10 @@ struct sof_man_fw_desc *platform_base_fw_get_manifest(void) return desc; } -int platform_basefw_modules_info_get(uint32_t *data_offset, char *data) +int basefw_vendor_modules_info_get(uint32_t *data_offset, char *data) { struct ipc4_modules_info *const module_info = (struct ipc4_modules_info *)data; - struct sof_man_fw_desc *desc = platform_base_fw_get_manifest(); + struct sof_man_fw_desc *desc = basefw_vendor_get_manifest(); if (!desc) return -EINVAL; @@ -228,12 +228,12 @@ static uint32_t basefw_get_ext_system_time(uint32_t *data_offset, char *data) return IPC4_UNAVAILABLE; } -int platform_basefw_get_large_config(struct comp_dev *dev, - uint32_t param_id, - bool first_block, - bool last_block, - uint32_t *data_offset, - char *data) +int basefw_vendor_get_large_config(struct comp_dev *dev, + uint32_t param_id, + bool first_block, + bool last_block, + uint32_t *data_offset, + char *data) { /* We can use extended param id for both extended and standard param id */ union ipc4_extended_param_id extended_param_id; @@ -298,12 +298,12 @@ static int basefw_set_fw_config(bool first_block, return 0; } -int platform_basefw_set_large_config(struct comp_dev *dev, - uint32_t param_id, - bool first_block, - bool last_block, - uint32_t data_offset, - const char *data) +int basefw_vendor_set_large_config(struct comp_dev *dev, + uint32_t param_id, + bool first_block, + bool last_block, + uint32_t data_offset, + const char *data) { switch (param_id) { case IPC4_FW_CONFIG: diff --git a/src/include/ipc4/base_fw_platform.h b/src/include/ipc4/base_fw_platform.h deleted file mode 100644 index a4694dde4708..000000000000 --- a/src/include/ipc4/base_fw_platform.h +++ /dev/null @@ -1,80 +0,0 @@ -/* SPDX-License-Identifier: BSD-3-Clause - * - * Copyright(c) 2024 Intel Corporation. - * - * Author: Kai Vehmanen - */ - -/** - * \file include/ipc4/base_fw_platform.h - * \brief Platform specific IPC4 base firmware functionality. - */ - -#ifndef __SOF_IPC4_BASE_FW_PLATFORM_H__ -#define __SOF_IPC4_BASE_FW_PLATFORM_H__ - -#include -#include -#include - -struct comp_dev; - -/** - * \brief Platform specific routine to add data tuples to FW_CONFIG - * structure sent to host via IPC. - * \param[out] data_offset data offset after tuples added - * \param[in] data pointer where to add new entries - * \return 0 if successful, error code otherwise. - */ -int platform_basefw_fw_config(uint32_t *data_offset, char *data); - -/** - * \brief Platform specific routine to add data tuples to HW_CONFIG - * structure sent to host via IPC. - * \param[out] data_offset data offset after tuples added - * \param[in] data pointer where to add new entries - * \return 0 if successful, error code otherwise. - */ -int platform_basefw_hw_config(uint32_t *data_offset, char *data); - -/** - * \brief Platform specific routine which return the pointer to - * the boot base manifest. - * \return pointer to struct if successful, null otherwise. - */ -struct sof_man_fw_desc *platform_base_fw_get_manifest(void); - -/** - * \brief Platform specific routine to get information about modules. - * Function add information and sent to host via IPC. - * \param[out] data_offset data offset after structure added - * \param[in] data pointer where to add new entries - * \return 0 if successful, error code otherwise. - */ -int platform_basefw_modules_info_get(uint32_t *data_offset, char *data); - -/** - * \brief Implement platform specific parameter for basefw module. - * This function is called for parameters not handled by - * generic base_fw code. - */ -int platform_basefw_get_large_config(struct comp_dev *dev, - uint32_t param_id, - bool first_block, - bool last_block, - uint32_t *data_offset, - char *data); - -/** - * \brief Implement platform specific parameter for basefw module. - * This function is called for parameters not handled by - * generic base_fw code. - */ -int platform_basefw_set_large_config(struct comp_dev *dev, - uint32_t param_id, - bool first_block, - bool last_block, - uint32_t data_offset, - const char *data); - -#endif /* __SOF_IPC4_BASE_FW_PLATFORM_H__ */ diff --git a/src/include/ipc4/base_fw_vendor.h b/src/include/ipc4/base_fw_vendor.h new file mode 100644 index 000000000000..4b02ad89c480 --- /dev/null +++ b/src/include/ipc4/base_fw_vendor.h @@ -0,0 +1,138 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * + * Copyright(c) 2024 Intel Corporation. + * + * Author: Kai Vehmanen + */ + +/** + * \file include/ipc4/base_fw_platform.h + * \brief Platform specific IPC4 base firmware functionality. + */ + +#ifndef __SOF_IPC4_BASE_FW_VENDOR_H__ +#define __SOF_IPC4_BASE_FW_VENDOR_H__ + +#include +#include +#include + +struct comp_dev; + +/* + * TODO: Add a more generic BASE_FW_VENDOR Kconfig if/when + * more vendor specific implementations are added. + */ +#ifdef CONFIG_IPC4_BASE_FW_INTEL + +/** + * \brief Vendor specific routine to add data tuples to FW_CONFIG + * structure sent to host via IPC. + * \param[out] data_offset data offset after tuples added + * \param[in] data pointer where to add new entries + * \return 0 if successful, error code otherwise. + */ +int basefw_vendor_fw_config(uint32_t *data_offset, char *data); + +/** + * \brief Vendor specific routine to add data tuples to HW_CONFIG + * structure sent to host via IPC. + * \param[out] data_offset data offset after tuples added + * \param[in] data pointer where to add new entries + * \return 0 if successful, error code otherwise. + */ +int basefw_vendor_hw_config(uint32_t *data_offset, char *data); + +/** + * \brief Vendor specific routine which return the pointer to + * the boot base manifest. + * \return pointer to struct if successful, null otherwise. + */ +struct sof_man_fw_desc *basefw_vendor_get_manifest(void); + +/** + * \brief Vendor specific routine to get information about modules. + * Function add information and sent to host via IPC. + * \param[out] data_offset data offset after structure added + * \param[in] data pointer where to add new entries + * \return 0 if successful, error code otherwise. + */ +int basefw_vendor_modules_info_get(uint32_t *data_offset, char *data); + +/** + * \brief Implement vendor specific parameter for basefw module. + * This function is called for parameters not handled by + * generic base_fw code. + */ +int basefw_vendor_get_large_config(struct comp_dev *dev, + uint32_t param_id, + bool first_block, + bool last_block, + uint32_t *data_offset, + char *data); + +/** + * \brief Implement vendor specific parameter for basefw module. + * This function is called for parameters not handled by + * generic base_fw code. + */ +int basefw_vendor_set_large_config(struct comp_dev *dev, + uint32_t param_id, + bool first_block, + bool last_block, + uint32_t data_offset, + const char *data); + +#else /* !CONFIG_IPC4_BASE_FW_INTEL */ + +static inline int basefw_vendor_fw_config(uint32_t *data_offset, char *data) +{ + *data_offset = 0; + + return 0; +} + +static inline int basefw_vendor_hw_config(uint32_t *data_offset, char *data) +{ + *data_offset = 0; + + return 0; +} + +static inline struct sof_man_fw_desc *basefw_vendor_get_manifest(void) +{ + struct sof_man_fw_desc *desc = NULL; + + return desc; +} + +static inline int basefw_vendor_modules_info_get(uint32_t *data_offset, char *data) +{ + *data_offset = 0; + + return 0; +} + +static inline int basefw_vendor_get_large_config(struct comp_dev *dev, + uint32_t param_id, + bool first_block, + bool last_block, + uint32_t *data_offset, + char *data) +{ + return -EINVAL; +} + +static inline int basefw_vendor_set_large_config(struct comp_dev *dev, + uint32_t param_id, + bool first_block, + bool last_block, + uint32_t data_offset, + const char *data) +{ + return IPC4_UNKNOWN_MESSAGE_TYPE; +} + +#endif + +#endif /* __SOF_IPC4_BASE_FW_VENDOR_H__ */ diff --git a/zephyr/CMakeLists.txt b/zephyr/CMakeLists.txt index e6889be1187b..e06df8954c61 100644 --- a/zephyr/CMakeLists.txt +++ b/zephyr/CMakeLists.txt @@ -163,7 +163,6 @@ if (CONFIG_SOC_SERIES_INTEL_CAVS_V25) # Platform sources zephyr_library_sources( ${SOF_PLATFORM_PATH}/intel/cavs/platform.c - ${SOF_PLATFORM_PATH}/intel/ace/lib/base_fw_platform.c ${SOF_PLATFORM_PATH}/tigerlake/lib/clk.c lib/pm_runtime.c lib/clk.c @@ -185,7 +184,6 @@ if (CONFIG_SOC_SERIES_INTEL_ADSP_ACE) # Platform sources zephyr_library_sources( ${SOF_PLATFORM_PATH}/intel/ace/platform.c - ${SOF_PLATFORM_PATH}/intel/ace/lib/base_fw_platform.c lib/pm_runtime.c lib/clk.c lib/dma.c @@ -335,7 +333,6 @@ zephyr_library_sources_ifdef(CONFIG_ZEPHYR_POSIX ${SOF_PLATFORM_PATH}/posix/dai.c ${SOF_PLATFORM_PATH}/posix/ipc.c ${SOF_PLATFORM_PATH}/posix/posix.c - ${SOF_PLATFORM_PATH}/posix/base_fw_platform.c ) zephyr_library_sources_ifdef(CONFIG_LIBRARY @@ -685,6 +682,10 @@ zephyr_library_sources_ifdef(CONFIG_COMP_BASEFW_IPC4 ${SOF_AUDIO_PATH}/base_fw.c ) +zephyr_library_sources_ifdef(CONFIG_IPC4_BASE_FW_INTEL + ${SOF_AUDIO_PATH}/base_fw_intel.c +) + zephyr_library_sources_ifdef(CONFIG_COMP_COPIER ${SOF_AUDIO_PATH}/copier/copier_generic.c ${SOF_AUDIO_PATH}/copier/copier_hifi.c