From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [4/5] dmaengine: sprd: Add Spreadtrum DMA configuration From: Vinod Koul Message-Id: <20180413034332.GI6014@localhost> Date: Fri, 13 Apr 2018 09:13:33 +0530 To: Baolin Wang Cc: Dan Williams , Eric Long , Mark Brown , dmaengine@vger.kernel.org, LKML List-ID: T24gVGh1LCBBcHIgMTIsIDIwMTggYXQgMDc6MzY6MzRQTSArMDgwMCwgQmFvbGluIFdhbmcgd3Jv dGU6Cj4gPj4+ID4+ICsvKgo+ID4+PiA+PiArICogc3RydWN0IHNwcmRfZG1hX2NvbmZpZyAtIERN QSBjb25maWd1cmF0aW9uIHN0cnVjdHVyZQo+ID4+PiA+PiArICogQGNvbmZpZzogZG1hIHNsYXZl IGNoYW5uZWwgY29uZmlnCj4gPj4+ID4+ICsgKiBAZnJhZ21lbnRfbGVuOiBzcGVjaWZ5IG9uZSBm cmFnbWVudCB0cmFuc2ZlciBsZW5ndGgKPiA+Pj4gPj4gKyAqIEBibG9ja19sZW46IHNwZWNpZnkg b25lIGJsb2NrIHRyYW5zZmVyIGxlbmd0aAo+ID4+PiA+PiArICogQHRyYW5zY2F0aW9uX2xlbjog c3BlY2lmeSBvbmUgdHJhbnNjYXRpb24gdHJhbnNmZXIgbGVuZ3RoCj4gPj4+ID4+ICsgKiBAd3Jh cF9wdHI6IHdyYXAgcG9pbnRlciBhZGRyZXNzLCBvbmNlIHRoZSB0cmFuc2ZlciBhZGRyZXNzIHJl YWNoZXMgdGhlCj4gPj4+ID4+ICsgKiAnd3JhcF9wdHInLCB0aGUgbmV4dCB0cmFuc2ZlciBhZGRy ZXNzIHdpbGwganVtcCB0byB0aGUgJ3dyYXBfdG8nIGFkZHJlc3MuCj4gPj4+ID4+ICsgKiBAd3Jh cF90bzogd3JhcCBqdW1wIHRvIGFkZHJlc3MKPiA+Pj4gPj4gKyAqIEByZXFfbW9kZTogc3BlY2lm eSB0aGUgRE1BIHJlcXVlc3QgbW9kZQo+ID4+PiA+PiArICogQGludF9tb2RlOiBzcGVjaWZ5IHRo ZSBETUEgaW50ZXJydXB0IHR5cGUKPiA+Pj4gPj4gKyAqLwo+ID4+PiA+PiArc3RydWN0IHNwcmRf ZG1hX2NvbmZpZyB7Cj4gPj4+ID4+ICsgICAgIHN0cnVjdCBkbWFfc2xhdmVfY29uZmlnIGNvbmZp ZzsKPiA+Pj4gPj4gKyAgICAgdTMyIGZyYWdtZW50X2xlbjsKPiA+Pj4gPgo+ID4+PiA+IHdoeSBu b3QgdXNlIF9tYXhidXJzdD8KPiA+Pj4KPiA+Pj4gWWVzLCBJIGNhbiB1c2UgbWF4YnVyc3QuCj4g Pj4+Cj4gPj4+ID4KPiA+Pj4gPj4gKyAgICAgdTMyIGJsb2NrX2xlbjsKPiA+Pj4gPj4gKyAgICAg dTMyIHRyYW5zY2F0aW9uX2xlbjsKPiA+Pj4gPgo+ID4+PiA+IHdoYXQgZG9lcyBibG9jayBhbmQg dHJhbnNhY3Rpb24gbGVuIHJlZmVyIHRvIGhlcmUKPiA+Pj4KPiA+Pj4gIE91ciBETUEgaGFzIDMg dHJhbnNmZXIgbW9kZTogdHJhbnNhY3Rpb24gdHJhbnNmZXIsIGJsb2NrIHRyYW5zZmVyIGFuZAo+ ID4+PiBmcmFnbWVudCB0cmFuc2Zlci4gT25lIHRyYW5zYWN0aW9uIHRyYW5zZmVyIGNhbiBjb250 YWluIHNldmVyYWwgYmxvY2tzCj4gPj4+IHRyYW5zZmVyLCBhbmQgZWFjaCBibG9jayBjYW4gYmUg c2V0IHByb3BlciBibG9jayBzdGVwLiBPbmUgYmxvY2sgY2FuCj4gPj4+IGNvbnRhaW4gc2V2ZXJh bCBmcmFnbWVudHMgdHJhbnNmZXIgd2l0aCBwcm9wZXIgZnJhZ21lbnQgc3RlcC4gSXQgY2FuCj4g Pj4+IGdlbmVyYXRlIGludGVycnVwdHMgd2hlbiBvbmUgdHJhbnNhY3Rpb24gdHJhbnNmZXIgb3Ig YmxvY2sgdHJhbnNmZXIgb3IKPiA+Pj4gZnJhZ21lbnQgdHJhbnNmZXIgaXMgY29tcGxldGVkIGlm IHVzZXIgc2V0IHRoZSBpbnRlcnJ1cHQgdHlwZS4gU28gaGVyZQo+ID4+PiB3ZSBzaG91bGQgc2V0 IHRoZSBsZW5ndGggZm9yIHRyYW5zYWN0aW9uIHRyYW5zZmVyLCBibG9jayB0cmFuc2ZlciBhbmQK PiA+Pj4gZnJhZ21lbnQgdHJhbnNmZXIuCj4gPj4KPiA+PiB3aGF0IGFyZSB0aGUgbWF4IHNpemUg dGhlc2UgdHlwZXMgc3VwcG9ydD8KPiA+Cj4gPiBUaGVzZSB0eXBlcyBtYXggc2l6ZSBkZWZpbml0 aW9uOgo+ID4KPiA+ICNkZWZpbmUgU1BSRF9ETUFfRlJHX0xFTl9NQVNLIEdFTk1BU0soMTYsIDAp Cj4gPgo+ID4gI2RlZmluZSBTUFJEX0RNQV9CTEtfTEVOX01BU0sgR0VOTUFTSygxNiwgMCkKPiA+ Cj4gPiAjZGVmaW5lIFNQUkRfRE1BX1RSU0NfTEVOX01BU0sgR0VOTUFTSygyNywgMCkKPiA+Cj4g Pj4+Cj4gPj4+ID4KPiA+Pj4gPj4gKyAgICAgcGh5c19hZGRyX3Qgd3JhcF9wdHI7Cj4gPj4+ID4+ ICsgICAgIHBoeXNfYWRkcl90IHdyYXBfdG87Cj4gPj4+ID4KPiA+Pj4gPiB0aGlzIHNvdW5kIHNn X2xpc3QgdG8gbWUsIHdoeSBhcmUgd2Ugbm90IHVzaW5nIHRoYXQgaGVyZQo+ID4+Pgo+ID4+PiBJ dCBpcyBzaW1pbGFyIHRvIHNnIGxpc3QsIGJ1dCBpdCBpcyBub3Qgb25lIHNvZnR3YXJlIGFjdGlv biwgd2UgaGF2ZQo+ID4+PiBoYXJkd2FyZSByZWdpc3RlcnMgdG8gaGVscCB0byBqdW1wIG9uZSBz cGVjaWZpZWQgYWRkcmVzcy4KPiA+Pj4KPiA+Pj4gPgo+ID4+PiA+PiArICAgICBlbnVtIHNwcmRf ZG1hX3JlcV9tb2RlIHJlcV9tb2RlOwo+ID4+PiA+Cj4gPj4+ID4gTG9va2luZyBhdCBkZWZpbml0 aW9uIG9mIHJlcXVlc3QgbW9kZSB3ZSBoYXZlIGZyYWcsIGJsb2NrLCB0cmFuc2FjdGlvbiBsaXN0 Cj4gPj4+ID4gZXRjLi4gVGhhdCBzaG91bGQgZGVwZW5kIHVwb24gZG1hIHJlcXVlc3QuIElmIHlv dSBoYXZlIGJlZW4gYXNrZWQgdG8KPiA+Pj4gPiB0cmFuc2ZlciBhIGxpc3QsIHlvdSBzaGFsbCBj b25maWd1cmUgbGlzdCBtb2RlLiBpZiBpdCBpcyBhIHNpbmdsZQo+ID4+PiA+IHRyYW5zYWN0aW9u IHRoZW4gaXQgc2hvdWxkIGJlIHRyYW5zYWN0aW9uIG1vZGUhCj4gPj4+Cj4gPj4+IElmIEkgdW5k ZXJzdGFuZCB5b3VyIHBvaW50cyBjb3JyZWN0bHksIHlvdSBtZWFuIHdlIGNhbiBzcGVjaWZ5IHRo ZQo+ID4+PiByZXF1ZXN0IG1vZGUgd2hlbiByZXF1ZXN0aW5nIG9uZSBzbGF2ZSBjaGFubmVsIGJ5 Cj4gPj4+ICdkbWFfcmVxdWVzdF9zbGF2ZV9jaGFubmVsKCknLiBCdXQgd2UgbmVlZCBjaGFuZ2Ug dGhlIHJlcXVlc3QgbW9kZQo+ID4+PiBkeW5hbWljYWxseSBmb2xsb3dpbmcgZGlmZmVyZW50IHRy YW5zZmVyIHRhc2sgZm9yIHRoaXMgY2hhbm5lbCwgc28gSQo+ID4+PiBhbSBhZnJhaWQgd2UgY2Fu IG5vdCBzcGVjaWZ5IHRoZSByZXF1ZXN0IG1vZGUgb2YgdGhpcyBjaGFubmVsIGF0Cj4gPj4+IHJl cXVlc3RpbmcgdGltZS4KPiA+Pgo+ID4+IE5vcGUgYSBjaGFubmVsIGhhcyBub3RoaW5nIHRvIGRv IHdpdGggcmVxdWVzdCB0eXBlLiBZb3UgcmVxdWVzdCBhbmQgZ3JhYiBhCj4gPj4gY2hhbm5lbC4g VGhlbiB5b3UgcHJlcGFyZSBhIGRlc2NyaXB0b3IgZm9yIGEgZG1hIHRyYW5zYWN0aW9uLiBCYXNl ZCBvbgo+ID4+IHRyYW5zYWN0aW9uIHJlcXVlc3RlZCB5b3Ugc2hvdWxkIGludGVsbGlnZW50bHkg YnJlYWsgaXQgZG93biBhbmQgY3JlYXRlIGEKPiA+PiBkZXNjcmlwdG9yIHdoaWNoIHVzZXMgdHJh bnNhY3Rpb24vYmxvY2svZnJhZ21lbnQgc28gdGhhdCBETUEgdGhyb3VnaHB1dCBpcwo+ID4+IGVm ZmljaWVudC4gSWYgcHJlcGFyZSBoYXMgc2cgbGlzdCB0aGVuIHlvdSBzaG91bGQgdXNlIGxpbmsg bGlzdCBtb2RlLgo+ID4+IEZ1cnRoZXIgaWYgeW91IHN1cHBvcnQgbWF4IGxlbmd0aCwgc2F5IDE2 S0IgYW5kIHJlcXVlc3QgaXMgZm9yIDIwS0IgeW91IG1heQo+ID4+IGJyZWFrIGl0IGRvd24gZm9y IGxpbmsgbGlzdCB3aXRoIHR3byBzZWdtZW50cy4KPiA+Cj4gPiBPSy4gU28gSSBjYW4gYWRkIG9u ZSBtb3JlIGNlbGwgdG8gc3BlY2lmeSB0aGUgcmVxdWVzdCBtb2RlIGZvciB0aGlzIGNoYW5uZWwu Cj4gPgo+ID4gZG1hcyA9IDwmYXBkbWEgMTEgU1BSRF9ETUFfQkxLX1JFUT4KPiA+Cj4gPj4KPiA+ PiBFYWNoIHByZXAgY2FsbCBoYXMgZmxhZ3MgYXNzb2NpYXRlZCwgdGhhdCBjYW4gaGVscCB5b3Ug Y29uZmlndXJlIGludGVycnVwdAo+ID4+IGJlaGF2aW91ci4KPiAKPiBBZnRlciBtb3JlIHRoaW5r aW5nLCBJIHRoaW5rIHRoaXMgd2lsbCBiZSBub3QgdXNlZnVsIGZvciB1c2Vycywgc2luY2UKPiB1 c2VycyBuZWVkIGNvbmZpZ3VyZSBvbmUgRE1BIGNoYW5uZWwgdGhyb3VnaCBkaWZmZXJlbnQgMyBw bGFjZXMsCj4gc3BlY2lmeSB0aGUgcmVxdWVzdCBtb2RlIGluIGR0cywgc3BlY2lmeSB0aGUgaW50 ZXJydXB0IHR5cGUgdGhyb3VnaAo+IHByZXAgY2FsbCBmbGFncywgYW5kIG90aGVyIGNvbmZpZ3Vy YXRpb24gbmVlZCB0byBiZSBjb25maWd1cmVkIHRocm91Z2gKPiAnc3RydWN0IHNwcmRfZG1hX2Nv bmZpZycuIFRoYXQgaXMgbm90IG9uZSBnb29kIGRlc2lnbiBmb3IgdXNlcnMuIFdoYXQKPiBkbyB5 b3UgdGhpbms/IFRoYW5rcy4KCkFncmVlZCwgdXNlcnMgb25seSBjYXJlIGFib3V0IGdyYWJiaW5n IGEgY2hhbm5lbCwgc2V0dGluZyBhIGRlc2NyaXB0b3IgYW5kCnN1Ym1pdHRpbmcgdGhhdC4KCkkg dGhpbmsgeW91IG5lZWQgdG8gZ28gYmFjayBhbmQgdGhpbmsgYWJvdXQgdGhpcyBhIGJpdCwgcGxl YXNlIGRvIGdvIHRocnUKZG1hZW5naW5lIGRvY3VtZW50YXRpb24gYW5kIHNlZSBvdGhlciBkcml2 ZXIgZXhhbXBsZXMuCgpXZSBkb24ndCB0eXBpY2FsbHkgZXhwb3NlIHRoZXNlIHRvIHVzZXJzLCB0 aGV5IGdpdmUgdXMgYSB0cmFuc2ZlciBhbmQgd2Ugc2V0CnRoYXQgdXAgaW4gaGFyZHdhcmUgZm9y IGVmZmljaWVudC4gSXRzIERNQSBzbyBwZW9wbGUgZXhwZWN0IHVzIHRvIHVzZSBmYXN0ZXN0Cm1l Y2hhbmlzbSBhdmFpbGFibGUuCgpJIHdvdWxkIHNheSBzZXR1cCBhcyBMaW5rIGxpc3QgZm9yIHNn IHRyYW5zZmVycyBhbmQgdXNlIG9uZSBvZiB0aGVtIG1vZGVzCihhZ2FpbiB0aGluayBlZmZpY2ll bmN5KSBmb3Igc2luZ2xlIHRyYW5zZmVycy4KCkFsc28gdXNlIGFsbCB0aGUgcGFyYW1ldGVycyBp biBkbWFfc2xhdmVfY29uZmlnIGFuZCBhbHNvIHNldHVwIGNhcGFiaWxpdGllcyBpZgpub3QgZG9u ZS4K From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753724AbeDMDjH (ORCPT ); Thu, 12 Apr 2018 23:39:07 -0400 Received: from mga09.intel.com ([134.134.136.24]:23921 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752170AbeDMDjF (ORCPT ); Thu, 12 Apr 2018 23:39:05 -0400 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,444,1517904000"; d="scan'208";a="42971379" Date: Fri, 13 Apr 2018 09:13:33 +0530 From: Vinod Koul To: Baolin Wang Cc: Dan Williams , Eric Long , Mark Brown , dmaengine@vger.kernel.org, LKML Subject: Re: [PATCH 4/5] dmaengine: sprd: Add Spreadtrum DMA configuration Message-ID: <20180413034332.GI6014@localhost> References: <0c2b76aba6a49e583f920ae582d6815fa9cc4361.1523346135.git.baolin.wang@linaro.org> <20180411093634.GC6014@localhost> <20180412093735.GF6014@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 12, 2018 at 07:36:34PM +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. Agreed, users only care about grabbing a channel, setting a descriptor and submitting that. I think you need to go back and think about this a bit, please do go thru dmaengine documentation and see other driver examples. We don't typically expose these to users, they give us a transfer and we set that up in hardware for efficient. Its DMA so people expect us to use fastest mechanism available. I would say setup as Link list for sg transfers and use one of them modes (again think efficiency) for single transfers. Also use all the parameters in dma_slave_config and also setup capabilities if not done. -- ~Vinod