thesofproject / sof

Sound Open Firmware
Other
541 stars 311 forks source link

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

Open johnylin76 opened 2 years ago

johnylin76 commented 2 years ago

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:

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

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).

lgirdwood commented 2 years ago

@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 commented 2 years ago

@ranj063 fyi

cujomalainey commented 1 year ago

@johnylin76 is this done?

johnylin76 commented 1 year ago

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.

lgirdwood commented 1 year ago

@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 ?

mwasko commented 1 year ago

@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

kv2019i commented 1 year ago

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

cujomalainey commented 1 year ago

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

kv2019i commented 1 year ago

@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.