On Thu, 16 Nov 2017 08:55:25 +0100, Kuninori Morimoto wrote:
Hi Takashi-san
commit 4821d914fe74 ("ASoC: rsnd: use dma_sync_single_for_xxx() for IOMMU") had supported IOMMU, but it breaks normal sound "recorde" and both PulseAudio's "playback/recorde". The sound will be noisy.
That commit was using dma_sync_single_for_xxx(), and driver should make sure memory is protected during CPU or Device are using it. But if driver returns current "residue" data size correctly on pointer function, player/recorder will access to protected memory.
IOMMU feature should be supported, but I don't know how to handle it without memory cache problem at this point. Thus, this patch simply revert it to avoid current noisy sound.
The driver may return the position at the last synced point via pointer callback while keeping the finer position by compensating with status delay field. In that way, user-space can avoid to touch the in-flight memory but can know the exact position.
BTW, if the problem is only about PA, the easiest workaround would be to put SNDRV_PCM_INFO_BATCH flag. Then PA switches from tsched to normal mode.
The issue (= noise) is not only for PulseAudio, but NormalAudio had capture noise issue.
OK, it indicates that returning the value with residue is actually wrong. Basically the pointer callback needs to give the position that has been already processed where you can safely scratch at this point at most.
Maybe we can use "delay" field, but this IOMMU patch is assuming many things which is no guarantee anyway, I can say dirty. And Renesas platform is not yet enabled IOMMU at this point. Thus, removing IOMMU support has no degrade, and will be fresh.
IOMMU support on this driver was created based on local prototype SoC IOMMU support. So, I want to fresh create new IOMMU support in the future
Sure, I'm not against the revert, which is the safest option, but just wanted to mention about a right implementation in such scenarios.
Takashi