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.

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

[DNM] POC: ] Direct use of cached / not-cached shared memory alias instead of coherent.h in case of buffers #8013

Conversation

marcinszkudlinski
Copy link
Contributor

@marcinszkudlinski marcinszkudlinski commented Aug 8, 2023

This PR is a quick POC for discussuion #8006

There are a lot of TODOs, yet it is ready to be tested in IPC4/MTL

TODOs:

  1. Check if there is no need to extra spinlocks!!!! in case of shared buffers probably - yes because buffer_aquire was also performing as a mutex when in shared mode.
    In not-shared there's no need for locks because LLs are processed one by one in a single thread
  2. remove buffer_aquire from the code (in inteligent way, keep point 1 in mind)
  3. remove lots of __sparse_cache
  4. add __sparse_cache to pointers pointing audio samples buffers

@lgirdwood
Copy link
Member

@marcinszkudlinski I assume you will continue with updates for teh TODOs here so that CI will turn all Green ?

marcinszkudlinski and others added 19 commits August 31, 2023 22:33
This commit is a POC that removes coherent.h from
struct comp_buffer

As sharing status of a buffer is known at creation time, it is
enough to create a buffer in shared (not cached mem alias)
when it will be used by several cores

Signed-off-by: Marcin Szkudlinski <[email protected]>
Removes all instances of buffer_acquire function usage

Signed-off-by: Tobiasz Dryjanski <[email protected]>
Source/sink interfce will be treated in the same way as
buffers, either cached or not as needed
Sparse markings should be removed

Signed-off-by: Marcin Szkudlinski <[email protected]>
Source/sink interfce will be treated in the same way as
buffers, either cached or not as needed
Sparse markings should be removed

Signed-off-by: Marcin Szkudlinski <[email protected]>
audio stream is no longer required to have __sparse mark
remove __sparse from container_of

Signed-off-by: Marcin Szkudlinski <[email protected]>
audio stream is no longer required to have __sparse mark
remove __sparse from all audio_stream functions

Signed-off-by: Marcin Szkudlinski <[email protected]>
Signed-off-by: Tobiasz Dryjanski <[email protected]>
buffer_acquire and buffer_release are no longer in use
remove stubs from the code

Signed-off-by: Marcin Szkudlinski <[email protected]>
@@ -319,7 +319,7 @@ __must_check static inline struct coherent __sparse_cache *coherent_acquire(stru
dcache_invalidate_region(cc, size);
}

return (__sparse_force struct coherent __sparse_cache *)c;
return c;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why these changes? I'd expect coherent.h to remain unchanged, unless there are bugs there, but fixing any would be a separate piece of work?

Copy link
Contributor Author

@marcinszkudlinski marcinszkudlinski Sep 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh I should not use this branch for CI check ;)
You're right, this change is incorrect

@marcinszkudlinski
Copy link
Contributor Author

Closing this POC as it was accidently used as CI test branch
The PR is almost ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants