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

[Andy Gross]

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@xxxxxxxxxxxxxx>
> > ---
> >  include/linux/dmaengine.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index cc535a4..6324c1f 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -692,6 +692,8 @@ struct dma_filter {
> >   *	be called after period_len bytes have been transferred.
> >   * @device_prep_interleaved_dma: Transfer expression in a generic way.
> >   * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst address
> > + * @device_prep_dma_custom_mapping: prepares a dma operation from dma driver
> > + *	specific custom data
> >   * @device_config: Pushes a new configuration to a channel, return 0 or an error
> >   *	code
> >   * @device_pause: Pauses any transfer happening on a channel. Returns
> > @@ -783,6 +785,9 @@ struct dma_device {
> >  	struct dma_async_tx_descriptor *(*device_prep_dma_imm_data)(
> >  		struct dma_chan *chan, dma_addr_t dst, u64 data,
> >  		unsigned long flags);
> > +	struct dma_async_tx_descriptor *(*device_prep_dma_custom_mapping)(
> > +		struct dma_chan *chan, void *data,
> > +		unsigned long flags);
> 
> 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?

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.

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

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?

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

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.

Regards,
Andy