Thread (16 messages) 16 messages, 5 authors, 2020-08-25

Re: pcm|dmaengine|imx-sdma race condition on i.MX6

From: Sascha Hauer <s.hauer@pengutronix.de>
Date: 2020-08-20 07:10:29
Also in: alsa-devel, dmaengine, lkml

On Wed, Aug 19, 2020 at 01:08:29PM +0200, Lars-Peter Clausen wrote:
quoted
For the first option, which is potentially more performant, we have to leave the atomic PCM context
and we are not sure if we are allowed to.
For the second option, we would have to divide the dma_device terminate_all into an atomic sync and
an async one, which would align with the dmaengine API, giving it the option to ensure termination
in an atomic context.
Based on my understanding, most of them are synchronous anyways, for the currently async ones we
would have to implement busy waits.
However, with this approach, we reach the WARN_ON [6] inside of an atomic context,
indicating we might not do the right thing.
I don't know how feasible this is to implement in the SDMA dmaengine driver.
But I think what is should do is to have some flag to indicate if a
terminate is in progress. If a new transfer is issued while terminate is in
progress the transfer should go on a list. Once terminate finishes it should
check the list and start the transfer if there are any on the list.
The list is already there in form of the vchan helpers the driver uses.

I think the big mistake the driver makes is to configure fields in
struct sdma_channel and also the hardware directly in
sdma_prep_memcpy(), sdma_prep_slave_sg() and sdma_prep_dma_cyclic(). All
information should be stored in the struct sdma_desc allocated in the
prep functions and only be used when it's time to fire that specific
descriptor.

More specifically sdma_config_write() may not be called from
sdma_prep_slave_sg() or sdma_prep_dma_cyclic(), but instead must be
called from sdma_start_desc().  sdma_config_ownership() also must be
called later in sdma_start_desc(). 'direction' must be a member of
struct sdma_desc, not of struct sdma_channel.

Overall this sounds like a fair amount of work to do, but should be
feasible and IMO is a step in the right direction.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help