Re: [PATCH 2/5] dmaengine: Add support for custom data mapping

[Abhishek Sahu <absahu@codeaurora.org>]

On 2016-12-19 23:22, Andy Gross wrote:
> On Mon, Dec 19, 2016 at 09:19:23PM +0530, Vinod Koul wrote:
>> On Sun, Dec 18, 2016 at 11:06:42PM -0600, Andy Gross wrote:
>> > On Sun, Dec 18, 2016 at 09:56:02PM +0530, Vinod Koul wrote:
>> > > On Thu, Dec 15, 2016 at 03:25:52PM +0530, Abhishek Sahu wrote:
>> > > > The current DMA APIs only support SGL or data in generic format.
>> > > > The QCA BAM DMA engine data cannot be mapped with already
>> > > > available APIs due to following reasons.
>> > > >
>> > > > 1. The QCA BAM DMA engine uses custom flags which cannot be
>> > > >    mapped with generic DMA engine flags.
>> > > > 2. Some peripheral driver like QCA QPIC NAND/LCD requires to
>> > > >    set specific flags (Like NWD, EOT) for some of the descriptors
>> > > >    in scatter gather list. The already available mapping APIs take
>> > > >    flags parameter in API itself and there is no support for
>> > > >    passing DMA specific flags for each SGL entry.
>> > > >
>> > > > Now this patch adds the support for making the DMA descriptor from
>> > > > custom data with new DMA mapping function prep_dma_custom_mapping.
>> > > > The peripheral driver will pass the custom data in this function and
>> > > > DMA engine driver will form the descriptor according to its own
>> > > > logic. In future, this API can be used by any other DMA engine
>> > > > drivers also which are unable to do DMA mapping with already
>> > > > available API’s.
>> > > >
>> > > > Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
>> > > > ---
>> > > >  include/linux/dmaengine.h | 5 +++++
>> > > >  1 file changed, 5 insertions(+)
>> > > >
>> > > This needs a discussion. Why do you need to add a new API for framework.
>> > >
>> > > What are NWD and EOT flags, cna you details out the flags?
>> >

The QCA BAM descriptor has multiple flags. Following is the detailed
explanation for these flags

1. EOT (End of Transfer) – this flag is used to specify end of 
transaction
    at the end of this descriptor.
2. EOB (End of Blcok) – this flag is used to specify end of block at the
    end of this descriptor.
3. NWD (Notify When Done) – when set, NWD provides a handshake between
    peripheral and BAM indicating the transaction is truly done and
    data/command has delivered its destination.

    SW can use the NWD feature in order to make the BAM to separate 
between
    executions of consecutive descriptors. This can be useful for 
features
    like Command Descriptor.
4. CMD (Command) - allows the SW to create descriptors of type Command 
which
    does not generate any data transmissions but configures registers in 
the
    Peripheral (write operations, and read registers operations).

>> > These are the notify when done and end of transaction flags.  I believe the last
>> > time we talked about this, we (Vinod and I)  agreed to just expose a QCOM only interface to set
>> > the special transaction flags.  You'd then have to have some other API to fixup
>> > the descriptor with the right qcom flags.
>> 
>> Okay, do you have pointer on that one, will avoid asking the same 
>> questions
>> again.
> 
> I'll see if I can find the correspondance and send to you directly.
> 
>> 
>> > Ahbishek, correct me where i am wrong on the following:
>> > So two main differences between a normal descriptor and a command descriptor:
>> > 1) size of the descriptor
>> > 2) the flag setting
>> > 3) data sent in is a modified scatter gather that includes flags , vs a normal
>> > scatter gather

Top level descriptor is same for both. Only difference is Command flag. 
The
command descriptor will contain list of register read/write instead of 
data address
The peripheral driver can form the list with helper function provided in 
patch 5
and submit it to BAM. The main issue is for other flag like EOT/NWD.

The top level descriptor is again in the form of list where BAM writes 
the
address of the list in register before starting of transfer. In this 
list,
each element will have different flags.

>> >
>> > So the CMD descriptors in a given sgl can all have varying flags set? I'd assume
>> > they all have CMD flag set.  Do the current users of the command descriptors
>> > coalesce all of their requests into a big list?
>> >

The main user for command descriptor is currently QPIC NAND/LCD. The 
NAND uses
3 BAM channels- tx, rx and command. NAND controller do the data transfer 
in
chunk of codeword (maximum 544 bytes). NAND chip does the data transfer 
on
page basis so each page read/write can have multiple codewords. The NAND
driver prepares command, tx(write) or rx(read) descriptor for complete 
page
, submit it to BAM and wait for its completion. So NAND driver coalesces
all of their requests into a big list. In this big list,

1. Some of the request for command channel requires NWD flag to be set.
2. TX request depends upon the setting of EOT flag so some of the TX 
request
    in complete page write will contain EOT flag and others will not. So 
this
    custom mapping will be used for data descriptor also in NAND driver.

>> > So a couple of thoughts on how to deal with this:
>> >
>> > 1) Define a virtual channel for the command descriptors vs a normal DMA
>> > transaction.  This lets you use the same hardware channel, but lets you discern
>> > which descriptor format you need to use.  The only issue I see with this is the
>> > required change in device tree binding to target the right type of channel (cmd
>> > vs normal).
>> 
>> Or mark the descriptor is cmd and write accordingly...
> 
> The only issue i see with that is knowing how much to pre-allocate 
> during the
> prep call.  The flag set API would be called on the allocated tx 
> descriptor.  So
> you'd have to know up front and be able to specify it.
> 
>> 
>> >
>> > 2) Provide an API to set flags for the descriptor on a whole descriptor basis.
>> >
>> > 3) If you have a set of transactions described by an sgl that has disparate use
>> > of flags, you split the list and use a separate transaction.  In other words, we
>> > need to enforce that the flag set API will be applied to all descriptors
>> > described by an sgl.  This means that the whole transaction may be comprised of
>> > multiple async TX descriptors.

Each async TX descriptor will generate the BAM interrupt so we are 
deviating
from main purpose of DMA where ideally we should get the interrupt at 
the end
of transfer. This is the main reason for going for this patch.

With the submitted patch, only 1 interrupt per channel is required for
complete NAND page and it solves the setting of BAM specific flags also.
Only issue with this patch is adding new API in DMA framework itself. 
But
this API can be used by other DMA engines in future for which mapping 
cannot
be done with available APIs and if this mapping is vendor specific.
> 
> Regards,
> 
> Andy

-- 
Abhishek