Re: [PATCH 2/2] dmaengine: ep93xx: Don't drain the transfers in terminate_all()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Vinod,

On 14/05/17 15:16, Vinod Koul 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.

you are right, otherwise device_free_chan_resources() can be called on running
channel... Will do!

>>  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");
>>  }
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux