Re: [PATCH v2 0/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously
On 07/30/2018 09:26 AM, Eugeniy Paltsev wrote:
> The ARC HS processor provides an IOC port (I/O coherency bus
> interface) that allows external devices such as DMA devices
> to access memory through the cache hierarchy, providing
> coherency between I/O transactions and the complete memory
> hierarchy.
You didn't pay attention to my previous comment on this !
IOC port can be considered a micro-architecture optimization (an important one
though). The main thing is hardware snooping DMA transactions which enabled IOC
in
first place.
> Some recent SoC with ARC HS (like HSDK) allow to select bus
> port (IOC or non-IOC port) for connecting DMA devices in runtime.
Again you mention the port but none of your 4 patches actually touch the port
itself in programming it.
> With this patch we can use both HW-coherent and regular DMA
> peripherals simultaneously.
>
> NOTE:
> This patch series was stress tested on HSDK with iperf3 (ethernet)
> and bonie++ (usb and sdio) in three configurations:
> * IOC enabled globaly
> * IOC disabled globaly
> * IOC enabled partially (USB & SDIO are connected via IOC AXI port,
>ethernet is connected to DDR AXI port (non-IOC port)
>
> NOTE:
> If you want to test some device without IOC it is not enough
> to remove "dma-coherent" property from dts. You had to remap this
> device to regular DDR AXI port intead of IOC AXI port.
Why are we not doing that for the GPU as part of this series.
> You also need to apply 3 following patches firstly:
> https://www.mail-archive.com/linux-snps-arc@lists.infradead.org/msg03865.html
> https://www.mail-archive.com/linux-snps-arc@lists.infradead.org/msg03889.html
> https://www.mail-archive.com/linux-snps-arc@lists.infradead.org/msg03887.html
>
> NOTE:
> We don't touch any aperture configuration in this patch series. So we
> don't switch any devices between IOC and non-IOC AXI ports on any board.
> It can be done later if it is required.
>
> Changes v1->v2 (Thanks to Christoph):
> * Don't select DMA_DIRECT_OPS explicitly as it is already selected by
>DMA_NONCOHERENT_OPS
> * Remove check for HIGHMEM pages from arch_dma_{alloc, free}
>
> Eugeniy Paltsev (4):
> ARC: DTS: mark DMA devices connected through IOC port as dma-coherent
> ARC: allow to use IOC and non-IOC DMA devices simultaneously
> ARC: IOC: panic if both IOC and ZONE_HIGHMEM enabled
> ARC: don't check for HIGHMEM pages in arch_dma_alloc
>
> arch/arc/boot/dts/axc003.dtsi | 26
> arch/arc/boot/dts/axc003_idu.dtsi | 26
> arch/arc/boot/dts/hsdk.dts | 4 +++
> arch/arc/include/asm/dma-mapping.h | 13
> arch/arc/mm/cache.c| 30 +-
> arch/arc/mm/dma.c | 62
> +++---
> 6 files changed, 115 insertions(+), 46 deletions(-)
> create mode 100644 arch/arc/include/asm/dma-mapping.h
>
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 2/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously
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) {
> + 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;
> diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c
> index cefb776a99ff..4d1466905e48 100644
> --- a/arch/arc/mm/dma.c
> +++ b/arch/arc/mm/dma.c
> @@ -33,19 +33,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
> dma_addr_t *dma_handle,
> if (!page)
> return NULL;
>
> - /*
> - * 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;
>
> /*
> @@ -95,8 +83,7 @@ void arch_dma_free(struct device *dev, size_t size, void
> *vaddr,
> struct page *page = virt_to_page(paddr);
> int is_non_coh = 1;
>
> - is_non_coh = (attrs & DMA_ATTR_NON_CONSISTENT) ||
> - (is_isa_arcv2() && ioc_enable);
> + is_non_coh = (attrs & DMA_ATTR_NON_CONSISTENT);
>
> if (PageHighMem(page) || !is_non_coh)
> iounmap((void __force __iomem *)vaddr);
> @@ -182,3 +169,20 @@ void arch_sync_dma_for_cpu(struct device *dev,
> phys_addr_t paddr,
> break;
> }
> }
I think we have some shenanigans with @ioc_enable now.
Do note that it was more of a debug hack using when the hw feature was
introduced
to be able to run same kernel on various FPGA bitfiles but just flicking a
global
variable via the debugger.
So per code below, if @ioc_enable is NOT set, we still use software assisted
cache
maintenance, but dma_{alloc,free} don't do use that variable. Have you tried
testing the combination when @ioc_enable is set to 0 before boot ? And is that
works ?
> +
> +void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> + const struct iommu_ops *iommu, bool coherent)
> +{
> + /*
> + * IOC hardware snoops all DMA traffic keeping the caches consistent
> + * with memory - eliding need for any explicit cache maintenance of
> + * DMA buffers - so we can use dma_direct cache ops.
> + */
> + if (is_isa_arcv2() && ioc_enable && coherent) {
> + set_dma_ops(dev, &dma_direct_ops);
> + dev_info(dev, "use dma_direct_ops cache ops\n");
> + } else {
> + set_dma_ops(dev, &dma_noncoherent_ops);
> + dev_info(dev, "use dma_noncoherent_ops cache ops\n");
> + }
> +}
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 2/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously
On Mon, 2018-08-13 at 16:24 +, 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) {
> > + 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;
> > diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c
> > index cefb776a99ff..4d1466905e48 100644
> > --- a/arch/arc/mm/dma.c
> > +++ b/arch/arc/mm/dma.c
> > @@ -33,19 +33,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
> > dma_addr_t *dma_handle,
> > if (!page)
> > return NULL;
> >
> > - /*
> > -* 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;
> >
> > /*
> > @@ -95,8 +83,7 @@ void arch_dma_free(struct device *dev, size_t size, void
> > *vaddr,
> > struct page *page = virt_to_page(paddr);
> > int is_non_coh = 1;
> >
> > - is_non_coh = (attrs & DMA_ATTR_NON_CONSISTENT) ||
> > - (is_isa_arcv2() && ioc_enable);
> > + is_non_coh = (attrs & DMA_ATTR_NON_CONSISTENT);
> >
> > if (PageHighMem(page) || !is_non_coh)
> > iounmap((void __force __iomem *)vaddr);
> > @@ -182,3 +169,20 @@ void arch_sync_dma_for_cpu(struct device *dev,
> > phys_addr_t paddr,
> > break;
> > }
> > }
>
> I think we have some shenanigans with @ioc_enable now.
> Do note that it was more of a debug hack using when the hw feature was
> introduced
> to be able to run same kernel on various FPGA bitfiles but just flicking a
> global
> variable via the debugger.
>
> So per code below, if @ioc_enable is NOT set, we still use software assisted
> cache
> maintenance, but dma_{alloc,free} don't do use that variable. Have you tried
> testing the combination when @ioc_enable is set to 0 before boot ? And is
> that works ?
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.
Previously [before this patch] we used this ops for both HW/SW assisted cache
maintenance
that's why we checked @ioc_enable in arch_dma_{alloc,free}. (in case of HW
assisted cache
maintenance we only allocate memory, and in case of SW assisted cache
maintenance we
allocate memory and do MMU mapping to enforce non-cachability)
> > +
> > +void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> > + const struct iommu_ops *iommu, bool coherent)
> > +{
> > + /*
> > +* IOC hardware snoops all DMA traffic keeping the caches consistent
> > +* with memory - eliding need for any explicit cache maintenance of
> > +* DMA buffers - so we can use dma_direct cache ops.
> > +*/
> > + if (is_isa_arcv2() && ioc_enable && coherent) {
> > + set_dma_ops(dev, &dma_direct_ops);
> > + dev_info(dev, "use dma_direct_ops cache ops\n");
> > + } else {
> > + set_dma_ops(dev, &dma_noncoherent_ops);
> > + dev_info(dev, "use dma_noncoherent_ops cache ops\n");
> > + }
> > +}
>
>
--
Eugeniy Paltsev
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 0/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously
On Mon, 2018-08-13 at 16:19 +, Vineet Gupta wrote:
> On 07/30/2018 09:26 AM, Eugeniy Paltsev wrote:
> > The ARC HS processor provides an IOC port (I/O coherency bus
> > interface) that allows external devices such as DMA devices
> > to access memory through the cache hierarchy, providing
> > coherency between I/O transactions and the complete memory
> > hierarchy.
>
> You didn't pay attention to my previous comment on this !
> IOC port can be considered a micro-architecture optimization (an important one
> though). The main thing is hardware snooping DMA transactions which enabled
> IOC in
> first place.
Ok, I'll rewrite this commit message.
> > Some recent SoC with ARC HS (like HSDK) allow to select bus
> > port (IOC or non-IOC port) for connecting DMA devices in runtime.
>
> Again you mention the port but none of your 4 patches actually touch the port
> itself in programming it.
Ok.
>
> > With this patch we can use both HW-coherent and regular DMA
> > peripherals simultaneously.
> >
> > NOTE:
> > This patch series was stress tested on HSDK with iperf3 (ethernet)
> > and bonie++ (usb and sdio) in three configurations:
> > * IOC enabled globaly
> > * IOC disabled globaly
> > * IOC enabled partially (USB & SDIO are connected via IOC AXI port,
> >ethernet is connected to DDR AXI port (non-IOC port)
> >
> > NOTE:
> > If you want to test some device without IOC it is not enough
> > to remove "dma-coherent" property from dts. You had to remap this
> > device to regular DDR AXI port intead of IOC AXI port.
>
> Why are we not doing that for the GPU as part of this series.
We still need to upstream couple of minor changes in Vivante driver before
enable
it for HSDK.
>
> > You also need to apply 3 following patches firstly:
> > https://www.mail-archive.com/linux-snps-arc@lists.infradead.org/msg03865.html
> > https://www.mail-archive.com/linux-snps-arc@lists.infradead.org/msg03889.html
> > https://www.mail-archive.com/linux-snps-arc@lists.infradead.org/msg03887.html
> >
> > NOTE:
> > We don't touch any aperture configuration in this patch series. So we
> > don't switch any devices between IOC and non-IOC AXI ports on any board.
> > It can be done later if it is required.
> >
> > Changes v1->v2 (Thanks to Christoph):
> > * Don't select DMA_DIRECT_OPS explicitly as it is already selected by
> >DMA_NONCOHERENT_OPS
> > * Remove check for HIGHMEM pages from arch_dma_{alloc, free}
> >
> > Eugeniy Paltsev (4):
> > ARC: DTS: mark DMA devices connected through IOC port as dma-coherent
> > ARC: allow to use IOC and non-IOC DMA devices simultaneously
> > ARC: IOC: panic if both IOC and ZONE_HIGHMEM enabled
> > ARC: don't check for HIGHMEM pages in arch_dma_alloc
> >
> > arch/arc/boot/dts/axc003.dtsi | 26
> > arch/arc/boot/dts/axc003_idu.dtsi | 26
> > arch/arc/boot/dts/hsdk.dts | 4 +++
> > arch/arc/include/asm/dma-mapping.h | 13
> > arch/arc/mm/cache.c| 30 +-
> > arch/arc/mm/dma.c | 62
> > +++---
> > 6 files changed, 115 insertions(+), 46 deletions(-)
> > create mode 100644 arch/arc/include/asm/dma-mapping.h
> >
>
>
--
Eugeniy Paltsev
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 0/4] ARC: allow to use IOC and non-IOC DMA devices simultaneously
On 08/13/2018 10:27 AM, Eugeniy Paltsev wrote: >> You didn't pay attention to my previous comment on this ! >> IOC port can be considered a micro-architecture optimization (an important >> one >> though). The main thing is hardware snooping DMA transactions which enabled >> IOC in >> first place. > Ok, I'll rewrite this commit message. No need since this is a cover letter and will get dropped anyways, but the reason I emphasize it is that it sets the wrong expectations for someone down the line googling for this stuff ! And that someone could be other or just us when we've long forgotten about this ;-) > >>> Some recent SoC with ARC HS (like HSDK) allow to select bus >>> port (IOC or non-IOC port) for connecting DMA devices in runtime. >> Again you mention the port but none of your 4 patches actually touch the port >> itself in programming it. > Ok. > ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
