From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eugeniy Paltsev Subject: Re: [PATCH v2 2/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously Date: Wed, 22 Aug 2018 18:40:27 +0000 Message-ID: <1534963226.3962.215.camel@synopsys.com> References: <20180730162636.3556-1-Eugeniy.Paltsev@synopsys.com> <20180730162636.3556-3-Eugeniy.Paltsev@synopsys.com> <1534180089.3962.68.camel@synopsys.com> <81ddd506-1f7e-db82-4c77-ff08b1c15dd3@synopsys.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <81ddd506-1f7e-db82-4c77-ff08b1c15dd3@synopsys.com> Content-Language: en-US Content-ID: <782B7B2CFC7553469BA8F1DD9EBA45B6@internal.synopsys.com> Sender: linux-kernel-owner@vger.kernel.org To: Vineet Gupta , "linux-snps-arc@lists.infradead.org" Cc: "hch@lst.de" , "linux-kernel@vger.kernel.org" , "linux-arch@vger.kernel.org" , "Alexey.Brodkin@synopsys.com" List-Id: linux-arch.vger.kernel.org SGkgVmluZWV0LA0KDQpPbiBNb24sIDIwMTgtMDgtMjAgYXQgMTU6MzQgLTA3MDAsIFZpbmVldCBH dXB0YSB3cm90ZToNCj4gT24gMDgvMTMvMjAxOCAxMDowOCBBTSwgRXVnZW5peSBQYWx0c2V2IHdy b3RlOg0KPiA+IE9uIE1vbiwgMjAxOC0wOC0xMyBhdCAxNjoyNCArMDAwMCwgVmluZWV0IEd1cHRh IHdyb3RlOg0KPiA+ID4gT24gMDcvMzAvMjAxOCAwOToyNiBBTSwgRXVnZW5peSBQYWx0c2V2IHdy b3RlOg0KPiA+ID4gPiBAQCAtMTI2MywxMSArMTI1NCw3IEBAIHZvaWQgX19pbml0IGFyY19jYWNo ZV9pbml0X21hc3Rlcih2b2lkKQ0KPiA+ID4gPiAgCWlmIChpc19pc2FfYXJjdjIoKSAmJiBpb2Nf ZW5hYmxlKQ0KPiA+ID4gPiAgCQlhcmNfaW9jX3NldHVwKCk7DQo+ID4gPiA+ICANCj4gPiA+ID4g LQlpZiAoaXNfaXNhX2FyY3YyKCkgJiYgaW9jX2VuYWJsZSkgew0KPiA+ID4gPiAtCQlfX2RtYV9j YWNoZV93YmFja19pbnYgPSBfX2RtYV9jYWNoZV93YmFja19pbnZfaW9jOw0KPiA+ID4gPiAtCQlf X2RtYV9jYWNoZV9pbnYgPSBfX2RtYV9jYWNoZV9pbnZfaW9jOw0KPiA+ID4gPiAtCQlfX2RtYV9j YWNoZV93YmFjayA9IF9fZG1hX2NhY2hlX3diYWNrX2lvYzsNCj4gPiA+ID4gLQl9IGVsc2UgaWYg KGlzX2lzYV9hcmN2MigpICYmIGwyX2xpbmVfc3ogJiYgc2xjX2VuYWJsZSkgew0KPiANCj4gRm9y IHRoZSBjYXN1YWwgcmVhZGVyIEknZCBhZGQgYSBjb21tZW50IHdoeSB0aGlzIHdhcyBkZWxldGVk Lg0KRG8geW91IG1lYW4gY29tbWVudCBpbiBjb21taXQgZGVzY3JpcHRpb24gb3IgY29tbWVudCBy aWdodCBpbiBzb3VyY2VzPw0KDQo+IA0KPiA+ID4gPiArCWlmIChpc19pc2FfYXJjdjIoKSAmJiBs Ml9saW5lX3N6ICYmIHNsY19lbmFibGUpIHsNCj4gPiA+ID4gIAkJX19kbWFfY2FjaGVfd2JhY2tf aW52ID0gX19kbWFfY2FjaGVfd2JhY2tfaW52X3NsYzsNCj4gPiA+ID4gIAkJX19kbWFfY2FjaGVf aW52ID0gX19kbWFfY2FjaGVfaW52X3NsYzsNCj4gPiA+ID4gIAkJX19kbWFfY2FjaGVfd2JhY2sg PSBfX2RtYV9jYWNoZV93YmFja19zbGM7DQo+IA0KPiBbc25pcF0NCj4gDQo+ID4gPiA+ICANCj4g PiA+ID4gLQkvKg0KPiA+ID4gPiAtCSAqIElPQyByZWxpZXMgb24gYWxsIGRhdGEgKGV2ZW4gY29o ZXJlbnQgRE1BIGRhdGEpIGJlaW5nIGluIGNhY2hlDQo+ID4gPiA+IC0JICogVGh1cyBhbGxvY2F0 ZSBub3JtYWwgY2FjaGVkIG1lbW9yeQ0KPiA+ID4gPiAtCSAqDQo+ID4gPiA+IC0JICogVGhlIGdh aW5zIHdpdGggSU9DIGFyZSB0d28gcHJvbmdlZDoNCj4gPiA+ID4gLQkgKiAgIC1Gb3Igc3RyZWFt aW5nIGRhdGEsIGVsaWRlcyBuZWVkIGZvciBjYWNoZSBtYWludGVuYW5jZSwgc2F2aW5nDQo+ID4g PiA+IC0JICogICAgY3ljbGVzIGluIGZsdXNoIGNvZGUsIGFuZCBidXMgYmFuZHdpZHRoIGFzIGFs bCB0aGUgbGluZXMgb2YgYQ0KPiA+ID4gPiAtCSAqICAgIGJ1ZmZlciBuZWVkIHRvIGJlIGZsdXNo ZWQgb3V0IHRvIG1lbW9yeQ0KPiA+ID4gPiAtCSAqICAgLUZvciBjb2hlcmVudCBkYXRhLCBSZWFk L1dyaXRlIHRvIGJ1ZmZlcnMgdGVybWluYXRlIGVhcmx5IGluIGNhY2hlDQo+ID4gPiA+IC0JICog ICAodnMuIGFsd2F5cyBnb2luZyB0byBtZW1vcnkgLSB0aHVzIGFyZSBmYXN0ZXIpDQo+ID4gPiA+ IC0JICovDQo+ID4gPiA+IC0JaWYgKChpc19pc2FfYXJjdjIoKSAmJiBpb2NfZW5hYmxlKSB8fA0K PiA+ID4gPiAtCSAgICAoYXR0cnMgJiBETUFfQVRUUl9OT05fQ09OU0lTVEVOVCkpDQo+ID4gPiA+ ICsJaWYgKGF0dHJzICYgRE1BX0FUVFJfTk9OX0NPTlNJU1RFTlQpDQo+ID4gPiA+ICAJCW5lZWRf Y29oID0gMDsNCj4gPiA+ID4gIA0KPiANCj4gW3NuaXBdDQo+IA0KPiA+IFllcCwgSSB0ZXN0ZWQg dGhhdC4NCj4gPiBBbmQgaXQgd29ya3MgZmluZSB3aXRoIGJvdGggQGlvY19lbmFibGUgPT0gMCBh bmQgQGlvY19lbmFibGUgPT0gMQ0KPiA+IE5vdGUgdGhhdCB3ZSBjaGVjayB0aGlzIHZhcmlhYmxl IGluIGFyY2hfc2V0dXBfZG1hX29wcygpIGZ1bmN0aW9uIG5vdy4NCj4gPiANCj4gPiBTbyB0aGlz IGFyY2hfZG1hX3thbGxvYyxmcmVlfSBhcmUgdXNlZCBPTkxZIGluIGNhc2Ugb2Ygc29mdHdhcmUg YXNzaXN0ZWQgY2FjaGUgbWFpbnRlbmFuY2UuDQo+ID4gVGhhdCdzIHdoeSB3ZSBoYWQgdG8gZG8g TU1VIG1hcHBpbmcgdG8gZW5mb3JjZSBub24tY2FjaGFiaWxpdHkgcmVnYXJkbGVzcyBvZiBAaW9j X2VuYWJsZS4NCj4gDQo+IFJlYWRpbmcga2VybmVsL2RtYS8qIEkgc2VlIHdoYXQgeW91IG1lYW4u IFdlIGNoZWNrIEBpb2NfZW5hYmxlIGF0IHRoZSB0aW1lIG9mDQo+IHJlZ2lzdGVyaW5nIHRoZSBk bWEgb3AgZm9yIGNvaGVyZW50IHZzLiBub24gY29oZXJlbnQgY2FzZSwgc28gdGhlcmUncyBjb21t b24gdnMuDQo+IEFSQyB2ZXJzaW9ucyBvZiBhbGxvYy9mcmVlIGZvciBjb2hlcmVudCB2cy4gbm9u Y29oZXJlbnQuDQoNCkp1c3QgdG8gYmUgc3VyZSB0aGF0IHdlIHVuZGVyc3RhbmQgYm90aCBlYWNo IG90aGVyIGFuZCBzb3VyY2UgY29kZSBjb3JyZWN0bHk6IA0KLSBJbiBjb2hlcmVudCBjYXNlIHdl IHVzZSBkbWFfZGlyZWN0Xyogb3BzIHdoaWNoIGRvZXNuJ3QgdXNlIEFSQyBhbGxvYyBmdW5jdGlv bnMgKGFyY2hfZG1hX3thbGxvY3xmcmVlfSkNCi0gSW4gbm9uLWNvaGVyZW50IGNhc2Ugd2UgdXNl IGRtYV9ub25jb2hlcmVudF8qIG9wcyB3aGljaCB1c2VzIEFSQyBhbGxvYyBmdW5jdGlvbnMgKGFy Y2hfZG1hX3thbGxvY3xmcmVlfSkNCg0KPiBCdXQgdGhlbiBJJ20gY3VyaW91cyB3aHkNCj4gZG8g d2UgYm90aGVyIHRvIGNoZWNrIHRoZSBmb2xsb3dpbmcgaW4gbmV3IGFyY2hfZG1hXyhhbGxvY3xm cmVlKSBhdCBhbGwuDQo+IA0KPiAJaWYgKGF0dHJzICYgRE1BX0FUVFJfTk9OX0NPTlNJU1RFTlQp DQo+IA0KPiBJc24ndCBpdCBzdXBwb3NlZCB0byBiZSBOT05fQ09OU0lTVEVOVCBhbHdheXMgZ2l2 ZW4gdGhlIHdheSBuZXcgY29kZSB3b3JrcyA/DQoNCkRNQV9BVFRSX05PTl9DT05TSVNURU5UIGZs YWcgaXMgbm90IHJlbGF0ZWQgdG8gSU9DIHRvcGljLg0KSXQgaXMgYSBmbGFnIHdoaWNoIHdlIGNh biBwYXNzIHRvIGRtYV97YWxsb2N8ZnJlZX1fYXR0cnMgZnVuY3Rpb24gZnJvbSBkcml2ZXIgc2lk ZS4NCg0KQWNjb3JkaW5nIHRvIGRvY3VtZW50YXRpb246DQogIERNQV9BVFRSX05PTl9DT05TSVNU RU5UOiBMZXRzIHRoZSBwbGF0Zm9ybSB0byBjaG9vc2UgdG8gcmV0dXJuIGVpdGhlcg0KICBjb25z aXN0ZW50IG9yIG5vbi1jb25zaXN0ZW50IG1lbW9yeSBhcyBpdCBzZWVzIGZpdC4NCg0KV2UgY2hl Y2sgdGhpcyBmbGFnIGluIGFyY2hfZG1hX2FsbG9jICh3aGljaCBhcmUgdXNlZCBpbiBub24tY29o ZXJlbnQgY2FzZSkgdG8NCnNraXAgTU1VIG1hcHBpbmcgaWYgd2UgYXJlIGFkdmVydGlzZWQgdGhh dCBjb25zaXN0ZW5jeSBpcyBub3QgcmVxdWlyZWQuDQoNClNvLCBhY3R1YWxseSB3ZSBjYW4gZ2V0 IHJpZCBvZiB0aGlzIGZsYWcgY2hlY2tpbmcgaW4gYXJjaF9kbWFfYWxsb2MgYW5kIA0Kc2ltcGx5 IGFsd2F5cyBkbyBNTVUgbWFwcGluZyB0byBlbmZvcmNlIG5vbi1jYWNoYWJpbGl0eSBhbmQgcmV0 dXJuDQpub24tY2FjaGVhYmxlIG1lbW9yeSBldmVuIGlmIERNQV9BVFRSX05PTl9DT05TSVNURU5U IGlzIHBhc3NlZC4NCkJ1dCBJIGRvbid0IHN1cmUgd2Ugd2FudCB0byBkbyB0aGF0Lg0KDQpCVFc6 IGRtYV9hbGxvY19jb2hlcmVudCBpcyBzaW1wbHkgZG1hX2FsbG9jX2F0dHJzIHdpdGggYXR0cnMg PT0gMC4NCg0KLS0gDQogRXVnZW5peSBQYWx0c2V2 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eugeniy.Paltsev@synopsys.com (Eugeniy Paltsev) Date: Wed, 22 Aug 2018 18:40:27 +0000 Subject: [PATCH v2 2/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously In-Reply-To: <81ddd506-1f7e-db82-4c77-ff08b1c15dd3@synopsys.com> References: <20180730162636.3556-1-Eugeniy.Paltsev@synopsys.com> <20180730162636.3556-3-Eugeniy.Paltsev@synopsys.com> <1534180089.3962.68.camel@synopsys.com> <81ddd506-1f7e-db82-4c77-ff08b1c15dd3@synopsys.com> List-ID: Message-ID: <1534963226.3962.215.camel@synopsys.com> To: linux-snps-arc@lists.infradead.org Hi Vineet, On Mon, 2018-08-20@15:34 -0700, Vineet Gupta wrote: > On 08/13/2018 10:08 AM, Eugeniy Paltsev wrote: > > On Mon, 2018-08-13@16:24 +0000, Vineet Gupta wrote: > > > On 07/30/2018 09:26 AM, Eugeniy Paltsev wrote: > > > > @@ -1263,11 +1254,7 @@ void __init arc_cache_init_master(void) > > > > if (is_isa_arcv2() && ioc_enable) > > > > arc_ioc_setup(); > > > > > > > > - if (is_isa_arcv2() && ioc_enable) { > > > > - __dma_cache_wback_inv = __dma_cache_wback_inv_ioc; > > > > - __dma_cache_inv = __dma_cache_inv_ioc; > > > > - __dma_cache_wback = __dma_cache_wback_ioc; > > > > - } else if (is_isa_arcv2() && l2_line_sz && slc_enable) { > > For the casual reader I'd add a comment why this was deleted. Do you mean comment in commit description or comment right in sources? > > > > > + if (is_isa_arcv2() && l2_line_sz && slc_enable) { > > > > __dma_cache_wback_inv = __dma_cache_wback_inv_slc; > > > > __dma_cache_inv = __dma_cache_inv_slc; > > > > __dma_cache_wback = __dma_cache_wback_slc; > > [snip] > > > > > > > > > - /* > > > > - * IOC relies on all data (even coherent DMA data) being in cache > > > > - * Thus allocate normal cached memory > > > > - * > > > > - * The gains with IOC are two pronged: > > > > - * -For streaming data, elides need for cache maintenance, saving > > > > - * cycles in flush code, and bus bandwidth as all the lines of a > > > > - * buffer need to be flushed out to memory > > > > - * -For coherent data, Read/Write to buffers terminate early in cache > > > > - * (vs. always going to memory - thus are faster) > > > > - */ > > > > - if ((is_isa_arcv2() && ioc_enable) || > > > > - (attrs & DMA_ATTR_NON_CONSISTENT)) > > > > + if (attrs & DMA_ATTR_NON_CONSISTENT) > > > > need_coh = 0; > > > > > > [snip] > > > Yep, I tested that. > > And it works fine with both @ioc_enable == 0 and @ioc_enable == 1 > > Note that we check this variable in arch_setup_dma_ops() function now. > > > > So this arch_dma_{alloc,free} are used ONLY in case of software assisted cache maintenance. > > That's why we had to do MMU mapping to enforce non-cachability regardless of @ioc_enable. > > Reading kernel/dma/* I see what you mean. We check @ioc_enable at the time of > registering the dma op for coherent vs. non coherent case, so there's common vs. > ARC versions of alloc/free for coherent vs. noncoherent. Just to be sure that we understand both each other and source code correctly: - In coherent case we use dma_direct_* ops which doesn't use ARC alloc functions (arch_dma_{alloc|free}) - In non-coherent case we use dma_noncoherent_* ops which uses ARC alloc functions (arch_dma_{alloc|free}) > But then I'm curious why > do we bother to check the following in new arch_dma_(alloc|free) at all. > > if (attrs & DMA_ATTR_NON_CONSISTENT) > > Isn't it supposed to be NON_CONSISTENT always given the way new code works ? DMA_ATTR_NON_CONSISTENT flag is not related to IOC topic. It is a flag which we can pass to dma_{alloc|free}_attrs function from driver side. According to documentation: DMA_ATTR_NON_CONSISTENT: Lets the platform to choose to return either consistent or non-consistent memory as it sees fit. We check this flag in arch_dma_alloc (which are used in non-coherent case) to skip MMU mapping if we are advertised that consistency is not required. So, actually we can get rid of this flag checking in arch_dma_alloc and simply always do MMU mapping to enforce non-cachability and return non-cacheable memory even if DMA_ATTR_NON_CONSISTENT is passed. But I don't sure we want to do that. BTW: dma_alloc_coherent is simply dma_alloc_attrs with attrs == 0. -- Eugeniy Paltsev