On Thursday 25 February 2016 18:49:06 Alexandre Courbot wrote: > On Wed, Feb 24, 2016 at 9:37 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > > On Wednesday 24 February 2016 18:11:19 Alexandre Courbot wrote: > >> On T210, the sdhci controller can address more than 32 bits of address > >> space. Failing to express this fact results in the use of bounce > >> buffers and affects performance. > >> > >> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx> > >> --- > >> I am pretty sure this one is wrong in some way, but just to get the ball > >> rolling as the use of bounce buffers is currently quite heavy on Jetson TX1. > >> > >> Thierry, Stephen, could you confirm that I got the DMA masks correctly? I > >> am not sure about the actual addressable size on TX1, and also suspect TK1 can > >> also address more than 32 bits. > >> > >> Also, I noticed that sdhci_host has a dma_mask member which I thought would do > >> the trick but actually doesn't seem to be used for anything useful. Could the > >> MMC maintainers comment on this and let me know if the DMA mask setting should > >> be moved at the core level instead of being done per-driver? > > > > So the question is what the DMA capabilities of the sdhci device are. > > > > Usually I think SDHCI should just support a 64-bit mask, and you can > > request that in the driver, but the platform might reject it, e.g. > > if the parent bus is lacking a dma-ranges property. > > > > On 32-bit platforms with no RAM above the 4GB boundary, setting a 64-bit > > mask typically succeeds, because there is no harm in using it. > > > > You should only set a 34-bit mask in the specific case that: > > > > * SDHCI reports that it supports 64-bit addressing > > * The parent bus supports 64-bit addressing and correctly sets up > > its dma-ranges property > > * The device is connected incorrectly to the parent bus and > > any access above 0x400000000ull fail to end up in the correct > > memory for this particular device, but not other devices on the > > same bus. > > Thanks for the clarification. The hardware is definitely capable of > > 32 bits addressing (how much more exactly, I still have to figure out > as Stephen pointed out). Parent bus is the platform bus (root node in > the DT) so I'm not too sure how the dma-ranges property should be used > in that case (as of today we have no dma-ranges property at all in > Tegra DTs). I meant the actual physical bus. AXI buses that are typically used can have either 32-bit or 64-bit addressing, but IIRC they wouldn't have anything inbetween based on the way this protocol works (i.e. not an actual set of address wires, but a data packet getting sent around on a point to point link). > We could maybe specify it at the root level (I see at > least one arm device doing that), but then we need to make sure every > platform device supports the same range. I think there is still a bug in the code that will lead to dma_set_mask just succeeding, regardless of what the bus supports, and this should really really be fixed. I think it will just keep working for platforms that accidentally don't set the dma-ranges property, but it may be slower as we fall back to swiotlb. > Actually even if we specify a dma-ranges on the parent DT node, the > DMA range will still be limited to 32 bits because of the following > code in of_dma_configure(): > > /* > * Set default coherent_dma_mask to 32 bit. Drivers are expected to > * setup the correct supported mask. > */ > if (!dev->coherent_dma_mask) > dev->coherent_dma_mask = DMA_BIT_MASK(32); > > /* > * Set it to coherent_dma_mask by default if the architecture > * code has not set it. > */ > if (!dev->dma_mask) > dev->dma_mask = &dev->coherent_dma_mask; > > .... > /* gets dma-ranges into dma_addr and size */ > .... > > > *dev->dma_mask = min((*dev->dma_mask), > DMA_BIT_MASK(ilog2(dma_addr + size))); > > So unless the DMA mask is set on the device before of_dma_configure() > is called, the min() statement will choose the 32 bits mask that has > been previously set. So IIUC in any case, the driver will need to call > dma_set_mask() Yes, the driver definitely has to call dma_set_mask(), the property of the parent bus is used to make that fail when the bus doesn't support it. > Can I have your thoughts on this? Am I missing something? One point: I think the dma_set_mask() probably should be in the generic part of the sdhci driver, not the tegra specific portion. I also forget how this really needs to interact with swiotlb. I know we have discussed this a couple of times, but the result currently is lost to me. Maybe the answer was that if swiotlb or iommu are enabled, then dma_set_mask() should always succeed, but the mask should not actually be updated? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
