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

coherent: fix cache flush logic if coherent is not the first member #7299

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Mar 17, 2023

If coherent object is at non-zero offset, the init call ends up invalidating data cache outside the allocated buffer.

All current users of "struct coherent" have the object as the first member, so nothing is broken with current code.

If coherent object is at non-zero offset, the init call ends
up invalidating data cache outside the allocated buffer.

All current users of "struct coherent" have the object as
the first member, so nothing is broken with current code.

Signed-off-by: Kai Vehmanen <[email protected]>
Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

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

yeah, fixing the same assumption in other locations like

dcache_invalidate_region(uncache_to_cache(c), size);
dcache_invalidate_region(cc, size);
dcache_writeback_invalidate_region(c, size);
(and more) would require some (internal) API changes

@lgirdwood
Copy link
Member

@kv2019i @lyakh I assume we need followups for

  1. coherent zalloc() alignment to cache line size ?
  2. remove the offset, i..e always at offset 0 ?

@lyakh
Copy link
Collaborator

lyakh commented Mar 20, 2023

@kv2019i @lyakh I assume we need followups for

1. coherent zalloc() alignment to cache line size ?

2. remove the offset, i..e always at offset 0 ?

@lgirdwood I don't think we need those because we so far aren't aware of any bugs that those changes would fix. In fact I think we could add support for the coherent header anywhere within the managed object, apart from type-casting convenience I don't see any reason why it has to be in the beginning.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Mar 20, 2023

SOFCI TEST

@kv2019i
Copy link
Collaborator Author

kv2019i commented Mar 20, 2023

A few failes in https://sof-ci.01.org/sofpr/PR7299/build4829/devicetest/index.html (PM fail, also seen on other PRs) an dhttps://sof-ci.01.org/sofpr/PR7299/build4830/devicetest/index.html . In the latter, once case of #7191 was hit (https://sof-ci.01.org/sofpr/PR7299/build4830/devicetest/index.html?model=TGLU_RVP_NOCODEC_IPC4ZPH&testcase=multiple-pause-resume-5 ). Neither related to this PR, so proceeding with merge.

@kv2019i kv2019i merged commit fd31818 into thesofproject:main Mar 20, 2023
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