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 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 1 of 3  [1] 2 3  Next page →

#1530193 — Re: Tearing down DMA transfer setup after DMA client has finished

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] | [next] | [standalone]

#1530232

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

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

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

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

[ 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

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

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

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

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

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

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]

#1537889

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!

-- 
~Vinod

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

#1537900

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.

-- 
Måns Rullgård

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

#1538419

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.

-- 
~Vinod

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

#1538430

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?

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

Regards.

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

#1538438

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.

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]

#1538460

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.

-- 
Måns Rullgård

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

#1538471

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).

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]

#1538474

On 08/12/2016 13:03, Geert Uytterhoeven wrote:

> Måns Rullgård wrote:
>
>> Geert Uytterhoeven writes:
>>
>>> Can you fall back to PIO if requesting a channel fails?
>>
>> 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).

FWIW, the ECC engine is tied to the DMA engine. So PIO means
not only taking a hit from tying up the CPU for a slooow
transfer, but also a huge hit if ECC must be computed in SW.
(A 100x perf degradation is not unlikely.)

Regards.

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

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web