Repository navigation
ipc message dropped if dsp is not ready #395
Description
in the call to sof_ipc_tx_message(), the work to send the ipc message is scheduled only if the dsp is ready(). What happens if the dsp is busy? Should the ipc be re-sent to the dsp again after a delay?
Activity
plbossart commentedon Dec 16, 2018
There's been no comments or PR on this, isn't this a valid point that needs to be fixed?
keyonjie commentedon Dec 17, 2018
Hi @ranj063 , after checked it more, I think this is not an issue if snd_sof_dsp_is_ready() is reliable, as not ready means there is already IPC(host->fw) being processing, the reply(tx_wait_done) for this ongoing IPC will continue to schedule any remaining message with snd_sof_ipc_msgs_tx() at the end of the function, that means we won't drop sending any IPC messages.
xiulipan commentedon Dec 17, 2018
@keyonjie
I think this issues maybe related to the
IRQ nobody careissue?The HOST will not handle any IRQ since it is being disabling.
ranj063 commentedon Dec 17, 2018
@keyonjie @xiulipan if the snd_sof_dsp_ready() is correct and the DSP is indeed busy you are right that the message will likely be sent again after the current IPC processing is done. But the reason I called this as a potential issue is that we kick off the wait before the message is actually sent to the DSP. What is the guarantee that the previous ipc processing will complete before the wait timeout expires?
keyonjie commentedon Dec 18, 2018
@ranj063 for the wait timeout issue, currently we set timeout to be 300ms, you are right our current design actually not guarantee 300ms waiting for all IPCs, but considering we are requiring rapid response in FW(doing TH and then return ASAP), the typical normal response is < 1ms(e.g.200~500us), so this deviation for several IPCs(e.g. 10 IPCs in the queue, typical timeout for the last one should be < 10*1ms) comparing to 300ms should be fine for us.
mengdonglin commentedon Dec 19, 2018
@ranj063 are we good to close this issue?
mengdonglin commentedon Jan 5, 2019
Comments of @ranj063
"In the current driver design, our ipc's are sequential. So the chances of the DSP being busy when a new ipc is sent out is probably minimal.
But I still think there is a small risk that this issue could be hit.
We probably need to alter the code to start the wait after the doorbell has been sent to the DSP"
ranj063 commentedon Jan 14, 2019
@mengdonglin im going to close this bug. Altering the timeout for IPC completion is harder than I thought. As it is scheduled in a separate thread, it will not be able to notify initiator of the outcome of the IPC. So, the alternative is to start the timeout for a long enough time and just hope it gets done.