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

pipeline:make the IPC API atomic. #104

Merged
merged 1 commit into from Jul 19, 2018
Merged

pipeline:make the IPC API atomic. #104

merged 1 commit into from Jul 19, 2018

Conversation

ghost
Copy link

@ghost ghost commented Jul 14, 2018

make the IPC API atomic, to avoid the interruption of
the scheduled pipeline.

Signed-off-by: Wu Zhigang zhigang.wu@linux.intel.com

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

The code changes look correct if we have to make prepare() atomic. However, I'm not following the commit message comments. Can you add more detail to each step above, include numbers on newlines like 1), 2) etc to make it easy to follow. Thanks

@ghost
Copy link
Author

ghost commented Jul 15, 2018

ok, I will change the comment and upload the patch again.

@ghost
Copy link
Author

ghost commented Jul 15, 2018

update the comment already. please check it.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Ok, so I assume we have a race with the IPC interrupt when handling XRUNs ?
Is this fixing a real bug today or just fixing a potential race ?

@ghost
Copy link
Author

ghost commented Jul 16, 2018

This is just fixing a potential race.
It is similarly as what i sent a patch before: 9112ee5

@lgirdwood
Copy link
Member

ok, so pipeline APIs are called by IPC handler (which is serialized and only called in idle state) and internally for XRUN and copy(). XRUN handler should be atomic (IRQs off) alongside the other IPC pipeline APIs.

@ghost
Copy link
Author

ghost commented Jul 16, 2018

exactly! this patch is good for the stress test.

@lgirdwood
Copy link
Member

please rename the PR and message to reflect the agreed changes alongside the new patches..

@ghost ghost changed the title pipeline:disable irq during prepare processing. pipeline:make prepare() atomic when handling xrun Jul 17, 2018
@ghost
Copy link
Author

ghost commented Jul 18, 2018

update the PR and message already.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

This patch should also make the other pipeline APIs called by IPC atomic. e.g.params and reset

@ghost
Copy link
Author

ghost commented Jul 18, 2018

I will do this after back to office.

make the IPC API atomic, to avoid the interruption of
the scheduled pipeline.

Signed-off-by: Wu Zhigang <zhigang.wu@linux.intel.com>
@ghost ghost changed the title pipeline:make prepare() atomic when handling xrun pipeline:make the IPC API atomic. Jul 19, 2018
@ghost
Copy link
Author

ghost commented Jul 19, 2018

updated the code and message already.
please check.

@lgirdwood lgirdwood merged commit d696d8f into thesofproject:master Jul 19, 2018
@jiawang6
Copy link

CI observed building failure.

Failed Platform: baytrail apollolake cannonlake

configure: error: in `/build/sof/src':
configure: error: C compiler cannot create executables

@lgirdwood
Copy link
Member

@jiawang6 I think your CI is broken as this patch builds for me and has nothing todo with autotools.

@ghost ghost deleted the topic/platform-stable1 branch September 11, 2018 09:45
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.

2 participants