On Sun, May 14, 2017 at 12:13:27AM +0200, Alexander Sverdlin wrote:
> As dmaengine.h suggests, device_terminate_all() "Aborts all transfers on a
> channel". Seems that no other driver tried to busy-wait and actually drain
> the data. Moreover, this happens with IRQs disabled, therefore induces huge
> latency:
Perhaps you should move current code to device_synchronize callback to allow
people to do so, if desired.
>
> irqsoff latency trace v1.1.5 on 4.11.0
> --------------------------------------------------------------------
> latency: 39770 us, #57/57, CPU#0 | (M:preempt VP:0, KP:0, SP:0 HP:0)
> -----------------
> | task: process-129 (uid:0 nice:0 policy:2 rt_prio:50)
> -----------------
> => started at: _snd_pcm_stream_lock_irqsave
> => ended at: snd_pcm_stream_unlock_irqrestore
>
> _------=> CPU#
> / _-----=> irqs-off
> | / _----=> need-resched
> || / _---=> hardirq/softirq
> ||| / _--=> preempt-depth
> |||| / delay
> cmd pid ||||| time | caller
> \ / ||||| \ | /
> process-129 0d.s. 3us : _snd_pcm_stream_lock_irqsave
> process-129 0d.s1 9us : snd_pcm_stream_lock <-_snd_pcm_stream_lock_irqsave
> process-129 0d.s1 15us : preempt_count_add <-snd_pcm_stream_lock
> process-129 0d.s2 22us : preempt_count_add <-snd_pcm_stream_lock
> process-129 0d.s3 32us : snd_pcm_update_hw_ptr0 <-snd_pcm_period_elapsed
> process-129 0d.s3 41us : soc_pcm_pointer <-snd_pcm_update_hw_ptr0
> process-129 0d.s3 50us : dmaengine_pcm_pointer <-soc_pcm_pointer
> process-129 0d.s3 58us+: snd_dmaengine_pcm_pointer_no_residue <-dmaengine_pcm_pointer
> process-129 0d.s3 96us : update_audio_tstamp <-snd_pcm_update_hw_ptr0
> process-129 0d.s3 103us : snd_pcm_update_state <-snd_pcm_update_hw_ptr0
> process-129 0d.s3 112us : xrun <-snd_pcm_update_state
> process-129 0d.s3 119us : snd_pcm_stop <-xrun
> process-129 0d.s3 126us : snd_pcm_action <-snd_pcm_stop
> process-129 0d.s3 134us : snd_pcm_action_single <-snd_pcm_action
> process-129 0d.s3 141us : snd_pcm_pre_stop <-snd_pcm_action_single
> process-129 0d.s3 150us : snd_pcm_do_stop <-snd_pcm_action_single
> process-129 0d.s3 157us : soc_pcm_trigger <-snd_pcm_do_stop
> process-129 0d.s3 166us : snd_dmaengine_pcm_trigger <-soc_pcm_trigger
> process-129 0d.s3 175us : ep93xx_dma_terminate_all <-snd_dmaengine_pcm_trigger
> process-129 0d.s3 182us : preempt_count_add <-ep93xx_dma_terminate_all
> process-129 0d.s4 189us*: m2p_hw_shutdown <-ep93xx_dma_terminate_all
> process-129 0d.s4 39472us : m2p_hw_setup <-ep93xx_dma_terminate_all
>
> ... rest skipped...
>
> process-129 0d.s. 40080us : <stack trace>
> => ep93xx_dma_tasklet
> => tasklet_action
> => __do_softirq
> => irq_exit
> => __handle_domain_irq
> => vic_handle_irq
> => __irq_usr
> => 0xb66c6668
>
> Just abort the transfers and warn if the HW state is not what we expect.
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> drivers/dma/ep93xx_dma.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
> index a3d946d2a8e1..e78d1fbf2179 100644
> --- a/drivers/dma/ep93xx_dma.c
> +++ b/drivers/dma/ep93xx_dma.c
> @@ -345,19 +345,10 @@ static inline u32 m2p_channel_state(struct ep93xx_dma_chan *edmac)
>
> static void m2p_hw_shutdown(struct ep93xx_dma_chan *edmac)
> {
> - u32 control;
> -
> - control = readl(edmac->regs + M2P_CONTROL);
> - control &= ~(M2P_CONTROL_STALLINT | M2P_CONTROL_NFBINT);
> - m2p_set_control(edmac, control);
> -
> - while (m2p_channel_state(edmac) >= M2P_STATE_ON)
> - cpu_relax();
> -
> m2p_set_control(edmac, 0);
>
> - while (m2p_channel_state(edmac) == M2P_STATE_STALL)
> - cpu_relax();
> + while (m2p_channel_state(edmac) != M2P_STATE_IDLE)
> + dev_warn(chan2dev(edmac), "M2P: Not yet IDLE\n");
> }
>
> static void m2p_fill_desc(struct ep93xx_dma_chan *edmac)
> --
> 2.12.2
>
--
~Vinod