Re: [PATCH 4/5] dmaengine: sprd: Add Spreadtrum DMA configuration

[Vinod Koul <vinod.koul@intel.com>]

On Wed, Apr 11, 2018 at 08:13:28PM +0800, Baolin Wang wrote:
> Hi Vinod,
> 
> On 11 April 2018 at 17:36, Vinod Koul <vinod.koul@intel.com> wrote:
> > On Tue, Apr 10, 2018 at 03:46:06PM +0800, Baolin Wang wrote:
> >
> >> +/*
> >> + * struct sprd_dma_config - DMA configuration structure
> >> + * @config: dma slave channel config
> >> + * @fragment_len: specify one fragment transfer length
> >> + * @block_len: specify one block transfer length
> >> + * @transcation_len: specify one transcation transfer length
> >> + * @wrap_ptr: wrap pointer address, once the transfer address reaches the
> >> + * 'wrap_ptr', the next transfer address will jump to the 'wrap_to' address.
> >> + * @wrap_to: wrap jump to address
> >> + * @req_mode: specify the DMA request mode
> >> + * @int_mode: specify the DMA interrupt type
> >> + */
> >> +struct sprd_dma_config {
> >> +     struct dma_slave_config config;
> >> +     u32 fragment_len;
> >
> > why not use _maxburst?
> 
> Yes, I can use maxburst.
> 
> >
> >> +     u32 block_len;
> >> +     u32 transcation_len;
> >
> > what does block and transaction len refer to here
> 
>  Our DMA has 3 transfer mode: transaction transfer, block transfer and
> fragment transfer. One transaction transfer can contain several blocks
> transfer, and each block can be set proper block step. One block can
> contain several fragments transfer with proper fragment step. It can
> generate interrupts when one transaction transfer or block transfer or
> fragment transfer is completed if user set the interrupt type. So here
> we should set the length for transaction transfer, block transfer and
> fragment transfer.

what are the max size these types support?

> 
> >
> >> +     phys_addr_t wrap_ptr;
> >> +     phys_addr_t wrap_to;
> >
> > this sound sg_list to me, why are we not using that here
> 
> It is similar to sg list, but it is not one software action, we have
> hardware registers to help to jump one specified address.
> 
> >
> >> +     enum sprd_dma_req_mode req_mode;
> >
> > Looking at definition of request mode we have frag, block, transaction list
> > etc.. That should depend upon dma request. If you have been asked to
> > transfer a list, you shall configure list mode. if it is a single
> > transaction then it should be transaction mode!
> 
> If I understand your points correctly, you mean we can specify the
> request mode when requesting one slave channel by
> 'dma_request_slave_channel()'. But we need change the request mode
> dynamically following different transfer task for this channel, so I
> am afraid we can not specify the request mode of this channel at
> requesting time.

Nope a channel has nothing to do with request type. You request and grab a
channel. Then you prepare a descriptor for a dma transaction. Based on
transaction requested you should intelligently break it down and create a
descriptor which uses transaction/block/fragment so that DMA throughput is
efficient. If prepare has sg list then you should use link list mode.
Further if you support max length, say 16KB and request is for 20KB you may
break it down for link list with two segments.

Each prep call has flags associated, that can help you configure interrupt
behaviour.

Btw other dma controllers have similar capabilities and driver manages these
:)

-- 
~Vinod