Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[BUG][Zephyr] assert hit in sof-test run with dmic or hda DAIs #6896

Closed
kv2019i opened this issue Dec 30, 2022 · 1 comment
Closed

[BUG][Zephyr] assert hit in sof-test run with dmic or hda DAIs #6896

kv2019i opened this issue Dec 30, 2022 · 1 comment
Assignees
@kv2019i
Labels
bug Something isn't working as expected P2 Critical bugs or normal features Zephyr Issues only observed with Zephyr integrated
Milestone
v2.5

Comments

@kv2019i
Copy link
Collaborator

kv2019i commented Dec 30, 2022

Describe the bug
When CONFIG_ASSERT=y is enabled in build (e.g. with #6530), assert is hit in multiple test cases with sof-test and on multiple platforms.

To Reproduce
Apply #6530 , run sof-test.

Reproduction Rate
100%

Expected behavior
Asserts should not be hit!

Impact
Unable to enable CONFIG_ASSERT in CI by default due to many failures.

Environment

  1. Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).
    • Kernel: e3707e4dd275bc731f6ed4067a2694534675ca52 (sof-dev of today, "ASoC: Intel: soc-acpi: add configuration for variant of 0C11 product")
    • SOF: 1447444 ("topology2: cavs-nocodec add wov support")
  2. Name of the topology file
    • Topology: sof-hda-generic.tplg (but based on SOF Ci results affects also many other topologies)
  3. Name of the platform(s) on which the bug is observed.
    • Platform: Intel TGL, ADL, MTL

Screenshots or console output
@kv2019i kv2019i added bug Something isn't working as expected P2 Critical bugs or normal features Zephyr Issues only observed with Zephyr integrated labels Dec 30, 2022
@kv2019i kv2019i self-assigned this Dec 30, 2022
kv2019i added a commit to kv2019i/sof that referenced this issue Dec 30, 2022
@kv2019i
dai-zephyr: handle NULL case when calling dai_config_get()
2658389 
As per interface documentation of dai_config_get(), the function
may return NULL if the device is not configured for the requested
direction. Handle this case explicitly.

Currently no upstream Zephyr driver returns NULL, but e.g. dmic
driver returns invalid configuration when dai_config_get() is
called with DAI_DIR_TX as direction.

Link: thesofproject#6896
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i mentioned this issue Dec 30, 2022
Merged
kv2019i added a commit to kv2019i/zephyr that referenced this issue Dec 30, 2022
@kv2019i
drivers: dai: intel: dmic: don't use assert for error handling
96366a3 
The dai.h interface does not prohibit calling dai_config_get()
with different direction values. The dmic driver should handle
invalid direction value explicitly and not rely on an assert.

Link: thesofproject/sof#6896
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i mentioned this issue Dec 30, 2022
Merged
kv2019i added a commit to kv2019i/zephyr that referenced this issue Dec 30, 2022
@kv2019i
intel_adsp: ace: power: fix build error with asserts enabled
60d11c9 
If defined(CONFIG_ASSERT) and !defined(CONFIG_ADSP_IMR_CONTEXT_SAVE),
build will fail as symbol "global_imr_ram_storage" is not defined.

Link: thesofproject/sof#6896
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i mentioned this issue Dec 30, 2022
Merged
kv2019i added a commit to kv2019i/zephyr that referenced this issue Dec 30, 2022
@kv2019i
drivers: dai: intel: dmic: don't use assert for error handling
cb06070 
The dai.h interface does not prohibit calling dai_config_get()
with different direction values. The dmic driver should handle
invalid direction value explicitly and not rely on an assert.

Link: thesofproject/sof#6896
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i mentioned this issue Jan 2, 2023
Merged
nashif pushed a commit to zephyrproject-rtos/zephyr that referenced this issue Jan 3, 2023
@kv2019i @nashif
intel_adsp: ace: power: fix build error with asserts enabled
e753af0 
If defined(CONFIG_ASSERT) and !defined(CONFIG_ADSP_IMR_CONTEXT_SAVE),
build will fail as symbol "global_imr_ram_storage" is not defined.

Link: thesofproject/sof#6896
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i added a commit to kv2019i/sof that referenced this issue Jan 4, 2023
@kv2019i
module_adapter: fix buffer consumption logic in simple_copy mode
bd21b3a 
When simple_copy mode is used, some of the module sinks may
be skipped (e.g. source in different state) and the number of
inputs passed to module_process() can thus be less than the number
of sinks (dev->bsink_list).

As a result, the call to comp_update_buffer_consume() may be
done with an incorrect pair of comp_buffer (source_c[i]) and matching
consumed data (mod->input_buffers[i].consumed).

This will lead to asserts triggering as the consumed value can
be undefined.

Link: thesofproject#6896
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i mentioned this issue Jan 4, 2023
Closed
@lgirdwood lgirdwood added this to the v2.5 milestone Jan 4, 2023
coreboot-org-bot pushed a commit to coreboot/zephyr-cros that referenced this issue Jan 4, 2023
@kv2019i
intel_adsp: ace: power: fix build error with asserts enabled
ce87148 
If defined(CONFIG_ASSERT) and !defined(CONFIG_ADSP_IMR_CONTEXT_SAVE),
build will fail as symbol "global_imr_ram_storage" is not defined.

(cherry picked from commit e753af0)

Original-Link: thesofproject/sof#6896
Original-Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
GitOrigin-RevId: e753af0
Change-Id: I37341e961369f9730fbe0322cfbde6f3df669810
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/4136258
Tested-by: CopyBot Service Account <copybot.service@gmail.com>
Tested-by: Keith Short <keithshort@chromium.org>
Reviewed-by: Keith Short <keithshort@chromium.org>
Commit-Queue: Keith Short <keithshort@chromium.org>
kv2019i added a commit that referenced this issue Jan 9, 2023
@kv2019i
dai-zephyr: handle NULL case when calling dai_config_get()
90c14e5 
As per interface documentation of dai_config_get(), the function
may return NULL if the device is not configured for the requested
direction. Handle this case explicitly.

Currently no upstream Zephyr driver returns NULL, but e.g. dmic
driver returns invalid configuration when dai_config_get() is
called with DAI_DIR_TX as direction.

Link: #6896
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
carlescufi pushed a commit to zephyrproject-rtos/zephyr that referenced this issue Jan 12, 2023
@kv2019i @carlescufi
drivers: dai: intel: dmic: don't use assert for error handling
8374325 
The dai.h interface does not prohibit calling dai_config_get()
with different direction values. The dmic driver should handle
invalid direction value explicitly and not rely on an assert.

Link: thesofproject/sof#6896
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
coreboot-org-bot pushed a commit to coreboot/zephyr-cros that referenced this issue Jan 16, 2023
@kv2019i
drivers: dai: intel: dmic: don't use assert for error handling
472bb8f 
The dai.h interface does not prohibit calling dai_config_get()
with different direction values. The dmic driver should handle
invalid direction value explicitly and not rely on an assert.

(cherry picked from commit 8374325)

Original-Link: thesofproject/sof#6896
Original-Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
GitOrigin-RevId: 8374325
Change-Id: Idbb95ba2eb60adb3446d2571faaeef6114468191
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/4163126
Reviewed-by: Tristan Honscheid <honscheid@google.com>
Reviewed-by: Fabio Baltieri <fabiobaltieri@google.com>
Tested-by: Tristan Honscheid <honscheid@google.com>
Commit-Queue: Fabio Baltieri <fabiobaltieri@google.com>
Tested-by: CopyBot Service Account <copybot.service@gmail.com>
Tested-by: Fabio Baltieri <fabiobaltieri@google.com>
kv2019i added a commit to kv2019i/sof that referenced this issue Feb 7, 2023
@kv2019i
mixin-mixout: handle congestion corner case without assert
5452607 
If one mixout (A) is not able to process data, the 'sinks_free_frames'
will be truncated compared other mixout buffers. When the mixin process
is producing data to another mixout buffer (B), the number of already
mixed frames may be larger than 'sinks_free_frames'. Further it is
possible this mixout (B) has pending frames, leading to a larger
'start_frame' value.

This is a valid and possible scenario, but will trigger
  assert(sinks_free_frames >= start_frame)
in current code.

As 'frames_to_copy' is already correctly calculated as limited to
available free frames across all mixouts, no extra logic is needed when
this condition happens. Mixin is not able to proceed until the congested
mixout (A) has room again for further samples.

Link: thesofproject#6896
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i added a commit that referenced this issue Feb 8, 2023
@kv2019i
mixin-mixout: handle congestion corner case without assert
ad9f659 
If one mixout (A) is not able to process data, the 'sinks_free_frames'
will be truncated compared other mixout buffers. When the mixin process
is producing data to another mixout buffer (B), the number of already
mixed frames may be larger than 'sinks_free_frames'. Further it is
possible this mixout (B) has pending frames, leading to a larger
'start_frame' value.

This is a valid and possible scenario, but will trigger
  assert(sinks_free_frames >= start_frame)
in current code.

As 'frames_to_copy' is already correctly calculated as limited to
available free frames across all mixouts, no extra logic is needed when
this condition happens. Mixin is not able to proceed until the congested
mixout (A) has room again for further samples.

Link: #6896
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Mar 6, 2023

All patches merged, issue now resolved in SOF main.

@kv2019i kv2019i closed this as completed Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kv2019i kv2019i

Labels
bug Something isn't working as expected P2 Critical bugs or normal features Zephyr Issues only observed with Zephyr integrated
Projects
None yet
Milestone
v2.5
Development

No branches or pull requests
2 participants
@lgirdwood @kv2019i