Skip to content

module_adapter : audio : fix DTS support on MTL #8379

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

Merged
cujomalainey merged 7 commits into from
Nov 14, 2023
Merged

module_adapter : audio : fix DTS support on MTL #8379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9074a05
dts : add LOG_MODULE_REGISTER()
joechengxperi Oct 25, 2023
d0b1e70
dts : fix set_configuration() fail on MTL
joechengxperi Oct 25, 2023
f07d601
dts : add more error logs in set_configuration()
joechengxperi Oct 25, 2023
8bb4a05
dts : update DTS version to V1.1.3
joechengxperi Oct 25, 2023
04991be
audio : dts : add DTS support for MTL
joechengxperi Nov 1, 2023
7d5cc30
zephyr : import dts library
joechengxperi Oct 3, 2023
ac2f487
app: overlays: Add DTS support to MTL
joechengxperi Oct 2, 2023
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
3 changes: 3 additions & 0 deletions app/overlays/mtl/dts_overlay.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
CONFIG_COMP_IIR=y
Copy link
Contributor

@btian1 btian1 Oct 25, 2023

Choose a reason for hiding this comment

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

I noticed this may be followed from:
./src/arch/xtensa/configs/override/mt8195_chrome_dts.config:4:CONFIG_DTS_CODEC=y
./src/arch/xtensa/configs/override/renoir_chrome_dts.config:3:CONFIG_DTS_CODEC=y
./src/arch/xtensa/configs/override/mt8186_chrome_dts.config:3:CONFIG_DTS_CODEC=y

my question is: is IIR must follow with DTS codec? they should have no relation from module perspecitve.

Copy link
Contributor Author

@joechengxperi joechengxperi Oct 25, 2023

Choose a reason for hiding this comment

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

We use EQ IIR + DTS to support customer project. For different platforms, EQ IIR might be default enabled or disabled. So we add it to make sure IIR is enabled. On MTL, IIR is default enabled. I could remove it if you think it's better.

Copy link
Member

@lgirdwood lgirdwood Oct 25, 2023

Choose a reason for hiding this comment

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

So the Kconfig can set a dependecy, we could use depends in Kconfig to state this relationship between DTS and IIR.
@singalsu fyi.

CONFIG_COMP_MODULE_ADAPTER=y
CONFIG_DTS_CODEC=y
22 changes: 18 additions & 4 deletions src/audio/module_adapter/module/dts/dts.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <sof/audio/dts/DtsSofInterface.h>


LOG_MODULE_REGISTER(dts, CONFIG_SOF_LOG_LEVEL);

/* d95fc34f-370f-4ac7-bc86-bfdc5be241e6 */
DECLARE_SOF_RT_UUID("dts_codec", dts_uuid, 0xd95fc34f, 0x370f, 0x4ac7,
0xbc, 0x86, 0xbf, 0xdc, 0x5b, 0xe2, 0x41, 0xe6);
Expand Down Expand Up @@ -425,13 +427,25 @@ dts_codec_set_configuration(struct processing_module *mod, uint32_t config_id,

ret = module_set_configuration(mod, config_id, pos, data_offset_size, fragment,
fragment_size, response, response_size);
if (ret < 0)
if (ret < 0) {
Copy link
Collaborator

@lyakh lyakh Oct 30, 2023

Choose a reason for hiding this comment

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

yes, both braces must be in the same commit to avoid breaking bisection

Copy link
Contributor Author

@joechengxperi joechengxperi Oct 30, 2023

Choose a reason for hiding this comment

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

Done. Put them in the same commit now.

comp_err(dev, "dts_codec_set_configuration(): error %x from module_set_configuration()",
ret);
return ret;
}
Copy link
Collaborator

@lyakh lyakh Oct 30, 2023

Choose a reason for hiding this comment

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

is this removed in a later commit again?


/* return if more fragments are expected or if the module is not prepared */
if ((pos != MODULE_CFG_FRAGMENT_LAST && pos != MODULE_CFG_FRAGMENT_SINGLE) ||
md->state < MODULE_INITIALIZED)
/* return if more fragments are expected */
if (pos != MODULE_CFG_FRAGMENT_LAST && pos != MODULE_CFG_FRAGMENT_SINGLE) {
comp_err(dev, "dts_codec_set_configuration(): pos %d error", pos);
return 0;
}

#if CONFIG_IPC_MAJOR_3
// return if the module is not prepared
if (md->state < MODULE_INITIALIZED) {
comp_err(dev, "dts_codec_set_configuration(): state %d error", md->state);
return 0;
}
#endif

/* whole configuration received, apply it now */
ret = dts_codec_apply_config(mod);
Expand Down
1 change: 1 addition & 0 deletions src/include/sof/audio/component.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,7 @@ void sys_comp_module_crossover_interface_init(void);
void sys_comp_module_dcblock_interface_init(void);
void sys_comp_module_demux_interface_init(void);
void sys_comp_module_drc_interface_init(void);
void sys_comp_module_dts_interface_init(void);
void sys_comp_module_eq_fir_interface_init(void);
void sys_comp_module_eq_iir_interface_init(void);
void sys_comp_module_mfcc_interface_init(void);
Expand Down
6 changes: 3 additions & 3 deletions src/include/sof/audio/dts/DtsSofInterfaceVersion.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@

#define DTS_SOF_INTERFACE_PRODUCT_NAME "DtsSofInterface"
#define DTS_SOF_INTERFACE_PRODUCT_VERSION_MAJOR 1
#define DTS_SOF_INTERFACE_PRODUCT_VERSION_MINOR 0
#define DTS_SOF_INTERFACE_PRODUCT_VERSION_PATCH 7
#define DTS_SOF_INTERFACE_PRODUCT_VERSION_MINOR 1
#define DTS_SOF_INTERFACE_PRODUCT_VERSION_PATCH 3
#define DTS_SOF_INTERFACE_PRODUCT_VERSION_BUILD 0
#define DTS_SOF_INTERFACE_PRODUCT_VERSION_EXTRA "Dev"
#define DTS_SOF_INTERFACE_PRODUCT_VERSION_EXTRA "RC2"

#if defined(__cplusplus)
extern "C" {
Expand Down
13 changes: 9 additions & 4 deletions zephyr/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,10 +711,15 @@ zephyr_library_sources_ifdef(CONFIG_COMP_GOOGLE_HOTWORD_DETECT
zephyr_library_sources_ifdef(CONFIG_DTS_CODEC
${SOF_AUDIO_PATH}/module_adapter/module/dts/dts.c
)

zephyr_library_sources_ifdef(CONFIG_DTS_CODEC_STUB
${SOF_AUDIO_PATH}/module_adapter/module/dts/dts_stub.c
)
if (CONFIG_DTS_CODEC)
Copy link
Contributor

@cujomalainey cujomalainey Nov 13, 2023

Choose a reason for hiding this comment

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

This broke the stub build. Please fix

Copy link
Collaborator

@marc-hb marc-hb Nov 13, 2023

Choose a reason for hiding this comment

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

This broke the stub build. Please fix

@joechengxperi it is the responsibility of the submitter to look at failures, not the reviewers. Ask for help if you don't understand some failures (or worse: if you don't have access to some)

https://github.com/thesofproject/sof/actions/runs/6718130608/job/18257259780?pr=8379

The last error messages can be misleading. Always scroll up and look at the first errors first. Parallel builds may require scrolling quite a way up.

Copy link
Contributor Author

@joechengxperi joechengxperi Nov 14, 2023

Choose a reason for hiding this comment

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

Fixed. Apologize that I did not notice that build error.

if (CONFIG_DTS_CODEC_STUB)
zephyr_library_sources(
${SOF_AUDIO_PATH}/module_adapter/module/dts/dts_stub.c
)
else()
zephyr_library_import(DtsCodec ${SOF_AUDIO_PATH}/module_adapter/lib/release/libdts-sof-interface-i32.a)
endif()
endif()

zephyr_library_sources_ifdef(CONFIG_WAVES_CODEC
${SOF_AUDIO_PATH}/module_adapter/module/waves/waves.c
Expand Down