On Thu, Aug 27, 2020 at 10:05:14AM +0300, Dmitry Osipenko wrote: > 24.08.2020 17:01, Robin Murphy пишет: > ... > >> Robin, thank you very much for the clarifications! > >> > >> In accordance to yours comments, this patch can't be applied until Tegra > >> SMMU will support IOMMU_DOMAIN_IDENTITY and implement def_domain_type() > >> callback that returns IOMMU_DOMAIN_IDENTITY for the VDE device. > >> > >> Otherwise you're breaking the VDE driver because > >> dma_buf_map_attachment() [1] returns the IOMMU SGT of the implicit > >> domain which is then mapped into the VDE's explicit domain [2], and this > >> is a nonsense. > > > > It's true that iommu_dma_ops will do some work in the unattached default > > domain, but non-coherent cache maintenance will still be performed > > correctly on the underlying memory, which is really all that you care > > about for this case. As for tegra_vde_iommu_map(), that seems to do the > > right thing in only referencing the physical side of the scatterlist > > (via iommu_map_sg()) and ignoring the DMA side, so things ought to work > > out OK even if it is a little non-obvious. > > I'll need to double-check this, it's indeed not clear to me right now. > > I see that if Tegra DRM driver uses implicit IOMMU domain, then when VDE > driver imports DMA-buf from Terga DRM and the imported buffer will be > auto-mapped to the implicit VDE IOVA [1]. > > [1] > https://elixir.bootlin.com/linux/v5.9-rc2/source/drivers/gpu/drm/tegra/gem.c#L574 > > >> Hence, either VDE driver should bypass iommu_dma_ops from the start or > >> it needs a way to kick out the ops, like it does this using ARM's > >> arm_iommu_detach_device(). > >> > >> > >> The same applies to the Tegra GPU devices, otherwise you're breaking > >> them as well because Tegra DRM is sensible to implicit vs explicit > >> domain. > > > > Note that Tegra DRM will only be as broken as its current state on > > arm64, and I was under the impression that that was OK now - at least I > > don't recall seeing any complaints since 43c5bf11a610. Although that > > commit and the one before it are resolving the scalability issue that > > they describe, it was very much in my mind at the time that they also > > have the happy side-effect described above - the default domain isn't > > *completely* out of the way, but it's far enough that sensible cases > > should be able to work as expected. > > The Tegra DRM has a very special quirk for ARM32 that was added in this > commit [2] and driver relies on checking of whether explicit or implicit > IOMMU is used in order to activate the quirk. > > [2] > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=273da5a046965ccf0ec79eb63f2d5173467e20fa > > Once the implicit IOMMU is used for the DRM driver, the quirk no longer > works (if I'm not missing something). This problem needs to be resolved > before implicit IOMMU could be used by the Tegra DRM on ARM32. > > >> BTW, I tried to apply this series and T30 doesn't boot anymore. I don't > >> have more info for now. > > > > Yeah, I'm still trying to get to the bottom of whether it's actually > > working as intended at all, even on my RK3288. So far my debugging > > instrumentation has been confusingly inconclusive :/ > > Surely it will take some time to resolve all the problems and it's great > that you're pushing this work! > > I'll try to help with fixing the ARM32 Tegra side of the problems. I > added this to my "TODO" list and should be able to take a closer look > during of this/next weeks! I do have a patch lying around somewhere that implements the mapping cache that was referenced in the above commit. Let me know if I should dig that up and send it out. Thierry
Attachment:
signature.asc
Description: PGP signature
