[xiulipan]

Add new platform specific ops to dump IPC regs when IPC timeout happen.

This is only a framework RFC, please help to review and will add more detail for the logs.

Only add APL and CNL+ platforms with raw data.

[plbossart]

@xiulipan thanks for starting this, it's a good idea. Couple of suggested fixes below

[plbossart]

I don't get what the %s does here?

[xiulipan]

Please see the comment I made below.

[plbossart]

parameter alignment seems inconsistent?

[xiulipan]

not sure if we really need them. Since not all of them are useful

[plbossart]

same, why do you need the %s for?

[plbossart]

snd_sof_ipc_dump? we have IPC registers but others may have more stuff to log?

[xiulipan]

I also thinks about snd_sof_dsp_dbg_dump(ipc->sdev, SOF_DBG_REGS | SOF_DBG_MBOX | SOF_DBG_IPC);
but I found that we never used the flag in snd_sof_dsp_dbg_dump(struct snd_sof_dev *sdev, u32 flags)
So I just create a new function for this.

[plbossart]

should we use rate_limited versions?

[xiulipan]

what is a rate_limited version here?

[plbossart]

dev_err_ratelimited()

[xiulipan]

I did not think we need here. Since IPC timed out will be a blocker for the FW, the FW will be dead if an timed out happen. I think there will not be a flooding.

[xiulipan]

@lgirdwood @ranj063
Could you have a look if this as you expected. I am not sure which regs I need to check and parse.

[xiulipan]

@plbossart

This actually some proposal from @lgirdwood. The %s is some reservation for future parse to string to help understand.
But when I try to do the implement, I found that it seems we did not have so many bits for this in one reg.

@lgirdwood @ranj063 Any idea

#846 (comment)

IPC timeouts are often difficult to debug as we have two state machines running, one within the driver and the other within the FW.

This feature is to add useful debug output to the kernel when it detects and IPC timeout. i.e.

<< dmesg after current IPC timeout is printed >>
error: IPC host state: HOST_TO_DSP_CMD, DSP_TO_HOST_NOTIFICATION
error: IPC host mask: DSP_TO_HOST_NOTIFICATION
error: IPC host raw: status 0x00000003 mask 0x0000001
error: IPC dsp state: DSP_TO_HOST_NOTIFICATION
error: IPC dsp mask: DSP_TO_HOST_NOTIFICATION
error: IPC dsp raw: status 0x00000001 mask 0x0000001

The intention here is to show the IPC state machines for both DSP and host and show the pending operations in a high level manner alongside the raw status and mask.

[ranj063]

This actually some proposal from @lgirdwood. The %s is some reservation for future parse to string to help understand.
But when I try to do the implement, I found that it seems we did not have so many bits for this in one reg.

@xiulipan the host/dsp state is something we will have to infer from the IPC registers I think. For example, hipct & HDA_DSP_REG_HIPCT_BUSY indicates a new message from DSP, so this means the dsp state is DSP_TO_HOST_CMD.

@lgirdwood please correct me if I am wrong

[plbossart]

This actually some proposal from @lgirdwood. The %s is some reservation for future parse to string to help understand.
But when I try to do the implement, I found that it seems we did not have so many bits for this in one reg.

I don't want 'reservation for future stuff', since these kind of things tend to never come and are left as placeholder until a janitor (me in most cases) comes in. Please submit what exists today and an update if and when an extension is needed.

[xiulipan]

@plbossart @ranj063
I have test with IPC timeout issue. And will update the PR withe the RAW register now.
I think the IPC state can be added later. I will remove the placeholder here.

[plbossart]

Looks good to me, but someone else must approve.

[plbossart]

edited title to remove the RFC/WIP tags