Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
A zero mask aborts loading before custom feature topologies can load, preventing the intended custom-only configuration.
What changed in this PR
This PR adds configurable function-topology selection to Intel SOF, aiming to let custom topologies supplied through feature_topologies replace standard pipelines.
Changes:
- Adds a read-only
function_tplg_maskmodule parameter, defaulting to all devices. - Applies the mask to SoundWire and I2S topology discovery with best-effort loading.
- Returns an error when an explicit mask yields no function topologies.
| File | Description |
|---|---|
| sound/soc/intel/common/sof-function-topology-lib.c | Adds mask-based function-topology selection and empty-selection checks. |
Review effort: Balanced
5878b6d to
01cbec4
Compare
| { | ||
| char *filename = NULL; | ||
| const struct firmware *fw; | ||
| char *variant_name = NULL; |
There was a problem hiding this comment.
char *variant_name __free(kfree) = NULL; ?
| filename = devm_kasprintf(dev, GFP_KERNEL, "%s/sof-%s-%s-id%d", | ||
| prefix, platform, tplg_dev_name, dai_link_id); | ||
| if (function_variant_count) | ||
| variant_name = devm_kasprintf(dev, GFP_KERNEL, |
There was a problem hiding this comment.
why do you use devm variant? The variant_name is only used locally.
| const struct firmware *fw; | ||
| char *variant_name = NULL; | ||
| char *tplg_filename; | ||
| char *filename; |
There was a problem hiding this comment.
char *filename __free(kfree) = NULL; and you save on free all around the place?
| break; | ||
| default: | ||
| filename = devm_kasprintf(dev, GFP_KERNEL, "%s/sof-%s-id%d.tplg", | ||
| filename = devm_kasprintf(dev, GFP_KERNEL, "%s/sof-%s-id%d", |
There was a problem hiding this comment.
only used locally, no need for devm_ ?
| int dai_link_id, int tplg_dev) | ||
| { | ||
| char *filename = NULL; | ||
| const struct firmware *fw; |
There was a problem hiding this comment.
you only use this under the if() within the loop, why not move it there?
| static int function_tplg_mask = -1; | ||
| module_param(function_tplg_mask, int, 0444); | ||
| MODULE_PARM_DESC(function_tplg_mask, | ||
| "Function topology selection mask, BIT(tplg_device_id); -1: all devices"); |
There was a problem hiding this comment.
why this is useful? Disabling BT on non Chrome products is one that I can think of, but other than that?
There was a problem hiding this comment.
Indeed, there is no strong reason to have it. I will drop the commit for now.
01cbec4 to
acc8a6c
Compare
While we can load feature topologies to add features to the generial function topology, we can not change the modules in the function topology. This commit allow selecting function topology variants through the feature_variant module parameter to use the variant function topology instead of the generic function topology. Fall back to the generic topology when a configured variant is unavailable. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
acc8a6c to
d111f14
Compare
A function topology provides a standard pipeline. Although feature topologies allow appending widgets, they do not support modifying or inserting widgets into the middle of the pipeline. To achieve this, users must deploy a customized topology.
Add support to bypass loading function topologies so that custom topologies can be loaded directly through the 'feature_topologies' module parameter.