Skip to content

Conversation

@ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Sep 28, 2021

Pipeline trigger is now handled as part of the pipeline
task. Keeping the PRE_START/PRE_RELEASE as the last options
in the switch case to get the current component state results
in task taking taking too long during PRE_START leading to an
xrun with the first copy in the pipeline task. Move the
PRE_START/PRE_RELEASE options as the first cases to fix this.

Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com

Pipeline trigger is now handled as part of the pipeline
task. Keeping the PRE_START/PRE_RELEASE as the last options
in the switch case to get the current component state results
in task taking taking too long during PRE_START leading to an
xrun with the first copy in the pipeline task. Move the
PRE_START/PRE_RELEASE options as the first cases to fix this.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
Copy link
Collaborator Author

ranj063 commented Sep 28, 2021

@lyakh this fixes my problem with deep buffer pipelines and hda topology for me. But I feel like maybe we need to speed up the trigger now that it is part of the pipeline task. Maybe we should have an array look-up for translating trigger commands to the requested state. What do you think?

@lyakh
Copy link
Collaborator

lyakh commented Sep 28, 2021

@lyakh this fixes my problem with deep buffer pipelines and hda topology for me. But I feel like maybe we need to speed up the trigger now that it is part of the pipeline task. Maybe we should have an array look-up for translating trigger commands to the requested state. What do you think?

This really shouldn't be a fix. The difference should be just a couple of DSP cycles, a couple of assembly instructions, we really shouldn't be down to that. If we are, we have a bigger problem to solve. Also effects of such reordering are compiler specific. So, no, I'm very suspicious that this actually is making a difference in your tests...

@ranj063
Copy link
Collaborator Author

ranj063 commented Sep 28, 2021

This really shouldn't be a fix. The difference should be just a couple of DSP cycles, a couple of assembly instructions, we really shouldn't be down to that. If we are, we have a bigger problem to solve. Also effects of such reordering are compiler specific. So, no, I'm very suspicious that this actually is making a difference in your tests...

@lyakh that is my worry too but this does fix my problem. maybe what we need to do is time how long the trigger set state takes per comp and see how to proceed

@lyakh
Copy link
Collaborator

lyakh commented Sep 28, 2021

This really shouldn't be a fix. The difference should be just a couple of DSP cycles, a couple of assembly instructions, we really shouldn't be down to that. If we are, we have a bigger problem to solve. Also effects of such reordering are compiler specific. So, no, I'm very suspicious that this actually is making a difference in your tests...

@lyakh that is my worry too but this does fix my problem. maybe what we need to do is time how long the trigger set state takes per comp and see how to proceed

I'll eat my hat if this is the case (mmh, no, I won't, but still). Is this in multicore configurations? Can there be a race? I'd much rather suspect a race with an interrupt or something, where by moving the instructions earlier the racing interrupt now hits after them or something.

@aiChaoSONG
Copy link
Collaborator

SOFCI TEST

@plbossart
Copy link
Member

@ranj063 I would agree with @lyakh. If the xrun happens on start, the xrun will be thrown 2s after the trigger. There is no real way moving the switch cases would account for such an long delay?

@plbossart
Copy link
Member

@ranj063 I would agree with @lyakh. If the xrun happens on start, the xrun will be thrown 2s after the trigger. There is no real way moving the switch cases would account for such an long delay?

And I still get an xrun...

firmware: #4611 + #4812
kernel: thesofproject/linux#3146

test with this script that starts two streams going to the same mixer.

set -e

while true; do
    speaker-test -Dhw:0,0 -c2 -r48000 -f S16_LE -l 1 &
    speaker-test -Dhw:0,31 -c2 -r48000 -f S16_LE -l 1 &
    sleep 10
done
Using 16 octaves of pink noise
Rate set to 48000Hz (requested 48000Hz)
Buffer size range from 96 to 16384
Period size range from 48 to 4096
Using max buffer size 16384
Periods = 4
Rate set to 48000Hz (requested 48000Hz)
Buffer size range from 96 to 16384
Period size range from 48 to 4096
Using max buffer size 16384
Periods = 4
was set period_size = 4096
was set buffer_size = 16384
 0 - Front Left
was set period_size = 4096
was set buffer_size = 16384
 0 - Front Left
 1 - Front Right
 1 - Front Right
Write error: -5,Input/output error
xrun_recovery failed: -5,Input/output error
Transfer failed: Input/output error
Write error: -5,Input/output error
xrun_recovery failed: -5,Input/output error
Transfer failed: Input/output error

@ranj063
Copy link
Collaborator Author

ranj063 commented Sep 28, 2021

@ranj063 I would agree with @lyakh. If the xrun happens on start, the xrun will be thrown 2s after the trigger. There is no real way moving the switch cases would account for such an long delay?

@plbossart agree. Its just bizarre that this has an effect. @lyakh and I agreed for him to look into race conditions with the pipeline trigger in this case. I will close this PR and open an issue instead

@ranj063 ranj063 closed this Sep 28, 2021
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