lkml.org 
[lkml]   [2018]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 4/5] dmaengine: sprd: Add Spreadtrum DMA configuration
    On 12 April 2018 at 19:30, Baolin Wang <baolin.wang@linaro.org> wrote:
    > Hi Vinod,
    >
    > On 12 April 2018 at 17:37, Vinod Koul <vinod.koul@intel.com> wrote:
    >> 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?
    >
    > These types max size definition:
    >
    > #define SPRD_DMA_FRG_LEN_MASK GENMASK(16, 0)
    >
    > #define SPRD_DMA_BLK_LEN_MASK GENMASK(16, 0)
    >
    > #define SPRD_DMA_TRSC_LEN_MASK GENMASK(27, 0)
    >
    >>>
    >>> >
    >>> >> + 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.
    >
    > OK. So I can add one more cell to specify the request mode for this channel.
    >
    > dmas = <&apdma 11 SPRD_DMA_BLK_REQ>
    >
    >>
    >> Each prep call has flags associated, that can help you configure interrupt
    >> behaviour.

    After more thinking, I think this will be not useful for users, since
    users need configure one DMA channel through different 3 places,
    specify the request mode in dts, specify the interrupt type through
    prep call flags, and other configuration need to be configured through
    'struct sprd_dma_config'. That is not one good design for users. What
    do you think? Thanks.

    --
    Baolin.wang
    Best Regards

    \
     
     \ /
      Last update: 2018-04-12 13:36    [W:4.847 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site