From: Vineet Gupta <Vineet.Gupta1@synopsys.com> To: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>, "linux-snps-arc@lists.infradead.org" <linux-snps-arc@lists.infradead.org> Cc: "hch@lst.de" <hch@lst.de>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>, Alexey Brodkin <Alexey.Brodkin@synopsys.com> Subject: Re: [PATCH v2 2/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously Date: Mon, 20 Aug 2018 15:34:31 -0700 [thread overview] Message-ID: <81ddd506-1f7e-db82-4c77-ff08b1c15dd3@synopsys.com> (raw) In-Reply-To: <1534180089.3962.68.camel@synopsys.com> On 08/13/2018 10:08 AM, Eugeniy Paltsev wrote: > On Mon, 2018-08-13 at 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. >>> + 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. 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 ?
WARNING: multiple messages have this Message-ID (diff)
From: Vineet.Gupta1@synopsys.com (Vineet Gupta) To: linux-snps-arc@lists.infradead.org Subject: [PATCH v2 2/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously Date: Mon, 20 Aug 2018 15:34:31 -0700 [thread overview] Message-ID: <81ddd506-1f7e-db82-4c77-ff08b1c15dd3@synopsys.com> (raw) In-Reply-To: <1534180089.3962.68.camel@synopsys.com> 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. >>> + 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. 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 ?
next prev parent reply other threads:[~2018-08-20 22:34 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-30 16:26 [PATCH v2 0/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously Eugeniy Paltsev 2018-07-30 16:26 ` Eugeniy Paltsev 2018-07-30 16:26 ` [PATCH v2 1/4] ARC: DTS: mark DMA devices connected through IOC port as dma-coherent Eugeniy Paltsev 2018-07-30 16:26 ` Eugeniy Paltsev 2018-07-30 16:26 ` [PATCH v2 2/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously Eugeniy Paltsev 2018-07-30 16:26 ` Eugeniy Paltsev 2018-08-13 16:24 ` Vineet Gupta 2018-08-13 16:24 ` Vineet Gupta 2018-08-13 16:24 ` Vineet Gupta 2018-08-13 17:08 ` Eugeniy Paltsev 2018-08-13 17:08 ` Eugeniy Paltsev 2018-08-13 17:08 ` Eugeniy Paltsev 2018-08-20 22:34 ` Vineet Gupta [this message] 2018-08-20 22:34 ` Vineet Gupta 2018-08-20 22:34 ` Vineet Gupta 2018-08-22 18:40 ` Eugeniy Paltsev 2018-08-22 18:40 ` Eugeniy Paltsev 2018-08-22 18:40 ` Eugeniy Paltsev 2018-08-23 14:05 ` hch 2018-08-23 14:05 ` hch 2018-08-23 14:05 ` hch 2018-09-04 18:13 ` Vineet Gupta 2018-09-04 18:13 ` Vineet Gupta 2018-09-04 18:13 ` Vineet Gupta 2018-07-30 16:26 ` [PATCH v2 3/4] ARC: IOC: panic if both IOC and ZONE_HIGHMEM enabled Eugeniy Paltsev 2018-07-30 16:26 ` Eugeniy Paltsev 2018-07-30 16:26 ` [PATCH v2 4/4] ARC: don't check for HIGHMEM pages in arch_dma_alloc Eugeniy Paltsev 2018-07-30 16:26 ` Eugeniy Paltsev 2018-07-31 8:08 ` Christoph Hellwig 2018-07-31 8:08 ` Christoph Hellwig 2018-08-13 16:19 ` [PATCH v2 0/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously Vineet Gupta 2018-08-13 16:19 ` Vineet Gupta 2018-08-13 16:19 ` Vineet Gupta 2018-08-13 17:27 ` Eugeniy Paltsev 2018-08-13 17:27 ` Eugeniy Paltsev 2018-08-13 17:27 ` Eugeniy Paltsev 2018-08-13 17:39 ` Vineet Gupta 2018-08-13 17:39 ` Vineet Gupta 2018-08-13 17:39 ` Vineet Gupta 2018-09-04 20:14 ` Vineet Gupta 2018-09-04 20:14 ` Vineet Gupta 2018-09-04 20:14 ` Vineet Gupta 2018-09-04 21:11 ` Christoph Hellwig 2018-09-04 21:11 ` Christoph Hellwig 2018-09-04 21:34 ` Vineet Gupta 2018-09-04 21:34 ` Vineet Gupta 2018-09-04 21:34 ` Vineet Gupta 2018-09-04 21:38 ` Christoph Hellwig 2018-09-04 21:38 ` Christoph Hellwig 2018-09-04 21:38 ` Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=81ddd506-1f7e-db82-4c77-ff08b1c15dd3@synopsys.com \ --to=vineet.gupta1@synopsys.com \ --cc=Alexey.Brodkin@synopsys.com \ --cc=Eugeniy.Paltsev@synopsys.com \ --cc=hch@lst.de \ --cc=linux-arch@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-snps-arc@lists.infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.