On Fri, Nov 29, 2019 at 11:33:45AM +0100, Thierry Reding wrote:
On Fri, Nov 29, 2019 at 10:12:44AM +0100, Daniel Vetter wrote:
On Thu, Nov 28, 2019 at 04:37:35PM +0100, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
It's not known at import time whether or not all users of a DMA-BUF will be able to deal with non-contiguous memory. Each user needs to verify at map-time whether it can access the buffer.
Signed-off-by: Thierry Reding treding@nvidia.com
I'm not seeing any other check for nents ... does this mean that there's not actually any block that requires contig mem?
All the blocks require contiguous memory. However, they are all behind an IOMMU and in practice will always end up mapping the buffers through the IOMMU. Techically this check should now be in tegra_dc_pin(), which is called by the ->prepare_fb() callback. I didn't add it because there are no practical use-cases where this happens, although I guess you could come up with a kernel and DTB combination where this is actually possible by jumping through some hoops.
This fix here is to make Tegra DRM interoperation with Nouveau work again since that's currently broken after moving to the IOMMU-backed DMA API as an alternative to explicit IOMMU usage. With explicit IOMMU usage (that's the if corresponding to the else removed below) the IOMMU domain was shared between the display controllers at the driver level, so it was fine to make this determination in the else branch because this was the case where no IOMMU was in play. After the move to the DMA API, this else branch is also taken when the DMA API is backed by an IOMMU and at it is unfortunately not known at import time which display controller ends up scanning out the DMA BUF, nor if that display controller is behind an IOMMU. We only know that when the actual mapping takes place, so we'd need to look at sgt->nents after dma_map_sg() in in tegra_dc_pin().
I'll add that check there, just in case anyone manages to conjure up such a configuration.
Imo just paste the above explanation into the commit message and
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Or also add the check, but imo the explanation here is the important part. -Daniel
Thierry
-Daniel
drivers/gpu/drm/tegra/gem.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c index 6dfad56eee2b..bc15b430156d 100644 --- a/drivers/gpu/drm/tegra/gem.c +++ b/drivers/gpu/drm/tegra/gem.c @@ -440,13 +440,6 @@ static struct tegra_bo *tegra_bo_import(struct drm_device *drm, err = tegra_bo_iommu_map(tegra, bo); if (err < 0) goto detach;
} else {
if (bo->sgt->nents > 1) {err = -EINVAL;goto detach;}bo->iova = sg_dma_address(bo->sgt->sgl);}
bo->gem.import_attach = attach;
-- 2.23.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch