Re: [PATCH 8/8] etnaviv: remove IOMMU dependency

[Wladimir J. van der Laan]

On Fri, Sep 15, 2017 at 07:04:39PM +0200, Lucas Stach wrote:
> Using the IOMMU API to manage the internal GPU MMU has been an
> historical accident and it keeps getting in the way, as well as
> entangling the driver with the inner workings of the IOMMU
> subsystem.
> 
> Clean this up by removing the usage of iommu_domain, which is the
> last piece linking etnaviv to the IOMMU subsystem.
> 
> Signed-off-by: Lucas Stach 

Reviewed-by: Wladimir J. van der Laan 

> ---
>  drivers/gpu/drm/etnaviv/Kconfig|  2 -
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h  |  1 -
>  drivers/gpu/drm/etnaviv/etnaviv_iommu.c| 71 
> +++---
>  drivers/gpu/drm/etnaviv/etnaviv_iommu.h|  6 ++-
>  drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c | 55 +++
>  drivers/gpu/drm/etnaviv/etnaviv_mmu.c  | 38 +++-
>  drivers/gpu/drm/etnaviv/etnaviv_mmu.h  | 20 ++---
>  7 files changed, 93 insertions(+), 100 deletions(-)
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 8/8] etnaviv: remove IOMMU dependency

[Philipp Zabel]

On Fri, 2017-09-15 at 19:04 +0200, Lucas Stach wrote:
> Using the IOMMU API to manage the internal GPU MMU has been an
> historical accident and it keeps getting in the way, as well as
> entangling the driver with the inner workings of the IOMMU
> subsystem.
> 
> Clean this up by removing the usage of iommu_domain, which is the
> last piece linking etnaviv to the IOMMU subsystem.
> 
> Signed-off-by: Lucas Stach 
> ---
>  drivers/gpu/drm/etnaviv/Kconfig|  2 -
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h  |  1 -
>  drivers/gpu/drm/etnaviv/etnaviv_iommu.c| 71 +++
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_iommu.h|  6 ++-
>  drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c | 55 +++
> 
>  drivers/gpu/drm/etnaviv/etnaviv_mmu.c  | 38 +++-
>  drivers/gpu/drm/etnaviv/etnaviv_mmu.h  | 20 ++---
>  7 files changed, 93 insertions(+), 100 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/Kconfig
> b/drivers/gpu/drm/etnaviv/Kconfig
> index 71cee4e9fefb..b0a4ee27eb05 100644
> --- a/drivers/gpu/drm/etnaviv/Kconfig
> +++ b/drivers/gpu/drm/etnaviv/Kconfig
> @@ -7,8 +7,6 @@ config DRM_ETNAVIV
>   select SHMEM
>   select SYNC_FILE
>   select TMPFS
> - select IOMMU_API
> - select IOMMU_SUPPORT
>   select WANT_DEV_COREDUMP
>   help
>     DRM driver for Vivante GPUs.
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index 058389f93b69..d157d9379e68 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -26,7 +26,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
> index 43a0508bdbd7..f0390842441a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
> @@ -14,7 +14,6 @@
>   * this program.  If not, see .
>   */
>  
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -36,20 +35,21 @@ struct etnaviv_iommu_domain_pgtable {

Should this be renamed to etnaviv_iommuv2_domain_pgtable for
consistency?

>   dma_addr_t paddr;
>  };
>  
> -struct etnaviv_iommu_domain {
> - struct iommu_domain domain;
> +struct etnaviv_iommuv1_domain {
> + struct etnaviv_iommu_domain base;
>   struct device *dev;
>   void *bad_page_cpu;
>   dma_addr_t bad_page_dma;

Now that base is etnaviv specific, these three could be moved up into
etnaviv_iommu_domain.

>   struct etnaviv_iommu_domain_pgtable pgtable;
>  };
>  
> -static struct etnaviv_iommu_domain *to_etnaviv_domain(struct
> iommu_domain *domain)
> +static struct etnaviv_iommuv1_domain *
> +to_etnaviv_domain(struct etnaviv_iommu_domain *domain)
>  {
> - return container_of(domain, struct etnaviv_iommu_domain,
> domain);
> + return container_of(domain, struct etnaviv_iommuv1_domain,
> base);
>  }
>  
> -static int __etnaviv_iommu_init(struct etnaviv_iommu_domain
> *etnaviv_domain)
> +static int __etnaviv_iommu_init(struct etnaviv_iommuv1_domain
> *etnaviv_domain)
>  {
>   u32 *p;
>   int ret, i;
> @@ -83,9 +83,10 @@ static int __etnaviv_iommu_init(struct
> etnaviv_iommu_domain *etnaviv_domain)
>   return 0;
>  }
>  
> -static void etnaviv_domain_free(struct iommu_domain *domain)
> +static void etnaviv_iommuv1_domain_free(struct etnaviv_iommu_domain
> *domain)
>  {
> - struct etnaviv_iommu_domain *etnaviv_domain =
> to_etnaviv_domain(domain);
> + struct etnaviv_iommuv1_domain *etnaviv_domain =
> + to_etnaviv_domain(domain);
>  
>   dma_free_coherent(etnaviv_domain->dev, PT_SIZE,
>     etnaviv_domain->pgtable.pgtable,
> @@ -98,10 +99,11 @@ static void etnaviv_domain_free(struct
> iommu_domain *domain)
>   kfree(etnaviv_domain);
>  }
>  
> -static int etnaviv_iommuv1_map(struct iommu_domain *domain, unsigned
> long iova,
> -    phys_addr_t paddr, size_t size, int prot)
> +static int etnaviv_iommuv1_map(struct etnaviv_iommu_domain *domain,
> +    unsigned long iova, phys_addr_t paddr,
> +    size_t size, int prot)
>  {
> - struct etnaviv_iommu_domain *etnaviv_domain =
> to_etnaviv_domain(domain);
> + struct etnaviv_iommuv1_domain *etnaviv_domain =
> to_etnaviv_domain(domain);
>   unsigned int index = (iova - GPU_MEM_START) / SZ_4K;
>  
>   if (size != SZ_4K)
> @@ -112,10 +114,11 @@ static int etnaviv_iommuv1_map(struct
> iommu_domain *domain, unsigned long iova,
>   return 0;
>  }
>  
> -static size_t etnaviv_iommuv1_unmap(struct iommu_domain *domain,
> +static size_t etnaviv_iommuv1_unmap(struct etnaviv_iommu_domain
> *domain,
>   unsigned long iova, size_t size)
>  {
> - struct etnaviv_iommu_domain *etnaviv_domain =
> to_etnaviv_domain(domain);
> + struct 

[PATCH 8/8] etnaviv: remove IOMMU dependency

[Lucas Stach]

Using the IOMMU API to manage the internal GPU MMU has been an
historical accident and it keeps getting in the way, as well as
entangling the driver with the inner workings of the IOMMU
subsystem.

Clean this up by removing the usage of iommu_domain, which is the
last piece linking etnaviv to the IOMMU subsystem.

Signed-off-by: Lucas Stach 
---
 drivers/gpu/drm/etnaviv/Kconfig|  2 -
 drivers/gpu/drm/etnaviv/etnaviv_drv.h  |  1 -
 drivers/gpu/drm/etnaviv/etnaviv_iommu.c| 71 +++---
 drivers/gpu/drm/etnaviv/etnaviv_iommu.h|  6 ++-
 drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c | 55 +++
 drivers/gpu/drm/etnaviv/etnaviv_mmu.c  | 38 +++-
 drivers/gpu/drm/etnaviv/etnaviv_mmu.h  | 20 ++---
 7 files changed, 93 insertions(+), 100 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
index 71cee4e9fefb..b0a4ee27eb05 100644
--- a/drivers/gpu/drm/etnaviv/Kconfig
+++ b/drivers/gpu/drm/etnaviv/Kconfig
@@ -7,8 +7,6 @@ config DRM_ETNAVIV
select SHMEM
select SYNC_FILE
select TMPFS
-   select IOMMU_API
-   select IOMMU_SUPPORT
select WANT_DEV_COREDUMP
help
  DRM driver for Vivante GPUs.
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h 
b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 058389f93b69..d157d9379e68 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -26,7 +26,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c 
b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
index 43a0508bdbd7..f0390842441a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
@@ -14,7 +14,6 @@
  * this program.  If not, see .
  */
 
-#include 
 #include 
 #include 
 #include 
@@ -36,20 +35,21 @@ struct etnaviv_iommu_domain_pgtable {
dma_addr_t paddr;
 };
 
-struct etnaviv_iommu_domain {
-   struct iommu_domain domain;
+struct etnaviv_iommuv1_domain {
+   struct etnaviv_iommu_domain base;
struct device *dev;
void *bad_page_cpu;
dma_addr_t bad_page_dma;
struct etnaviv_iommu_domain_pgtable pgtable;
 };
 
-static struct etnaviv_iommu_domain *to_etnaviv_domain(struct iommu_domain 
*domain)
+static struct etnaviv_iommuv1_domain *
+to_etnaviv_domain(struct etnaviv_iommu_domain *domain)
 {
-   return container_of(domain, struct etnaviv_iommu_domain, domain);
+   return container_of(domain, struct etnaviv_iommuv1_domain, base);
 }
 
-static int __etnaviv_iommu_init(struct etnaviv_iommu_domain *etnaviv_domain)
+static int __etnaviv_iommu_init(struct etnaviv_iommuv1_domain *etnaviv_domain)
 {
u32 *p;
int ret, i;
@@ -83,9 +83,10 @@ static int __etnaviv_iommu_init(struct etnaviv_iommu_domain 
*etnaviv_domain)
return 0;
 }
 
-static void etnaviv_domain_free(struct iommu_domain *domain)
+static void etnaviv_iommuv1_domain_free(struct etnaviv_iommu_domain *domain)
 {
-   struct etnaviv_iommu_domain *etnaviv_domain = to_etnaviv_domain(domain);
+   struct etnaviv_iommuv1_domain *etnaviv_domain =
+   to_etnaviv_domain(domain);
 
dma_free_coherent(etnaviv_domain->dev, PT_SIZE,
  etnaviv_domain->pgtable.pgtable,
@@ -98,10 +99,11 @@ static void etnaviv_domain_free(struct iommu_domain *domain)
kfree(etnaviv_domain);
 }
 
-static int etnaviv_iommuv1_map(struct iommu_domain *domain, unsigned long iova,
-  phys_addr_t paddr, size_t size, int prot)
+static int etnaviv_iommuv1_map(struct etnaviv_iommu_domain *domain,
+  unsigned long iova, phys_addr_t paddr,
+  size_t size, int prot)
 {
-   struct etnaviv_iommu_domain *etnaviv_domain = to_etnaviv_domain(domain);
+   struct etnaviv_iommuv1_domain *etnaviv_domain = 
to_etnaviv_domain(domain);
unsigned int index = (iova - GPU_MEM_START) / SZ_4K;
 
if (size != SZ_4K)
@@ -112,10 +114,11 @@ static int etnaviv_iommuv1_map(struct iommu_domain 
*domain, unsigned long iova,
return 0;
 }
 
-static size_t etnaviv_iommuv1_unmap(struct iommu_domain *domain,
+static size_t etnaviv_iommuv1_unmap(struct etnaviv_iommu_domain *domain,
unsigned long iova, size_t size)
 {
-   struct etnaviv_iommu_domain *etnaviv_domain = to_etnaviv_domain(domain);
+   struct etnaviv_iommuv1_domain *etnaviv_domain =
+   to_etnaviv_domain(domain);
unsigned int index = (iova - GPU_MEM_START) / SZ_4K;
 
if (size != SZ_4K)
@@ -126,32 +129,22 @@ static size_t etnaviv_iommuv1_unmap(struct iommu_domain 
*domain,
return SZ_4K;
 }
 
-static size_t etnaviv_iommuv1_dump_size(struct iommu_domain *domain)
+static size_t etnaviv_iommuv1_dump_size(struct etnaviv_iommu_domain *domain)
 {