-
Notifications
You must be signed in to change notification settings - Fork 321
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
LLEXT: fix failures and make DRC an LLEXT module by default on MTL #9116
Changes from all commits
167c303
d6c2b5a
1f3b74f
ebdab32
9cc1afb
ccd41ba
6e84874
6249a3b
7015939
95517f8
a529d61
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
CONFIG_SAMPLE_SMART_AMP=m | ||
CONFIG_COMP_DRC=m |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
CONFIG_SAMPLE_SMART_AMP=m | ||
CONFIG_COMP_MIXIN_MIXOUT=m | ||
CONFIG_COMP_IIR=m | ||
CONFIG_COMP_DRC=m |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
CONFIG_OUTPUT_DISASSEMBLY=y | ||
CONFIG_OUTPUT_DISASSEMBLY_WITH_SOURCE=n | ||
CONFIG_MODULES=n |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# Copyright (c) 2024 Intel Corporation. | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
# Hard-coded .text address to be moved to a common place | ||
sof_llext_build("drc" | ||
SOURCES ../drc.c | ||
../drc_generic.c | ||
../drc_math_generic.c | ||
../drc_hifi3.c | ||
../drc_hifi4.c | ||
../drc_math_hifi3.c | ||
TEXT_ADDR 0xa068a000 | ||
) |
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 "../drc.toml" | ||
|
||
[module] | ||
count = __COUNTER__ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,8 +100,6 @@ static int eq_iir_free(struct processing_module *mod) | |
{ | ||
struct comp_data *cd = module_get_private_data(mod); | ||
|
||
comp_info(mod->dev, "eq_iir_free()"); | ||
|
||
eq_iir_free_delaylines(cd); | ||
comp_data_blob_handler_free(cd->model_handler); | ||
|
||
|
@@ -234,8 +232,6 @@ static int eq_iir_reset(struct processing_module *mod) | |
struct comp_data *cd = module_get_private_data(mod); | ||
int i; | ||
|
||
comp_info(mod->dev, "eq_iir_reset()"); | ||
|
||
eq_iir_free_delaylines(cd); | ||
|
||
cd->eq_iir_func = NULL; | ||
|
@@ -271,7 +267,7 @@ SOF_MODULE_INIT(eq_iir, sys_comp_module_eq_iir_interface_init); | |
SOF_LLEXT_MOD_ENTRY(eq_iir, &eq_iir_interface); | ||
|
||
static const struct sof_man_module_manifest mod_manifest __section(".module") __used = | ||
SOF_LLEXT_MODULE_MANIFEST("EQIIR", eq_iir_llext_entry, 1, UUID_EQIIR); | ||
SOF_LLEXT_MODULE_MANIFEST("EQIIR", eq_iir_llext_entry, 1, UUID_EQIIR, 40); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we do a -1 i.e. dont care ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lgirdwood actually we do care now. Not because LLEXT modules need this, but because it's used by the infrastructure. We should remove this for LLEXT, but that should be coordinated with other loadable module implementations There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, lets circle back on this, we need to avoid artificial hard coded limitations There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lgirdwood this is where it's used and this is where it used to fail without these two commits: https://github.com/thesofproject/linux/blob/8245a1411fef45a208f182c176660107b7f582aa/sound/soc/sof/ipc4-topology.c#L1119 So we'd need to modify the driver to allow arbitrarily large instance IDs, or we can define and use a macro like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can as well use |
||
|
||
SOF_LLEXT_BUILDINFO; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -344,7 +344,15 @@ static int pipeline_calc_cps_consumption(struct comp_dev *current, | |
|
||
if (cd->cpc == 0) { | ||
/* Use maximum clock budget, assume 1ms chunk size */ | ||
ppl_data->kcps[comp_core] = CLK_MAX_CPU_HZ / 1000; | ||
uint32_t core_kcps = core_kcps_get(comp_core); | ||
|
||
if (!current->kcps_inc[comp_core]) { | ||
current->kcps_inc[comp_core] = core_kcps; | ||
ppl_data->kcps[comp_core] = CLK_MAX_CPU_HZ / 1000 - core_kcps; | ||
} else { | ||
ppl_data->kcps[comp_core] = core_kcps - current->kcps_inc[comp_core]; | ||
current->kcps_inc[comp_core] = 0; | ||
} | ||
tr_warn(pipe, | ||
"0 CPS requested for module: %#x, core: %d using safe max KCPS: %u", | ||
current->ipc_config.id, comp_core, ppl_data->kcps[comp_core]); | ||
|
@@ -367,6 +375,9 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) | |
{ | ||
int ret; | ||
#if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL | ||
/* FIXME: this must be a platform-specific parameter or a Kconfig option */ | ||
#define DSP_MIN_KCPS 50000 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no point in keeping KCPS feature with such limit because it means we cannot go lower than highest available clock which is not true because there are basic topologies which we can handle on 38.4. So why is this change required for llext module? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @abonislawski what do you mean "lower than highest available clock?" Our highest available clock is hundreds of MHz, i.e. hundreds of thousands of KCPS, isn't it? So this is definitely lower than that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. KCPS higher than 38400 will switch to our highest clock and DSP_MIN_KCPS is already higher than that. PRIMARY_CORE_BASE_CPS_USAGE is currently 20000 (higher than needed). |
||
|
||
struct pipeline_data data = { | ||
.start = p->source_comp, | ||
.p = p, | ||
|
@@ -431,8 +442,15 @@ int pipeline_trigger(struct pipeline *p, struct comp_dev *host, int cmd) | |
|
||
for (int i = 0; i < arch_num_cpus(); i++) { | ||
if (data.kcps[i] > 0) { | ||
uint32_t core_kcps = core_kcps_get(i); | ||
|
||
/* Tests showed, that we cannot go below 40000kcps on MTL */ | ||
if (data.kcps[i] > core_kcps - DSP_MIN_KCPS) | ||
data.kcps[i] = core_kcps - DSP_MIN_KCPS; | ||
|
||
core_kcps_adjust(i, -data.kcps[i]); | ||
tr_info(pipe, "Sum of KCPS consumption: %d, core: %d", core_kcps_get(i), i); | ||
tr_info(pipe, "Sum of KCPS consumption: %d, core: %d", | ||
core_kcps, i); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andyross is this needed with your changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's compatible (UUIDs aren't required to be registered), but yes, this should be added to the registry.