thesofproject
[RFC]: Fix pause-release not working on i.MX
Currently, when we're trying to pause and then resume a stream we get an error message from the FW saying that a comp_trigger() operation failed. This was caused by the fact that edma_release would set the channel state to COMP_STATE_ACTIVE and edma_start would not allow the transition from that state.
In order to fix this, edma_release now changes the state to COMP_STATE_PRE_ACTIVE and edma_start allow the transition from COMP_STATE_PRE_ACTIVE to COMP_STATE_ACTIVE. This way, the EDMA states will be consistent with the comp state diagram found at https://thesofproject.github.io/latest/architectures/firmware/components/component-overview.html.
Another problem with edma_start was the fact that it was allowing the transition from COMP_STATE_SUSPEND to COMP_STATE_ACTIVE which is wrong because that state is never set in the edma driver.
Another solution to the pause-resume problem would be to simply set the state of the edma channel to COMP_STATE_PREPARE on edma_release. This would also work but it would be inconsistent with the aforementioned comp state diagram.
@LaurentiuM1234 very good description of the issues. Looking at the other drivers I would suggest using COMP_STATE_PREPARE. I think the documentation with the diagram state is outdated.
Fwiw, @lyakh is working out how we can simplify the pipeline state machine by reducing/merging some states.
Update:
After the first 3 commits, the problem with pause-release operation not working at all was fixed, but after a sequence of pause-release operations a static sound would play along with the song. This static sound would sometimes disappear after another sequence of pause-release operations and would appear again after another sequence of those operations.
In order to fix this, I had to add a new function in the i.MX SAI driver: sai_release() which would handle the release operation on sai_trigger(RELEASE). Initially, release would be treated just as start which is not really correct because:
-
we don't need to do a software reset of the SAI (according to the documentation, apart from resetting the SAI internal logic, it was also resetting the the FIFO read and write pointers which was not required)
-
we don't need to enable the flags that were not disabled by
sai_stop
Apart from the static sound problem there seemed to be another one: after some pause-release operations the sound would play with higher volume in one earpiece compared to the other one. This seemed to happen only during a certain part of some song (the beginning). By removing the unnecessary set of CSR_START register in edma_start the problem seemed to have disappeared (I did 20 tests with the beginning of the song and this problem didn't occur). Setting CSR_START register in edma_start is not necessary because we don't need to explicitly start a channel in the software. The hardware will take care of it.
@LaurentiuM1234 can you please capture your last comment inside the commit messages. People usually look at git log when investigating code not at github comments.
Update:
- Changed some of the commit messages in order to better explain the issues the patches are meant to solve.
- Reordered the last 3 commits to better reflect the order in which the problems were discovered.
thanks @LaurentiuM1234 this also fixes the pause/release problem on i.MX8MP for PCM playback. I will have more testing for compress on Monday then will merge the changes.
@LaurentiuM1234 this looks all self contained within iMX code, I'm assuming you were satisfied that there were no errors in the core state machine ?
Yes.
@LaurentiuM1234 pls let us know when fully tested and ready for merge as still RFC. @paulstelian97 good for you too ?
@lgirdwood this is good for us. @paulstelian97 is on vacation for the next two weeks so we need an approval for you.