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

Groups > linux.kernel > #1530193 > unrolled thread

Re: Tearing down DMA transfer setup after DMA client has finished

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  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 Vinod Koul <vinod.koul@intel.com> - 2016-12-12 06:10 +0100
                                    Re: Tearing down DMA transfer setup after DMA client has finished Mark Brown <broonie@kernel.org> - 2016-12-15 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 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 3 — ← Prev page 1 [2] 3  Next page →

#1538487

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Måns,
>
> On Thu, Dec 8, 2016 at 12:47 PM, Måns Rullgård <mans@mansr.com> wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>> On Thu, Dec 8, 2016 at 11:54 AM, Mason <slash.tmp@free.fr> wrote:
>>>> On 08/12/2016 11:39, Vinod Koul wrote:
>>>>> On Wed, Dec 07, 2016 at 04:45:58PM +0000, Måns Rullgård wrote:
>>>>>> Vinod Koul <vinod.koul@intel.com> writes:
>>>>>>> On Tue, Dec 06, 2016 at 01:14:20PM +0000, Måns Rullgård wrote:
>>>>>>>> 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.
>>>>>>>
>>>>>>> No that doesn't make sense at all, you should get a channel only when you
>>>>>>> want to use it and not in probe!
>>>>>>
>>>>>> Tell that to just about every single driver ever written.
>>>>>
>>>>> Not really, few do yes which is wrong but not _all_ do that.
>>>>
>>>> Vinod,
>>>>
>>>> Could you explain something to me in layman's terms?
>>>>
>>>> I have a NAND Flash Controller driver that depends on the
>>>> DMA driver under discussion.
>>>>
>>>> Suppose I move the dma_request_chan() call from the driver's
>>>> probe function, to the actual DMA transfer function.
>>>>
>>>> I would want dma_request_chan() to put the calling thread
>>>> to sleep until a channel becomes available (possibly with
>>>> a timeout value).
>>>>
>>>> But Maxime told me dma_request_chan() will just return
>>>> -EBUSY if no channels are available.
>>>>
>>>> Am I supposed to busy wait in my driver's DMA function
>>>> until a channel becomes available?
>>>
>>> Can you fall back to PIO if requesting a channel fails?
>>>
>>> Alternatively, dma_request_chan() could always succeed, and
>>> dmaengine_prep_slave_sg() could fail if the channel is currently not
>>> available due to a limitation on the number of active channels, and
>>> the driver could fall back to PIO for that transfer.
>>
>> Why are we debating this nonsense?  There is an easy fix that doesn't
>> require changing the semantics of existing functions or falling back to
>> slow pio.
>
> You still want to fall back to PIO if the DMA engine is not available at all
> (e.g. DMA engine driver not compiled in, or module not loaded).

That's a choice for each device driver to make.  Some devices don't have
a pio mode at all.

-- 
Måns Rullgård

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

#1538711

On Thu, Dec 08, 2016 at 11:54:51AM +0100, Mason wrote:
> On 08/12/2016 11:39, Vinod Koul wrote:
> 
> > On Wed, Dec 07, 2016 at 04:45:58PM +0000, Måns Rullgård wrote:
> >
> >> Vinod Koul <vinod.koul@intel.com> writes:
> >>
> >>> On Tue, Dec 06, 2016 at 01:14:20PM +0000, Måns Rullgård wrote:
> >>>>
> >>>> 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.
> >>>
> >>> No that doesn't make sense at all, you should get a channel only when you
> >>> want to use it and not in probe!
> >>
> >> Tell that to just about every single driver ever written.
> > 
> > Not really, few do yes which is wrong but not _all_ do that.
> 
> Vinod,
> 
> Could you explain something to me in layman's terms?
> 
> I have a NAND Flash Controller driver that depends on the
> DMA driver under discussion.
> 
> Suppose I move the dma_request_chan() call from the driver's
> probe function, to the actual DMA transfer function.
> 
> I would want dma_request_chan() to put the calling thread
> to sleep until a channel becomes available (possibly with
> a timeout value).
> 
> But Maxime told me dma_request_chan() will just return
> -EBUSY if no channels are available.

That is correct

> Am I supposed to busy wait in my driver's DMA function
> until a channel becomes available?

If someone else is using the channels then the bust wait will not help, so
in this case you should fall back to PIO, few drivers do that

> I don't understand how the multiplexing of few memory
> channels to many clients is supposed to happen efficiently?

To make it efficient, disregarding your Sbox HW issue, the solution is
virtual channels. You can delink physical channels and virtual channels. If
one has SW controlled MUX then a channel can service any client. For few
controllers request lines are hard wired so they cant use any channel. But
if you dont have this restriction then driver can queue up many transactions
from different controllers.

-- 
~Vinod

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

#1538722

Vinod Koul <vinod.koul@intel.com> writes:

> To make it efficient, disregarding your Sbox HW issue, the solution is
> virtual channels. You can delink physical channels and virtual channels. If
> one has SW controlled MUX then a channel can service any client. For few
> controllers request lines are hard wired so they cant use any channel. But
> if you dont have this restriction then driver can queue up many transactions
> from different controllers.

Have you been paying attention at all?  This exactly what the driver
ALREADY DOES.

-- 
Måns Rullgård

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

#1539144

On Thu, Dec 08, 2016 at 04:48:18PM +0000, Måns Rullgård wrote:
> Vinod Koul <vinod.koul@intel.com> writes:
> 
> > To make it efficient, disregarding your Sbox HW issue, the solution is
> > virtual channels. You can delink physical channels and virtual channels. If
> > one has SW controlled MUX then a channel can service any client. For few
> > controllers request lines are hard wired so they cant use any channel. But
> > if you dont have this restriction then driver can queue up many transactions
> > from different controllers.
> 
> Have you been paying attention at all?  This exactly what the driver
> ALREADY DOES.

And have you read what the question was?

-- 
~Vinod

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

#1539243

On 09/12/16 07:59, Vinod Koul wrote:
> On Thu, Dec 08, 2016 at 04:48:18PM +0000, Måns Rullgård wrote:
>> Vinod Koul <vinod.koul@intel.com> writes:
>>
>>> To make it efficient, disregarding your Sbox HW issue, the solution is
>>> virtual channels. You can delink physical channels and virtual channels. If
>>> one has SW controlled MUX then a channel can service any client. For few
>>> controllers request lines are hard wired so they cant use any channel. But
>>> if you dont have this restriction then driver can queue up many transactions
>>> from different controllers.
>>
>> Have you been paying attention at all?  This exactly what the driver
>> ALREADY DOES.
> 
> And have you read what the question was?
> 

I think many people appreciate the quick turn around time and responsiveness of
knowledgeable people making constructive remarks in this thread, but it looks we
are slowly drifting away from the main problem.

If we had to sum up the discussion, the current DMA API/framework in Linux seems
to lack a way to properly handle this HW (or if it has a way, the information got
lost somewhere).

What concrete solution do you propose?

Alternatively, one can think of the current issue (i.e.: the fact that the IRQ
arrives "too soon") in a different way.
Instead of thinking the IRQ indicates "transfer complete", it is indicating "ready
to accept another command", which in practice (and with proper API support) can
translate into efficient queuing of DMA operations.

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

#1539292

Sebastian Frias <sf84@laposte.net> writes:

> On 09/12/16 07:59, Vinod Koul wrote:
>> On Thu, Dec 08, 2016 at 04:48:18PM +0000, Måns Rullgård wrote:
>>> Vinod Koul <vinod.koul@intel.com> writes:
>>>
>>>> To make it efficient, disregarding your Sbox HW issue, the solution is
>>>> virtual channels. You can delink physical channels and virtual channels. If
>>>> one has SW controlled MUX then a channel can service any client. For few
>>>> controllers request lines are hard wired so they cant use any channel. But
>>>> if you dont have this restriction then driver can queue up many transactions
>>>> from different controllers.
>>>
>>> Have you been paying attention at all?  This exactly what the driver
>>> ALREADY DOES.
>> 
>> And have you read what the question was?

I wrote the driver.  I think I know what Mason and I are asking.

> I think many people appreciate the quick turn around time and
> responsiveness of knowledgeable people making constructive remarks in
> this thread, but it looks we are slowly drifting away from the main
> problem.
>
> If we had to sum up the discussion, the current DMA API/framework in
> Linux seems to lack a way to properly handle this HW (or if it has a
> way, the information got lost somewhere).
>
> What concrete solution do you propose?
>
> Alternatively, one can think of the current issue (i.e.: the fact that
> the IRQ arrives "too soon") in a different way.  Instead of thinking
> the IRQ indicates "transfer complete", it is indicating "ready to
> accept another command", which in practice (and with proper API
> support) can translate into efficient queuing of DMA operations.

For multiple back to back transfers to the same peripheral, it is indeed
a slight optimisation.  What's apparently lacking is some way of doing a
full flush 

-- 
Måns Rullgård

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

#1539294

Sebastian Frias <sf84@laposte.net> writes:

> On 09/12/16 07:59, Vinod Koul wrote:
>> On Thu, Dec 08, 2016 at 04:48:18PM +0000, Måns Rullgård wrote:
>>> Vinod Koul <vinod.koul@intel.com> writes:
>>>
>>>> To make it efficient, disregarding your Sbox HW issue, the solution is
>>>> virtual channels. You can delink physical channels and virtual channels. If
>>>> one has SW controlled MUX then a channel can service any client. For few
>>>> controllers request lines are hard wired so they cant use any channel. But
>>>> if you dont have this restriction then driver can queue up many transactions
>>>> from different controllers.
>>>
>>> Have you been paying attention at all?  This exactly what the driver
>>> ALREADY DOES.
>> 
>> And have you read what the question was?

I wrote the driver.  I think I know what Mason and I are asking.

> I think many people appreciate the quick turn around time and
> responsiveness of knowledgeable people making constructive remarks in
> this thread, but it looks we are slowly drifting away from the main
> problem.
>
> If we had to sum up the discussion, the current DMA API/framework in
> Linux seems to lack a way to properly handle this HW (or if it has a
> way, the information got lost somewhere).
>
> What concrete solution do you propose?
>
> Alternatively, one can think of the current issue (i.e.: the fact that
> the IRQ arrives "too soon") in a different way.  Instead of thinking
> the IRQ indicates "transfer complete", it is indicating "ready to
> accept another command", which in practice (and with proper API
> support) can translate into efficient queuing of DMA operations.

For multiple back to back transfers to the same peripheral, it is indeed
a slight optimisation.  What's apparently lacking is some way of doing a
full flush at the end of a series of transfers, before switching the
channel to another device.

-- 
Måns Rullgård

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

#1539561

On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> 
> What concrete solution do you propose?

I have already proposed two solutions.

A) Request a channel only when you need it. Obviously we can't do virtual
channels with this (though we should still use virt-channels framework).
The sbox setup and teardown can be done as part of channel request and
freeup. PL08x already does this.

Downside is that we can only have as many consumers at a time as channels.

I have not heard any technical reason for not doing this apart from drivers
grab the channel at probe, which is incorrect and needs to be fixed
irrespective of the problem at hand.

This is my preferred option.

B) Create a custom driver specific API. This API for example:
	sbox_setup(bool enable, ...)
can be called by client to explicitly setup and clear up the sbox setting.

This way we can have transactions muxed.

I have not heard any arguments on why we shouldn't do this except Russell's
comment that A) solves this.

> Alternatively, one can think of the current issue (i.e.: the fact that the IRQ
> arrives "too soon") in a different way.
> Instead of thinking the IRQ indicates "transfer complete", it is indicating "ready
> to accept another command", which in practice (and with proper API support) can
> translate into efficient queuing of DMA operations.

That IMO is a better understanding of this issue. But based on discussion, I
think the issue is that submitting next transaction cannot be done until
sbox is setup (as a consequence torn down for previous). Tear down can be
down only when client knows that transfer is done, from dma controller data
has been pushed and in-flight.


-- 
~Vinod

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

#1539570

Vinod Koul <vinod.koul@intel.com> writes:

> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>> 
>> What concrete solution do you propose?
>
> I have already proposed two solutions.
>
> A) Request a channel only when you need it. Obviously we can't do virtual
> channels with this (though we should still use virt-channels framework).
> The sbox setup and teardown can be done as part of channel request and
> freeup. PL08x already does this.
>
> Downside is that we can only have as many consumers at a time as channels.
>
> I have not heard any technical reason for not doing this apart from drivers
> grab the channel at probe, which is incorrect and needs to be fixed
> irrespective of the problem at hand.
>
> This is my preferred option.

Sorry, but this is not acceptable.

> B) Create a custom driver specific API. This API for example:
> 	sbox_setup(bool enable, ...)
> can be called by client to explicitly setup and clear up the sbox setting.
>
> This way we can have transactions muxed.
>
> I have not heard any arguments on why we shouldn't do this except Russell's
> comment that A) solves this.

Driver-specific interfaces are not the solution.  That way lies chaos
and madness.

This would all be so much easier if you all would just shut up for a
moment and let me fix it properly.

-- 
Måns Rullgård

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

#1539577

On Fri, Dec 09, 2016 at 05:28:01PM +0000, Måns Rullgård wrote:
> Vinod Koul <vinod.koul@intel.com> writes:
> 
> > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> >> 
> >> What concrete solution do you propose?
> >
> > I have already proposed two solutions.
> >
> > A) Request a channel only when you need it. Obviously we can't do virtual
> > channels with this (though we should still use virt-channels framework).
> > The sbox setup and teardown can be done as part of channel request and
> > freeup. PL08x already does this.
> >
> > Downside is that we can only have as many consumers at a time as channels.
> >
> > I have not heard any technical reason for not doing this apart from drivers
> > grab the channel at probe, which is incorrect and needs to be fixed
> > irrespective of the problem at hand.
> >
> > This is my preferred option.
> 
> Sorry, but this is not acceptable.

without outlining why..

> 
> > B) Create a custom driver specific API. This API for example:
> > 	sbox_setup(bool enable, ...)
> > can be called by client to explicitly setup and clear up the sbox setting.
> >
> > This way we can have transactions muxed.
> >
> > I have not heard any arguments on why we shouldn't do this except Russell's
> > comment that A) solves this.
> 
> Driver-specific interfaces are not the solution.  That way lies chaos
> and madness.

Yes fair enough. So would API change which 99% world doesnt need.

> This would all be so much easier if you all would just shut up for a
> moment and let me fix it properly.

Oh please go away, noone is asking you to reply!

-- 
~Vinod

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

#1539572

On 09/12/2016 18:17, Vinod Koul wrote:

> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>>
>> What concrete solution do you propose?
> 
> I have already proposed two solutions.
> 
> A) Request a channel only when you need it. Obviously we can't do virtual
> channels with this (though we should still use virt-channels framework).
> The sbox setup and teardown can be done as part of channel request and
> freeup. PL08x already does this.
> 
> Downside is that we can only have as many consumers at a time as channels.
> 
> I have not heard any technical reason for not doing this apart from drivers
> grab the channel at probe, which is incorrect and needs to be fixed
> irrespective of the problem at hand.
> 
> This is my preferred option.

There is one important drawback with this solution. If a driver calls
dma_request_chan() when no channels are currently available, it will
get -EBUSY. If there were a flag in dma_request_chan to be put to
sleep (with timeout) until a channel is available, then it would
work. But busy waiting in the client driver is a waste of power.

Regards.

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

#1539579

On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> On 09/12/2016 18:17, Vinod Koul wrote:
> 
> > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> >>
> >> What concrete solution do you propose?
> > 
> > I have already proposed two solutions.
> > 
> > A) Request a channel only when you need it. Obviously we can't do virtual
> > channels with this (though we should still use virt-channels framework).
> > The sbox setup and teardown can be done as part of channel request and
> > freeup. PL08x already does this.
> > 
> > Downside is that we can only have as many consumers at a time as channels.
> > 
> > I have not heard any technical reason for not doing this apart from drivers
> > grab the channel at probe, which is incorrect and needs to be fixed
> > irrespective of the problem at hand.
> > 
> > This is my preferred option.
> 
> There is one important drawback with this solution. If a driver calls
> dma_request_chan() when no channels are currently available, it will
> get -EBUSY. If there were a flag in dma_request_chan to be put to
> sleep (with timeout) until a channel is available, then it would
> work. But busy waiting in the client driver is a waste of power.

Right, but in that case the fallback would be PIO mode, and if that is
not availble (IIRC some f your devices don't) then reject the usage with
EAGAIN.


-- 
~Vinod

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

#1539595

On Fri, Dec 09, 2016 at 11:26:06PM +0530, Vinod Koul wrote:
> On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> > On 09/12/2016 18:17, Vinod Koul wrote:
> > 
> > > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> > >>
> > >> What concrete solution do you propose?
> > > 
> > > I have already proposed two solutions.
> > > 
> > > A) Request a channel only when you need it. Obviously we can't do virtual
> > > channels with this (though we should still use virt-channels framework).
> > > The sbox setup and teardown can be done as part of channel request and
> > > freeup. PL08x already does this.
> > > 
> > > Downside is that we can only have as many consumers at a time as channels.
> > > 
> > > I have not heard any technical reason for not doing this apart from drivers
> > > grab the channel at probe, which is incorrect and needs to be fixed
> > > irrespective of the problem at hand.
> > > 
> > > This is my preferred option.
> > 
> > There is one important drawback with this solution. If a driver calls
> > dma_request_chan() when no channels are currently available, it will
> > get -EBUSY. If there were a flag in dma_request_chan to be put to
> > sleep (with timeout) until a channel is available, then it would
> > work. But busy waiting in the client driver is a waste of power.
> 
> Right, but in that case the fallback would be PIO mode, and if that is
> not availble (IIRC some f your devices don't) then reject the usage with
> EAGAIN.

Alternatively I can think of one more way.

If there is fixed delay or maximum delay predicted between ISR being
fired and transaction being completed from client, then we can use that
magic value and degrade the performance a bit but make a simpler system
than other two suggestions.

The idea here is that typically the subsequent transaction should be
issued as soon as possible, best case being in the ISR. But we can
degrade that performance a bit and issue in the tasklet. But that can be
done after introducing a delay, that too only in the case where new sbox
configuration is different from previous one (so performance degrade is
only on the switch and not for txn for same setup). You can possible
optimize even further by issuing in ISR for same sbox setup and issuing
in tasklet if configuration is different.

Yes this is bit iffy and adds more burden on driver, but lets us get
away with decent performance and being able to handle the hardware
condition.

Would that work for your case...?

-- 
~Vinod

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

#1539609

[ Dropping Mans to preserve his peace-of-mind ]

On 09/12/2016 18:56, Vinod Koul wrote:
> On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
>> On 09/12/2016 18:17, Vinod Koul wrote:
>>
>>> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>>>>
>>>> What concrete solution do you propose?
>>>
>>> I have already proposed two solutions.
>>>
>>> A) Request a channel only when you need it. Obviously we can't do virtual
>>> channels with this (though we should still use virt-channels framework).
>>> The sbox setup and teardown can be done as part of channel request and
>>> freeup. PL08x already does this.
>>>
>>> Downside is that we can only have as many consumers at a time as channels.
>>>
>>> I have not heard any technical reason for not doing this apart from drivers
>>> grab the channel at probe, which is incorrect and needs to be fixed
>>> irrespective of the problem at hand.
>>>
>>> This is my preferred option.
>>
>> There is one important drawback with this solution. If a driver calls
>> dma_request_chan() when no channels are currently available, it will
>> get -EBUSY. If there were a flag in dma_request_chan to be put to
>> sleep (with timeout) until a channel is available, then it would
>> work. But busy waiting in the client driver is a waste of power.
> 
> Right, but in that case the fallback would be PIO mode, and if that is
> not availble (IIRC some f your devices don't) then reject the usage with
> EAGAIN.

Maybe I'm missing something, but I don't see how that would help.
Take the NAND Flash controller driver, for instance. PIO is not
an option, because the ECC engine is tied to DMA.

And failing with -EAGAIN doesn't help the busy looping situation.
The caller should be put on some kind of queue to wait for a
"channel ready" event.

Regards.

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

#1540133

On Fri, Dec 09, 2016 at 07:23:17PM +0100, Mason wrote:
> [ Dropping Mans to preserve his peace-of-mind ]
> 
> On 09/12/2016 18:56, Vinod Koul wrote:
> > On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> >> On 09/12/2016 18:17, Vinod Koul wrote:
> >>
> >>> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> >>>>
> >>>> What concrete solution do you propose?
> >>>
> >>> I have already proposed two solutions.
> >>>
> >>> A) Request a channel only when you need it. Obviously we can't do virtual
> >>> channels with this (though we should still use virt-channels framework).
> >>> The sbox setup and teardown can be done as part of channel request and
> >>> freeup. PL08x already does this.
> >>>
> >>> Downside is that we can only have as many consumers at a time as channels.
> >>>
> >>> I have not heard any technical reason for not doing this apart from drivers
> >>> grab the channel at probe, which is incorrect and needs to be fixed
> >>> irrespective of the problem at hand.
> >>>
> >>> This is my preferred option.
> >>
> >> There is one important drawback with this solution. If a driver calls
> >> dma_request_chan() when no channels are currently available, it will
> >> get -EBUSY. If there were a flag in dma_request_chan to be put to
> >> sleep (with timeout) until a channel is available, then it would
> >> work. But busy waiting in the client driver is a waste of power.
> > 
> > Right, but in that case the fallback would be PIO mode, and if that is
> > not availble (IIRC some f your devices don't) then reject the usage with
> > EAGAIN.
> 
> Maybe I'm missing something, but I don't see how that would help.
> Take the NAND Flash controller driver, for instance. PIO is not
> an option, because the ECC engine is tied to DMA.
> 
> And failing with -EAGAIN doesn't help the busy looping situation.
> The caller should be put on some kind of queue to wait for a
> "channel ready" event.

So if you go down this route then we have do a bit of engineering for this
solutions to solve the hardware issue.

Can you tell me the clients for this controller and channels available?

In this case possibly we can dedicate one for NAND and keep the ones with
PIO mode dynamic in nature.. But yes it is not an elegant one.

-- 
~Vinod

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

#1542646

[Multipart message — attachments visible in raw view] — view raw

On Fri, Dec 09, 2016 at 07:23:17PM +0100, Mason wrote:
> On 09/12/2016 18:56, Vinod Koul wrote:

> > Right, but in that case the fallback would be PIO mode, and if that is
> > not availble (IIRC some f your devices don't) then reject the usage with
> > EAGAIN.

> Maybe I'm missing something, but I don't see how that would help.
> Take the NAND Flash controller driver, for instance. PIO is not
> an option, because the ECC engine is tied to DMA.

> And failing with -EAGAIN doesn't help the busy looping situation.
> The caller should be put on some kind of queue to wait for a
> "channel ready" event.

Even without the tie into ECC being an issue it seems like falling back
to PIO could produce poor results at the system level - it's likely that
the PIO would be very expensive and take longer than waiting would've
done, increasing the burden on the CPU and slowing things down overall
for userspace.

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

#1538458

Vinod Koul <vinod.koul@intel.com> writes:

> On Wed, Dec 07, 2016 at 04:45:58PM +0000, Måns Rullgård wrote:
>> Vinod Koul <vinod.koul@intel.com> writes:
>> 
>> > On Tue, Dec 06, 2016 at 01:14:20PM +0000, Måns Rullgård wrote:
>> >> 
>> >> 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.
>> >
>> > No that doesnt make sense at all, you should get a channel only when you
>> > want to use it and not in probe!
>> 
>> Tell that to just about every single driver ever written.
>
> Not really, few do yes which is wrong but not _all_ do that.

Every driver I ever looked at does.  Name one you consider "correct."

-- 
Måns Rullgård

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

#1538470

On Thu, Dec 8, 2016 at 12:44 PM, Måns Rullgård <mans@mansr.com> wrote:
> Vinod Koul <vinod.koul@intel.com> writes:
>> On Wed, Dec 07, 2016 at 04:45:58PM +0000, Måns Rullgård wrote:
>>> Vinod Koul <vinod.koul@intel.com> writes:
>>> > On Tue, Dec 06, 2016 at 01:14:20PM +0000, Måns Rullgård wrote:
>>> >> 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.
>>> >
>>> > No that doesnt make sense at all, you should get a channel only when you
>>> > want to use it and not in probe!
>>>
>>> Tell that to just about every single driver ever written.
>>
>> Not really, few do yes which is wrong but not _all_ do that.
>
> Every driver I ever looked at does.  Name one you consider "correct."

I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but it does
request DMA channels at open time, not at probe time.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

#1538477

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> On Thu, Dec 8, 2016 at 12:44 PM, Måns Rullgård <mans@mansr.com> wrote:
>> Vinod Koul <vinod.koul@intel.com> writes:
>>> On Wed, Dec 07, 2016 at 04:45:58PM +0000, Måns Rullgård wrote:
>>>> Vinod Koul <vinod.koul@intel.com> writes:
>>>> > On Tue, Dec 06, 2016 at 01:14:20PM +0000, Måns Rullgård wrote:
>>>> >> 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.
>>>> >
>>>> > No that doesnt make sense at all, you should get a channel only when you
>>>> > want to use it and not in probe!
>>>>
>>>> Tell that to just about every single driver ever written.
>>>
>>> Not really, few do yes which is wrong but not _all_ do that.
>>
>> Every driver I ever looked at does.  Name one you consider "correct."
>
> I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
> it does request DMA channels at open time, not at probe time.

In the part quoted above, I said most drivers request dma channels in
their probe or open functions.  For the purposes of this discussion,
that distinction is irrelevant.  In either case, the channel is held
indefinitely.  If this wasn't the correct way to use the dmaengine,
there would be no need for the virt-dma helpers which are specifically
designed for cases the one currently at hand.

The only problem we have is that nobody envisioned hardware where the
dma engine indicates completion slightly too soon.  I suspect there's a
fifo or such somewhere, and the interrupt is triggered when the last
byte has been placed in the fifo rather than when it has been removed
which would have been more correct.

-- 
Måns Rullgård

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

#1538489

On Thu, Dec 8, 2016 at 1:20 PM, Måns Rullgård <mans@mansr.com> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> On Thu, Dec 8, 2016 at 12:44 PM, Måns Rullgård <mans@mansr.com> wrote:
>>> Vinod Koul <vinod.koul@intel.com> writes:
>>>> On Wed, Dec 07, 2016 at 04:45:58PM +0000, Måns Rullgård wrote:
>>>>> Vinod Koul <vinod.koul@intel.com> writes:
>>>>> > On Tue, Dec 06, 2016 at 01:14:20PM +0000, Måns Rullgård wrote:
>>>>> >> 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.
>>>>> >
>>>>> > No that doesnt make sense at all, you should get a channel only when you
>>>>> > want to use it and not in probe!
>>>>>
>>>>> Tell that to just about every single driver ever written.
>>>>
>>>> Not really, few do yes which is wrong but not _all_ do that.
>>>
>>> Every driver I ever looked at does.  Name one you consider "correct."
>>
>> I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
>> it does request DMA channels at open time, not at probe time.
>
> In the part quoted above, I said most drivers request dma channels in
> their probe or open functions.  For the purposes of this discussion,
> that distinction is irrelevant.  In either case, the channel is held
> indefinitely.  If this wasn't the correct way to use the dmaengine,
> there would be no need for the virt-dma helpers which are specifically
> designed for cases the one currently at hand.

Sorry, I mainly read Vinod's "not in probe", and missed your "or when the
device is opened".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

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

Back to top | Article view | linux.kernel

csiph-web