Repository navigation
Conversation
There was a problem hiding this comment.
Is the comment above still necessary with this change?
There was a problem hiding this comment.
@paulstelian97 I thought about it, but in the same time this comment is still valid.
There was a problem hiding this comment.
Didn't we want to remove xrun recovery at all from fw?
There was a problem hiding this comment.
I think the comment is wrong and it's only used to notify the DAI we are in xrun, but I haven't verified the logic behind it.
There was a problem hiding this comment.
What is the definition of started ? This looks like we are duplicating state information ?
There was a problem hiding this comment.
@lgirdwood Component is started, but it hasn't yet copied any data. It's really just used to avoid reporting an unecessary xrun. Couldn't come up with any another name and adding additional comp state was too much.
There was a problem hiding this comment.
This just seems like a shortcut by adding a new state indirectly and cheaply and will ultimately lead to confusion for integrators. I suggest we have STARTED and RUNNING states for components and pipeline.
There was a problem hiding this comment.
Right now this is something internal for DAI, so I don't see integrators looking into it, since it's generic component. Adding additional state will add even more confusion, because it will be used only in this one place and I will need to set it pretty much in every component's copy.
There was a problem hiding this comment.
@tlauda pipeline and component states should all be in the one place period (especially since this is a generic state that should be useful for others too), we have one user of this new state today but that does not mean there wont be other users in the future.
|
SOFCI TEST |
1 similar comment
|
SOFCI TEST |
Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Adds new component state named COMP_STATE_STARTED. This new state indicates that the component has been started, but hasn't copied any data yet. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
2b01be9 to
2666277
Compare
2666277 to
46d352e
Compare
|
@lgirdwood Patch updated. Please check if it's what you've expected. |
Uses new COMP_STATE_STARTED and sets it on START and RELEASE. After that component can use their own logic to perform its internal operations based on current state. API automatically sets component's state to COMP_STATE_ACTIVE, if copy function returned success. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Fixes rare cases of xruns reported on release with timer scheduled pipelines. With timer scheduling the timer tick can happen so quickly after the release, that the DMA hasn't yet copied any data. Since position is not zero, we will report an xrun. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Uses comp_is_started in pipeline_comp_params instead of manually checking the state. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
46d352e to
a54a820
Compare
|
@lgirdwood Closing this, because I've implemented different version in #1945. After giving it a little thought adding new comp state is not the right way to go in this particular case. |
Adds new started field based on which we assess if the pipeline
has been just started/released or maybe we should throw an xrun.
It should fix rare cases of xruns on release with timer scheduled
pipelines.
Signed-off-by: Tomasz Lauda tomasz.lauda@linux.intel.com