cache / sparse: next lot by lyakh · Pull Request #5957 · thesofproject/sof

lyakh · 2022-06-28T14:53:41Z

next 7 commits from the cache / coherency / sparse work

marc-hb · 2022-06-28T18:16:38Z

These really do not look like sparse fixes.

@marc-hb I have to update drc and other components for cache / sparse and that only works with Zephyr, so I need to add them to Zephyr builds

I'm sure you know what you're doing but this seems really buried in a large PR that seems to be mostly about something else. It's not mentioned in the description and I could not find a commit message mentioning it either?

yeah, better in the future to have a new PR for this but we've already reviewing for this PR.

dbaluta · 2022-06-29T06:31:19Z

Looks like this doesn't belong to this commit. Same with some other changes related to __sparse_cache

@dbaluta this very much does belong to it. Maybe the commit description isn't very clear, but that's exactly what you need to do to acquire and release buffers properly: with acquired buffers you need to use the proper sparse annotation, specifically this one.

dbaluta · 2022-06-29T06:33:39Z

I don't understand this change. Do you enable DRC for Zephyr? Then it should be a separate commit with proper explanation.

@dbaluta its a workaround atm for a Zephyr linker bug. We are not sure the module section is not being stitched into the Zephyr image (like it does with native SOF image)..

lgirdwood

Need to make the Zephyr build updates as another patch or PR. Sparse changes look good.

Add multiple third-party and optional audio processing components to the Zephyr cmake and initialisation lists. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Add the __sparse_cache sparse annotation to sample audio tests. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Instead of acquiring and releasing buffers locally multiple times, do that once for .copy(), .trigger() and .prepare() methods. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Instead of acquiring and releasing buffers locally multiple times, do that once for .copy(), .params() and .prepare() methods. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Instead of acquiring and releasing buffers locally multiple times, do that once for the .copy() method. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Instead of acquiring and releasing buffers locally multiple times, do that once for .copy() and .prepare() methods. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

lyakh · 2022-07-01T07:40:46Z

CI: only one suspend-resume failure https://sof-ci.01.org/sofpr/PR5957/build787/devicetest/?model=WHL_UPEXT_HDA_ZEPHYR&testcase=check-suspend-resume-with-capture-5

lgirdwood · 2022-07-01T10:11:01Z

@lyakh two things

Can you prep the next sparse PR for review (we need to get to the point where we can start CI using Sparse soon).
Can you create an issue for the Zephyr linker - your workaround is fine, but long term we need t use the ELF section like native SOF for all the processing modules. Thanks.
We need to start using sparse for multicore cache/uncache objects in more places (including Zephyr)

lyakh · 2022-07-01T11:03:40Z

Can you prep the next sparse PR for review (we need to get to the point where we can start CI using Sparse soon).

@lgirdwood #5976

Can you create an issue for the Zephyr linker - your workaround is fine, but long term we need t use the ELF section like native SOF for all the processing modules. Thanks.

For the record: it isn't my workaround. It's been there from day one of SOF Zephyr port. Yes, we have to do that, it should be somewhere in our plans...

We need to start using sparse for multicore cache/uncache objects in more places (including Zephyr)

Yes, but maybe (1) merge the rest of comp_buffer sharing in #5976, (2) collect some experience with that, (3) benchmark performance differences with CONFIG_INCOHERENT disabled.

lyakh requested review from a team, aborisovich, dbaluta, lbetlej, lgirdwood, marc-hb, mmaka1, plbossart and singalsu as code owners June 28, 2022 14:53

lyakh force-pushed the sparse-6 branch from daefc90 to 6b53fdcCompare June 28, 2022 15:32

marc-hb reviewed Jun 28, 2022
View reviewed changes

dbaluta reviewed Jun 29, 2022
View reviewed changes

Comment threadzephyr/CMakeLists.txt Outdated

dbaluta reviewed Jun 29, 2022
View reviewed changes

lgirdwood reviewed Jun 29, 2022
View reviewed changes

Comment threadzephyr/CMakeLists.txt Outdated
Comment threadzephyr/CMakeLists.txt Outdated

lyakh force-pushed the sparse-6 branch from 6b53fdc to f9ef414Compare June 30, 2022 08:01

lyakh added 8 commits June 30, 2022 17:01

zephyr: add third-party components
bce3d60
Add multiple third-party and optional audio processing components to the Zephyr cmake and initialisation lists. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

samples: use __sparse_cache sparse annotation
fee8936
Add the __sparse_cache sparse annotation to sample audio tests. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

smart_amp: fix buffer acquisition
3d49cde
Instead of acquiring and releasing buffers locally multiple times, do that once for .copy(), .trigger() and .prepare() methods. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

tone: fix buffer acquisition
d0c851d
Instead of acquiring and releasing buffers locally multiple times, do that once for .copy(), .params() and .prepare() methods. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

aria: fix buffer acquisition
a632be6
Instead of acquiring and releasing buffers locally multiple times, do that once for the .copy() method. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

crossover: fix buffer acquisition
5f74f24
Instead of acquiring and releasing buffers locally multiple times, do that once for .copy() and .prepare() methods. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

drc: fix buffer acquisition
a1c4823
Instead of acquiring and releasing buffers locally multiple times, do that once for .copy() and .prepare() methods. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

multiband-drc: fix buffer acquisition
a616d41
Instead of acquiring and releasing buffers locally multiple times, do that once for .copy() and .prepare() methods. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

lyakh force-pushed the sparse-6 branch from f9ef414 to a616d41Compare June 30, 2022 15:04

lgirdwood approved these changes Jul 1, 2022
View reviewed changes

lgirdwood merged commit 7a2e0e1 into thesofproject:mainJul 1, 2022

lyakh deleted the sparse-6 branch July 1, 2022 10:44

Uh oh!

Conversation

lyakh commented Jun 28, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lyakh commented Jul 1, 2022

Uh oh!

lgirdwood commented Jul 1, 2022

Uh oh!

lyakh commented Jul 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants