Hello Morimoto-san, Thank you for the patch.
On Tuesday 07 Mar 2017 01:10:32 Kuninori Morimoto wrote: > From: Kuninori Morimoto <[email protected]> > > SYS-DMAC can use 40bit address transfer, and it supports Descriptor > Mode too. Current SYS-DMAC driver disables Descriptor Mode if it was > 40bit address today. But it can use Descriptor Mode with 40bit if > transfer Source/Destination address are located in same 4GiB region > in the 40 bit address space. > This patch enables it if all condition was clear > > Signed-off-by: Kuninori Morimoto <[email protected]> > --- > drivers/dma/sh/rcar-dmac.c | 65 +++++++++++++++++++++++------------------- > 1 file changed, 36 insertions(+), 29 deletions(-) > > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c > index 48b22d5..2ebd2be 100644 > --- a/drivers/dma/sh/rcar-dmac.c > +++ b/drivers/dma/sh/rcar-dmac.c [snip] > @@ -855,8 +859,12 @@ static void rcar_dmac_chan_configure_desc(struct > rcar_dmac_chan *chan, unsigned int nchunks = 0; > unsigned int max_chunk_size; > unsigned int full_size = 0; > - bool highmem = false; > + bool cross_boundary = false; > unsigned int i; > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT > + u32 high_src_addr = 0; > + u32 high_dst_addr = 0; > +#endif > > desc = rcar_dmac_desc_get(chan); > if (!desc) > @@ -885,25 +893,6 @@ static void rcar_dmac_chan_configure_desc(struct > rcar_dmac_chan *chan, while (len) { > unsigned int size = min(len, max_chunk_size); > > -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT > - /* > - * Prevent individual transfers from crossing 4GB > - * boundaries. > - */ > - if (dev_addr >> 32 != (dev_addr + size - 1) >> 32) > - size = ALIGN(dev_addr, 1ULL << 32) - dev_addr; > - if (mem_addr >> 32 != (mem_addr + size - 1) >> 32) > - size = ALIGN(mem_addr, 1ULL << 32) - mem_addr; > - > - /* > - * Check if either of the source or destination address > - * can't be expressed in 32 bits. If so we can't use > - * hardware descriptor lists. > - */ > - if (dev_addr >> 32 || mem_addr >> 32) > - highmem = true; > -#endif > - > chunk = rcar_dmac_xfer_chunk_get(chan); > if (!chunk) { > rcar_dmac_desc_put(chan, desc); > @@ -918,6 +907,26 @@ static void rcar_dmac_chan_configure_desc(struct > rcar_dmac_chan *chan, chunk->dst_addr = dev_addr; > } > > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT > + if (i == 0) { > + high_src_addr = chunk->src_addr >> 32; > + high_dst_addr = chunk->dst_addr >> 32; > + } This will be triggered for every iteration of the while loop, so if one sg entry crosses a 32-bit boundary, you won't detect it. I know it will mean one additional #ifdef, but I think you should move that code out of the while loop as I previously proposed. > + /* > + * Prevent individual transfers from crossing 4GB > + * boundaries. > + */ > + if (dev_addr >> 32 != (dev_addr + size - 1) >> 32) > + size = ALIGN(dev_addr, 1ULL << 32) - dev_addr; > + if (mem_addr >> 32 != (mem_addr + size - 1) >> 32) > + size = ALIGN(mem_addr, 1ULL << 32) - mem_addr; This would then become if (dev_addr >> 32 != (dev_addr + size - 1) >> 32) { size = ALIGN(dev_addr, 1ULL << 32) - dev_addr; cross_boundary = true; } if (mem_addr >> 32 != (mem_addr + size - 1) >> 32) { size = ALIGN(mem_addr, 1ULL << 32) - mem_addr; cross_boundary = true; } > + if ((chunk->src_addr >> 32 != high_src_addr) || > + (chunk->dst_addr >> 32 != high_dst_addr)) > + cross_boundary = true; And this check can also be moved just before the while loop. > +#endif > + > chunk->size = size; > > dev_dbg(chan->chan.device->dev, > @@ -943,13 +952,11 @@ static void rcar_dmac_chan_configure_desc(struct > rcar_dmac_chan *chan, * Use hardware descriptor lists if possible when more > than one chunk * needs to be transferred (otherwise they don't make much > sense). * > - * The highmem check currently covers the whole transfer. As an > - * optimization we could use descriptor lists for consecutive lowmem > - * chunks and direct manual mode for highmem chunks. Whether the > - * performance improvement would be significant enough compared to the > - * additional complexity remains to be investigated. > + * Source/Destination address should be located in same 4GiB region > + * in the 40bit address space when it uses Hardware descriptor, > + * and cross_boundary is checking it. > */ > - desc->hwdescs.use = !highmem && nchunks > 1; > + desc->hwdescs.use = !cross_boundary && nchunks > 1; > if (desc->hwdescs.use) { > if (rcar_dmac_fill_hwdesc(chan, desc) < 0) > desc->hwdescs.use = false; -- Regards, Laurent Pinchart
