lkml.org 
[lkml]   [2020]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH] ASoC: Intel: sst: ipc command timeout
Date


On 4/21/2020 6:16 PM, Lu, Brent wrote:
>>
>> Regs width difference between BDW and BYT comes from specification. BDW
>> has IPC registers which are 32 wide. This fact ain't exactly the reason to modify
>> sst_shim32_read64.
>>
>> I'm sharing Amadeo's point of view. Your change should slow down execution a
>> bit - but that might be just what handlers needed to make everything work again.
>> Debug prints also slow down the execution what could have prevented you from
>> spotting the real problem.
>> Let's ignore the memcpy stuff for a moment - could you focus on elaborating
>> the scenario where such issue occurs? Your initial commit message also skips
>> important bits such as platform used when reproducing and so on, please add
>> them.
>>
>> Thanks,
>> Czarek
>
> This issue is reported to happen on BYT Chrome book (rambi) on Chrome-v4.4
> branch after changing toolchain from clang10 to clang11.
>
> The reproduce step is simple. Just run arecord multiple times (<10) then you will
> see error message for hw_params ioctl failure.
>
> $ arecord -D hw:1,0 -f S16_LE -c 2 /dev/null
> $ ctrl+c
>
> The error message suggests the stream commit (IPC_IA_ALLOC_STREAM, 0x20) IPC
> command failed due to timeout but the msg id field of ipcd read after the timeout is
> also 0x20. It seems to me that the command is success but the host driver does not
> know it for some reason.
>
> 2020-03-15T23:02:06.614151+00:00 ERR kernel: [ 852.023766] baytrail-pcm-audio baytrail-pcm-audio: ipc: --message timeout-- ipcx 0x2220 isr 0xf0020 ipcd 0x2220 imrx 0x0
> 2020-03-15T23:02:06.614209+00:00 ERR kernel: [ 852.023796] baytrail-pcm-audio baytrail-pcm-audio: ipc: error stream commit failed
> 2020-03-15T23:02:06.614218+00:00 ERR kernel: [ 852.023813] Baytrail Audio: PCM: failed stream commit -110
> 2020-03-15T23:02:06.614225+00:00 ERR kernel: [ 852.023832] baytrail-pcm-audio baytrail-pcm-audio: ASoC: baytrail-pcm-audio hw params failed: -110
>
> I add some messages to make the log easier to read. You can check the gerrit link if you
> are interested:
> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2131507
>
> In the test patch I read the SST_IPCD register in sst_byt_irq_thread() twice and found
> the value could be different. The IPC_IA_FREE_STREAM(0x21) seems to be the last IPC
> command sent in the pcm_release of previous arecord command.
>
> [ 125.009724] sound pcmC1D0c: snd_pcm_release:
> [ 125.009732] baytrail-pcm-audio baytrail-pcm-audio: ipc_tx_message: header 0x8000000000000221 wait 1
> [ 125.009760] baytrail-pcm-audio baytrail-pcm-audio: byt_tx_msg: header 0x8000000000000226
> [ 125.009803] baytrail-pcm-audio baytrail-pcm-audio: sst_byt_irq_thread: header 0x297
> [ 125.009823] baytrail-pcm-audio baytrail-pcm-audio: byt_tx_msg: header 0x8000000000000221
> [ 125.009872] baytrail-pcm-audio baytrail-pcm-audio: sst_byt_irq_thread: header 0x297
> [ 125.010442] baytrail-pcm-audio baytrail-pcm-audio: sst_byt_irq_thread: header 0x80000000000c2226
> [ 125.010456] baytrail-pcm-audio baytrail-pcm-audio: sst_ipc_reply_find_msg: not match, msg header 0x8000000000000221
> [ 125.010511] baytrail-pcm-audio baytrail-pcm-audio: sst_byt_irq_thread: header 0x8000000000000221
> [ 125.011355] baytrail-pcm-audio baytrail-pcm-audio: tx_wait_done: ipc success, header 0x8000000000000221
> [ 125.380506] sound pcmC1D0c: snd_pcm_capture_open:
> [ 125.380544] sound pcmC1D0c: snd_pcm_capture_ioctl1: INFO
> [ 125.380554] sound pcmC1D0c: snd_pcm_capture_ioctl1: PVERSION
> [ 125.380561] sound pcmC1D0c: snd_pcm_capture_ioctl1: TTSTAMP
> [ 125.380581] sound pcmC1D0c: snd_pcm_mmap:
> [ 125.380622] sound pcmC1D0c: snd_pcm_mmap:
> [ 125.380648] sound pcmC1D0c: snd_pcm_capture_ioctl1: INFO
> [ 125.380717] sound pcmC1D0c: snd_pcm_capture_ioctl1: HW_REFINE
> [ 125.380737] sound pcmC1D0c: snd_pcm_capture_ioctl1: HW_REFINE
> [ 125.380766] sound pcmC1D0c: snd_pcm_capture_ioctl1: HW_REFINE
> [ 125.380779] sound pcmC1D0c: snd_pcm_capture_ioctl1: HW_REFINE
> [ 125.380789] sound pcmC1D0c: snd_pcm_capture_ioctl1: HW_REFINE
> [ 125.380799] sound pcmC1D0c: snd_pcm_capture_ioctl1: HW_REFINE
> [ 125.380808] sound pcmC1D0c: snd_pcm_capture_ioctl1: HW_REFINE
> [ 125.380815] sound pcmC1D0c: snd_pcm_capture_ioctl1: HW_PARAMS
> [ 125.380839] baytrail-pcm-audio baytrail-pcm-audio: ipc_tx_message: header 0x8000000000642220 wait 1
> [ 125.380854] baytrail-pcm-audio baytrail-pcm-audio: byt_tx_msg: header 0x8000000000642220
> => write IPC_IA_ALLOC_STREAM message (0x20) to SST_IPCX
> [ 125.380931] baytrail-pcm-audio baytrail-pcm-audio: sst_byt_irq_thread: header 0x8000000000000221
> => read message reply from SST_IPCD and an IPC_IA_FREE_STREAM(0x21) is returned
> [ 125.380942] baytrail-pcm-audio baytrail-pcm-audio: sst_byt_irq_thread: ipcd 0x8000000000602220
> => read SST_IPCD again, this time the message id is what we are expected (0x20)
> [ 125.380948] baytrail-pcm-audio baytrail-pcm-audio: sst_ipc_reply_find_msg: not match, msg header 0x8000000000642220
> [ 125.681171] baytrail-pcm-audio baytrail-pcm-audio: tx_wait_done: ipc timeout, header 0x8000000000642220
> [ 125.681204] baytrail-pcm-audio baytrail-pcm-audio: ipc: --message timeout-- ipcx 0x2220 isr 0x50020 ipcd 0x2220 imrx 0x0
> [ 125.681212] baytrail-pcm-audio baytrail-pcm-audio: ipc: error stream commit failed
> [ 125.681217] Baytrail Audio: PCM: failed stream commit -110
> [ 125.681223] baytrail-pcm-audio baytrail-pcm-audio: ASoC: baytrail-pcm-audio hw params failed: -110
>
> Regards,
> Brent
>

I've looked at the code and byt_is_dsp_busy seems suspicious to me.
Can you check if following change fixes problem for you:

diff --git a/sound/soc/intel/baytrail/sst-baytrail-ipc.c
b/sound/soc/intel/baytrail/sst-baytrail-ipc.c
index 74274bd38f7a..34746fd871b0 100644
--- a/sound/soc/intel/baytrail/sst-baytrail-ipc.c
+++ b/sound/soc/intel/baytrail/sst-baytrail-ipc.c
@@ -666,8 +666,8 @@ static bool byt_is_dsp_busy(struct sst_dsp *dsp)
{
u64 ipcx;

- ipcx = sst_dsp_shim_read_unlocked(dsp, SST_IPCX);
- return (ipcx & (SST_IPCX_BUSY | SST_IPCX_DONE));
+ ipcx = sst_dsp_shim_read64_unlocked(dsp, SST_IPCX);
+ return (ipcx & (SST_BYT_IPCX_BUSY | SST_BYT_IPCX_DONE));
}

int sst_byt_dsp_init(struct device *dev, struct sst_pdata *pdata)
We seem to treat SST_IPCX as 32 bit register instead of 64 one, which
may explain wrong behaviour. (Specification says it is 64 bit register).

Thanks,
Amadeusz

\
 
 \ /
  Last update: 2020-04-23 10:34    [W:0.093 / U:0.676 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site