Re: Tegra-DRM: Tegra30: DMA mapping API (was: About dma mapping apis)

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

On Mon, 2012-07-09 at 23:28 +0800, Hiroshi Doyu wrote:
> Hi Mark, adding linux-tegra on Cc:.... 
> 
> Mark Zhang <markz-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote @ Mon, 9 Jul 2012 14:13:13 +0200:
> 
> > > Hi Mark, Cc: Thierry & Ken,
> > >
> > > Can GEM allow DRM to use contigious memory? That could be easier.
> > >
> > > Otherwise, I think that you need to make drv->dev iommu'able before
> > > calling DMA API if you want to use IOMMU. To make drv->dev iommu'able,
> > > you need to call arm_iommu_create_mapping() and arm_iommu_attach_device().
> > 
> > Thank you. Yes, we need to call these functions otherwise iommu will not be enabled at all.
> > So my question is:
> > 1. in arm_iommu_create_mapping, we need to specify the base address
> > of the iova. Is these an API which can get this base address? I
> > mean, SMMU serves a lot of clients and maybe every client will
> > specify a range of iova which it needs. So as the center point of
> > these clients, does SMMU module has a function which can be used by
> > client to get this base address?
> 
> The SMMU patch was sent but it depends on the following framework.
> 
>   [PATCH 0/5] IOMMU: Make IOMMU-API ready for GART-like hardware
>     https://lkml.org/lkml/2012/1/19/170
> 

Got it. Seems this patch hasn't been integrated yet. Besides, SMMU
module should do some implementations for this set/get attribute
functions as well, right?

> > 2. I took a quick glance of tegra-smmu.c, I noticed that there is a
> > dts property named dma-window, which it's value is: <0x0
> > 0x40000000>. Does this means the iova which SMMU handles is from 0 -
> > 1G? IIRC, SMMU handles 4G address space...
> 
> It tries to avoid overwrapping 1-1 iovirt-phys mapping, which is
> necessary for some drivers right now.
> 

Sorry I didn't get it. What is overwrapping 1-1 iovirt-phys mapping?

> > 3. Right now in my drm driver, arm_iommu_attach_device failed with
> > an error code: -22(Invalid Argument). I think this is caused by
> > these codes in function smmu_iommu_attach_dev:
> > 
> > map = (unsigned long)dev->platform_data;
> >         if (!map)
> >                 return -EINVAL;
> > 
> > So, does this mean the drm_device should have a "platform_data"(right now it is NULL so -EINVAL returned)? 
> > I skimmed the codes of tegra-smmu.c, seems this "platform_data" has specific usage and should be composed in some rules.
> > Could you explain a little bit about this?
> 
> You can skip that check with the following patch.
> 

Yes, it works. Right now the framebuffer works on my T30.
Thank you!

> From 42a03ff88ed133db613389d2cc4c4ad33cd5d7e3 Mon Sep 17 00:00:00 2001
> From: Hiroshi DOYU <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Date: Tue, 28 Feb 2012 15:33:17 +0200
> Subject: [PATCH 1/1] iommu/tegra: smmu: Enable all SWGRP by default
> 
> This should be revisited along with successors, which would
> introduce different configurations.
> 
> Signed-off-by: Hiroshi DOYU <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/tegra-smmu.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index bf33c03..3bc5c91 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -38,6 +38,8 @@
>  #include <mach/smmu.h>
>  #include <mach/tegra_smmu.h>
>  
> +#define SKIP_SWGRP_CHECK /* FIXME: SWGRP should be passed from DT. */
> +
>  /* bitmap of the page sizes currently supported */
>  #define SMMU_IOMMU_PGSIZES	(SZ_4K)
>  
> @@ -316,11 +318,15 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
>  		offs = HWGRP_ASID_REG(i);
>  		val = smmu_read(smmu, offs);
>  		if (on) {
> +#if !defined(SKIP_SWGRP_CHECK)
>  			if (WARN_ON(val & mask))
>  				goto err_hw_busy;
> +#endif
>  			val |= mask;
>  		} else {
> +#if !defined(SKIP_SWGRP_CHECK)
>  			WARN_ON((val & mask) == mask);
> +#endif
>  			val &= ~mask;
>  		}
>  		smmu_write(smmu, val, offs);
> @@ -701,9 +707,15 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
>  		return -ENOMEM;
>  	client->dev = dev;
>  	client->as = as;
> +
> +#ifdef SKIP_SWGRP_CHECK
> +	/* Enable all SWGRP blindly by default */
> +	map = (1 << HWGRP_COUNT) - 1;
> +#else
>  	map = (unsigned long)dev->platform_data;
>  	if (!map)
>  		return -EINVAL;
> +#endif
>  
>  	err = smmu_client_enable_hwgrp(client, map);
>  	if (err)