diff mbox

Gen2 IPMMU and DU prototype

Message ID 20150930142242.25755.81946.sendpatchset@little-apple (mailing list archive)
State RFC
Headers show

Commit Message

Magnus Damm Sept. 30, 2015, 2:22 p.m. UTC 
From: Magnus Damm <damm+renesas@opensource.se>

Here is a simple hack to enable the IPMMU on R-Car Gen2.

The VGA port on r8a7790 Lager may be driven by the DU
and the IPMMU via the modetest utility to output some
test image.

Actually the r8a7790 DU device is connected to the
IPMMU using two separate uTLBs, but the DU driver
is currently supporting all DU instances using a
single device. To make testing easily the r8a7790 DU
driver code is with this patch hacked to just cover
a single DU instance with a single IPMMU uTLB.

The map/umap debug printk() will show you if the IPMMU
is used or not. Another way could be to XOR the physical
address in the map routine to adjust the test image.

Several sources have told me that the DU on Lager is not
working as expected. With this hack (with or without IPMMU)
at least the VGA output seems fine. So it would be good
to look into HDMI and LVDS as well as unmodified VGA
support for the DU on the Lager board.

Also, we need to figure out how to modify the DU driver
to support multiple uTLBs.

Not for upstream merge.

Not-Yet-Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 arch/arm/boot/dts/r8a7790.dtsi        |    3 ++-
 arch/arm/boot/dts/r8a7791.dtsi        |    3 ++-
 arch/arm/mm/dma-mapping.c             |    4 ++--
 drivers/gpu/drm/rcar-du/rcar_du_drv.c |    6 +++---
 drivers/iommu/ipmmu-vmsa.c            |    8 ++++++--
 drivers/of/device.c                   |    6 +++---
 6 files changed, 18 insertions(+), 12 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Laurent Pinchart Oct. 1, 2015, 2:30 p.m. UTC | #1 
Hi Magnus,

Thank you for the patch.

On Wednesday 30 September 2015 23:22:42 Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Here is a simple hack to enable the IPMMU on R-Car Gen2.
> 
> The VGA port on r8a7790 Lager may be driven by the DU
> and the IPMMU via the modetest utility to output some
> test image.
> 
> Actually the r8a7790 DU device is connected to the
> IPMMU using two separate uTLBs, but the DU driver
> is currently supporting all DU instances using a
> single device. To make testing easily the r8a7790 DU
> driver code is with this patch hacked to just cover
> a single DU instance with a single IPMMU uTLB.

You can reference multiple uTLBs in the DT ipmmus property. The ipmmu-vmsa 
driver currently requires all uTLBs to belong to the same IPMMU instance, 
which shouldn't be an issue here.

> The map/umap debug printk() will show you if the IPMMU
> is used or not. Another way could be to XOR the physical
> address in the map routine to adjust the test image.
> 
> Several sources have told me that the DU on Lager is not
> working as expected. With this hack (with or without IPMMU)
> at least the VGA output seems fine. So it would be good
> to look into HDMI and LVDS as well as unmodified VGA
> support for the DU on the Lager board.
> 
> Also, we need to figure out how to modify the DU driver
> to support multiple uTLBs.
> 
> Not for upstream merge.
> 
> Not-Yet-Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  arch/arm/boot/dts/r8a7790.dtsi        |    3 ++-
>  arch/arm/boot/dts/r8a7791.dtsi        |    3 ++-
>  arch/arm/mm/dma-mapping.c             |    4 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c |    6 +++---
>  drivers/iommu/ipmmu-vmsa.c            |    8 ++++++--
>  drivers/of/device.c                   |    6 +++---
>  6 files changed, 18 insertions(+), 12 deletions(-)
> 
> --- 0001/arch/arm/boot/dts/r8a7790.dtsi
> +++ work/arch/arm/boot/dts/r8a7790.dtsi	2015-09-29 15:04:37.540513000 +0900
> @@ -882,6 +882,7 @@
>  			 <&mstp7_clks R8A7790_CLK_LVDS0>,
>  			 <&mstp7_clks R8A7790_CLK_LVDS1>;
>  		clock-names = "du.0", "du.1", "du.2", "lvds.0", "lvds.1";
> +		iommus = <&ipmmu_mx 15>;
>  		status = "disabled";
> 
>  		ports {
> @@ -1805,7 +1806,7 @@
>  		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>,
>  			     <0 221 IRQ_TYPE_LEVEL_HIGH>;
>  		#iommu-cells = <1>;
> -		status = "disabled";
> +		status = "okay";
>  	};
> 
>  	ipmmu_rt: mmu@ffc80000 {
> --- 0001/arch/arm/boot/dts/r8a7791.dtsi
> +++ work/arch/arm/boot/dts/r8a7791.dtsi	2015-09-29 14:15:26.610513000 +0900
> @@ -919,6 +919,7 @@
>  			 <&mstp7_clks R8A7791_CLK_DU1>,
>  			 <&mstp7_clks R8A7791_CLK_LVDS0>;
>  		clock-names = "du.0", "du.1", "lvds.0";
> +		iommus = <&ipmmu_mx 15>;
>  		status = "disabled";
> 
>  		ports {
> @@ -1618,7 +1619,7 @@
>  		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>,
>  			     <0 221 IRQ_TYPE_LEVEL_HIGH>;
>  		#iommu-cells = <1>;
> -		status = "disabled";
> +		status = "okay";
>  	};
> 
>  	ipmmu_rt: mmu@ffc80000 {
> --- 0001/arch/arm/mm/dma-mapping.c
> +++ work/arch/arm/mm/dma-mapping.c	2015-09-29 14:58:51.150513000 +0900
> @@ -1998,7 +1998,7 @@ static int __arm_iommu_attach_device(str
>  	kref_get(&mapping->kref);
>  	to_dma_iommu_mapping(dev) = mapping;
> 
> -	pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
> +	pr_info("Attached IOMMU controller to %s device.\n", dev_name(dev));
>  	return 0;
>  }
> 
> @@ -2043,7 +2043,7 @@ static void __arm_iommu_detach_device(st
>  	kref_put(&mapping->kref, release_iommu_mapping);
>  	to_dma_iommu_mapping(dev) = NULL;
> 
> -	pr_debug("Detached IOMMU controller from %s device.\n", dev_name(dev));
> +	pr_info("Detached IOMMU controller from %s device.\n", dev_name(dev));
>  }
> 
>  /**
> --- 0001/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ work/drivers/gpu/drm/rcar-du/rcar_du_drv.c	2015-09-29 
15:05:31.520513000
> +0900 @@ -60,13 +60,13 @@ static const struct rcar_du_device_info
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> 
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
> 
>  	.quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES,
> -	.num_crtcs = 3,
> +	.num_crtcs = 2,
>  	.routes = {
>  		/* R8A7790 has one RGB output, two LVDS outputs and one
>  		 * (currently unsupported) TCON output.
>  		 */
>  		[RCAR_DU_OUTPUT_DPAD0] = {
> -			.possible_crtcs = BIT(2) | BIT(1) | BIT(0),
> +			.possible_crtcs = BIT(1) | BIT(0),
>  			.encoder_type = DRM_MODE_ENCODER_NONE,
>  			.port = 0,
>  		},
> @@ -76,7 +76,7 @@ static const struct rcar_du_device_info
>  			.port = 1,
>  		},
>  		[RCAR_DU_OUTPUT_LVDS1] = {
> -			.possible_crtcs = BIT(2) | BIT(1),
> +			.possible_crtcs = BIT(1),
>  			.encoder_type = DRM_MODE_ENCODER_LVDS,
>  			.port = 2,
>  		},
> --- 0001/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c	2015-09-29 15:12:31.610513000 +0900
> @@ -7,7 +7,7 @@
>   * it under the terms of the GNU General Public License as published by
>   * the Free Software Foundation; version 2 of the License.
>   */
> -
> +#define DEBUG
>  #include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/err.h>
> @@ -544,9 +544,11 @@ static int ipmmu_map(struct iommu_domain
>  {
>  	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
> 
> +	printk("xxx map 0x%08lx <-> %pad %zu\n", iova, &paddr, size);
> +
>  	if (!domain)
>  		return -ENODEV;
> -
> +
>  	return domain->iop->map(domain->iop, iova, paddr, size, prot);
>  }
> 
> @@ -555,6 +557,8 @@ static size_t ipmmu_unmap(struct iommu_d
>  {
>  	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
> 
> +	printk("xxx unmap 0x%08lx %zu\n", iova, size);
> +
>  	return domain->iop->unmap(domain->iop, iova, size);
>  }
> 
> --- 0001/drivers/of/device.c
> +++ work/drivers/of/device.c	2015-09-29 15:00:10.680513000 +0900
> @@ -124,7 +124,7 @@ void of_dma_configure(struct device *dev
>  			dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
>  			return;
>  		}
> -		dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> +		dev_info(dev, "dma_pfn_offset(%#08lx)\n", offset);
>  	}
> 
>  	dev->dma_pfn_offset = offset;
> @@ -139,11 +139,11 @@ void of_dma_configure(struct device *dev
>  			     DMA_BIT_MASK(ilog2(dma_addr + size)));
> 
>  	coherent = of_dma_is_coherent(np);
> -	dev_dbg(dev, "device is%sdma coherent\n",
> +	dev_info(dev, "device is%sdma coherent\n",
>  		coherent ? " " : " not ");
> 
>  	iommu = of_iommu_configure(dev, np);
> -	dev_dbg(dev, "device is%sbehind an iommu\n",
> +	dev_info(dev, "device is%sbehind an iommu\n",
>  		iommu ? " " : " not ");
> 
>  	arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
Magnus Damm Oct. 2, 2015, 3:12 a.m. UTC | #2 
Hi Laurent,

On Thu, Oct 1, 2015 at 11:30 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.

Thanks for feedback!!

> On Wednesday 30 September 2015 23:22:42 Magnus Damm wrote:
>> From: Magnus Damm <damm+renesas@opensource.se>
>>
>> Here is a simple hack to enable the IPMMU on R-Car Gen2.
>>
>> The VGA port on r8a7790 Lager may be driven by the DU
>> and the IPMMU via the modetest utility to output some
>> test image.
>>
>> Actually the r8a7790 DU device is connected to the
>> IPMMU using two separate uTLBs, but the DU driver
>> is currently supporting all DU instances using a
>> single device. To make testing easily the r8a7790 DU
>> driver code is with this patch hacked to just cover
>> a single DU instance with a single IPMMU uTLB.
>
> You can reference multiple uTLBs in the DT ipmmus property. The ipmmu-vmsa
> driver currently requires all uTLBs to belong to the same IPMMU instance,
> which shouldn't be an issue here.

Sure, from a DT property point of view it seems possible to assign
multiple uTLBs to the DU node. I would like us to start integrating
the IPMMU on Gen2 and passing multiple uTLBs in case of r8a7790 must
be the right way to describe the hardware.

When it comes to the DU driver and memory management, from my point of
view Gen2 also seems to pass a single device pointer passed to
dma_alloc_writecombine(). I may be wrong, but to support multiple
uTLBs it looks like we need to pass different device pointers to
dma_alloc_writecombine() - unless some index is also passed how can
the IPMMU code otherwise know which uTLB to use?

So it looks like the device pointer issue both affects Gen2 and Gen3.
Of course I wish that wasn't the case, so please prove me wrong. =)

Cheers,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart Oct. 2, 2015, 8:13 a.m. UTC | #3 
Hi Magnus,

On Friday 02 October 2015 12:12:22 Magnus Damm wrote:
> On Thu, Oct 1, 2015 at 11:30 PM, Laurent Pinchart wrote:
> > On Wednesday 30 September 2015 23:22:42 Magnus Damm wrote:
> >> From: Magnus Damm <damm+renesas@opensource.se>
> >> 
> >> Here is a simple hack to enable the IPMMU on R-Car Gen2.
> >> 
> >> The VGA port on r8a7790 Lager may be driven by the DU
> >> and the IPMMU via the modetest utility to output some
> >> test image.
> >> 
> >> Actually the r8a7790 DU device is connected to the
> >> IPMMU using two separate uTLBs, but the DU driver
> >> is currently supporting all DU instances using a
> >> single device. To make testing easily the r8a7790 DU
> >> driver code is with this patch hacked to just cover
> >> a single DU instance with a single IPMMU uTLB.
> > 
> > You can reference multiple uTLBs in the DT ipmmus property. The ipmmu-vmsa
> > driver currently requires all uTLBs to belong to the same IPMMU instance,
> > which shouldn't be an issue here.
> 
> Sure, from a DT property point of view it seems possible to assign
> multiple uTLBs to the DU node. I would like us to start integrating
> the IPMMU on Gen2 and passing multiple uTLBs in case of r8a7790 must
> be the right way to describe the hardware.
> 
> When it comes to the DU driver and memory management, from my point of
> view Gen2 also seems to pass a single device pointer passed to
> dma_alloc_writecombine(). I may be wrong, but to support multiple
> uTLBs it looks like we need to pass different device pointers to
> dma_alloc_writecombine() - unless some index is also passed how can
> the IPMMU code otherwise know which uTLB to use?
> 
> So it looks like the device pointer issue both affects Gen2 and Gen3.
> Of course I wish that wasn't the case, so please prove me wrong. =)

The IPMMU driver currently enables all uTLBs associated with a bus master 
device. As the driver uses a single page table for all uTLBs we shouldn't have 
any issue on Gen2. Problems will of course appear if/when we start using 
multiple page tables for a single IPMMU instance.
diff mbox

Patch

--- 0001/arch/arm/boot/dts/r8a7790.dtsi
+++ work/arch/arm/boot/dts/r8a7790.dtsi	2015-09-29 15:04:37.540513000 +0900
@@ -882,6 +882,7 @@ 
 			 <&mstp7_clks R8A7790_CLK_LVDS0>,
 			 <&mstp7_clks R8A7790_CLK_LVDS1>;
 		clock-names = "du.0", "du.1", "du.2", "lvds.0", "lvds.1";
+		iommus = <&ipmmu_mx 15>;
 		status = "disabled";
 
 		ports {
@@ -1805,7 +1806,7 @@ 
 		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>,
 			     <0 221 IRQ_TYPE_LEVEL_HIGH>;
 		#iommu-cells = <1>;
-		status = "disabled";
+		status = "okay";
 	};
 
 	ipmmu_rt: mmu@ffc80000 {
--- 0001/arch/arm/boot/dts/r8a7791.dtsi
+++ work/arch/arm/boot/dts/r8a7791.dtsi	2015-09-29 14:15:26.610513000 +0900
@@ -919,6 +919,7 @@ 
 			 <&mstp7_clks R8A7791_CLK_DU1>,
 			 <&mstp7_clks R8A7791_CLK_LVDS0>;
 		clock-names = "du.0", "du.1", "lvds.0";
+		iommus = <&ipmmu_mx 15>;
 		status = "disabled";
 
 		ports {
@@ -1618,7 +1619,7 @@ 
 		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>,
 			     <0 221 IRQ_TYPE_LEVEL_HIGH>;
 		#iommu-cells = <1>;
-		status = "disabled";
+		status = "okay";
 	};
 
 	ipmmu_rt: mmu@ffc80000 {
--- 0001/arch/arm/mm/dma-mapping.c
+++ work/arch/arm/mm/dma-mapping.c	2015-09-29 14:58:51.150513000 +0900
@@ -1998,7 +1998,7 @@  static int __arm_iommu_attach_device(str
 	kref_get(&mapping->kref);
 	to_dma_iommu_mapping(dev) = mapping;
 
-	pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
+	pr_info("Attached IOMMU controller to %s device.\n", dev_name(dev));
 	return 0;
 }
 
@@ -2043,7 +2043,7 @@  static void __arm_iommu_detach_device(st
 	kref_put(&mapping->kref, release_iommu_mapping);
 	to_dma_iommu_mapping(dev) = NULL;
 
-	pr_debug("Detached IOMMU controller from %s device.\n", dev_name(dev));
+	pr_info("Detached IOMMU controller from %s device.\n", dev_name(dev));
 }
 
 /**
--- 0001/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ work/drivers/gpu/drm/rcar-du/rcar_du_drv.c	2015-09-29 15:05:31.520513000 +0900
@@ -60,13 +60,13 @@  static const struct rcar_du_device_info
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
 	.quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES,
-	.num_crtcs = 3,
+	.num_crtcs = 2,
 	.routes = {
 		/* R8A7790 has one RGB output, two LVDS outputs and one
 		 * (currently unsupported) TCON output.
 		 */
 		[RCAR_DU_OUTPUT_DPAD0] = {
-			.possible_crtcs = BIT(2) | BIT(1) | BIT(0),
+			.possible_crtcs = BIT(1) | BIT(0),
 			.encoder_type = DRM_MODE_ENCODER_NONE,
 			.port = 0,
 		},
@@ -76,7 +76,7 @@  static const struct rcar_du_device_info
 			.port = 1,
 		},
 		[RCAR_DU_OUTPUT_LVDS1] = {
-			.possible_crtcs = BIT(2) | BIT(1),
+			.possible_crtcs = BIT(1),
 			.encoder_type = DRM_MODE_ENCODER_LVDS,
 			.port = 2,
 		},
--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2015-09-29 15:12:31.610513000 +0900
@@ -7,7 +7,7 @@ 
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; version 2 of the License.
  */
-
+#define DEBUG
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
@@ -544,9 +544,11 @@  static int ipmmu_map(struct iommu_domain
 {
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
 
+	printk("xxx map 0x%08lx <-> %pad %zu\n", iova, &paddr, size);
+
 	if (!domain)
 		return -ENODEV;
-
+	
 	return domain->iop->map(domain->iop, iova, paddr, size, prot);
 }
 
@@ -555,6 +557,8 @@  static size_t ipmmu_unmap(struct iommu_d
 {
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
 
+	printk("xxx unmap 0x%08lx %zu\n", iova, size);
+
 	return domain->iop->unmap(domain->iop, iova, size);
 }
 
--- 0001/drivers/of/device.c
+++ work/drivers/of/device.c	2015-09-29 15:00:10.680513000 +0900
@@ -124,7 +124,7 @@  void of_dma_configure(struct device *dev
 			dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
 			return;
 		}
-		dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
+		dev_info(dev, "dma_pfn_offset(%#08lx)\n", offset);
 	}
 
 	dev->dma_pfn_offset = offset;
@@ -139,11 +139,11 @@  void of_dma_configure(struct device *dev
 			     DMA_BIT_MASK(ilog2(dma_addr + size)));
 
 	coherent = of_dma_is_coherent(np);
-	dev_dbg(dev, "device is%sdma coherent\n",
+	dev_info(dev, "device is%sdma coherent\n",
 		coherent ? " " : " not ");
 
 	iommu = of_iommu_configure(dev, np);
-	dev_dbg(dev, "device is%sbehind an iommu\n",
+	dev_info(dev, "device is%sbehind an iommu\n",
 		iommu ? " " : " not ");
 
 	arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);