sof icon indicating copy to clipboard operation
sof copied to clipboard

[FEATURE] Align the latest prepare/reset API flow update of module_adapter for 3P modules WAVES and DTS

Open johnylin76 opened this issue 2 years ago • 9 comments

Is your feature request related to a problem? Please describe. PR#6230, which modifies prepare/reset API definition for internal modules in module_adapter, has landed on mainstream recently. It's a critical fix of module_adapter and required for the issue of Google auto-test item: ALSAConformance. Therefore, PR#6230 is planned to back-port on branches supporting SOF firmware builds for Google projects.

** Describe the solution you'd like** (AI: @johnylin76 ) PR#6230 back-porting to the following branches:

  • [x] amd_rn_dts (and amd-rn-prod-001)
  • [x] mt8195/v0.4 (where it called codec_adapter)
  • [x] mt8186/v0.2
  • [x] adl-004-drop-stable (where it called codec_adapter) --> uprev to rpl-001-drop-stable
  • [x] tgl-012-drop-stable (where it called codec_adapter) --> commits have been back-ported, although similar to ADL that the adaptation on TGL Chrome devices is deferred to the branch uprev.

3P modules align the updated prepare/reset API of module_adapter (or codec_adapter). More details on Additional context:

  • [x] (AI: Waves) alignment of waves module
  • [x] (AI: Xperi) alignment of dts module

Additional context The main change by PR#6230 to be aligned is that module-specific prepare will be called every time on prepare of module_adapter (stream start trigger), which is only called once in the beginning before.

Regarding the flow of module-specfic API/state: Present:

API call: -------> init ------------> prepare -----> (*init_process) --> process... --> reset -----> (*init_process) --> process... --> reset -----> free
State:   DISABLED        INITIALIZED           IDLE                     PROCESSING             IDLE                     PROCESSING             IDLE

(*init_process is valid in codec_adapter only. It has been removed in module_adapter)

After PR#6230:

API call: -------> init ------------> prepare -----> (*init_process) --> process... --> reset ------------> prepare -----> (*init_process) --> process... --> reset ------------> free
State:   DISABLED        INITIALIZED           IDLE                     PROCESSING             INITIALIZED           IDLE                     PROCESSING             INITIALIZED

The change makes the module able to re-configure and re-allocate data buffers according to the stream parameter for the new stream at that time. Under this flow, the module should free data buffers on module-specific reset, while data buffers are retained at present flow.

For modules to align the updated API flow, there are two items to check:

  1. on module-specific reset, the module should be reset and runtime data buffers should be freed.
  2. runtime config application considering:
    • if config blob is set each time before stream start trigger (by UCM EnableSequence of SectionDevice in Google projects), then it should be fine with new flow.
    • if config blob is set during sound card initialization (by UCM EnableSequence of SectionVerb (initial sequence)), and the module requires the config to be applied again after reset, it will have problems because the config may not be set during the intermediate module reset and prepare. Moreover, the config buffer from module_adapter cannot be re-referenced which is freed after use.
      • module itself should cache the config data and shouldn't free the cache on module-specific reset.

Verification To verify the alignment to the updated API flow, run the command:

alsa_conformance_test -f S16_LE && alsa_conformance_test -f S24_LE

then wait at least 10 seconds, or reboot device, or suspend/resume, run the command:

alsa_conformance_test -f S24_LE && alsa_conformance_test -f S16_LE

check whether both of them are passed.

Note that the module (and the pipeline) will be released while going to power state S0ix, such as device suspend, reboot, or after the active streams are flushed and stopped. So && is used to run the test with S16 and S24 format continuously, where the module will be reset then prepare with varied stream format between two tests (not being released and re-created).

johnylin76 avatar Sep 25 '22 10:09 johnylin76

@johnylin76 fwiw, @mwasko will be converging the Intel CAVS 2.5 branches soon i.e. ADL, TGL and RPL. So this should make future alignments simpler.

lgirdwood avatar Sep 26 '22 10:09 lgirdwood

@ranj063 fyi

lgirdwood avatar Sep 26 '22 10:09 lgirdwood

@johnylin76 is this done?

cujomalainey avatar Dec 13 '22 01:12 cujomalainey

It's all done except for TGL devices. However since TGL is planned for rpl-001 migration as well as ADL and rpl-001 is done, we could also consider it's done.

johnylin76 avatar Jan 18 '23 04:01 johnylin76

@mwasko can you confirm changes are all done in correct branches and @johnylin76 does NOT need to do TGL as it's migrated to rpl-01 ?

lgirdwood avatar Jan 18 '23 15:01 lgirdwood

@mwasko can you confirm changes are all done in correct branches and @johnylin76 does NOT need to do TGL as it's migrated to rpl-01 ?

The maintenance branch for cAVS 2.5 targets (TGL, ADL, RPL) is prepared based on rpl-01, but we need to complete full validation including E2E testing. I am expecting that we can make official announcement after Chinese New Year when we have positive test results feedback - https://github.com/thesofproject/sof/tree/cavs2.5-001-drop-stable

mwasko avatar Jan 19 '23 08:01 mwasko

This is affecting the product branches, so removing v2.5 tag (released from SOF main).

kv2019i avatar Mar 06 '23 10:03 kv2019i

@kv2019i this might have implications for stable v2.2 if it not already patched

cujomalainey avatar Mar 06 '23 18:03 cujomalainey

@cujomalainey wrote:

@kv2019i this might have implications for stable v2.2 if it not already patched

Ack. We should backport to origin/stable-v2.2 as well. @mwasko can you check this? I see the patch is in cavs2.5-001-drop-stable already, so we should have the same in stable-v2.2 as well.

kv2019i avatar Mar 07 '23 18:03 kv2019i