ASoC: SOF: Intel: revisit module/namespace split for SoundWire/BPT support #4703
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
plbossart
force-pushed
the
hda/fix-module-split
branch
from
adfd3de to
2d18371
Compare
plbossart
requested review from
bardliao,
lgirdwood,
ranj063,
kv2019i,
dbaluta and
ujfalusi
as code owners
|
All module load/remove failed, let's retest now that thesofproject/sof-test#1130 is merged |
|
SOFCI TEST |
|
maybe one day we'll get test results in less than 5 days. Oh well, 'main-cavs' is shown as pending. |
|
SOFCI TEST |
1 similar comment
|
SOFCI TEST |
To avoid circular dependencies between SOF/Intel and SoundWire/Intel, we need to split the top-level hda.c from the rest of the code. This patch first regroups all SoundWire related code in hda.c. Signed-off-by: Pierre-Louis Bossart <[email protected]>
The existing code relies on the 'HDA_COMMON' module and namespace. We
need to start splitting top-level parts from the low-level ones,
otherwise we will not be able to reuse the low-level parts DMA support
for SoundWire/BPT.
In the end the dependencies will be:
+----------------------------------------------+
| |
| v
sof-pci-intel-xxx --> sof-intel-hda ------------> sof-hda-common
| ^
| |
+-> soundwire_intel --> sof_hda_sdw_bpt
This patch adds the initial split between the sof-pci-intel-xxx
modules and the common parts, in a follow-up patch we will further
split the HDA_COMMON parts
Since the PCI modules are not all independent, i.e. the CNL parts are
also used in JSL and TGL, additional Kconfig and namespace modules
were added.
Signed-off-by: Pierre-Louis Bossart <[email protected]>
To avoid circular dependencies when moving hda.c to a separate module, we need to move the common code to hda-ipc.c and hda-dsp.c No functionality change, just code move. Signed-off-by: Pierre-Louis Bossart <[email protected]>
CREATE_TRACEPOINTS is supposed to be used once. To avoid modpost issues when creating modules, let's move the tracepoint creation in a single object file. Signed-off-by: Pierre-Louis Bossart <[email protected]>
hda_sdw_process_wakeen() is used in hda-loader.c, but defined in hda.c. This code split will create a circular dependency when hda.c is moved to a different module. Rather than an invasive code change, this patch follows the model used for sdw_check_wakeen_irq() with an abstraction. For now all abstractions point to the same common routine, which is arguably not great, but this also provides us with a future-proof way of addressing platform-specific wake processing. Signed-off-by: Pierre-Louis Bossart <[email protected]>
Now that most of the code moves are done, we can add a new module and the required EXPORT_SYMBOL definitions. No functionality change, just a new module added. Signed-off-by: Pierre-Louis Bossart <[email protected]>
plbossart
force-pushed
the
hda/fix-module-split
branch
from
2d18371 to
ec6244b
Compare
plbossart
added a commit
to plbossart/sof-test
that referenced
this pull request
Nov 30, 2023
With thesofproject/linux#4703, TGL and ICL depend on CNL, and LNL on MTL. The order in which the modules are removed needs to respend those dependencies. Signed-off-by: Pierre-Louis Bossart <[email protected]>
plbossart
added a commit
to plbossart/sof-test
that referenced
this pull request
Nov 30, 2023
With thesofproject/linux#4703, TGL and ICL depend on CNL, and LNL on MTL. The order in which the modules are removed needs to respect those dependencies. Link: https://sof-ci.01.org/linuxpr/PR4703/build611/devicetest/index.html Signed-off-by: Pierre-Louis Bossart <[email protected]>
plbossart
added a commit
to thesofproject/sof-test
that referenced
this pull request
Nov 30, 2023
With thesofproject/linux#4703, TGL and ICL depend on CNL, and LNL on MTL. The order in which the modules are removed needs to respect those dependencies. Link: https://sof-ci.01.org/linuxpr/PR4703/build611/devicetest/index.html Signed-off-by: Pierre-Louis Bossart <[email protected]>
|
dependencies issues reported in https://sof-ci.01.org/linuxpr/PR4703/build610/devicetest/index.html, fixed now by thesofproject/sof-test#1133 |
|
SOFCI TEST |
|
I don't think CI took my changes https://github.com/thesofproject/sof-test/blob/main/tools/kmod/sof_remove.sh#L87 remove_module snd_sof_pci_intel_tgl What a mess. |
|
SOFCI TEST |
@plbossart I think this is because CI needs time to synchronize the changes to the storage gitweb after merging PR. |
|
All module tests are now passing now with updated sof-test and lots of waiting. @ujfalusi @bardliao @ranj063 do you mind reviewing, this needs to be merged soon and sent upstream since it's very invasive. We want enough time to fix any misses reported by randconfig and other bots on intel next. Thanks! |
bardliao
approved these changes
Dec 4, 2023
ranj063
approved these changes
Dec 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Unfortunately without a complete rework of module support, we will have a circular dependency:
depmod: ERROR: Cycle detected: soundwire_intel -> snd_sof_intel_hda_sdw_bpt -> snd_sof_intel_hda_common -> soundwire_intel
This might have been avoided if the SoundWire Intel parts were moved to ASoC/SOF...