Re: Tegra-DRM: Tegra30: DMA mapping API

[markz <markz-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]

On Thu, 2012-07-12 at 15:11 +0800, Hiroshi Doyu wrote:
> On Thu, 12 Jul 2012 08:49:05 +0200
> markz <markz-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> .....
> > > > After Joerg's patch and yours are merged into linux-next, I can submit
> > > > my drm patch into review process.
> > > 
> > > You can also send them now for early review with some note mentioning
> > > that your patches depend on the above "[PATCH 6/7] iommu/tegra:
> > > Implement DOMAIN_ATTR_GEOMETRY attribute" in the commit log.
> > 
> > Thank you. I merged this patch set locally but seems it has problems:
> > The requirement of the drm driver is: It needs a parameter named "base"
> > while calling function "arm_iommu_create_mapping" to reserve a block of
> > iova from GART/SMMU.
> > This patch set defines function "iommu_domain_get_attr" &
> > "iommu_domain_set_attr", but drm driver can't use them:
> > 1. These 2 functions need a iommu_domain param which means the
> > iommu_domain should be created before calling them. But the iommu_domain
> > is created in function "arm_iommu_create_mapping".
> > 2. It's not a good practice to make drm driver calls iommu apis
> > directly.
> > 
> > Actually, I'm curious that why function "arm_iommu_create_mapping" needs
> > this "base" param. AIUI, drm driver just needs to tell IOMMU the size of
> > the iova address to reserve, then the base address can be checked out in
> > a lot of ways. For example: 
> > 1. Via "dma_map_sg" interface -- dma_map_sg defines we can check out the
> > mapped iova address from "dma_address" member in scatterlist structure.
> > 2. Use this iommu get/set attr interface. Of course, we need some dma
> > mapping api wrappers on them.
> 
> arm_iommu_attach_device() should be called independently from DRM
> driver in advance. In DRM driver, "struct device" should be connected
> to map. Usually this can be done in board files, but with DT, we need
> new framework to deal with this at once. Especially in Tegra, we have
> many IOMMU'able devices, and it's not good idea to do respectively.
> 

arm_iommu_attach_device() needs "struct dma_iommu_mapping *mapping"
which is the return value of arm_iommu_create_mapping(). So we should
call arm_iommu_create_mapping in advance.
"In DRM driver, "struct device" should be connected to map". What does
this "map" means? How "struct device" connects to map? Why we need this?

> For temporary solution, you can call arm_iommu_create_mapping() and
> arm_iommu_attach_device() in advance before DRM uses DMA API.

Yes, this is what I do right now.