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.

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

Tools: Testbench: Convert file component to module adapter #8811

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/audio/pipeline/pipeline-stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ static int pipeline_comp_trigger(struct comp_dev *current,
* Initialization delay is only used with SSP, where we
* don't use more than one DAI per copier
*/
#if !CONFIG_LIBRARY
struct dai_data *dd;
#if CONFIG_IPC_MAJOR_3
dd = comp_get_drvdata(current);
Expand All @@ -383,6 +384,7 @@ static int pipeline_comp_trigger(struct comp_dev *current,
#error Unknown IPC major version
#endif
ppl_data->delay_ms = dai_get_init_delay_ms(dd->dai);
#endif
}
break;
default:
Expand Down
8 changes: 8 additions & 0 deletions src/include/ipc/topology.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,16 @@ struct sof_ipc_comp_process {

/* IPC file component used by testbench only */
struct sof_ipc_comp_file {
/* These need to be the same as in above sof_ipc_comp_process */
struct sof_ipc_comp comp;
struct sof_ipc_comp_config config;
uint32_t size; /**< size of bespoke data section in bytes */
uint32_t type; /**< sof_ipc_process_type */

/* reserved for future use */
uint32_t reserved[7];
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest put reserved to the bottom of struct definition.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did it this way since currently module adapter defaults to process type IPC unless I add exception handling to module adapter. The extra IPC data for file component is passed in the end of this struct.

I think I will try to another approach with CONFIG_LIBRARY exceptions added for SOF_COMP_FILEREAD and SOF_COMP_FILEWRITE. Not sure which is better.

Also this will be bit different with IPC4. I'm keeping this draft until I know better.


/* These are additional parameters for file */
uint32_t rate;
uint32_t channels;
char *fn;
Expand Down
1 change: 1 addition & 0 deletions src/include/sof/audio/ipc-config.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ struct ipc_config_process {

/* file IO ipc comp */
struct ipc_comp_file {
struct ipc_config_process module_header; /* Needed for module_adapter_init_data() */
uint32_t rate;
uint32_t channels;
char *fn;
Expand Down
18 changes: 10 additions & 8 deletions src/ipc/ipc3/helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,9 @@ static int comp_specific_builder(struct sof_ipc_comp *comp,
{
#if CONFIG_LIBRARY
struct sof_ipc_comp_file *file = (struct sof_ipc_comp_file *)comp;
#else
#endif
struct sof_ipc_comp_host *host = (struct sof_ipc_comp_host *)comp;
struct sof_ipc_comp_dai *dai = (struct sof_ipc_comp_dai *)comp;
#endif
struct sof_ipc_comp_volume *vol = (struct sof_ipc_comp_volume *)comp;
struct sof_ipc_comp_process *proc = (struct sof_ipc_comp_process *)comp;
struct sof_ipc_comp_src *src = (struct sof_ipc_comp_src *)comp;
Expand All @@ -211,18 +210,22 @@ static int comp_specific_builder(struct sof_ipc_comp *comp,
switch (comp->type) {
#if CONFIG_LIBRARY
/* test bench maps host and DAIs to a file */
case SOF_COMP_HOST:
case SOF_COMP_SG_HOST:
case SOF_COMP_DAI:
case SOF_COMP_SG_DAI:
case SOF_COMP_FILEREAD:
case SOF_COMP_FILEWRITE:
config->file.channels = file->channels;
config->file.fn = file->fn;
config->file.frame_fmt = file->frame_fmt;
config->file.mode = file->mode;
config->file.rate = file->rate;
config->file.direction = file->direction;

/* For module_adapter_init_data() ipc_module_adapter compatibility */
config->file.module_header.type = proc->type;
config->file.module_header.size = proc->size;
config->file.module_header.data = (uint8_t *)proc->data -
sizeof(struct ipc_config_process);
break;
#else
#endif
case SOF_COMP_HOST:
case SOF_COMP_SG_HOST:
config->host.direction = host->direction;
Expand All @@ -235,7 +238,6 @@ static int comp_specific_builder(struct sof_ipc_comp *comp,
config->dai.direction = dai->direction;
config->dai.type = dai->type;
break;
#endif
case SOF_COMP_VOLUME:
config->volume.channels = vol->channels;
config->volume.initial_ramp = vol->initial_ramp;
Expand Down
34 changes: 18 additions & 16 deletions tools/testbench/common_test.c
Original file line number Diff line number Diff line change
@@ -1,33 +1,35 @@
// SPDX-License-Identifier: BSD-3-Clause
//
// Copyright(c) 2018 Intel Corporation. All rights reserved.
// Copyright(c) 2018-2024 Intel Corporation. All rights reserved.

#include <stdint.h>
#include <stddef.h>
#include <time.h>
#include <stdio.h>
#include <stdlib.h>
#include <rtos/string.h>
#include <math.h>
#include <rtos/sof.h>
#include <rtos/task.h>
#include <rtos/alloc.h>
#include <sof/lib/notifier.h>
#include <sof/audio/component_ext.h>
#include <sof/audio/pipeline.h>
#include <sof/ipc/driver.h>
#include <sof/ipc/topology.h>
#include <sof/lib/agent.h>
#include <sof/lib/dai.h>
#include <sof/lib/dma.h>
#include <sof/lib/notifier.h>
#include <sof/schedule/edf_schedule.h>
#include <sof/schedule/ll_schedule.h>
#include <sof/schedule/ll_schedule_domain.h>
#include <sof/schedule/schedule.h>
#include <rtos/alloc.h>
#include <rtos/sof.h>
#include <rtos/string.h>
#include <rtos/task.h>
#include <rtos/wait.h>
#include <sof/audio/pipeline.h>
#include <sof/audio/component_ext.h>
#include <tplg_parser/topology.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. maybe it is a good time to check whether all the headers are all must to put it here.
  2. suggest a sequence for header include, from long to short.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm following a rule for alphabetic sort for 4 levels, 3 levels, 2 levels, standard C headers last. I got such suggestion some time ago in review. Though these could go to other cosmetic changes patch.

#include <math.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#include "testbench/common_test.h"
#include "testbench/trace.h"
#include <tplg_parser/topology.h>
#include "testbench/file.h"

#if defined __XCC__
#include <xtensa/tie/xt_timer.h>
Expand All @@ -43,7 +45,6 @@ int tb_setup(struct sof *sof, struct testbench_prm *tp)

/* init components */
sys_comp_init(sof);
sys_comp_file_init();
sys_comp_selector_init();

/* Module adapter components */
Expand All @@ -53,6 +54,7 @@ int tb_setup(struct sof *sof, struct testbench_prm *tp)
sys_comp_module_drc_interface_init();
sys_comp_module_eq_fir_interface_init();
sys_comp_module_eq_iir_interface_init();
sys_comp_module_file_interface_init();
sys_comp_module_google_rtc_audio_processing_interface_init();
sys_comp_module_igo_nr_interface_init();
sys_comp_module_multiband_drc_interface_init();
Expand Down
Loading
Loading