On Tue, Nov 27, 2018 at 03:58:13PM -0800, Jeykumar Sankaran wrote: > Specify geometry for DPU iommu domain which sets > the address space for gem allocations. > > Signed-off-by: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx> > Suggested-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> > Suggested-by: Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx> There is a lot of confusion surrounding all of this, so let me spend a bit of time to explain. Like other platform devices with a iommu attached the mdss and gpu devices have a default DMA IOMMU domain attached to them at enumeration time. The intent is that if a device uses the DMA API it hides the mechanics of iova management. However in the case of the GPU (and by extension the display) we *DO* want to manage our own IOMMU space. This is partially for historical reasons (we started doing so in the era just before the DMA API became a thing) and also for future reasons. Eventually we will want to be able to do per-process pagetables for the GPU and for that we need manual control of the iova domain. So long story shortx, we don't want to be involved with the DMA domain. In fact, we would GREATLY prefer if we didn't have it, but thats much wider reaching discussion that doesn't make sense here (but if you want to get involved in the fight let me know). In the place of the dma domain we have the internal concept of address spaces which manages iova allocation for a specific IOMMU domain. Today this is either the mdss or the gpu domain (because we use different iommus) but in the future there will be a new address space for each process for per-process pagetables. In the upstream code we reuse the domain geometry struct to set up the range for the address space allocator which is why this patch is needed because the dpu needs to provide that clue to the address space. That said, some folks are getting this patch confused with the patch that Vivek sent out [1] (drm: msm: Replace dma_map_sg with dma_sync_sg). A pre-release iteration of this patch did an additional thing - instead of creating a new iommu domain it re-used the DMA domain and registered it with the msm iommu engine which meant that we started crossing the streams and mapping the GEM buffers manually onto the DMA space. This is bad. In fact it started to conflict with the DMA domain which lead directly to Vivek's patch. This isn't to say that Vivek's patch isn't appropriate - we knew all along we were abusing the DMA API for fun and profit, and we might as well abuse dma_sync_sg instead of dma_map_sg. But it isn't strictly needed if we create a new domain and not reuse the DMA domain. Hopefully this clarifies things. If not, hit me up in IRC and I'll try to explain it again. In the meantime, this patch is correct and useful so: Acked-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> [1] https://patchwork.freedesktop.org/patch/263914/ > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 7d931ae..efceddf1 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -823,6 +823,9 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms) > if (!domain) > return 0; > > + domain->geometry.aperture_start = 0x1000; > + domain->geometry.aperture_end = 0xffffffff; > + > aspace = msm_gem_address_space_create(dpu_kms->dev->dev, > domain, "dpu1"); > if (IS_ERR(aspace)) { -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
