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


Groups > linux.kernel > #1528286 > unrolled thread

Tearing down DMA transfer setup after DMA client has finished

Started byMason <slash.tmp@free.fr>
First post2016-11-23 11:30 +0100
Last post2016-12-08 12:50 +0100
Articles 20 on this page of 80 — 7 participants

Back to article view | Back to linux.kernel


Contents

  Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-23 11:30 +0100
    Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-23 13:20 +0100
      Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-23 13:50 +0100
        Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-23 18:30 +0100
          Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-24 12:00 +0100
            Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-24 15:20 +0100
              Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-24 16:30 +0100
                Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-24 17:40 +0100
    Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-11-25 05:50 +0100
      Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 13:00 +0100
        Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-25 15:10 +0100
          Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 15:20 +0100
            Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-25 15:30 +0100
              Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 15:50 +0100
      Re: Tearing down DMA transfer setup after DMA client has finished Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-25 13:50 +0100
        Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 14:10 +0100
          Re: Tearing down DMA transfer setup after DMA client has finished Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-25 14:40 +0100
            Re: Tearing down DMA transfer setup after DMA client has finished Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-25 15:00 +0100
              Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 15:10 +0100
                Re: Tearing down DMA transfer setup after DMA client has finished Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-25 15:30 +0100
                  Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 15:50 +0100
                    Re: Tearing down DMA transfer setup after DMA client has finished Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-25 16:00 +0100
                      Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 16:30 +0100
                  Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-25 16:10 +0100
                    Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 16:20 +0100
                      Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 16:30 +0100
                      Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-25 16:30 +0100
            Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 15:00 +0100
      Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-25 13:50 +0100
        Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 14:20 +0100
          Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-25 15:30 +0100
            Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-25 15:40 +0100
              Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-25 16:50 +0100
        Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-29 19:30 +0100
          Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-06 06:10 +0100
            Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-12-06 13:50 +0100
              Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-06 14:20 +0100
                Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-12-06 16:30 +0100
                  Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-06 16:40 +0100
                    Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-12-07 00:00 +0100
                Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-07 17:40 +0100
                  Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-07 17:50 +0100
                    Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-08 11:40 +0100
                      Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-12-08 12:00 +0100
                        Re: Tearing down DMA transfer setup after DMA client has finished Geert Uytterhoeven <geert@linux-m68k.org> - 2016-12-08 12:20 +0100
                          Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-08 12:50 +0100
                            Re: Tearing down DMA transfer setup after DMA client has finished Geert Uytterhoeven <geert@linux-m68k.org> - 2016-12-08 13:10 +0100
                              Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-12-08 13:20 +0100
                              Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-08 13:30 +0100
                        Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-08 17:40 +0100
                          Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-08 17:50 +0100
                            Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-09 08:10 +0100
                              Re: Tearing down DMA transfer setup after DMA client has finished Sebastian Frias <sf84@laposte.net> - 2016-12-09 11:30 +0100
                                Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-09 12:40 +0100
                                Re: Tearing down DMA transfer setup after DMA client has finished 1Måns Rullgård <mans@mansr.com> - 2016-12-09 12:40 +0100
                                Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-09 18:20 +0100
                                  Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-09 18:30 +0100
                                    Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-09 19:00 +0100
                                  Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-12-09 18:40 +0100
                                    Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-09 19:00 +0100
                                      Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-09 19:20 +0100
                                      Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-12-09 19:30 +0100
                      Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-08 12:50 +0100
                        Re: Tearing down DMA transfer setup after DMA client has finished Geert Uytterhoeven <geert@linux-m68k.org> - 2016-12-08 13:10 +0100
                          Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-08 13:30 +0100
                            Re: Tearing down DMA transfer setup after DMA client has finished Geert Uytterhoeven <geert@linux-m68k.org> - 2016-12-08 13:40 +0100
                            Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-08 13:50 +0100
                              Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-12-08 14:40 +0100
                                Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-08 14:40 +0100
                            Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-12-08 13:50 +0100
                            Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-08 16:50 +0100
                              Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-08 17:40 +0100
                        Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-08 16:40 +0100
                          Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-12-08 16:50 +0100
                            Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-08 17:30 +0100
                          Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-08 17:50 +0100
              Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-07 17:40 +0100
                Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-07 17:50 +0100
                  Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-12-08 11:30 +0100
                    Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-12-08 12:50 +0100

Page 2 of 4 — ← Prev page 1 [2] 3 4  Next page →


#1530296

FromMåns Rullgård <mans@mansr.com>
Date2016-11-25 15:50 +0100
Message-ID<sHpZ8-1P4-27@gated-at.bofh.it>
In reply to#1530274
Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> On Fri, Nov 25, 2016 at 02:03:20PM +0000, Måns Rullgård wrote:
>> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>> 
>> > On Fri, Nov 25, 2016 at 01:50:35PM +0000, Måns Rullgård wrote:
>> >> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>> >> > It would be unfair to augment the API and add the burden on everyone
>> >> > for the new API when 99.999% of the world doesn't require it.
>> >> 
>> >> I don't think making this particular dma driver wait for the descriptor
>> >> callback to return before reusing a channel quite amounts to a horrid
>> >> hack.  It certainly wouldn't burden anyone other than the poor drivers
>> >> for devices connected to it, all of which are specific to Sigma AFAIK.
>> >
>> > Except when you stop to think that delaying in a tasklet is exactly
>> > the same as randomly delaying in an interrupt handler - the tasklet
>> > runs on the return path back to the parent context of an interrupt
>> > handler.  Even if you sleep in the tasklet, you're sleeping on behalf
>> > of the currently executing thread - if it's a RT thread, you effectively
>> > destroy the RT-ness of the thread.  Let's hope no one cares about RT
>> > performance on that hardware...
>> 
>> That's why I suggested to do this only if the needed delay is known to
>> be no more than a few bus cycles.  The completion callback is currently
>> the only post-transfer interaction we have between the dma and device
>> drivers.  To handle an arbitrarily long delay, some new interface will
>> be required.
>
> And now we're back at the point I made a few emails ago about undue
> burden which is just about quoted above...

So what do you suggest?  Stick our heads in the sand and pretend
everything is perfect?

-- 
Måns Rullgård

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


#1530299

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2016-11-25 16:00 +0100
Message-ID<sHq8N-1SA-7@gated-at.bofh.it>
In reply to#1530296
On Fri, Nov 25, 2016 at 02:40:21PM +0000, Måns Rullgård wrote:
> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> 
> > On Fri, Nov 25, 2016 at 02:03:20PM +0000, Måns Rullgård wrote:
> >> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> >> 
> >> > On Fri, Nov 25, 2016 at 01:50:35PM +0000, Måns Rullgård wrote:
> >> >> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> >> >> > It would be unfair to augment the API and add the burden on everyone
> >> >> > for the new API when 99.999% of the world doesn't require it.
> >> >> 
> >> >> I don't think making this particular dma driver wait for the descriptor
> >> >> callback to return before reusing a channel quite amounts to a horrid
> >> >> hack.  It certainly wouldn't burden anyone other than the poor drivers
> >> >> for devices connected to it, all of which are specific to Sigma AFAIK.
> >> >
> >> > Except when you stop to think that delaying in a tasklet is exactly
> >> > the same as randomly delaying in an interrupt handler - the tasklet
> >> > runs on the return path back to the parent context of an interrupt
> >> > handler.  Even if you sleep in the tasklet, you're sleeping on behalf
> >> > of the currently executing thread - if it's a RT thread, you effectively
> >> > destroy the RT-ness of the thread.  Let's hope no one cares about RT
> >> > performance on that hardware...
> >> 
> >> That's why I suggested to do this only if the needed delay is known to
> >> be no more than a few bus cycles.  The completion callback is currently
> >> the only post-transfer interaction we have between the dma and device
> >> drivers.  To handle an arbitrarily long delay, some new interface will
> >> be required.
> >
> > And now we're back at the point I made a few emails ago about undue
> > burden which is just about quoted above...
> 
> So what do you suggest?  Stick our heads in the sand and pretend
> everything is perfect?

Look, if you're going to be arsey, don't be surprised if I start getting
the urge to repeat previous comments.

Let's try and keep this on a technical basis for once, rather than
decending into insults.

So, wind back to my original email where I started talking about PL08x
already doing something along these lines.  Before a DMA user can make
use of a DMA channel, it has to be requested.  Once a DMA user has
finished, it can free up the channel.

What this means is that there's already a solution here - but it depends
how many DMA channels and how many active DMA users there are.  It's
entirely possible to set the mapping up when a DMA user requests a
DMA channel, leave it setup, and only tear it down when the channel
is eventually freed.

At that point, there's no need to spin-wait or sleep to delay the
tear-down of the channel - and I'd suggest that approach _until_
such time that there are more users than there are DMA channels.  This
has minimal overhead, it doesn't screw up RT threads (which include
IRQ threads), and it doesn't spread the maintanence burden across
drivers with a new custom API just for one SoC.

If (or when) the number of active users exceeds the number of hardware
DMA channels, then there's a decision to be made:

1) either limit the number of peripherals that we support DMA on for
   the SoC.
2) add the delay or API as necessary and switch to dynamic channel
   allocation to incoming requests.

Until that point is reached, there's no point inventing new APIs for
something that isn't actually a problem yet.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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


#1530351

FromMåns Rullgård <mans@mansr.com>
Date2016-11-25 16:30 +0100
Message-ID<sHqBQ-2hw-19@gated-at.bofh.it>
In reply to#1530299
Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> On Fri, Nov 25, 2016 at 02:40:21PM +0000, Måns Rullgård wrote:
>> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>> 
>> > On Fri, Nov 25, 2016 at 02:03:20PM +0000, Måns Rullgård wrote:
>> >> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>> >> 
>> >> > On Fri, Nov 25, 2016 at 01:50:35PM +0000, Måns Rullgård wrote:
>> >> >> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>> >> >> > It would be unfair to augment the API and add the burden on everyone
>> >> >> > for the new API when 99.999% of the world doesn't require it.
>> >> >> 
>> >> >> I don't think making this particular dma driver wait for the descriptor
>> >> >> callback to return before reusing a channel quite amounts to a horrid
>> >> >> hack.  It certainly wouldn't burden anyone other than the poor drivers
>> >> >> for devices connected to it, all of which are specific to Sigma AFAIK.
>> >> >
>> >> > Except when you stop to think that delaying in a tasklet is exactly
>> >> > the same as randomly delaying in an interrupt handler - the tasklet
>> >> > runs on the return path back to the parent context of an interrupt
>> >> > handler.  Even if you sleep in the tasklet, you're sleeping on behalf
>> >> > of the currently executing thread - if it's a RT thread, you effectively
>> >> > destroy the RT-ness of the thread.  Let's hope no one cares about RT
>> >> > performance on that hardware...
>> >> 
>> >> That's why I suggested to do this only if the needed delay is known to
>> >> be no more than a few bus cycles.  The completion callback is currently
>> >> the only post-transfer interaction we have between the dma and device
>> >> drivers.  To handle an arbitrarily long delay, some new interface will
>> >> be required.
>> >
>> > And now we're back at the point I made a few emails ago about undue
>> > burden which is just about quoted above...
>> 
>> So what do you suggest?  Stick our heads in the sand and pretend
>> everything is perfect?
>
> Look, if you're going to be arsey, don't be surprised if I start getting
> the urge to repeat previous comments.
>
> Let's try and keep this on a technical basis for once, rather than
> decending into insults.

You're the one who constantly insults people.  I'd be happy for you to
stop.

> So, wind back to my original email where I started talking about PL08x
> already doing something along these lines.  Before a DMA user can make
> use of a DMA channel, it has to be requested.  Once a DMA user has
> finished, it can free up the channel.
>
> What this means is that there's already a solution here - but it depends
> how many DMA channels and how many active DMA users there are.  It's
> entirely possible to set the mapping up when a DMA user requests a
> DMA channel, leave it setup, and only tear it down when the channel
> is eventually freed.
>
> At that point, there's no need to spin-wait or sleep to delay the
> tear-down of the channel - and I'd suggest that approach _until_
> such time that there are more users than there are DMA channels.  This
> has minimal overhead, it doesn't screw up RT threads (which include
> IRQ threads), and it doesn't spread the maintanence burden across
> drivers with a new custom API just for one SoC.

I never suggested a custom API for one SoC.

> If (or when) the number of active users exceeds the number of hardware
> DMA channels, then there's a decision to be made:
>
> 1) either limit the number of peripherals that we support DMA on for
>    the SoC.

I don't think people would like being forced to choose between, say,
SATA and NAND flash.

> 2) add the delay or API as necessary and switch to dynamic channel
>    allocation to incoming requests.

A fixed delay doesn't seem right.  Since we don't know the exact amount
required, we'll need to make a guess and make it conservative enough
that it never ends up being too short.  This will most likely end up
delaying things far more than is actually necessary.

The reality of the situation is that the current dmaengine api doesn't
adequately cover all real hardware situations.  You seem to be of the
opinion that fixing this is an "undue burden."

> Until that point is reached, there's no point inventing new APIs for
> something that isn't actually a problem yet.

We're already at that point.  The hardware has many more devices than
physical channels.

-- 
Måns Rullgård

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


#1530325

FromMason <slash.tmp@free.fr>
Date2016-11-25 16:10 +0100
Message-ID<sHqiu-2aY-43@gated-at.bofh.it>
In reply to#1530274
On 25/11/2016 15:17, Russell King - ARM Linux wrote:
> On Fri, Nov 25, 2016 at 02:03:20PM +0000, Måns Rullgård wrote:
>> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>>
>>> On Fri, Nov 25, 2016 at 01:50:35PM +0000, Måns Rullgård wrote:
>>>> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>>>>> It would be unfair to augment the API and add the burden on everyone
>>>>> for the new API when 99.999% of the world doesn't require it.
>>>>
>>>> I don't think making this particular dma driver wait for the descriptor
>>>> callback to return before reusing a channel quite amounts to a horrid
>>>> hack.  It certainly wouldn't burden anyone other than the poor drivers
>>>> for devices connected to it, all of which are specific to Sigma AFAIK.
>>>
>>> Except when you stop to think that delaying in a tasklet is exactly
>>> the same as randomly delaying in an interrupt handler - the tasklet
>>> runs on the return path back to the parent context of an interrupt
>>> handler.  Even if you sleep in the tasklet, you're sleeping on behalf
>>> of the currently executing thread - if it's a RT thread, you effectively
>>> destroy the RT-ness of the thread.  Let's hope no one cares about RT
>>> performance on that hardware...
>>
>> That's why I suggested to do this only if the needed delay is known to
>> be no more than a few bus cycles.  The completion callback is currently
>> the only post-transfer interaction we have between the dma and device
>> drivers.  To handle an arbitrarily long delay, some new interface will
>> be required.
> 
> And now we're back at the point I made a few emails ago about undue
> burden which is just about quoted above...

I've had several talks with the HW dev, and I don't think they
anticipated the need to mux the 3 channels. In their minds,
customers would choose at most 3 devices to support, and
assign one channel to each device statically.

In fact, in tango4, supported devices are:
A) NAND Flash controllers 0 and 1
NB: the upstream driver only uses controller 0
B) IDE or SATA controllers 0 and 1
C) a few crypto HW blocks which do not work as expected (unused)

Customers typically use 1 channel for NAND, maybe 1 for SATA,
and 1 channel remains unused.

I understand the desire to solve the general case in the
driver, but actual use-cases are much more trivial.

Regards.

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


#1530342

FromMåns Rullgård <mans@mansr.com>
Date2016-11-25 16:20 +0100
Message-ID<sHqsb-2er-51@gated-at.bofh.it>
In reply to#1530325
Mason <slash.tmp@free.fr> writes:

> On 25/11/2016 15:17, Russell King - ARM Linux wrote:
>> On Fri, Nov 25, 2016 at 02:03:20PM +0000, Måns Rullgård wrote:
>>> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>>>
>>>> On Fri, Nov 25, 2016 at 01:50:35PM +0000, Måns Rullgård wrote:
>>>>> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>>>>>> It would be unfair to augment the API and add the burden on everyone
>>>>>> for the new API when 99.999% of the world doesn't require it.
>>>>>
>>>>> I don't think making this particular dma driver wait for the descriptor
>>>>> callback to return before reusing a channel quite amounts to a horrid
>>>>> hack.  It certainly wouldn't burden anyone other than the poor drivers
>>>>> for devices connected to it, all of which are specific to Sigma AFAIK.
>>>>
>>>> Except when you stop to think that delaying in a tasklet is exactly
>>>> the same as randomly delaying in an interrupt handler - the tasklet
>>>> runs on the return path back to the parent context of an interrupt
>>>> handler.  Even if you sleep in the tasklet, you're sleeping on behalf
>>>> of the currently executing thread - if it's a RT thread, you effectively
>>>> destroy the RT-ness of the thread.  Let's hope no one cares about RT
>>>> performance on that hardware...
>>>
>>> That's why I suggested to do this only if the needed delay is known to
>>> be no more than a few bus cycles.  The completion callback is currently
>>> the only post-transfer interaction we have between the dma and device
>>> drivers.  To handle an arbitrarily long delay, some new interface will
>>> be required.
>> 
>> And now we're back at the point I made a few emails ago about undue
>> burden which is just about quoted above...
>
> I've had several talks with the HW dev, and I don't think they
> anticipated the need to mux the 3 channels. In their minds,
> customers would choose at most 3 devices to support, and
> assign one channel to each device statically.
>
> In fact, in tango4, supported devices are:
> A) NAND Flash controllers 0 and 1
> NB: the upstream driver only uses controller 0
> B) IDE or SATA controllers 0 and 1
> C) a few crypto HW blocks which do not work as expected (unused)
>
> Customers typically use 1 channel for NAND, maybe 1 for SATA,
> and 1 channel remains unused.

The hardware has two sata controllers, and I have a board that uses both.

-- 
Måns Rullgård

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


#1530352

FromMåns Rullgård <mans@mansr.com>
Date2016-11-25 16:30 +0100
Message-ID<sHqBQ-2hw-21@gated-at.bofh.it>
In reply to#1530342
Mason <slash.tmp@free.fr> writes:

> On 25/11/2016 16:12, Måns Rullgård wrote:
>
>> Mason writes:
>> 
>>> I've had several talks with the HW dev, and I don't think they
>>> anticipated the need to mux the 3 channels. In their minds,
>>> customers would choose at most 3 devices to support, and
>>> assign one channel to each device statically.
>>>
>>> In fact, in tango4, supported devices are:
>>> A) NAND Flash controllers 0 and 1
>>> NB: the upstream driver only uses controller 0
>>> B) IDE or SATA controllers 0 and 1
>>> C) a few crypto HW blocks which do not work as expected (unused)
>>>
>>> Customers typically use 1 channel for NAND, maybe 1 for SATA,
>>> and 1 channel remains unused.
>> 
>> The hardware has two sata controllers, and I have a board that uses both.
>
> I don't have the tango3 client devices in mind, but
> 1 NAND + 2 SATA works out alright for 3 channels, right?

There are only two usable channels.

Besides, your 3.4 kernel allocates the channels dynamically, sort of,
but since it has a completely custom api, this particular timing issue
doesn't arise there.

-- 
Måns Rullgård

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


#1530353

FromMason <slash.tmp@free.fr>
Date2016-11-25 16:30 +0100
Message-ID<sHqBP-2hw-13@gated-at.bofh.it>
In reply to#1530342
On 25/11/2016 16:12, Måns Rullgård wrote:

> Mason writes:
> 
>> I've had several talks with the HW dev, and I don't think they
>> anticipated the need to mux the 3 channels. In their minds,
>> customers would choose at most 3 devices to support, and
>> assign one channel to each device statically.
>>
>> In fact, in tango4, supported devices are:
>> A) NAND Flash controllers 0 and 1
>> NB: the upstream driver only uses controller 0
>> B) IDE or SATA controllers 0 and 1
>> C) a few crypto HW blocks which do not work as expected (unused)
>>
>> Customers typically use 1 channel for NAND, maybe 1 for SATA,
>> and 1 channel remains unused.
> 
> The hardware has two sata controllers, and I have a board that uses both.

I don't have the tango3 client devices in mind, but
1 NAND + 2 SATA works out alright for 3 channels, right?

Regards.

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


#1530261

FromMåns Rullgård <mans@mansr.com>
Date2016-11-25 15:00 +0100
Message-ID<sHpcK-1fd-27@gated-at.bofh.it>
In reply to#1530247
Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> On Fri, Nov 25, 2016 at 01:07:05PM +0000, Måns Rullgård wrote:
>> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>> 
>> > On Fri, Nov 25, 2016 at 10:25:49AM +0530, Vinod Koul wrote:
>> >> Looking at thread and discussion now, first thinking would be to ensure
>> >> the transaction is completed properly and then isr fired. You may need
>> >> to talk to your HW designers to find a way for that. It is quite common
>> >> that DMA controllers will fire and complete whereas the transaction is
>> >> still in flight.
>> >> 
>> >> If that is not doable, then since you claim this is custom part which
>> >> other vendors wont use (hope we are wrong down the line), then we can
>> >> have a custom api,
>> >> 
>> >> foo_sbox_configure(bool enable, ...);
>> >> 
>> >> This can be invoked from NFC driver when required for configuration and
>> >> teardown. For very specific cases where people need some specific
>> >> configuration we do allow custom APIs.
>> >> 
>> >> Only problem with that would be it wont be a generic solution and you
>> >> seem to be fine with that.
>> >
>> > Isn't this just the same problem as PL08x or any other system which
>> > has multiple requests from devices, but only a limited number of
>> > hardware channels - so you have to route the request signals to the
>> > appropriate hardware channels according to the requests queued up?
>> >
>> > If so, no new "custom" APIs are required, it's already able to be
>> > solved within the DMA engine drivers...
>> 
>> That isn't the problem.  The multiplexing of many devices on a limited
>> number of hardware channels is working fine.  The problem is that (some)
>> client devices need the routing to remain for some time after the dma
>> interrupt signals completion.  I'd characterise this hardware as broken,
>> but there's nothing we can do about that.
>> 
>> The fix has to provide some way for the dma driver to delay reusing a
>> hardware channel until the client device indicates completion.  If only
>> a short delay (a few bus cycles) is needed, it is probably acceptable to
>> rework the driver such that the descriptor completion callback can do
>> the necessary waiting (e.g. by busy-polling a device status register).
>> If the delay can be longer, some other method needs to be devised.
>
> What I understood from the original mail is:
>
> | The problem is that the DMA driver tears down the sbox setup
> | as soon as it receives the IRQ. However, when writing to the
> | device, the interrupt only means "I have pushed all data from
> | memory to the memory channel". These data have not reached
> | the device yet, and may still be "in flight". Thus the sbox
> | setup can only be torn down after the NFC is idle.
>
> The interrupt comes in after it's read the the last data from memory,
> but the data is still sitting in the engine's buffers and has not yet
> been passed to the device.
>
> It sounds like the DMA engine buffers the data on its way to the device,
> and it's not clear from the description whether that is done in
> response to a request from the device or whether the data is prefetched.
> IOW, what the lifetime of the data in the dma engine is.

It's not clear from the information I have exactly when the interrupt
fires, only that it appears to be somewhat too early.

> It seems odd that the DMA engine provides no way to know whether the
> channel still contains data that is in-flight to the device, whether
> by interrupt (from the descriptions the IRQ is way too early) or by
> polling some status register within the DMA engine itself.
>
> If the delay is predictable, why not use a delayed workqueue or a
> hrtimer to wait a period after the IRQ before completing the DMA
> transaction?  If it's not predictable and you haven't some status
> register in the DMA engine hardware that indicates whether there's
> remaining data, then the design really is screwed up, and I don't
> think there's a reasonable solution to the problem - anything
> would be a horrid hack that would be specific to this SoC.

This would hardly be the first screwed up hardware design.  There is a
completion indicator, just not in the dma engine.  For some idiotic
reason, the designers put this responsibility on the client devices and
their respective drivers.

> It would be unfair to augment the API and add the burden on everyone
> for the new API when 99.999% of the world doesn't require it.

I don't think making this particular dma driver wait for the descriptor
callback to return before reusing a channel quite amounts to a horrid
hack.  It certainly wouldn't burden anyone other than the poor drivers
for devices connected to it, all of which are specific to Sigma AFAIK.

-- 
Måns Rullgård

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


#1530193

FromMason <slash.tmp@free.fr>
Date2016-11-25 13:50 +0100
Message-ID<sHo70-CC-55@gated-at.bofh.it>
In reply to#1529792
On 25/11/2016 05:55, Vinod Koul wrote:

> On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
>
>> On my platform, setting up a DMA transfer is a two-step process:
>>
>> 1) configure the "switch box" to connect a device to a memory channel
>> 2) configure the transfer details (address, size, command)
>>
>> When the transfer is done, the sbox setup can be torn down,
>> and the DMA driver can start another transfer.
>>
>> The current software architecture for my NFC (NAND Flash controller)
>> driver is as follows (for one DMA transfer).
>>
>>   sg_init_one
>>   dma_map_sg
>>   dmaengine_prep_slave_sg
>>   dmaengine_submit
>>   dma_async_issue_pending
>>   configure_NFC_transfer
>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>   wait_for_NFC_idle
>>   dma_unmap_sg
> 
> Looking at thread and discussion now, first thinking would be to ensure
> the transaction is completed properly and then isr fired. You may need
> to talk to your HW designers to find a way for that. It is quite common
> that DMA controllers will fire and complete whereas the transaction is
> still in flight.

It seems there is a disconnect between what Linux expects - an IRQ
when the transfer is complete - and the quirks of this HW :-(

On this system, there are MBUS "agents" connected via a "switch box".
An agent fires an IRQ when it has dealt with its *half* of the transfer.

SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT

Here are the steps for a transfer, in the general case:

1) setup the sbox to connect SOURCE TO DEST
2) configure source to send N bytes
3) configure dest to receive N bytes

When SOURCE_AGENT has sent N bytes, it fires an IRQ
When DEST_AGENT has received N bytes, it fires an IRQ
The sbox connection can be torn down only when the destination
agent has received all bytes.
(And the twist is that some agents do not have an IRQ line.)

The system provides 3 RAM-to-sbox agents (read channels)
and 3 sbox-to-RAM agents (write channels).

The NAND Flash controller read and write agents do not have
IRQ lines.

So for a NAND-to-memory transfer (read from device)
- nothing happens when the NFC has finished sending N bytes to the sbox
- the write channel fires an IRQ when it has received N bytes

In that case, one IRQ fires when the transfer is complete,
like Linux expects.

For a memory-to-NAND transfer (write to device)
- the read channel fires an IRQ when it has sent N bytes
- the NFC driver is supposed to poll the NFC to determine
when the controller has finished writing N bytes

In that case, the IRQ does not indicate that the transfer
is complete, merely that the sending half has finished
its part.

For a memory-to-memory transfer (memcpy)
- the read channel fires an IRQ when it has sent N bytes
- the write channel fires an IRQ when it has received N bytes

So you actually get two IRQs in that case, which I don't
think Linux (or the current DMA driver) expects.

I'm not sure how we're supposed to handle this kind of HW
in Linux? (That's why I started this thread.)


> If that is not doable, then since you claim this is custom part which
> other vendors won't use (hope we are wrong down the line),

I'm not sure how to interpret "you claim this is custom part".
Do you mean I may be wrong, that it is not custom?
I don't know if other vendors may have HW with the same
quirky behavior. What do you mean about being wrong down
the line?

> then we can have a custom api,
> 
> foo_sbox_configure(bool enable, ...);
> 
> This can be invoked from NFC driver when required for configuration and
> teardown. For very specific cases where people need some specific
> configuration we do allow custom APIs.

I don't think that would work. The fundamental issue is
that Linux expects a single IRQ to indicate "transfer
complete". And the driver (as written) starts a new
transfer as soon as the IRQ fires.

But the HW may generate 0, 1, or even 2 IRQs for a single
transfer. And when there is a single IRQ, it may not
indicate "transfer complete" (as seen above).

> Only problem with that would be it wont be a generic solution
> and you seem to be fine with that.

I think it is possible to have a generic solution:
Right now, the callback is called from tasklet context.
If we can have a new flag to have the callback invoked
directly from the ISR, then the driver for the client
device can do what is required.

For example, the NFC driver waits for the IRQ from the
memory agent, and then polls the controller itself.

I can whip up a proof-of-concept if it's better to
illustrate with a patch?

Regards.

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


#1530232

FromMåns Rullgård <mans@mansr.com>
Date2016-11-25 14:20 +0100
Message-ID<sHoA2-129-33@gated-at.bofh.it>
In reply to#1530193
Mason <slash.tmp@free.fr> writes:

> On 25/11/2016 05:55, Vinod Koul wrote:
>
>> On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
>>
>>> On my platform, setting up a DMA transfer is a two-step process:
>>>
>>> 1) configure the "switch box" to connect a device to a memory channel
>>> 2) configure the transfer details (address, size, command)
>>>
>>> When the transfer is done, the sbox setup can be torn down,
>>> and the DMA driver can start another transfer.
>>>
>>> The current software architecture for my NFC (NAND Flash controller)
>>> driver is as follows (for one DMA transfer).
>>>
>>>   sg_init_one
>>>   dma_map_sg
>>>   dmaengine_prep_slave_sg
>>>   dmaengine_submit
>>>   dma_async_issue_pending
>>>   configure_NFC_transfer
>>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>>   wait_for_NFC_idle
>>>   dma_unmap_sg
>> 
>> Looking at thread and discussion now, first thinking would be to ensure
>> the transaction is completed properly and then isr fired. You may need
>> to talk to your HW designers to find a way for that. It is quite common
>> that DMA controllers will fire and complete whereas the transaction is
>> still in flight.
>
> It seems there is a disconnect between what Linux expects - an IRQ
> when the transfer is complete - and the quirks of this HW :-(
>
> On this system, there are MBUS "agents" connected via a "switch box".
> An agent fires an IRQ when it has dealt with its *half* of the transfer.
>
> SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
>
> Here are the steps for a transfer, in the general case:
>
> 1) setup the sbox to connect SOURCE TO DEST
> 2) configure source to send N bytes
> 3) configure dest to receive N bytes
>
> When SOURCE_AGENT has sent N bytes, it fires an IRQ
> When DEST_AGENT has received N bytes, it fires an IRQ
> The sbox connection can be torn down only when the destination
> agent has received all bytes.
> (And the twist is that some agents do not have an IRQ line.)
>
> The system provides 3 RAM-to-sbox agents (read channels)
> and 3 sbox-to-RAM agents (write channels).
>
> The NAND Flash controller read and write agents do not have
> IRQ lines.
>
> So for a NAND-to-memory transfer (read from device)
> - nothing happens when the NFC has finished sending N bytes to the sbox
> - the write channel fires an IRQ when it has received N bytes
>
> In that case, one IRQ fires when the transfer is complete,
> like Linux expects.
>
> For a memory-to-NAND transfer (write to device)
> - the read channel fires an IRQ when it has sent N bytes
> - the NFC driver is supposed to poll the NFC to determine
> when the controller has finished writing N bytes
>
> In that case, the IRQ does not indicate that the transfer
> is complete, merely that the sending half has finished
> its part.

When does your NAND controller signal completion?  When it has received
the DMA data, or only when it has finished the actual write operation?

> I think it is possible to have a generic solution:
> Right now, the callback is called from tasklet context.
> If we can have a new flag to have the callback invoked
> directly from the ISR, then the driver for the client
> device can do what is required.

No, that won't work.  The callback shouldn't run in interrupt context.

-- 
Måns Rullgård

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


#1530276

FromMason <slash.tmp@free.fr>
Date2016-11-25 15:30 +0100
Message-ID<sHpFM-1In-23@gated-at.bofh.it>
In reply to#1530232
On 25/11/2016 14:11, Måns Rullgård wrote:

> Mason writes:
> 
>> It seems there is a disconnect between what Linux expects - an IRQ
>> when the transfer is complete - and the quirks of this HW :-(
>>
>> On this system, there are MBUS "agents" connected via a "switch box".
>> An agent fires an IRQ when it has dealt with its *half* of the transfer.
>>
>> SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
>>
>> Here are the steps for a transfer, in the general case:
>>
>> 1) setup the sbox to connect SOURCE TO DEST
>> 2) configure source to send N bytes
>> 3) configure dest to receive N bytes
>>
>> When SOURCE_AGENT has sent N bytes, it fires an IRQ
>> When DEST_AGENT has received N bytes, it fires an IRQ
>> The sbox connection can be torn down only when the destination
>> agent has received all bytes.
>> (And the twist is that some agents do not have an IRQ line.)
>>
>> The system provides 3 RAM-to-sbox agents (read channels)
>> and 3 sbox-to-RAM agents (write channels).
>>
>> The NAND Flash controller read and write agents do not have
>> IRQ lines.
>>
>> So for a NAND-to-memory transfer (read from device)
>> - nothing happens when the NFC has finished sending N bytes to the sbox
>> - the write channel fires an IRQ when it has received N bytes
>>
>> In that case, one IRQ fires when the transfer is complete,
>> like Linux expects.
>>
>> For a memory-to-NAND transfer (write to device)
>> - the read channel fires an IRQ when it has sent N bytes
>> - the NFC driver is supposed to poll the NFC to determine
>> when the controller has finished writing N bytes
>>
>> In that case, the IRQ does not indicate that the transfer
>> is complete, merely that the sending half has finished
>> its part.
> 
> When does your NAND controller signal completion?  When it has received
> the DMA data, or only when it has finished the actual write operation?

The NAND controller provides a STATUS register.
Bit 31 is the CMD_READY bit.
This bit goes to 0 when the controller is busy, and to 1
when the controller is ready to accept the next command.

The NFC driver is doing:

	res = wait_for_completion_timeout(&tx_done, HZ);
	if (res > 0)
		err = readl_poll_timeout(addr, val, val & CMD_READY, 0, 1000);

So basically, sleep until the memory agent IRQ falls,
then spin until the controller is idle.

Did you see that adding a 10 µs delay at the start of
tangox_dma_pchan_detach() makes the system no longer
fail (passes an mtd_speedtest).

>> I think it is possible to have a generic solution:
>> Right now, the callback is called from tasklet context.
>> If we can have a new flag to have the callback invoked
>> directly from the ISR, then the driver for the client
>> device can do what is required.
> 
> No, that won't work.  The callback shouldn't run in interrupt context.

What if the callback only spun for, at most, 10 µs ?

	readl_poll_timeout(addr, val, val & CMD_READY, 0, 10);

Regards.

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


#1530287

FromMåns Rullgård <mans@mansr.com>
Date2016-11-25 15:40 +0100
Message-ID<sHpPs-1LD-31@gated-at.bofh.it>
In reply to#1530276
Mason <slash.tmp@free.fr> writes:

> On 25/11/2016 14:11, Måns Rullgård wrote:
>
>> Mason writes:
>> 
>>> It seems there is a disconnect between what Linux expects - an IRQ
>>> when the transfer is complete - and the quirks of this HW :-(
>>>
>>> On this system, there are MBUS "agents" connected via a "switch box".
>>> An agent fires an IRQ when it has dealt with its *half* of the transfer.
>>>
>>> SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
>>>
>>> Here are the steps for a transfer, in the general case:
>>>
>>> 1) setup the sbox to connect SOURCE TO DEST
>>> 2) configure source to send N bytes
>>> 3) configure dest to receive N bytes
>>>
>>> When SOURCE_AGENT has sent N bytes, it fires an IRQ
>>> When DEST_AGENT has received N bytes, it fires an IRQ
>>> The sbox connection can be torn down only when the destination
>>> agent has received all bytes.
>>> (And the twist is that some agents do not have an IRQ line.)
>>>
>>> The system provides 3 RAM-to-sbox agents (read channels)
>>> and 3 sbox-to-RAM agents (write channels).
>>>
>>> The NAND Flash controller read and write agents do not have
>>> IRQ lines.
>>>
>>> So for a NAND-to-memory transfer (read from device)
>>> - nothing happens when the NFC has finished sending N bytes to the sbox
>>> - the write channel fires an IRQ when it has received N bytes
>>>
>>> In that case, one IRQ fires when the transfer is complete,
>>> like Linux expects.
>>>
>>> For a memory-to-NAND transfer (write to device)
>>> - the read channel fires an IRQ when it has sent N bytes
>>> - the NFC driver is supposed to poll the NFC to determine
>>> when the controller has finished writing N bytes
>>>
>>> In that case, the IRQ does not indicate that the transfer
>>> is complete, merely that the sending half has finished
>>> its part.
>> 
>> When does your NAND controller signal completion?  When it has received
>> the DMA data, or only when it has finished the actual write operation?
>
> The NAND controller provides a STATUS register.
> Bit 31 is the CMD_READY bit.
> This bit goes to 0 when the controller is busy, and to 1
> when the controller is ready to accept the next command.
>
> The NFC driver is doing:
>
> 	res = wait_for_completion_timeout(&tx_done, HZ);
> 	if (res > 0)
> 		err = readl_poll_timeout(addr, val, val & CMD_READY, 0, 1000);
>
> So basically, sleep until the memory agent IRQ falls,
> then spin until the controller is idle.

This doesn't answer my question.  Waiting for the entire operation to
finish isn't necessary.  The dma driver only needs to wait until all the
data has been received by the nand controller, not until the controller
is completely finished with the command.  Does the nand controller
provide an indication for completion of the dma independently of the
progress of the write command?  The dma glue Sigma added to the
Designware sata controller does this.

> Did you see that adding a 10 µs delay at the start of
> tangox_dma_pchan_detach() makes the system no longer
> fail (passes an mtd_speedtest).

Yes, but maybe that's much longer than is actually necessary.

>>> I think it is possible to have a generic solution:
>>> Right now, the callback is called from tasklet context.
>>> If we can have a new flag to have the callback invoked
>>> directly from the ISR, then the driver for the client
>>> device can do what is required.
>> 
>> No, that won't work.  The callback shouldn't run in interrupt context.
>
> What if the callback only spun for, at most, 10 µs ?
>
> 	readl_poll_timeout(addr, val, val & CMD_READY, 0, 10);

That's far too long to wait in interrupt of tasklet context.

-- 
Måns Rullgård

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


#1530372

FromMason <slash.tmp@free.fr>
Date2016-11-25 16:50 +0100
Message-ID<sHqVc-2o9-31@gated-at.bofh.it>
In reply to#1530287
On 25/11/2016 15:37, Måns Rullgård wrote:

> Mason writes:
> 
>> On 25/11/2016 14:11, Måns Rullgård wrote:
>>
>>> Mason writes:
>>>
>>>> It seems there is a disconnect between what Linux expects - an IRQ
>>>> when the transfer is complete - and the quirks of this HW :-(
>>>>
>>>> On this system, there are MBUS "agents" connected via a "switch box".
>>>> An agent fires an IRQ when it has dealt with its *half* of the transfer.
>>>>
>>>> SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
>>>>
>>>> Here are the steps for a transfer, in the general case:
>>>>
>>>> 1) setup the sbox to connect SOURCE TO DEST
>>>> 2) configure source to send N bytes
>>>> 3) configure dest to receive N bytes
>>>>
>>>> When SOURCE_AGENT has sent N bytes, it fires an IRQ
>>>> When DEST_AGENT has received N bytes, it fires an IRQ
>>>> The sbox connection can be torn down only when the destination
>>>> agent has received all bytes.
>>>> (And the twist is that some agents do not have an IRQ line.)
>>>>
>>>> The system provides 3 RAM-to-sbox agents (read channels)
>>>> and 3 sbox-to-RAM agents (write channels).
>>>>
>>>> The NAND Flash controller read and write agents do not have
>>>> IRQ lines.
>>>>
>>>> So for a NAND-to-memory transfer (read from device)
>>>> - nothing happens when the NFC has finished sending N bytes to the sbox
>>>> - the write channel fires an IRQ when it has received N bytes
>>>>
>>>> In that case, one IRQ fires when the transfer is complete,
>>>> like Linux expects.
>>>>
>>>> For a memory-to-NAND transfer (write to device)
>>>> - the read channel fires an IRQ when it has sent N bytes
>>>> - the NFC driver is supposed to poll the NFC to determine
>>>> when the controller has finished writing N bytes
>>>>
>>>> In that case, the IRQ does not indicate that the transfer
>>>> is complete, merely that the sending half has finished
>>>> its part.
>>>
>>> When does your NAND controller signal completion?  When it has received
>>> the DMA data, or only when it has finished the actual write operation?
>>
>> The NAND controller provides a STATUS register.
>> Bit 31 is the CMD_READY bit.
>> This bit goes to 0 when the controller is busy, and to 1
>> when the controller is ready to accept the next command.
>>
>> The NFC driver is doing:
>>
>> 	res = wait_for_completion_timeout(&tx_done, HZ);
>> 	if (res > 0)
>> 		err = readl_poll_timeout(addr, val, val & CMD_READY, 0, 1000);
>>
>> So basically, sleep until the memory agent IRQ falls,
>> then spin until the controller is idle.
> 
> This doesn't answer my question.  Waiting for the entire operation to
> finish isn't necessary.  The dma driver only needs to wait until all the
> data has been received by the nand controller, not until the controller
> is completely finished with the command.  Does the nand controller
> provide an indication for completion of the dma independently of the
> progress of the write command?  The dma glue Sigma added to the
> Designware sata controller does this.

I called the HW dev. He told me the NFC block does not have
buffers to store the incoming data; so they remain in the
MBUS FIFOs until the NFC consumes them, i.e. when it has
finished writing them to a NAND chip, which could take
a "long time" when writing to a slow chip.

So the answer to your question is: "the NAND controller
signals completion only when it has finished the actual
write operation."

>> Did you see that adding a 10 µs delay at the start of
>> tangox_dma_pchan_detach() makes the system no longer
>> fail (passes an mtd_speedtest).
> 
> Yes, but maybe that's much longer than is actually necessary.

I could instrument my spin loop to record how long we had
to wait between the IRQ and CMD_READY.

Regards.

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


#1532643

FromMason <slash.tmp@free.fr>
Date2016-11-29 19:30 +0100
Message-ID<sIVke-3Ce-33@gated-at.bofh.it>
In reply to#1530193
[ Nothing new added below.
  Vinod, was the description of my HW's quirks clear enough?
  Is there a way to write a driver within the existing framework?
  How can I get that HW block supported upstream?
  Regards. ]

On 25/11/2016 13:46, Mason wrote:

> On 25/11/2016 05:55, Vinod Koul wrote:
> 
>> On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
>>
>>> On my platform, setting up a DMA transfer is a two-step process:
>>>
>>> 1) configure the "switch box" to connect a device to a memory channel
>>> 2) configure the transfer details (address, size, command)
>>>
>>> When the transfer is done, the sbox setup can be torn down,
>>> and the DMA driver can start another transfer.
>>>
>>> The current software architecture for my NFC (NAND Flash controller)
>>> driver is as follows (for one DMA transfer).
>>>
>>>   sg_init_one
>>>   dma_map_sg
>>>   dmaengine_prep_slave_sg
>>>   dmaengine_submit
>>>   dma_async_issue_pending
>>>   configure_NFC_transfer
>>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>>   wait_for_NFC_idle
>>>   dma_unmap_sg
>>
>> Looking at thread and discussion now, first thinking would be to ensure
>> the transaction is completed properly and then isr fired. You may need
>> to talk to your HW designers to find a way for that. It is quite common
>> that DMA controllers will fire and complete whereas the transaction is
>> still in flight.
> 
> It seems there is a disconnect between what Linux expects - an IRQ
> when the transfer is complete - and the quirks of this HW :-(
> 
> On this system, there are MBUS "agents" connected via a "switch box".
> An agent fires an IRQ when it has dealt with its *half* of the transfer.
> 
> SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
> 
> Here are the steps for a transfer, in the general case:
> 
> 1) setup the sbox to connect SOURCE TO DEST
> 2) configure source to send N bytes
> 3) configure dest to receive N bytes
> 
> When SOURCE_AGENT has sent N bytes, it fires an IRQ
> When DEST_AGENT has received N bytes, it fires an IRQ
> The sbox connection can be torn down only when the destination
> agent has received all bytes.
> (And the twist is that some agents do not have an IRQ line.)
> 
> The system provides 3 RAM-to-sbox agents (read channels)
> and 3 sbox-to-RAM agents (write channels).
> 
> The NAND Flash controller read and write agents do not have
> IRQ lines.
> 
> So for a NAND-to-memory transfer (read from device)
> - nothing happens when the NFC has finished sending N bytes to the sbox
> - the write channel fires an IRQ when it has received N bytes
> 
> In that case, one IRQ fires when the transfer is complete,
> like Linux expects.
> 
> For a memory-to-NAND transfer (write to device)
> - the read channel fires an IRQ when it has sent N bytes
> - the NFC driver is supposed to poll the NFC to determine
> when the controller has finished writing N bytes
> 
> In that case, the IRQ does not indicate that the transfer
> is complete, merely that the sending half has finished
> its part.
> 
> For a memory-to-memory transfer (memcpy)
> - the read channel fires an IRQ when it has sent N bytes
> - the write channel fires an IRQ when it has received N bytes
> 
> So you actually get two IRQs in that case, which I don't
> think Linux (or the current DMA driver) expects.
> 
> I'm not sure how we're supposed to handle this kind of HW
> in Linux? (That's why I started this thread.)
> 
> 
>> If that is not doable, then since you claim this is custom part which
>> other vendors won't use (hope we are wrong down the line),
> 
> I'm not sure how to interpret "you claim this is custom part".
> Do you mean I may be wrong, that it is not custom?
> I don't know if other vendors may have HW with the same
> quirky behavior. What do you mean about being wrong down
> the line?
> 
>> then we can have a custom api,
>>
>> foo_sbox_configure(bool enable, ...);
>>
>> This can be invoked from NFC driver when required for configuration and
>> teardown. For very specific cases where people need some specific
>> configuration we do allow custom APIs.
> 
> I don't think that would work. The fundamental issue is
> that Linux expects a single IRQ to indicate "transfer
> complete". And the driver (as written) starts a new
> transfer as soon as the IRQ fires.
> 
> But the HW may generate 0, 1, or even 2 IRQs for a single
> transfer. And when there is a single IRQ, it may not
> indicate "transfer complete" (as seen above).
> 
>> Only problem with that would be it wont be a generic solution
>> and you seem to be fine with that.
> 
> I think it is possible to have a generic solution:
> Right now, the callback is called from tasklet context.
> If we can have a new flag to have the callback invoked
> directly from the ISR, then the driver for the client
> device can do what is required.
> 
> For example, the NFC driver waits for the IRQ from the
> memory agent, and then polls the controller itself.
> 
> I can whip up a proof-of-concept if it's better to
> illustrate with a patch?

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


#1536671

FromVinod Koul <vinod.koul@intel.com>
Date2016-12-06 06:10 +0100
Message-ID<sLgaS-62M-15@gated-at.bofh.it>
In reply to#1532643
On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:

Sorry I was away for a week in meeting with laptop down.

> [ Nothing new added below.
>   Vinod, was the description of my HW's quirks clear enough?

Yes

>   Is there a way to write a driver within the existing framework?

I think so, looking back at comments from Russell, I do tend to agree with
that. Is there a specfic reason why sbox can't be tied to alloc and free
channels?

>   How can I get that HW block supported upstream?
>   Regards. ]
> 
> On 25/11/2016 13:46, Mason wrote:
> 
> > On 25/11/2016 05:55, Vinod Koul wrote:
> > 
> >> On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
> >>
> >>> On my platform, setting up a DMA transfer is a two-step process:
> >>>
> >>> 1) configure the "switch box" to connect a device to a memory channel
> >>> 2) configure the transfer details (address, size, command)
> >>>
> >>> When the transfer is done, the sbox setup can be torn down,
> >>> and the DMA driver can start another transfer.
> >>>
> >>> The current software architecture for my NFC (NAND Flash controller)
> >>> driver is as follows (for one DMA transfer).
> >>>
> >>>   sg_init_one
> >>>   dma_map_sg
> >>>   dmaengine_prep_slave_sg
> >>>   dmaengine_submit
> >>>   dma_async_issue_pending
> >>>   configure_NFC_transfer
> >>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
> >>>   wait_for_NFC_idle
> >>>   dma_unmap_sg
> >>
> >> Looking at thread and discussion now, first thinking would be to ensure
> >> the transaction is completed properly and then isr fired. You may need
> >> to talk to your HW designers to find a way for that. It is quite common
> >> that DMA controllers will fire and complete whereas the transaction is
> >> still in flight.
> > 
> > It seems there is a disconnect between what Linux expects - an IRQ
> > when the transfer is complete - and the quirks of this HW :-(
> > 
> > On this system, there are MBUS "agents" connected via a "switch box".
> > An agent fires an IRQ when it has dealt with its *half* of the transfer.
> > 
> > SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
> > 
> > Here are the steps for a transfer, in the general case:
> > 
> > 1) setup the sbox to connect SOURCE TO DEST
> > 2) configure source to send N bytes
> > 3) configure dest to receive N bytes
> > 
> > When SOURCE_AGENT has sent N bytes, it fires an IRQ
> > When DEST_AGENT has received N bytes, it fires an IRQ
> > The sbox connection can be torn down only when the destination
> > agent has received all bytes.
> > (And the twist is that some agents do not have an IRQ line.)
> > 
> > The system provides 3 RAM-to-sbox agents (read channels)
> > and 3 sbox-to-RAM agents (write channels).
> > 
> > The NAND Flash controller read and write agents do not have
> > IRQ lines.
> > 
> > So for a NAND-to-memory transfer (read from device)
> > - nothing happens when the NFC has finished sending N bytes to the sbox
> > - the write channel fires an IRQ when it has received N bytes
> > 
> > In that case, one IRQ fires when the transfer is complete,
> > like Linux expects.
> > 
> > For a memory-to-NAND transfer (write to device)
> > - the read channel fires an IRQ when it has sent N bytes
> > - the NFC driver is supposed to poll the NFC to determine
> > when the controller has finished writing N bytes
> > 
> > In that case, the IRQ does not indicate that the transfer
> > is complete, merely that the sending half has finished
> > its part.
> > 
> > For a memory-to-memory transfer (memcpy)
> > - the read channel fires an IRQ when it has sent N bytes
> > - the write channel fires an IRQ when it has received N bytes
> > 
> > So you actually get two IRQs in that case, which I don't
> > think Linux (or the current DMA driver) expects.
> > 
> > I'm not sure how we're supposed to handle this kind of HW
> > in Linux? (That's why I started this thread.)
> > 
> > 
> >> If that is not doable, then since you claim this is custom part which
> >> other vendors won't use (hope we are wrong down the line),
> > 
> > I'm not sure how to interpret "you claim this is custom part".
> > Do you mean I may be wrong, that it is not custom?
> > I don't know if other vendors may have HW with the same
> > quirky behavior. What do you mean about being wrong down
> > the line?
> > 
> >> then we can have a custom api,
> >>
> >> foo_sbox_configure(bool enable, ...);
> >>
> >> This can be invoked from NFC driver when required for configuration and
> >> teardown. For very specific cases where people need some specific
> >> configuration we do allow custom APIs.
> > 
> > I don't think that would work. The fundamental issue is
> > that Linux expects a single IRQ to indicate "transfer
> > complete". And the driver (as written) starts a new
> > transfer as soon as the IRQ fires.
> > 
> > But the HW may generate 0, 1, or even 2 IRQs for a single
> > transfer. And when there is a single IRQ, it may not
> > indicate "transfer complete" (as seen above).
> > 
> >> Only problem with that would be it wont be a generic solution
> >> and you seem to be fine with that.
> > 
> > I think it is possible to have a generic solution:
> > Right now, the callback is called from tasklet context.
> > If we can have a new flag to have the callback invoked
> > directly from the ISR, then the driver for the client
> > device can do what is required.
> > 
> > For example, the NFC driver waits for the IRQ from the
> > memory agent, and then polls the controller itself.
> > 
> > I can whip up a proof-of-concept if it's better to
> > illustrate with a patch?
> 

-- 
~Vinod

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


#1536932

FromMason <slash.tmp@free.fr>
Date2016-12-06 13:50 +0100
Message-ID<sLnm3-26m-69@gated-at.bofh.it>
In reply to#1536671
On 06/12/2016 06:12, Vinod Koul wrote:

> On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
> 
>> Is there a way to write a driver within the existing framework?
> 
> I think so, looking back at comments from Russell, I do tend to agree with
> that. Is there a specific reason why sbox can't be tied to alloc and free
> channels?

Here's a recap of the situation.

The "SBOX+MBUS" HW is used in several iterations of the tango SoC:

tango3
  2 memory channels available
  6 devices ("clients"?) may request an MBUS channel

tango4 (one more channel)
  3 memory channels available
  7 devices may request an MBUS channel :
    NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)

Notes:
The current NFC driver supports only one controller.
IDE is mostly obsolete at this point.

tango5 (SATA gets own dedicated MBUS channel pair)
  3 memory channels available
  5 devices may request an MBUS channel :
    NFC0, NFC1, memcpy, (IDE0, IDE1)


If I understand the current DMA driver (written by Mans), client
drivers are instructed to use a specific channel in the DT, and
the DMA driver muxes access to that channel. The DMA driver
manages a per-channel queue of outstanding DMA transfer requests,
and a new transfer is started friom within the DMA ISR
(modulo the fact that the interrupt does not signal completion
of the transfer, as explained else-thread).

What you're proposing, Vinod, is to make a channel exclusive
to a driver, as long as the driver has not explicitly released
the channel, via dma_release_channel(), right?

Regards.

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


#1536958

FromMåns Rullgård <mans@mansr.com>
Date2016-12-06 14:20 +0100
Message-ID<sLnP3-2vS-17@gated-at.bofh.it>
In reply to#1536932
Mason <slash.tmp@free.fr> writes:

> On 06/12/2016 06:12, Vinod Koul wrote:
>
>> On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
>> 
>>> Is there a way to write a driver within the existing framework?
>> 
>> I think so, looking back at comments from Russell, I do tend to agree with
>> that. Is there a specific reason why sbox can't be tied to alloc and free
>> channels?
>
> Here's a recap of the situation.
>
> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>
> tango3
>   2 memory channels available
>   6 devices ("clients"?) may request an MBUS channel
>
> tango4 (one more channel)
>   3 memory channels available
>   7 devices may request an MBUS channel :
>     NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>
> Notes:
> The current NFC driver supports only one controller.

I consider that a bug.

> IDE is mostly obsolete at this point.
>
> tango5 (SATA gets own dedicated MBUS channel pair)
>   3 memory channels available
>   5 devices may request an MBUS channel :
>     NFC0, NFC1, memcpy, (IDE0, IDE1)

Some of the chip variants can also use this DMA engine for PCI devices.

> If I understand the current DMA driver (written by Mans), client
> drivers are instructed to use a specific channel in the DT, and
> the DMA driver muxes access to that channel.

Almost.  The DT indicates the sbox ID of each device.  The driver
multiplexes requests from all devices across all channels.

> The DMA driver manages a per-channel queue of outstanding DMA transfer
> requests, and a new transfer is started friom within the DMA ISR
> (modulo the fact that the interrupt does not signal completion of the
> transfer, as explained else-thread).

We need to somehow let the device driver signal the dma driver when a
transfer has been fully completed.  Currently the only post-transfer
interaction between the dma engine and the device driver is through the
descriptor callback, which is not suitable for this purpose.

This is starting to look like one of those situations where someone just
needs to implement a solution, or we'll be forever bickering about
hypotheticals.

> What you're proposing, Vinod, is to make a channel exclusive
> to a driver, as long as the driver has not explicitly released
> the channel, via dma_release_channel(), right?

That's not going to work very well.  Device drivers typically request
dma channels in their probe functions or when the device is opened.
This means that reserving one of the few channels there will inevitably
make some other device fail to operate.

Doing a request/release per transfer really doesn't fit with the
intended usage of the dmaengine api.  For starters, what should a driver
do if all the channels are currently busy?

Since the hardware actually does support multiplexing the dma channels,
I think it would be misguided to deliberately cripple the software
support in order to shoehorn it into an incomplete model of how hardware
ought to work.  While I agree it would be nicer if all hardware actually
did work that way, this isn't the reality we're living in.

-- 
Måns Rullgård

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


#1537041

FromMason <slash.tmp@free.fr>
Date2016-12-06 16:30 +0100
Message-ID<sLpQS-3Md-33@gated-at.bofh.it>
In reply to#1536958
On 06/12/2016 14:14, Måns Rullgård wrote:

> Mason wrote:
> 
>> On 06/12/2016 06:12, Vinod Koul wrote:
>>
>>> On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
>>>
>>>> Is there a way to write a driver within the existing framework?
>>>
>>> I think so, looking back at comments from Russell, I do tend to agree with
>>> that. Is there a specific reason why sbox can't be tied to alloc and free
>>> channels?
>>
>> Here's a recap of the situation.
>>
>> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>>
>> tango3
>>   2 memory channels available
>>   6 devices ("clients"?) may request an MBUS channel
>>
>> tango4 (one more channel)
>>   3 memory channels available
>>   7 devices may request an MBUS channel :
>>     NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>>
>> Notes:
>> The current NFC driver supports only one controller.
> 
> I consider that a bug.

Meh. The two controller blocks share the I/O pins to the outside
world, so it's not possible to have two concurrent accesses.
Moreover, the current NAND framework does not currently support
such a setup. (I discussed this with the maintainer.)


>> IDE is mostly obsolete at this point.
>>
>> tango5 (SATA gets own dedicated MBUS channel pair)
>>   3 memory channels available
>>   5 devices may request an MBUS channel :
>>     NFC0, NFC1, memcpy, (IDE0, IDE1)
> 
> Some of the chip variants can also use this DMA engine for PCI devices.

Note: PCI support was dropped with tango4.


>> If I understand the current DMA driver (written by Mans), client
>> drivers are instructed to use a specific channel in the DT, and
>> the DMA driver muxes access to that channel.
> 
> Almost.  The DT indicates the sbox ID of each device.  The driver
> multiplexes requests from all devices across all channels.

Thanks for pointing that out. I misremembered the DT.
So a client's DT node specifies the client's SBOX port.
And the DMA node specifies all available MBUS channels.

So when an interrupt fires, the DMA driver (re)uses that
channel for the next transfer in line?


>> The DMA driver manages a per-channel queue of outstanding DMA transfer
>> requests, and a new transfer is started from within the DMA ISR
>> (modulo the fact that the interrupt does not signal completion of the
>> transfer, as explained else-thread).
> 
> We need to somehow let the device driver signal the dma driver when a
> transfer has been fully completed.  Currently the only post-transfer
> interaction between the dma engine and the device driver is through the
> descriptor callback, which is not suitable for this purpose.

The callback is called from vchan_complete() right?
Is that running from interrupt context?

What's the relationship between vchan_complete() and
tangox_dma_irq() -- does one call the other? Are they
asynchronous?


> This is starting to look like one of those situations where someone just
> needs to implement a solution, or we'll be forever bickering about
> hypotheticals.

I can give that a shot (if you're busy with real work).


>> What you're proposing, Vinod, is to make a channel exclusive
>> to a driver, as long as the driver has not explicitly released
>> the channel, via dma_release_channel(), right?
> 
> That's not going to work very well.  Device drivers typically request
> dma channels in their probe functions or when the device is opened.
> This means that reserving one of the few channels there will inevitably
> make some other device fail to operate.

This is true for tango3. Less so for tango4. And no longer
an issue for tango5.


> Doing a request/release per transfer really doesn't fit with the
> intended usage of the dmaengine api.  For starters, what should a driver
> do if all the channels are currently busy?

Why can't we queue channel requests the same way we queue
transfer requests?


> Since the hardware actually does support multiplexing the dma channels,
> I think it would be misguided to deliberately cripple the software
> support in order to shoehorn it into an incomplete model of how hardware
> ought to work.  While I agree it would be nicer if all hardware actually
> did work that way, this isn't the reality we're living in.

I agree with you that it would be nice to have a general solution,
since the HW supports it.

Regards.

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


#1537047

FromMåns Rullgård <mans@mansr.com>
Date2016-12-06 16:40 +0100
Message-ID<sLq0y-3PO-43@gated-at.bofh.it>
In reply to#1537041
Mason <slash.tmp@free.fr> writes:

> On 06/12/2016 14:14, Måns Rullgård wrote:
>
>> Mason wrote:
>> 
>>> On 06/12/2016 06:12, Vinod Koul wrote:
>>>
>>>> On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
>>>>
>>>>> Is there a way to write a driver within the existing framework?
>>>>
>>>> I think so, looking back at comments from Russell, I do tend to agree with
>>>> that. Is there a specific reason why sbox can't be tied to alloc and free
>>>> channels?
>>>
>>> Here's a recap of the situation.
>>>
>>> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>>>
>>> tango3
>>>   2 memory channels available
>>>   6 devices ("clients"?) may request an MBUS channel
>>>
>>> tango4 (one more channel)
>>>   3 memory channels available
>>>   7 devices may request an MBUS channel :
>>>     NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>>>
>>> Notes:
>>> The current NFC driver supports only one controller.
>> 
>> I consider that a bug.
>
> Meh. The two controller blocks share the I/O pins to the outside
> world, so it's not possible to have two concurrent accesses.

OK, you failed to mention that part.  Why are there two controllers at
all if only one or the other can be used?

>>> If I understand the current DMA driver (written by Mans), client
>>> drivers are instructed to use a specific channel in the DT, and
>>> the DMA driver muxes access to that channel.
>> 
>> Almost.  The DT indicates the sbox ID of each device.  The driver
>> multiplexes requests from all devices across all channels.
>
> Thanks for pointing that out. I misremembered the DT.
> So a client's DT node specifies the client's SBOX port.
> And the DMA node specifies all available MBUS channels.
>
> So when an interrupt fires, the DMA driver (re)uses that
> channel for the next transfer in line?

Correct.

>>> The DMA driver manages a per-channel queue of outstanding DMA transfer
>>> requests, and a new transfer is started from within the DMA ISR
>>> (modulo the fact that the interrupt does not signal completion of the
>>> transfer, as explained else-thread).
>> 
>> We need to somehow let the device driver signal the dma driver when a
>> transfer has been fully completed.  Currently the only post-transfer
>> interaction between the dma engine and the device driver is through the
>> descriptor callback, which is not suitable for this purpose.
>
> The callback is called from vchan_complete() right?
> Is that running from interrupt context?

It runs from a tasklet which is almost the same thing.

> What's the relationship between vchan_complete() and
> tangox_dma_irq() -- does one call the other? Are they
> asynchronous?
>
>> This is starting to look like one of those situations where someone just
>> needs to implement a solution, or we'll be forever bickering about
>> hypotheticals.
>
> I can give that a shot (if you're busy with real work).

I have an idea I'd like to try out over the weekend.  If I don't come
back with something by next week, go for it.

>>> What you're proposing, Vinod, is to make a channel exclusive
>>> to a driver, as long as the driver has not explicitly released
>>> the channel, via dma_release_channel(), right?
>> 
>> That's not going to work very well.  Device drivers typically request
>> dma channels in their probe functions or when the device is opened.
>> This means that reserving one of the few channels there will inevitably
>> make some other device fail to operate.
>
> This is true for tango3. Less so for tango4. And no longer
> an issue for tango5.
>
>> Doing a request/release per transfer really doesn't fit with the
>> intended usage of the dmaengine api.  For starters, what should a driver
>> do if all the channels are currently busy?
>
> Why can't we queue channel requests the same way we queue
> transfer requests?

That's in effect what we're doing.  Calling it by another name doesn't
really solve anything.

-- 
Måns Rullgård

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


#1537309

FromMason <slash.tmp@free.fr>
Date2016-12-07 00:00 +0100
Message-ID<sLwSm-8cn-19@gated-at.bofh.it>
In reply to#1537047
On 06/12/2016 16:34, Måns Rullgård wrote:

> Mason writes:
> 
>> Meh. The two controller blocks share the I/O pins to the outside
>> world, so it's not possible to have two concurrent accesses.
> 
> OK, you failed to mention that part.  Why are there two controllers at
> all if only one or the other can be used?

I'd have to ask the HW designer what types of use-cases he had
in mind. Perhaps looking at what is *not* shared provides clues.
Configuration registers are duplicated, meaning it is possible
to decide "channel A for chip0, channel B for chip1" if there
are two NAND chips in the system (as is the case on dev boards).
In that case, it is unnecessary to rewrite the chip parameters
every time the driver switches chips. (I don't think the perf
impact is even measurable.)

The ECC engines are duplicated, but I don't know how long it
takes to run the BCH algorithm in HW vs performing I/O over
a slow 8-bit bus.


>> The callback is called from vchan_complete() right?
>> Is that running from interrupt context?
> 
> It runs from a tasklet which is almost the same thing.

I'll read up on tasklets tomorrow.


>> I can give that a shot (if you're busy with real work).
> 
> I have an idea I'd like to try out over the weekend.  If I don't come
> back with something by next week, go for it.

I do have my plate full this week, with XHCI and AHCI :-)


>> Why can't we queue channel requests the same way we queue
>> transfer requests?
> 
> That's in effect what we're doing.  Calling it by another name doesn't
> really solve anything.

Hmmm... the difference is that "tear down" would be explicit
in the release function.

Current implementation for single transfer:

dmaengine_prep_slave_sg()
dmaengine_submit()
dma_async_issue_pending()	/* A */
wait for read channel IRQ	/* B */
spin until NFC idle		/* C */

Setup SBOX route and program MBUS transfer happen in A.
The SBOX route is torn down a little before B (thus before C).


With the proposed implementation, where request_chan
sets up the route and release_chan tears it down:

request_chan()			/* X */
dmaengine_prep_slave_sg()
dmaengine_submit()
dma_async_issue_pending()	/* A */
wait for read channel IRQ	/* B */
spin until NFC idle		/* C */
release_chan()			/* Y */

Now, the SBOX route is setup in X.
(If no MBUS channel are available, thread is put to sleep
until one becomes available.)
Program MBUS transfer in A.
When IRQ falls, cannot start new transfer yet.
vchan_complete() will at some point run the client callback.
The client driver can now spin however long until NFC idle.
In Y, we release the channel, thus calling back into the
DMA driver, at which point a new transfer can be started.

What did I get wrong in this pseudo-code?


I do see one problem with the approach:
It's no big deal for me to convert the NFC driver to
do it that way, but the Synopsys (I think) SATA driver
in tango3 and tango4 is shared across multiple SoCs,
and they probably call request_chan() only in the
probe function, as you mentioned at some point.

http://lxr.free-electrons.com/source/drivers/ata/sata_dwc_460ex.c#L366

BTW, can someone explain what the DMA_CTRL_ACK flag means?


Regards.

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


Page 2 of 4 — ← Prev page 1 [2] 3 4  Next page →

Back to top | Article view | linux.kernel


csiph-web