Hi Arnd, On Friday 16 Sep 2016 14:02:35 Arnd Bergmann wrote: > On Friday, September 16, 2016 12:48:23 PM CEST Laurent Pinchart wrote: > > On Friday 16 Sep 2016 11:07:48 Arnd Bergmann wrote: > >> On Thursday, September 15, 2016 9:56:51 PM CEST Vinod Koul wrote: > >>> On Wed, Aug 10, 2016 at 11:07:10PM +0530, Vinod Koul wrote: > >>>> On Wed, Aug 10, 2016 at 01:22:13PM +0200, Niklas Söderlund wrote: > >> > >> I had not looked at the series earlier, but this version looks entirely > >> reasonable to me, so > >> > >> Acked-by: Arnd Bergmann <arnd@xxxxxxxx> > >> > >> > >> One concern I have is that we might get an awkward situation if we ever > >> encounter one DMA engine hardware that is used in different systems that > >> all have an IOMMU, but on some of them the connection between the DMA > >> master and the slave FIFO bypasses the IOMMU while on others the IOMMU > >> is required. > > > > Do you mean systems where some of the channels of a specific DMA engine go > > through the IOMMU while others do not ? We indeed have no solution today > > for such a situation. > > I wasn't thinking quite that far, though that is also a theoretical > problem. However, the simple solution would be to have a bit in the DMA > specifier let the driver know whether translation is needed or not. > > The simpler case I was thinking of is where the entire DMA engine > either goes through an IOMMU or doesn't (depending on the integration > into the SoC), so we'd have to find out through some DT property > or compatible string in the DMA enginen driver. Don't we already get that information from the iommus DT property ? If the DMA engine goes through an IOMMU the property will be set, otherwise it will not. > > The problem is a bit broader than that, we'll also have an issue with DMA > > engines that have different channels served by different IOMMUs. > > Do you mean a theoretical problem, or a chip that you already know exists? That's theoretical. The problem I'm facing today is a DMA engine whose channels are served by different ports of the same IOMMU. This works in a suboptimal way because I have to keep all the IOMMU ports enabled regardless of whether they're used or not, as the DMA engine and IOMMU APIs don't carry channel information. > > I recall discussing this in the past with you, and the solution you > > proposed was to add a channel index to struct dma_attrs seems good to me. > > To support the case where some channels don't go through an IOMMU we would > > only need support for null entries in the IOMMUs list associated with a > > device (for instance in the DT case null entries in the iommus property). > > > > Now I see that struct dma_attrs has been replaced by unsigned long in > > > > commit 00085f1efa387a8ce100e3734920f7639c80caa3 > > Author: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx> > > Date: Wed Aug 3 13:46:00 2016 -0700 > > > > dma-mapping: use unsigned long for dma_attrs > > > > We still have enough bits to reserve some of them for a channel number, > > but I'm not very happy with that patch as I can see how a future proposal > > to handle the channel number through the DMA attributes will get rejected > > on the grounds of bits starvation then :-( > > Agreed, that can become interesting. Does the above-mentioned patch really fix a performance, memory consumption or other issue ? -- Regards, Laurent Pinchart