Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1233195 > unrolled thread

[PATCH 1/1] dmaengine: dw: resolve recursion lock when audio playback

Started by"yitian" <yitian.bu@tangramtek.com>
First post2015-09-26 14:40 +0200
Last post2015-09-28 11:20 +0200
Articles 6 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/1] dmaengine: dw: resolve recursion lock when audio playback "yitian" <yitian.bu@tangramtek.com> - 2015-09-26 14:40 +0200
    Re: [PATCH 1/1] dmaengine: dw: resolve recursion lock when audio playback Viresh Kumar <viresh.kumar@linaro.org> - 2015-09-28 05:30 +0200
      Re: [PATCH 1/1] dmaengine: dw: resolve recursion lock when audio playback Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-09-28 09:10 +0200
        RE: [PATCH 1/1] dmaengine: dw: resolve recursion lock when audio playback "yitian" <yitian.bu@tangramtek.com> - 2015-09-28 09:40 +0200
          Re: [PATCH 1/1] dmaengine: dw: resolve recursion lock when audio  playback Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-09-28 10:50 +0200
            RE: [PATCH 1/1] dmaengine: dw: resolve recursion lock when audio playback "yitian" <yitian.bu@tangramtek.com> - 2015-09-28 11:20 +0200

#1233195 — [PATCH 1/1] dmaengine: dw: resolve recursion lock when audio playback

From"yitian" <yitian.bu@tangramtek.com>
Date2015-09-26 14:40 +0200
Subject[PATCH 1/1] dmaengine: dw: resolve recursion lock when audio playback
Message-ID<qcWVH-8tA-15@gated-at.bofh.it>
when audio playback is running, user space will call
snd_pcm_lib_write1() function, it will first get pcm stream lock,
after that it may call function snd_pcm_update_hw_ptr(), the call
stack will be as below:
  snd_pcm_lib_write1  <-- got pcm stream lock
   --> snd_pcm_update_hw_ptr
     --> dwc_tx_status
      --> dwc_scan_descriptors
       --> callback
        --> dmaengine_pcm_dma_complete
         --> snd_pcm_period_elapsed <-- get stream lock again
recursion lock occurs in snd_pcm_period_elapsed function.
remove dwc_scan_descriptors from dwc_tx_status can fix this issue.

Signed-off-by: Yitian Bu <yitian.bu@tangramtek.com>
---
 drivers/dma/dw/core.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index cf1c87f..5341b44 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1078,11 +1078,7 @@ dwc_tx_status(struct dma_chan *chan,
 	ret = dma_cookie_status(chan, cookie, txstate);
 	if (ret == DMA_COMPLETE)
 		return ret;
-
-	dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
-
-	ret = dma_cookie_status(chan, cookie, txstate);
-	if (ret != DMA_COMPLETE)
+	else
 		dma_set_residue(txstate, dwc_get_residue(dwc));
 
 	if (dwc->paused && ret == DMA_IN_PROGRESS)
-- 
1.7.12.4



Yitian
Best Regards




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1233830

FromViresh Kumar <viresh.kumar@linaro.org>
Date2015-09-28 05:30 +0200
Message-ID<qdxix-1Yl-5@gated-at.bofh.it>
In reply to#1233195
On 26 September 2015 at 05:38, yitian <yitian.bu@tangramtek.com> wrote:
> when audio playback is running, user space will call
> snd_pcm_lib_write1() function, it will first get pcm stream lock,
> after that it may call function snd_pcm_update_hw_ptr(), the call
> stack will be as below:
>   snd_pcm_lib_write1  <-- got pcm stream lock
>    --> snd_pcm_update_hw_ptr
>      --> dwc_tx_status
>       --> dwc_scan_descriptors
>        --> callback
>         --> dmaengine_pcm_dma_complete
>          --> snd_pcm_period_elapsed <-- get stream lock again
> recursion lock occurs in snd_pcm_period_elapsed function.
> remove dwc_scan_descriptors from dwc_tx_status can fix this issue.
>
> Signed-off-by: Yitian Bu <yitian.bu@tangramtek.com>
> ---
>  drivers/dma/dw/core.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)

I am not sure if this is a sane way of doing that, and we were scanning
the descriptors for some valid reason..

Though, I am on vacation for few days now and will be able to look
at the details later.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233894

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2015-09-28 09:10 +0200
Message-ID<qdAJr-6Z2-11@gated-at.bofh.it>
In reply to#1233830
On Mon, Sep 28, 2015 at 6:23 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 26 September 2015 at 05:38, yitian <yitian.bu@tangramtek.com> wrote:
>> when audio playback is running, user space will call
>> snd_pcm_lib_write1() function, it will first get pcm stream lock,
>> after that it may call function snd_pcm_update_hw_ptr(), the call
>> stack will be as below:
>>   snd_pcm_lib_write1  <-- got pcm stream lock
>>    --> snd_pcm_update_hw_ptr
>>      --> dwc_tx_status
>>       --> dwc_scan_descriptors
>>        --> callback
>>         --> dmaengine_pcm_dma_complete
>>          --> snd_pcm_period_elapsed <-- get stream lock again
>> recursion lock occurs in snd_pcm_period_elapsed function.
>> remove dwc_scan_descriptors from dwc_tx_status can fix this issue.
>>
>> Signed-off-by: Yitian Bu <yitian.bu@tangramtek.com>
>> ---
>>  drivers/dma/dw/core.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> I am not sure if this is a sane way of doing that, and we were scanning
> the descriptors for some valid reason..

Actually one of the patches in a pile sitting in my private repo is
also including similar change. In my case the reason is to support
cyclic transfers natively.

> Though, I am on vacation for few days now and will be able to look
> at the details later.

Have a good rest then!

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233914

From"yitian" <yitian.bu@tangramtek.com>
Date2015-09-28 09:40 +0200
Message-ID<qdBcu-7x9-7@gated-at.bofh.it>
In reply to#1233894
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Monday, September 28, 2015 3:06 PM
> To: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: yitian <yitian.bu@tangramtek.com>; Viresh Kumar
> <vireshk@kernel.org>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>; Vinod Koul <vinod.koul@intel.com>;
> Dan Williams <dan.j.williams@intel.com>; dmaengine
> <dmaengine@vger.kernel.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH 1/1] dmaengine: dw: resolve recursion 
> > I am not sure if this is a sane way of doing that, and we were scanning
> > the descriptors for some valid reason..
> 
> Actually one of the patches in a pile sitting in my private repo is
> also including similar change. In my case the reason is to support
> cyclic transfers natively.
> 

Yes, i am using DW DMAC to support cyclic transfer.
Currently it is very easy to get recursion lock crash, but
after this patch everything is fine on my device.
I am waiting for your further guide.
Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233948 — Re: [PATCH 1/1] dmaengine: dw: resolve recursion lock when audio playback

FromAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Date2015-09-28 10:50 +0200
SubjectRe: [PATCH 1/1] dmaengine: dw: resolve recursion lock when audio playback
Message-ID<qdCie-Fm-13@gated-at.bofh.it>
In reply to#1233914
On Mon, 2015-09-28 at 15:23 +0800, yitian wrote:
> > 
> > From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> > Sent: Monday, September 28, 2015 3:06 PM
> > To: Viresh Kumar <viresh.kumar@linaro.org>
> > Cc: yitian <yitian.bu@tangramtek.com>; Viresh Kumar
> > <vireshk@kernel.org>; Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com>; Vinod Koul <
> > vinod.koul@intel.com>;
> > Dan Williams <dan.j.williams@intel.com>; dmaengine
> > <dmaengine@vger.kernel.org>; Linux Kernel Mailing List
> > <linux-kernel@vger.kernel.org>
> > Subject: Re: [PATCH 1/1] dmaengine: dw: resolve recursion 
> > > I am not sure if this is a sane way of doing that, and we were 
> > > scanning
> > > the descriptors for some valid reason..
> > 
> > Actually one of the patches in a pile sitting in my private repo is
> > also including similar change. In my case the reason is to support
> > cyclic transfers natively.
> > 
> 
> Yes, i am using DW DMAC to support cyclic transfer.
> Currently it is very easy to get recursion lock crash, but
> after this patch everything is fine on my device.

What is an actual hardware you are running kernel on?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1233969

From"yitian" <yitian.bu@tangramtek.com>
Date2015-09-28 11:20 +0200
Message-ID<qdCLg-1sd-25@gated-at.bofh.it>
In reply to#1233948
> From: Andy Shevchenko [mailto:andriy.shevchenko@linux.intel.com]
> Sent: Monday, September 28, 2015 4:46 PM
> To: yitian <yitian.bu@tangramtek.com>; 'Andy Shevchenko'
> <andy.shevchenko@gmail.com>; 'Viresh Kumar' <viresh.kumar@linaro.org>
> Cc: 'Viresh Kumar' <vireshk@kernel.org>; 'Vinod Koul'
> <vinod.koul@intel.com>; 'Dan Williams' <dan.j.williams@intel.com>;
> 'dmaengine' <dmaengine@vger.kernel.org>; 'Linux Kernel Mailing List'
> <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH 1/1] dmaengine: dw: resolve recursion lock when
> audio playback
> 
> On Mon, 2015-09-28 at 15:23 +0800, yitian wrote:
> > >
> > > From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> > > Sent: Monday, September 28, 2015 3:06 PM
> > > To: Viresh Kumar <viresh.kumar@linaro.org>
> > > Cc: yitian <yitian.bu@tangramtek.com>; Viresh Kumar
> > > <vireshk@kernel.org>; Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com>; Vinod Koul <
> > > vinod.koul@intel.com>;
> > > Dan Williams <dan.j.williams@intel.com>; dmaengine
> > > <dmaengine@vger.kernel.org>; Linux Kernel Mailing List
> > > <linux-kernel@vger.kernel.org>
> > > Subject: Re: [PATCH 1/1] dmaengine: dw: resolve recursion
> > > > I am not sure if this is a sane way of doing that, and we were
> > > > scanning
> > > > the descriptors for some valid reason..
> > >
> > > Actually one of the patches in a pile sitting in my private repo is
> > > also including similar change. In my case the reason is to support
> > > cyclic transfers natively.
> > >
> >
> > Yes, i am using DW DMAC to support cyclic transfer.
> > Currently it is very easy to get recursion lock crash, but
> > after this patch everything is fine on my device.
> 
> What is an actual hardware you are running kernel on?
> 
Hi Andy:

I am using a FPGA, with Cortex-A5 core, Designware I2S IP, Designware
DMAC IP. What I was done is to run tinyplay and tinycap to test the
playback and capture function on the FPGA. With my change, both of
them are okay now. I didn't push the patch which added cyclic DMA
support for dw DMAC because I didn't make it decent enough yet.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web