sof
sof copied to clipboard
thesofproject
dai: remove bogus IRQ disabling in DAI group triggering
Disabling IRQs when sending a notification doesn't seem to be required and is potentially risky if that notification causes re-scheduling.
Who's the consumer of the notifier message ?
@lgirdwood dai_atomic_trigger() - the notification is used to handle DAI groups. When all DAIs have handled the trigger, that callback is called. TBH I don't even understand why not just call that function directly. The original DAI group commit says, all the DAIs should run on the same core, so effectively anyway the same thing happens: the notifier just calls the callback inline in-place.
Who's the consumer of the notifier message ?
@lgirdwood
dai_atomic_trigger()- the notification is used to handle DAI groups. When all DAIs have handled the trigger, that callback is called. TBH I don't even understand why not just call that function directly. The original DAI group commit says, all the DAIs should run on the same core, so effectively anyway the same thing happens: the notifier just calls the callback inline in-place.
Ok, so if it's all on the same core then we still need local IRQs off e.g. we get an IPC half way through the DAI enable bit setting between SSP0 and SSP1 meaning that SSP ports could be out of sync by 1 or more frames ?
Who's the consumer of the notifier message ?
@lgirdwood
dai_atomic_trigger()- the notification is used to handle DAI groups. When all DAIs have handled the trigger, that callback is called. TBH I don't even understand why not just call that function directly. The original DAI group commit says, all the DAIs should run on the same core, so effectively anyway the same thing happens: the notifier just calls the callback inline in-place.Ok, so if it's all on the same core then we still need local IRQs off e.g. we get an IPC half way through the DAI enable bit setting between SSP0 and SSP1 meaning that SSP ports could be out of sync by 1 or more frames ?
@lgirdwood then it should be done inside the callback - where it's really needed, not around the notifier call
Who's the consumer of the notifier message ?
@lgirdwood
dai_atomic_trigger()- the notification is used to handle DAI groups. When all DAIs have handled the trigger, that callback is called. TBH I don't even understand why not just call that function directly. The original DAI group commit says, all the DAIs should run on the same core, so effectively anyway the same thing happens: the notifier just calls the callback inline in-place.Ok, so if it's all on the same core then we still need local IRQs off e.g. we get an IPC half way through the DAI enable bit setting between SSP0 and SSP1 meaning that SSP ports could be out of sync by 1 or more frames ?
@lgirdwood then it should be done inside the callback - where it's really needed, not around the notifier call
The callback is run for each single physical DAI instance in a loop. How would moving IRQs handling inside the callback help to protect the entire loop for all DAIs from pre-emption?
The callback is run for each single physical DAI instance in a loop. How would moving IRQs handling inside the callback help to protect the entire loop for all DAIs from pre-emption?
@mmaka1 ok, can we then make it a simple list with a head in the group and a loop over that list? Not using the notifier with its potential scheduling calls
