Thread (50 messages) 50 messages, 12 authors, 2020-09-28

Re: [PATCH 16/18] staging/media/tegra-vde: Clean up IOMMU workaround

From: Dmitry Osipenko <digetx@gmail.com>
Date: 2020-08-27 07:05:58
Also in: dri-devel, linux-arm-msm, linux-iommu, linux-media, linux-mediatek, linux-samsung-soc, linux-tegra, lkml

24.08.2020 17:01, Robin Murphy пишет:
...
quoted
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
quoted
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.
quoted
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!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help