On 2023-04-10 19:52, Dmitry Baryshkov wrote:
If the Adreno SMMU is dma-coherent, allocation will fail unless we
disable IO_PGTABLE_QUIRK_ARM_OUTER_WBWA. Skip setting this quirk for the
coherent SMMUs (like we have on sm8350 platform).
Hmm, but is it right that it should fail in the first place? The fact is
that if the SMMU is coherent then walks *will* be outer-WBWA, so I
honestly can't see why the io-pgtable code is going out of its way to
explicitly reject a request to give them the same attribute it's already
giving then anyway :/
Even if the original intent was for the quirk to have an over-specific
implication of representing inner-NC as well, that hardly seems useful
if what we've ended up with in practice is a nonsensical-looking check
in one place and then a weird hacky bodge in another purely to work
around it.
Does anyone know a good reason why this is the way it is?
[ just came across this code in the tree while trying to figure out what
to do with iommu_set_pgtable_quirks()... ]
Thanks,
Robin.
Fixes: 54af0ceb7595 ("arm64: dts: qcom: sm8350: add GPU, GMU, GPU CC and SMMU nodes")
Reported-by: David Heidelberg <david@xxxxxxx>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 2942d2548ce6..f74495dcbd96 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1793,7 +1793,8 @@ a6xx_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev)
* This allows GPU to set the bus attributes required to use system
* cache on behalf of the iommu page table walker.
*/
- if (!IS_ERR_OR_NULL(a6xx_gpu->htw_llc_slice))
+ if (!IS_ERR_OR_NULL(a6xx_gpu->htw_llc_slice) &&
+ !device_iommu_capable(&pdev->dev, IOMMU_CAP_CACHE_COHERENCY))
quirks |= IO_PGTABLE_QUIRK_ARM_OUTER_WBWA;
return adreno_iommu_create_address_space(gpu, pdev, quirks);
