Repository navigation
dma: alloc separate buffers for DMAs - #1984
Conversation
|
How should I update my Dummy DMA driver so it is friendly with caches? If I just enable caching over the buffer memory I'm getting serious issues (repeated noise of length similar to a host period) which are potentially caused by my copying only happening in said cache and not getting flushed to actual RAM. (note: issue is present with and without this PR, it's unrelated to it but the fix might be different) |
|
@paulstelian97 Are you talking about DSP L1 cache? SOF should automatically writeback and invalidate buffers. Maybe your buffer is not aligned to cache line size? |
|
@tlauda The problem is that I suspect Host invalidates the host DMA buffer, which is the correct thing to do when we're talking about a hardware DMA but the wrong thing to do with the Dummy DMA (which writes its data in the cache rather than directly in RAM). So how should I solve this? (I have to fix it if I want to enable caching of the buffers, which would alleviate the performance issues I'm getting) |
How do you enable host buffer's cache? Isn't it controlled by the kernel? I would assume that host will expose uncached memory range to the DSP, since the DSP has the direct access to the memory. |
|
I meant enabling the cache for the buffer memory region inside the DSP. The Linux side is properly allocating DMA region and putting data in there, bypassing any cache it needs to bypass. I mean the actual buffer from there is accessible just fine. I mean the buffer between the host component and, say, volume component (that's what I have in the topology I'm testing with) and I referred to that as the "host buffer". I'm thinking that the interactions on that buffer between Dummy DMA and the caching code (particularly the flush inside host.c) aren't exactly the correct ones and this causes the breakage when I enable caching. Just as a detail, the buffer memory is in fact 8MB of DRAM (well, almost 8MB as a small part of it is used for SOF's other heaps and its stack but some 7MB+ of it is buffers, and the Linux kernel knows to treat that memory as a no-touch zone) |
|
@paulstelian97 Pleasy try to writeback copied data after memcpy in dummy dma: dcache_writeback_region of w_ptr. |
|
Of course only for playback. |
|
Somehow it doesn't seem to solve the problem. Does the DAI component (audio/dai.c) perform a writeback before triggering my driver code? I don't have cache_to_uncache and uncache_to_cache (I'm currently on an internal discussion about the memory map if we have any aliasing usable to implement these). |
|
Ah, what solved my problem was to ALSO invalidate the "read" region. It fell within the cached region. Now I should check if this caching will resolve my performance problems from the mailing list (EDIT: It seems to do something, will measure it tomorrow but it seems that the workaround I mentioned in the mailing list is no longer needed <3 ). Anyway, I think I'm done spamming this thread, this particular PR I approve. |
|
@dbaluta Unfortunately there is no way I can split commit |
@paulstelian97 thanks for taking care of this! @tlauda so basically a part for the system RAM is reserved by the kernel and can be mapped in the physical address space of the DSP (actually it can also be accessible from the kernel side). Now the DSP, can set caching per 512MB regions. So far this System RAM region was uncached on the DSP side which we think causes the performance issues when processing a simple pipeline with Volume component. Enabling the cache seems that causes us some other problems, and I think @paulstelian97 fixed them by flushing the cache whenever is appropriate. |
|
@tlauda this is an excellent PR which makes code more clear and easier to understand! I would like to make sure we won't break anything and also allow some time for other people to review. |
|
@dbaluta Our problems were especially because the buffer on the Linux side was around address 0x9601.... which sits well within the same 512MB region as the 16MB reserved for the DSP. So I had to flush that one too from the cache. I will submit a separate pull request enabling the cache these days. |
Adds and initializes id, pipeline_id and caps fields in buffer struct. This is the only data needed after buffer creation. It allows us to remove the whole sof_ipc_buffer struct from the buffers. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Extracts allocation part of buffer_new to separate function. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Adds new DMA attribute DMA_ATTR_BUFFER_PERIOD_COUNT and implements it for every DMA. This attribute says how many periods should DMA buffer consist of. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Implements simple buffer copy functions for 16 and 32 bit data. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Implements copy functions from/to DMA buffer. They take care of cache operations and also can perform custom processing during copy. By default it will be usual buffer copy, but in the future it can be replaced with pcm conversions, so we don't need volume into the pipelines just for that operation. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
DW-DMA has no specific alignments regarding buffer and copy sizes. However setting it to 32 bytes breaks stream formats with period sizes lower than 32 bytes, as linked list is being configured incorrectly. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Currently DMAs are using buffers on pipelines connected to either host or dai components. This causes many problems in firmware logic e.g. we need to preload playback pipelines with data or we need to manually override buffer sizes, if the buffer isn't complaint with specific DMA requirements. Also it requires from topology creators to know some hardware specific things on each platform like that for timer based scheduling on cAVS platforms we need to have 3 periods in buffers connected to dai components. This patch allocates separate buffers for DMAs, which are made complaint by using different DMA attributes first. We no longer need to preload playback streams, because we can start DMA right away and just let it transfer zeroes before any valid data will come. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Removes buffer cache operations done based on the fact, whether buffer is used by DMA or not. From now on DMAs have their own dedicated buffers with dedicated copy functions, so this code is no longer needed. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Change flow of components to not resize sink buffers. We should just check the size of the allocated buffers and let the topology define the maximum sizes. Components can easily work with non aligned buffer sizes. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Removes code related to preload as with separate DMA buffers we no longer need it. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Changes flow of dai_copy to call dma_copy even if there is no data available. This way we make sure that all DMA needed procedures are executed, since buffers pointers won't be changed anyway. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Reverts all previosly changed cAVS pipelines from 3 to 2 periods. Now we have separate buffers for DMA, so there is no need to make DAI buffers consist of 3 periods. DMA will take care of any internal hardware requirements. Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
ac4e27c to
f0f762d
Compare
|
This is an excellent PR which fixed a lot of problems found while working on probes (#1852 ) and probably will fix echo reference. |
|
Deleted this post - the issue is my test setup. |
|
@singalsu It also happens with host - dai pipeline? |
If you mean pipeline line PCM -> buffer -> DAI, I need to make such topology. This pipeline contained volume component. (Edit: Actually two volume components and a mixer.) |
|
It looks like some kind of codec issue. Could you set ssp to internal loopback and confirm? |
|
Sorry, I'm now suspecting it's my recording setup specific problem. Please ignore my comments for now. |
|
Yes, it's my development computer that messed up the recording. I switched to jackd instead of pulseaudio and now the recording is clean. I'm sorry @tlauda ! I'll delete the screenshots from confusing the discussion. |
Currently DMAs are using buffers on pipelines connected to either
host or dai components. This causes many problems in firmware logic
e.g. we need to preload playback pipelines with data or we need
to manually override buffer sizes, if the buffer isn't complaint
with specific DMA requirements. Also it requires from topology
creators to know some hardware specific things on each platform
like that for timer based scheduling on cAVS platforms we need
to have 3 periods in buffers connected to dai components.
This patch allocates separate buffers for DMAs, which are made
complaint by using different DMA attributes first. We no longer
need to preload playback streams, because we can start DMA
right away and just let it transfer zeroes before any valid
data will come.
Signed-off-by: Tomasz Lauda tomasz.lauda@linux.intel.com