Debug module framework by marcinszkudlinski · Pull Request #9787 · thesofproject/sof · GitHub
Skip to content
Merged
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
1 change: 1 addition & 0 deletions app/boards/intel_adsp_ace15_mtpm.conf
1 change: 1 addition & 0 deletions app/boards/intel_adsp_ace20_lnl.conf
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ CONFIG_COMP_ARIA=y
CONFIG_COMP_CHAIN_DMA=y
CONFIG_COMP_DRC=m
CONFIG_COMP_KPB=y
CONFIG_COMP_TESTER=m
CONFIG_COMP_SRC_IPC4_FULL_MATRIX=y
CONFIG_COMP_UP_DOWN_MIXER=y
CONFIG_COMP_VOLUME_WINDOWS_FADE=y
Expand Down
1 change: 1 addition & 0 deletions app/boards/intel_adsp_ace30_ptl.conf
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ CONFIG_COMP_ARIA=y
CONFIG_COMP_CHAIN_DMA=y
CONFIG_COMP_DRC=y
CONFIG_COMP_KPB=y
CONFIG_COMP_TESTER=m
Comment thread
kv2019i marked this conversation as resolved.
CONFIG_COMP_SRC_IPC4_FULL_MATRIX=y
CONFIG_COMP_UP_DOWN_MIXER=y
CONFIG_COMP_VOLUME_WINDOWS_FADE=y
Expand Down
1 change: 1 addition & 0 deletions app/configs/mtl/modules.conf
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ CONFIG_COMP_CROSSOVER=m
CONFIG_COMP_MULTIBAND_DRC=m
CONFIG_COMP_GOOGLE_CTC_AUDIO_PROCESSING=m
CONFIG_COMP_GOOGLE_RTC_AUDIO_PROCESSING=m
CONFIG_COMP_TESTER=m

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It would be nice to have the common non-intel changes in one commit and these Intel board changes in a separate.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@marcinszkudlinski Not a blocking thing, but this board file change is still in the same commit as the tester module implementation. So a comment would be nice if you just prefer not to do this.

2 changes: 2 additions & 0 deletions src/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,5 @@ rsource "library_manager/Kconfig"
rsource "debug/telemetry/Kconfig"

rsource "debug/debug_stream/Kconfig"

rsource "debug/tester/Kconfig"
1 change: 1 addition & 0 deletions src/audio/module_adapter/module_adapter_ipc4.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ int module_adapter_set_state(struct processing_module *mod, struct comp_dev *dev
{
return comp_set_state(dev, cmd);
}
EXPORT_SYMBOL(module_adapter_set_state);

int module_set_large_config(struct comp_dev *dev, uint32_t param_id, bool first_block,
bool last_block, uint32_t data_offset_size, const char *data)
Expand Down
60 changes: 60 additions & 0 deletions src/audio/sink_source_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,63 @@ int source_to_sink_copy(struct sof_source *source,
return 0;
}
EXPORT_SYMBOL(source_to_sink_copy);

int sink_fill_with_silence(struct sof_sink *sink, size_t size)
{
uint8_t *dst_ptr;
uint8_t *dst_begin;
uint8_t *dst_end;
size_t dst_size;
int ret;

if (!size)
return 0;
if (size > sink_get_free_size(sink))
return -ENOSPC;

Comment thread
dbaluta marked this conversation as resolved.
ret = sink_get_buffer(sink, size, (void **)&dst_ptr, (void **)&dst_begin, &dst_size);
if (ret)
Comment thread
marcinszkudlinski marked this conversation as resolved.
return ret;

dst_end = dst_begin + dst_size;
while (size) {
uint32_t dst_to_buf_overlap = (uintptr_t)dst_end - (uintptr_t)dst_ptr;
uint32_t to_fill = MIN(dst_to_buf_overlap, size);

ret = memset_s(dst_ptr, dst_to_buf_overlap, 0, to_fill);
assert(!ret);

size -= to_fill;
dst_ptr += to_fill;
if (to_fill == dst_to_buf_overlap)
dst_ptr = dst_begin;
}

sink_commit_buffer(sink, INT_MAX);
return 0;
}
EXPORT_SYMBOL(sink_fill_with_silence);

int source_drop_data(struct sof_source *source, size_t size)
{
uint8_t const *src_ptr;
uint8_t const *src_begin;
size_t src_size;
int ret;

if (!size)
return 0;
if (size > source_get_data_available(source))
return -EFBIG;
Comment thread
marcinszkudlinski marked this conversation as resolved.

ret = source_get_data(source, size,
(void const **)&src_ptr,
(void const **)&src_begin,
&src_size);
if (ret)
return ret;

source_release_data(source, INT_MAX);
return 0;
}
EXPORT_SYMBOL(source_drop_data);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please place each EXPORT_SYMBOL() under respective function

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@marcinszkudlinski let's keep "unresolved" until actually resolved or at least explained why you the existing version is preferred

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well, I see it has been moved, it is now at the end of source_drop_data function, isn't it?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@marcinszkudlinski sorry for not explaining clearly. Every EXPORT_SYMBOL() should immediately follow the respective function. Like

void f1()
{
}
EXPORT_SYMBOL(f1);

void f2()
{
}
EXPORT_SYMBOL(f2);

etc. See https://github.com/thesofproject/sof/actions/runs/12891822335/job/35944645332?pr=9787 :

WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
#87: FILE: src/audio/sink_source_utils.c:134:
+EXPORT_SYMBOL(sink_fill_with_silence);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confusing, because I fixed it and in the PR it is exactly as you wrote :)
...except one detail, I put the fix in wrong commit,
ok, will fix

4 changes: 4 additions & 0 deletions src/debug/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ if(CONFIG_GDB_DEBUG)
add_subdirectory(gdb)
endif()

if(CONFIG_COMP_TESTER)
add_subdirectory(tester)
endif()

add_local_sources(sof panic.c)
2 changes: 2 additions & 0 deletions src/debug/tester/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
add_local_sources(tester.c)
add_local_sources(tester_dummy_test.c)
10 changes: 10 additions & 0 deletions src/debug/tester/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# SPDX-License-Identifier: BSD-3-Clause

config COMP_TESTER
tristate "tester module"
default n
depends on IPC_MAJOR_4
help
Build a tester module. To be used in continuous
integration process for special test cases that
require a test code injected into the system itself
7 changes: 7 additions & 0 deletions src/debug/tester/llext/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Copyright (c) 2024 Intel Corporation.
# SPDX-License-Identifier: Apache-2.0

sof_llext_build("tester"
SOURCES ../tester.c
../tester_dummy_test.c
)
6 changes: 6 additions & 0 deletions src/debug/tester/llext/llext.toml.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <tools/rimage/config/platform.toml>
#define LOAD_TYPE "2"
#include "../tester.toml"

[module]
count = __COUNTER__
252 changes: 252 additions & 0 deletions src/debug/tester/tester.c
Loading