| Message ID | 1423954053-21760-3-git-send-email-robert.jarzmik@free.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Saturday 14 February 2015 23:47:33 Robert Jarzmik wrote: > @@ -294,7 +294,8 @@ int pxa_request_dma (char *name, pxa_dma_prio prio, > /* try grabbing a DMA channel with the requested priority */ > for (i = 0; i < num_dma_channels; i++) { > if ((dma_channels[i].prio == prio) && > - !dma_channels[i].name) { > + !dma_channels[i].name && > + !mmp_pdma_toggle_reserved_channel(i)) { > found = 1; > break; > } > How is the order between the two enforced? I.e. can it be that the dmaengine driver uses the same channel for a different slave before we get here? If this is ensured to work, I'm fine with your approach. Arnd
----- Mail original ----- De: "Arnd Bergmann" <arnd@arndb.de> À: linux-arm-kernel@lists.infradead.org Cc: "Robert Jarzmik" <robert.jarzmik@free.fr>, "Vinod Koul" <vinod.koul@intel.com>, "Olof Johansson" <olof@lixom.net>, "Daniel Mack" <zonque@gmail.com>, "Haojian Zhuang" <haojian.zhuang@gmail.com>, dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org Envoyé: Lundi 16 Février 2015 11:28:57 Objet: Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1 On Saturday 14 February 2015 23:47:33 Robert Jarzmik wrote: >> @@ -294,7 +294,8 @@ int pxa_request_dma (char *name, pxa_dma_prio prio, >> /* try grabbing a DMA channel with the requested priority */ >> for (i = 0; i < num_dma_channels; i++) { >> if ((dma_channels[i].prio == prio) && >> - !dma_channels[i].name) { >> + !dma_channels[i].name && >> + !mmp_pdma_toggle_reserved_channel(i)) { >> found = 1; >> break; >> } >> > How is the order between the two enforced? I.e. can it be that the dmaengine > driver uses the same channel for a different slave before we get here? If a request is made for a "low prio channel", ie. channel 8, 9 or 10 if I remember correctly : - suppose dmaengine has transactions underway, and channel 8 is busy - this loop, for i == 8 : mmp_pdma_toggle_reserved_channel(8) -> -EBUSY - loop continues, i == 9 : mmp_pdma_toggle_reserved_channel(8) -> 0 => pxa_request_dma reserves channel 9 From now on, mmp_pdma will "skip" channel 9 from its candidates to serve requests. > If this is ensured to work, I'm fine with your approach. Actually it does. Not exactly this version, as the mmp_pdma interrupt was a bit amended to "skip" also reserved channels and not steal events from legacy code, but that will be for official submission. It's also designed to be race free, relying on the fact that there is only one CPU on pxa{2,3}xx SocS (irq_save covers). I'm testing it right now with 2 drivers : - pxa3xx_nand, which is converted to dmaengine - pxamci, which is not converted to dmaengine In 3 variants : - zylonite pxa3xx board booted with device-tree - zylonite pxa3xx board booted in legacy - mioa701 board booted in device-tree So far so good. If the mmp_pdma patch is accepted for the transition, it will open up my path towards dmaengine conversion of all pxa drivers (mmc, nand, pxa_camera and pxa2xx-pcm-lib). As Daniel has already done most of the work, and because I have kept it for 2 years, that part will be less a burden than it would seem ... only the camera will give me a small headache. Cheers. -- Robert
On Sun, Feb 15, 2015 at 1:47 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote: > In order to slowly transition pxa to dmaengine, the legacy code will now > rely on dmaengine to request a channel. Hi Robert, What about dropping old PXA DMA code completely? Daniel Mack did port for most of PXA drivers to dma engine, I've rebased his patches against 3.17 several months ago and fixed oopses in pxamci and asoc drivers, but I didn't resubmit whole series due to lack of time. My 3.17 tree is at [1], I've tested it on pxa270 machine (Zipit Z2), and everything works fine so far. I guess it won't be too much work to rebase it against linux-3.20. Regards, Vasily [1] https://github.com/anarsoul/linux-2.6/tree/v3.17-pxa-dmaengine-wip
On 02/16/2015 12:14 PM, Vasily Khoruzhick wrote: > What about dropping old PXA DMA code completely? Daniel Mack did port > for most of PXA drivers to dma engine, > I've rebased his patches against 3.17 several months ago and fixed > oopses in pxamci and asoc drivers, but I didn't resubmit whole series > due to lack of time. Yes, I did the same locally. > My 3.17 tree is at [1], I've tested it on pxa270 machine (Zipit Z2), > and everything works fine so far. I guess it won't be too much work to > rebase it against linux-3.20. Rebasing is easy, but there's more to that. We should at least try to convert all drivers over to dmaengine, otherwise they will just stop working. The biggest problem that I currently see is the pxa camera driver, which also uses an advanced way of descriptor hot-linking at runtime. Last time I checked, this wasn't possible to achieve with the dma-slave API. Hence, this driver might need to be completely rewritten, which only someone with such hardware at hand can work on. Maybe the cyclic DMA support in mmp-dma already suffices for that. Robert, weren't you already working on that? Thanks, Daniel
On Monday 16 February 2015 12:12:14 robert.jarzmik@free.fr wrote: > ----- Mail original ----- > De: "Arnd Bergmann" <arnd@arndb.de> > À: linux-arm-kernel@lists.infradead.org > Cc: "Robert Jarzmik" <robert.jarzmik@free.fr>, "Vinod Koul" <vinod.koul@intel.com>, "Olof Johansson" <olof@lixom.net>, "Daniel Mack" <zonque@gmail.com>, "Haojian Zhuang" <haojian.zhuang@gmail.com>, dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org > Envoyé: Lundi 16 Février 2015 11:28:57 > Objet: Re: [PATCH RFC 2/2] ARM: pxa: transition to dmaengine phase 1 > > On Saturday 14 February 2015 23:47:33 Robert Jarzmik wrote: > >> @@ -294,7 +294,8 @@ int pxa_request_dma (char *name, pxa_dma_prio prio, > >> /* try grabbing a DMA channel with the requested priority */ > >> for (i = 0; i < num_dma_channels; i++) { > >> if ((dma_channels[i].prio == prio) && > >> - !dma_channels[i].name) { > >> + !dma_channels[i].name && > >> + !mmp_pdma_toggle_reserved_channel(i)) { > >> found = 1; > >> break; > >> } > >> > > > How is the order between the two enforced? I.e. can it be that the dmaengine > > driver uses the same channel for a different slave before we get here? > > If a request is made for a "low prio channel", ie. channel 8, 9 or 10 if I remember > correctly : > - suppose dmaengine has transactions underway, and channel 8 is busy > - this loop, for i == 8 : mmp_pdma_toggle_reserved_channel(8) -> -EBUSY > - loop continues, i == 9 : mmp_pdma_toggle_reserved_channel(8) -> 0 > => pxa_request_dma reserves channel 9 > > From now on, mmp_pdma will "skip" channel 9 from its candidates to serve requests. Ah, so the function asks for just one channel of the right priority, not a particular channel. I missed that detail. > > If this is ensured to work, I'm fine with your approach. > Actually it does. Not exactly this version, as the mmp_pdma interrupt was a bit > amended to "skip" also reserved channels and not steal events from legacy code, > but that will be for official submission. > > It's also designed to be race free, relying on the fact that there is only one > CPU on pxa{2,3}xx SocS (irq_save covers). Ok. > I'm testing it right now with 2 drivers : > - pxa3xx_nand, which is converted to dmaengine > - pxamci, which is not converted to dmaengine > In 3 variants : > - zylonite pxa3xx board booted with device-tree > - zylonite pxa3xx board booted in legacy > - mioa701 board booted in device-tree > > So far so good. If the mmp_pdma patch is accepted for the transition, it will > open up my path towards dmaengine conversion of all pxa drivers (mmc, nand, > pxa_camera and pxa2xx-pcm-lib). As Daniel has already done most of the work, > and because I have kept it for 2 years, that part will be less a burden than > it would seem ... Ok, great! > only the camera will give me a small headache. Let's worry about that when all the other ones are done then ;-) Arnd
On 02/16/2015 12:12 PM, robert.jarzmik@free.fr wrote: > So far so good. If the mmp_pdma patch is accepted for the transition, it will > open up my path towards dmaengine conversion of all pxa drivers (mmc, nand, > pxa_camera and pxa2xx-pcm-lib). As Daniel has already done most of the work, > and because I have kept it for 2 years, that part will be less a burden than > it would seem ... only the camera will give me a small headache. Ah, sorry, missed this part of the thread. Good to know you're on it :) Thanks, Daniel
Vasily Khoruzhick <anarsoul@gmail.com> writes: > On Sun, Feb 15, 2015 at 1:47 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote: >> In order to slowly transition pxa to dmaengine, the legacy code will now >> rely on dmaengine to request a channel. > > Hi Robert, > > What about dropping old PXA DMA code completely? Daniel Mack did port > for most of PXA drivers to dma engine, > I've rebased his patches against 3.17 several months ago and fixed > oopses in pxamci and asoc drivers, but I didn't resubmit whole series > due to lack of time. Well, it's the last step, yes. But I want a "smooth transition" : if amongst the ported drivers one starts to bug, I want to be able to revert _only_ that driver port to dmaengine, and not _all_ the drivers. That's the rationale of this patch : - phase 1 : enable peacefull coexistence of legacy pxa_request_dma() and dmaengine for pxa, for both devicetree and legacy platforms - phase 2 : port the drivers, and ensure the work correctly This might take a couple of cycles Note that phase 1 ensures that submissions can go through each maintainer's tree without need for strong consistence. - phase 3 : revert the mmp_pdma patch, and drop arch/arm/plat-pxa/dma.c > My 3.17 tree is at [1], I've tested it on pxa270 machine (Zipit Z2), > and everything works fine so far. I guess it won't be too much work to > rebase it against linux-3.20. Oh, do you volunteer ? That would indeed ease up my burden. I only rebased pxa3xx_nand, so any help to submit and push is welcome. At least I can commit to review, and I would concentrate on pxa_camera.c in the meantime. Cheers.
On Mon, Feb 16, 2015 at 7:54 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote: > Oh, do you volunteer ? That would indeed ease up my burden. I only rebased > pxa3xx_nand, so any help to submit and push is welcome. At least I can commit to > review, and I would concentrate on pxa_camera.c in the meantime. I can rebase those patches on-top of Linus' tree and make sure they work on my machine. I can test pxamci, pxa2xx-spi and pxa2xx-pcm drivers. But I don't think that I have enough time now to submit it and go through review process. Btw, where I can find your git tree? Regards, Vasily > Cheers. > > -- > Robert
Vasily Khoruzhick <anarsoul@gmail.com> writes: > On Mon, Feb 16, 2015 at 7:54 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote: > >> Oh, do you volunteer ? That would indeed ease up my burden. I only rebased >> pxa3xx_nand, so any help to submit and push is welcome. At least I can commit to >> review, and I would concentrate on pxa_camera.c in the meantime. > > I can rebase those patches on-top of Linus' tree and make sure they > work on my machine. > I can test pxamci, pxa2xx-spi and pxa2xx-pcm drivers. But I don't > think that I have enough time now to submit it and > go through review process. Do your best, I'll take over when you'll meet your load limit. > Btw, where I can find your git tree? Here : git fetch https://github.com/rjarzmik/linux.git work/clocks-pxa Cheers.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index bb358d2..cc23f2b 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -617,10 +617,12 @@ config ARCH_PXA select CLKDEV_LOOKUP select CLKSRC_MMIO select CLKSRC_OF + select DMADEVICES select GENERIC_CLOCKEVENTS select GPIO_PXA select HAVE_IDE select IRQ_DOMAIN + select MMP_PDMA select MULTI_IRQ_HANDLER select PLAT_PXA select SPARSE_IRQ diff --git a/arch/arm/plat-pxa/dma.c b/arch/arm/plat-pxa/dma.c index 054fc5a..e5094a6 100644 --- a/arch/arm/plat-pxa/dma.c +++ b/arch/arm/plat-pxa/dma.c @@ -294,7 +294,8 @@ int pxa_request_dma (char *name, pxa_dma_prio prio, /* try grabbing a DMA channel with the requested priority */ for (i = 0; i < num_dma_channels; i++) { if ((dma_channels[i].prio == prio) && - !dma_channels[i].name) { + !dma_channels[i].name && + !mmp_pdma_toggle_reserved_channel(i)) { found = 1; break; } @@ -331,6 +332,7 @@ void pxa_free_dma (int dma_ch) local_irq_save(flags); DCSR(dma_ch) = DCSR_STARTINTR|DCSR_ENDINTR|DCSR_BUSERR; dma_channels[dma_ch].name = NULL; + mmp_pdma_toggle_reserved_channel(dma_ch); local_irq_restore(flags); } EXPORT_SYMBOL(pxa_free_dma); diff --git a/arch/arm/plat-pxa/include/plat/dma.h b/arch/arm/plat-pxa/include/plat/dma.h index a7b91dc..f4c6339 100644 --- a/arch/arm/plat-pxa/include/plat/dma.h +++ b/arch/arm/plat-pxa/include/plat/dma.h @@ -81,5 +81,6 @@ int pxa_request_dma (char *name, void *data); void pxa_free_dma (int dma_ch); +extern int mmp_pdma_toggle_reserved_channel(int legacy_channel); #endif /* __PLAT_DMA_H */
In order to slowly transition pxa to dmaengine, the legacy code will now rely on dmaengine to request a channel. This implies that PXA architecture selects DMADEVICES and MMP_PDMA, which is not pretty. Yet it enables PXA drivers to be ported one by one, with part of them using dmaengine, and the other part using the legacy code. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> --- arch/arm/Kconfig | 2 ++ arch/arm/plat-pxa/dma.c | 4 +++- arch/arm/plat-pxa/include/plat/dma.h | 1 + 3 files changed, 6 insertions(+), 1 deletion(-)