Repository navigation
dw-dma: change dw_dma_stop for byt and cht - #1257
Conversation
There was a problem hiding this comment.
Do I understand it correctly, that previously this function wasn't disabling the DMA channel actively at all, it was waiting for the enable bit to go to 0, presumably, as triggered by some other process, e.g. by dw_dma_verify_block(), right? But that function only exists if DW_USE_HW_LLI is defined, and it is not defined on BYT and CHT. So, in fact I don't see what would be disabling the channel previously in this function. Does this mean, that this call was always running on a timeout? Were we getting "ewt" errors? The current timeout on BYT is defined to 1333us. Or was that bit cleared in some other way?
There was a problem hiding this comment.
Non cAVS platform don't have support for hardware linked list, so linked list items are manually reloaded in function dw_dma_chan_reload_lli and also this function enables the channel again (which is disabled after every lli transfer). So yes, we were just waiting for the transfer to be over with the next DMA irq, which usually took from 0.5 ms to 0.8 ms, but it really depends on when you stop the stream.
There was a problem hiding this comment.
Do you know how the channel is disabled after each transfer? Automatically or is there a write to the CHAN_EN register somewhere that I'm not seeing?
There was a problem hiding this comment.
thanks! Unrelated to this patch, but it would be nice to add a comment explaining that to the driver.
There was a problem hiding this comment.
not sure we need to OR with .cfg_lo above, but it shouldn't hurt either.
There was a problem hiding this comment.
@lyakh This means we need to keep the other bits to the original status as we only changes few bits
|
@tlauda what's the issue number, I've never tested/experiences this? |
|
@juimonen You've mentioned it here: #1208 (comment). |
|
@tlauda aah ok, I've seen this before quite often, but testing couple of days with verbose tracing on it has been quite rare... we should have a separate issue about this, because now we have the 1) topology period count change, 2) the copy_lli change in resume and this mixed up. Anyway I can test this, takes little bit time as I'm now in nocodec kernel... |
|
@tlauda @ranj063 |
xiulipan
left a comment
xiulipan
left a comment
There was a problem hiding this comment.
This will helps, please give more inline comments.
|
@tlauda changed PLATFORM_DMA_TIMEOUT to 4000 Was this what you meant? It didn't help the timeout error in stop... |
|
@juimonen No, I meant IPC timeout on kernel side. |
Changes implementation of dw_dma_stop function for baytrail and cherrytrail platforms. On this platforms we cannot simply disable channel during transfer, because it will hang DMA. Previously we were waiting for transfer finish, which could take up to one period size (1 ms in most cases). This patch uses suspend and drain FIFO mechanism, which lowers time needed to stop DMA channel to steady ~0.34 ms. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
42e19b6 to
9e4832d
Compare
|
@tlauda ipc timeout in kernel side is 300ms... and dmesg also shows time between tx and timeout about 300 ms. Am I missing something here as I see it, kernel is waiting for DSP 300ms without answer... |
|
@juimonen If it's 300 ms, then we have additional problem in kernel with getting responses as @xiulipan stated in thesofproject/linux#808. |
|
@tlauda @lyakh |
Changes implementation of dw_dma_stop function for
baytrail and cherrytrail platforms. On this platforms
we cannot simply disable channel during transfer,
because it will hang DMA. Previously we were waiting
for transfer finish, which could take up to one period
size (1 ms in most cases). This patch uses suspend and
drain FIFO mechanism, which lowers time needed to stop
DMA channel to steady ~0.34 ms.
Signed-off-by: Tomasz Lauda tomasz.lauda@linux.intel.com