Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Audio: basefw: Implement ipc4 modules info get #9032

Merged
merged 2 commits into from
May 2, 2024
Merged

Audio: basefw: Implement ipc4 modules info get #9032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d02a74a
ipc: Change ipc4 message max size
gbernatxintel Apr 15, 2024
c429510
Audio: basefw: Implement ipc4 modules info get
gbernatxintel Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions src/audio/base_fw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,11 @@ static int basefw_libraries_info_get(uint32_t *data_offset, char *data)
return 0;
}

static int basefw_modules_info_get(uint32_t *data_offset, char *data)
{
return platform_basefw_modules_info_get(data_offset, data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why it is offloaded to platform function?
What is the difference between the module info and library info (basefw_libraries_info_get()) to demand this handling?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have generally separated the function platform_basefw_modules_info_get(...) instead of implementing everything in base_fw.c because I use the ipc4_modules_info structure which uses sof_man_module.
sof_man_module is defined in rimage/manifest.h

I had a problem with the tests because on non-Intel platforms there is no manifest.h file and the tests did not pass. Besides, this modular approach follows the convention that we create code in base_fw_platform.c

With regard to the differences between basefw_modules_info_get and basefw_libraries_info_get

This basefw_libraries_info_get returns the currently loaded library and other structure.
basefw_modules_info_get returns which modules are loaded and which are not, along with information about them.

In short, both commands return a different data type, different structure. In preparing this PR I looked at previous code and tests that need specific information.

}

int schedulers_info_get(uint32_t *data_off_size,
char *data,
uint32_t core_id)
Expand Down Expand Up @@ -458,15 +463,15 @@ static int basefw_get_large_config(struct comp_dev *dev,
extended_param_id.part.parameter_instance);
case IPC4_PIPELINE_LIST_INFO_GET:
return basefw_pipeline_list_info_get(data_offset, data);
case IPC4_MODULES_INFO_GET:
return basefw_modules_info_get(data_offset, data);
case IPC4_LIBRARIES_INFO_GET:
return basefw_libraries_info_get(data_offset, data);
/* TODO: add more support */
case IPC4_DSP_RESOURCE_STATE:
case IPC4_NOTIFICATION_MASK:
case IPC4_MODULES_INFO_GET:
case IPC4_PIPELINE_PROPS_GET:
case IPC4_GATEWAYS_INFO_GET:
break;
case IPC4_LIBRARIES_INFO_GET:
return basefw_libraries_info_get(data_offset, data);
case IPC4_PERF_MEASUREMENTS_STATE:
case IPC4_GLOBAL_PERF_DATA:
COMPILER_FALLTHROUGH;
Expand Down
5 changes: 4 additions & 1 deletion src/include/ipc/header.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,11 @@ struct ipc_cmd_hdr;
#define SOF_IPC_MESSAGE_ID(x) ((x) & 0xffff)

/** Maximum message size for mailbox Tx/Rx */
#if CONFIG_IPC_MAJOR_4
#define SOF_IPC_MSG_MAX_SIZE 0x1000
#else
#define SOF_IPC_MSG_MAX_SIZE 384
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a separate commit and rationale for the increase. This affects all IPC3 implementations as well and increases memory use as this is directly used to allocate memory (which so far has been 384 bytes).

Or is this something only for IPC4. Then tihs could be changed only if built for IPC4.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gbernatxintel We can play safe here. e.g.

#if IPC4
#define SIZE 0x1000
#else
#define SIZE 384
#endif

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IPC fuzz failed only for IPC3 so please try this @gbernatxintel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ujfalusi @plbossart This SOF_IPC_MSG_MAX_SIZE is only used in IPC3 code, so if this change is ifdeffed like this, I don't think we need to bump the ABI ver

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kv2019i, the kernel uses different defines for the message size (in mailbox):
include/sound/sof/header.h:#define SOF_IPC_MSG_MAX_SIZE 384
include/sound/sof/ipc4/header.h:#define SOF_IPC4_MSG_MAX_SIZE 4096

sound/soc/sof/ipc3.c: sdev->ipc->max_payload_size = SOF_IPC_MSG_MAX_SIZE;
sound/soc/sof/ipc4.c: sdev->ipc->max_payload_size = SOF_IPC4_MSG_MAX_SIZE;


#endif
/** @} */

/**
Expand Down
9 changes: 9 additions & 0 deletions src/include/ipc4/base_fw_platform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ int platform_basefw_hw_config(uint32_t *data_offset, char *data);
*/
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
Expand Down
28 changes: 28 additions & 0 deletions src/platform/intel/ace/lib/base_fw_platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
#endif

#include <ipc4/base_fw.h>
#include <rimage/sof/user/manifest.h>

struct ipc4_modules_info {
uint32_t modules_count;
struct sof_man_module modules[0];
} __packed __aligned(4);

LOG_MODULE_REGISTER(basefw_platform, CONFIG_SOF_LOG_LEVEL);

Expand Down Expand Up @@ -72,6 +78,28 @@ 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)
{
struct ipc4_modules_info *const module_info = (struct ipc4_modules_info *)data;
struct sof_man_fw_desc *desc = platform_base_fw_get_manifest();

if (!desc)
return -EINVAL;

module_info->modules_count = desc->header.num_module_entries;

for (int idx = 0; idx < module_info->modules_count; ++idx) {
struct sof_man_module *module_entry =
(struct sof_man_module *)((char *)desc + SOF_MAN_MODULE_OFFSET(idx));
memcpy_s(&module_info->modules[idx], sizeof(module_info->modules[idx]),
module_entry, sizeof(struct sof_man_module));
}

*data_offset = sizeof(*module_info) +
module_info->modules_count * sizeof(module_info->modules[0]);
return 0;
}

/* There are two types of sram memory : high power mode sram and
* low power mode sram. This function retures memory size in page
* , memory bank power and usage status of each sram to host driver
Expand Down
7 changes: 7 additions & 0 deletions src/platform/posix/base_fw_platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ 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)
{
*data_offset = 0;

return 0;
}

int platform_basefw_get_large_config(struct comp_dev *dev,
uint32_t param_id,
bool first_block,
Expand Down
Loading