[PATCH/RFC] ARM: shmobile: LPAE memory bank CMA assignment prototype

[PATCH/RFC] ARM: shmobile: LPAE memory bank CMA assignment prototype

[Magnus Damm]

From: Magnus Damm <damm+renesas@opensource.se>

This prototype patch extends the kernel to also reserve CMA memory
in the top memory bank on R-Car Gen2 boards and ties this larger
CMA area to the DU device for testing purpose.

This top portion of the memory requires 40-bits addressing support
in bus master devices including LPAE for the ARM CPU cores.

The patch assigns a 512 MiB CMA area to the DU device that may be
used with the IPMMU hardware to perform 40-bit bus master access.
Without IPMMU the DU hardware only supports 32-bit addresses.

Tested on r8a7791 Koelsch HDMI output using the modetest utility:
# modetest -M rcar-du -s 33:1024x768@AR24

Not for upstream merge.

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

 arch/arm/mach-shmobile/setup-rcar-gen2.c |   33 ++++++++-
 drivers/staging/board/Makefile           |    1 
 drivers/staging/board/rcar-gen2.c        |  104 ++++++++++++++++++++++++++++++
 3 files changed, 134 insertions(+), 4 deletions(-)

--- 0001/arch/arm/mach-shmobile/setup-rcar-gen2.c
+++ work/arch/arm/mach-shmobile/setup-rcar-gen2.c	2015-12-19 09:38:09.750513000 +0900
@@ -136,14 +136,14 @@ struct memory_reserve_config {
 	u64 base, size;
 };
 
-static int __init rcar_gen2_scan_mem(unsigned long node, const char *uname,
-				     int depth, void *data)
+static int __init __rcar_gen2_scan_mem(unsigned long node, const char *uname,
+				     int depth, void *data,
+				     u64 lpae_start)
 {
 	const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
 	const __be32 *reg, *endp;
 	int l;
 	struct memory_reserve_config *mrc = data;
-	u64 lpae_start = 1ULL << 32;
 
 	/* We are scanning "memory" nodes only */
 	if (type = NULL || strcmp(type, "memory"))
@@ -182,6 +182,20 @@ static int __init rcar_gen2_scan_mem(uns
 	return 0;
 }
 
+static int __init rcar_gen2_scan_mem_legacy(unsigned long node,
+					    const char *uname,
+					    int depth, void *data)
+{
+	return __rcar_gen2_scan_mem(node, uname, depth, data, 1ULL << 32);
+}
+
+static int __init rcar_gen2_scan_mem_lpae(unsigned long node,
+					  const char *uname,
+					  int depth, void *data)
+{
+	return __rcar_gen2_scan_mem(node, uname, depth, data, 1ULL << 40);
+}
+
 struct cma *rcar_gen2_dma_contiguous;
 
 void __init rcar_gen2_reserve(void)
@@ -192,7 +206,18 @@ void __init rcar_gen2_reserve(void)
 	memset(&mrc, 0, sizeof(mrc));
 	mrc.reserved = SZ_256M;
 
-	of_scan_flat_dt(rcar_gen2_scan_mem, &mrc);
+	of_scan_flat_dt(rcar_gen2_scan_mem_legacy, &mrc);
+#ifdef CONFIG_DMA_CMA
+	if (mrc.size && memblock_is_region_memory(mrc.base, mrc.size))
+		dma_contiguous_reserve_area(mrc.size, mrc.base, 0,
+					    &rcar_gen2_dma_contiguous, true);
+#endif
+
+	/* reserve 512 MiB at the top of the 40-bit memory space */
+	memset(&mrc, 0, sizeof(mrc));
+	mrc.reserved = SZ_512M;
+
+	of_scan_flat_dt(rcar_gen2_scan_mem_lpae, &mrc);
 #ifdef CONFIG_DMA_CMA
 	if (mrc.size && memblock_is_region_memory(mrc.base, mrc.size))
 		dma_contiguous_reserve_area(mrc.size, mrc.base, 0,
--- 0001/drivers/staging/board/Makefile
+++ work/drivers/staging/board/Makefile	2015-12-18 17:46:31.030513000 +0900
@@ -1,3 +1,4 @@
 obj-y	:= board.o
 obj-$(CONFIG_ARCH_EMEV2)	+= kzm9d.o
 obj-$(CONFIG_ARCH_R8A7740)	+= armadillo800eva.o
+obj-$(CONFIG_ARCH_RCAR_GEN2)	+= rcar-gen2.o
--- /dev/null
+++ work/drivers/staging/board/rcar-gen2.c	2015-12-19 10:07:35.820513000 +0900
@@ -0,0 +1,104 @@
+/* Staging board support for R-Car Gen2. Enable not-yet-DT-capable bits here. */
+
+#include <linux/kernel.h>
+#include <linux/cma.h>
+#include <linux/dma-contiguous.h>
+#include <linux/errno.h>
+#include <linux/notifier.h>
+#include <linux/platform_device.h>
+#include "board.h"
+#include "../../../mm/cma.h"
+
+#if defined(CONFIG_CMA) && defined(CONFIG_DMA_CMA)
+
+static struct cma *largest_cma_area;
+
+static int cma_assign_bus_notifier(struct notifier_block *nb,
+			      unsigned long action, void *data)
+{
+	struct device *dev = data;
+	const struct device_node *node = dev->of_node;
+
+	if (action = BUS_NOTIFY_BIND_DRIVER) {
+		if (of_device_is_compatible(node, "renesas,du-r8a7790") ||
+		    of_device_is_compatible(node, "renesas,du-r8a7791") ||
+		    of_device_is_compatible(node, "renesas,du-r8a7793") ||
+		    of_device_is_compatible(node, "renesas,du-r8a7794")) {
+
+			pr_info("Board Staging: Assigning CMA to %s\n",
+				of_node_full_name(node));
+			dev_set_cma_area(dev, largest_cma_area);
+		}
+  	}
+
+	return 0;
+}
+
+static struct notifier_block cma_assign_nb = {
+	.notifier_call = cma_assign_bus_notifier,
+};
+
+struct cma *find_largest_nondefault_cma(void)
+{
+	unsigned long largest_size;
+	int k, largest_idx;
+
+	largest_size = 0;
+	largest_idx = -ENOENT;
+
+	for (k = 0; k < cma_area_count; k++) {
+		if (&cma_areas[k] = dma_contiguous_default_area)
+			continue;
+
+		if (cma_get_size(&cma_areas[k]) > largest_size) {
+			largest_size = cma_get_size(&cma_areas[k]);
+			largest_idx = k;
+		}
+	}
+
+	if (largest_idx != -ENOENT)
+		return &cma_areas[largest_idx];
+
+	return NULL;
+}
+
+static void __init board_staging_init(void)
+{
+	struct cma *target;
+	phys_addr_t cma_base;
+
+	target = find_largest_nondefault_cma();
+
+	if (target) {
+		cma_base = cma_get_base(target);
+
+		pr_info("Board Staging: Located CMA at "
+			"%pa, size %ld MiB\n", &cma_base,
+			(unsigned long)cma_get_size(target) / SZ_1M);
+
+		largest_cma_area = target;
+		bus_register_notifier(&platform_bus_type, &cma_assign_nb);
+	}
+}
+#else
+static inline void __init board_staging_init(void) {}
+#endif
+
+static int __init runtime_board_check(void)
+{
+	if (of_machine_is_compatible("renesas,r8a7790"))
+	       board_staging_init();
+
+	if (of_machine_is_compatible("renesas,r8a7791"))
+	       board_staging_init();
+
+	if (of_machine_is_compatible("renesas,r8a7793"))
+	       board_staging_init();
+
+	if (of_machine_is_compatible("renesas,r8a7794"))
+	       board_staging_init();
+
+	return 0;
+}
+
+arch_initcall(runtime_board_check)

Re: [PATCH/RFC] ARM: shmobile: LPAE memory bank CMA assignment prototype

[Laurent Pinchart]

Hi Magnus,

Thank you for the patch.

On Saturday 19 December 2015 10:40:05 Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> This prototype patch extends the kernel to also reserve CMA memory
> in the top memory bank on R-Car Gen2 boards and ties this larger
> CMA area to the DU device for testing purpose.
> 
> This top portion of the memory requires 40-bits addressing support
> in bus master devices including LPAE for the ARM CPU cores.
> 
> The patch assigns a 512 MiB CMA area to the DU device that may be
> used with the IPMMU hardware to perform 40-bit bus master access.
> Without IPMMU the DU hardware only supports 32-bit addresses.
> 
> Tested on r8a7791 Koelsch HDMI output using the modetest utility:
> # modetest -M rcar-du -s 33:1024x768@AR24
> 
> Not for upstream merge.

I tried to understand where the setup-rcar-gen2.c code you're patching came 
from. Due to a rebase the commit message of 83850b04ae77 ("ARM: shmobile: 
rcar-gen2: Update for of_get_flat_dt_prop() update") is incorrect, but I've 
traced the original commit to ae8bf91c80b0 ("ARM: shmobile: Add shared R-Car 
Gen2 CMA reservation code") in Simon's tree.

That patch doesn't look like a very good approach to me, and neither does this 
one :-) drivers/staging/board is a hack, and we're starting to abuse it. I 
don't want to see board code coming back through the back door. What's wrong 
with just reserving memory in DT with the reserved-memory bindings and 
assigning it to the DU ?

> Not-Yet-Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  arch/arm/mach-shmobile/setup-rcar-gen2.c |   33 ++++++++-
>  drivers/staging/board/Makefile           |    1
>  drivers/staging/board/rcar-gen2.c        |  104 +++++++++++++++++++++++++++
>  3 files changed, 134 insertions(+), 4 deletions(-)
> 
> --- 0001/arch/arm/mach-shmobile/setup-rcar-gen2.c
> +++ work/arch/arm/mach-shmobile/setup-rcar-gen2.c
> 2015-12-19 09:38:09.750513000 +0900
> @@ -136,14 +136,14 @@ struct memory_reserve_config
> {
>  	u64 base, size;
>  };
> 
> -static int __init rcar_gen2_scan_mem(unsigned long node, const char *uname,
> -				     int depth, void *data)
> +static int __init __rcar_gen2_scan_mem(unsigned long node, const char
> *uname,
> +				     int depth, void *data,
> +				     u64 lpae_start)
>  {
>  	const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
>  	const __be32 *reg, *endp;
>  	int l;
>  	struct memory_reserve_config *mrc = data;
> -	u64 lpae_start = 1ULL << 32;
> 
>  	/* We are scanning "memory" nodes only */
>  	if (type = NULL || strcmp(type, "memory"))
> @@ -182,6 +182,20 @@ static int __init rcar_gen2_scan_mem(uns
>  	return 0;
>  }
> 
> +static int __init rcar_gen2_scan_mem_legacy(unsigned long node,
> +					    const char *uname,
> +					    int depth, void *data)
> +{
> +	return __rcar_gen2_scan_mem(node, uname, depth, data, 1ULL << 32);
> +}
> +
> +static int __init rcar_gen2_scan_mem_lpae(unsigned long node,
> +					  const char *uname,
> +					  int depth, void *data)
> +{
> +	return __rcar_gen2_scan_mem(node, uname, depth, data, 1ULL << 40);
> +}
> +
>  struct cma *rcar_gen2_dma_contiguous;
> 
>  void __init rcar_gen2_reserve(void)
> @@ -192,7 +206,18 @@ void __init rcar_gen2_reserve(void)
>  	memset(&mrc, 0, sizeof(mrc));
>  	mrc.reserved = SZ_256M;
> 
> -	of_scan_flat_dt(rcar_gen2_scan_mem, &mrc);
> +	of_scan_flat_dt(rcar_gen2_scan_mem_legacy, &mrc);
> +#ifdef CONFIG_DMA_CMA
> +	if (mrc.size && memblock_is_region_memory(mrc.base, mrc.size))
> +		dma_contiguous_reserve_area(mrc.size, mrc.base, 0,
> +					    &rcar_gen2_dma_contiguous, true);
> +#endif
> +
> +	/* reserve 512 MiB at the top of the 40-bit memory space */
> +	memset(&mrc, 0, sizeof(mrc));
> +	mrc.reserved = SZ_512M;
> +
> +	of_scan_flat_dt(rcar_gen2_scan_mem_lpae, &mrc);
>  #ifdef CONFIG_DMA_CMA
>  	if (mrc.size && memblock_is_region_memory(mrc.base, mrc.size))
>  		dma_contiguous_reserve_area(mrc.size, mrc.base, 0,
> --- 0001/drivers/staging/board/Makefile
> +++ work/drivers/staging/board/Makefile	2015-12-18 17:46:31.030513000 +0900
> @@ -1,3 +1,4 @@
>  obj-y	:= board.o
>  obj-$(CONFIG_ARCH_EMEV2)	+= kzm9d.o
>  obj-$(CONFIG_ARCH_R8A7740)	+= armadillo800eva.o
> +obj-$(CONFIG_ARCH_RCAR_GEN2)	+= rcar-gen2.o
> --- /dev/null
> +++ work/drivers/staging/board/rcar-gen2.c	2015-12-19 10:07:35.820513000
> +0900 @@ -0,0 +1,104 @@
> +/* Staging board support for R-Car Gen2. Enable not-yet-DT-capable bits
> here. */ +
> +#include <linux/kernel.h>
> +#include <linux/cma.h>
> +#include <linux/dma-contiguous.h>
> +#include <linux/errno.h>
> +#include <linux/notifier.h>
> +#include <linux/platform_device.h>
> +#include "board.h"
> +#include "../../../mm/cma.h"
> +
> +#if defined(CONFIG_CMA) && defined(CONFIG_DMA_CMA)
> +
> +static struct cma *largest_cma_area;
> +
> +static int cma_assign_bus_notifier(struct notifier_block *nb,
> +			      unsigned long action, void *data)
> +{
> +	struct device *dev = data;
> +	const struct device_node *node = dev->of_node;
> +
> +	if (action = BUS_NOTIFY_BIND_DRIVER) {
> +		if (of_device_is_compatible(node, "renesas,du-r8a7790") ||
> +		    of_device_is_compatible(node, "renesas,du-r8a7791") ||
> +		    of_device_is_compatible(node, "renesas,du-r8a7793") ||
> +		    of_device_is_compatible(node, "renesas,du-r8a7794")) {
> +
> +			pr_info("Board Staging: Assigning CMA to %s\n",
> +				of_node_full_name(node));
> +			dev_set_cma_area(dev, largest_cma_area);
> +		}
> +  	}
> +
> +	return 0;
> +}
> +
> +static struct notifier_block cma_assign_nb = {
> +	.notifier_call = cma_assign_bus_notifier,
> +};
> +
> +struct cma *find_largest_nondefault_cma(void)
> +{
> +	unsigned long largest_size;
> +	int k, largest_idx;
> +
> +	largest_size = 0;
> +	largest_idx = -ENOENT;
> +
> +	for (k = 0; k < cma_area_count; k++) {
> +		if (&cma_areas[k] = dma_contiguous_default_area)
> +			continue;
> +
> +		if (cma_get_size(&cma_areas[k]) > largest_size) {
> +			largest_size = cma_get_size(&cma_areas[k]);
> +			largest_idx = k;
> +		}
> +	}
> +
> +	if (largest_idx != -ENOENT)
> +		return &cma_areas[largest_idx];
> +
> +	return NULL;
> +}
> +
> +static void __init board_staging_init(void)
> +{
> +	struct cma *target;
> +	phys_addr_t cma_base;
> +
> +	target = find_largest_nondefault_cma();
> +
> +	if (target) {
> +		cma_base = cma_get_base(target);
> +
> +		pr_info("Board Staging: Located CMA at "
> +			"%pa, size %ld MiB\n", &cma_base,
> +			(unsigned long)cma_get_size(target) / SZ_1M);
> +
> +		largest_cma_area = target;
> +		bus_register_notifier(&platform_bus_type, &cma_assign_nb);
> +	}
> +}
> +#else
> +static inline void __init board_staging_init(void) {}
> +#endif
> +
> +static int __init runtime_board_check(void)
> +{
> +	if (of_machine_is_compatible("renesas,r8a7790"))
> +	       board_staging_init();
> +
> +	if (of_machine_is_compatible("renesas,r8a7791"))
> +	       board_staging_init();
> +
> +	if (of_machine_is_compatible("renesas,r8a7793"))
> +	       board_staging_init();
> +
> +	if (of_machine_is_compatible("renesas,r8a7794"))
> +	       board_staging_init();
> +
> +	return 0;
> +}
> +
> +arch_initcall(runtime_board_check)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC] ARM: shmobile: LPAE memory bank CMA assignment prototype

[Magnus Damm]

Hi Laurent,

On Sun, Dec 27, 2015 at 5:51 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.
>
> On Saturday 19 December 2015 10:40:05 Magnus Damm wrote:
>> From: Magnus Damm <damm+renesas@opensource.se>
>>
>> This prototype patch extends the kernel to also reserve CMA memory
>> in the top memory bank on R-Car Gen2 boards and ties this larger
>> CMA area to the DU device for testing purpose.
>>
>> This top portion of the memory requires 40-bits addressing support
>> in bus master devices including LPAE for the ARM CPU cores.
>>
>> The patch assigns a 512 MiB CMA area to the DU device that may be
>> used with the IPMMU hardware to perform 40-bit bus master access.
>> Without IPMMU the DU hardware only supports 32-bit addresses.
>>
>> Tested on r8a7791 Koelsch HDMI output using the modetest utility:
>> # modetest -M rcar-du -s 33:1024x768@AR24
>>
>> Not for upstream merge.
>
> I tried to understand where the setup-rcar-gen2.c code you're patching came
> from. Due to a rebase the commit message of 83850b04ae77 ("ARM: shmobile:
> rcar-gen2: Update for of_get_flat_dt_prop() update") is incorrect, but I've
> traced the original commit to ae8bf91c80b0 ("ARM: shmobile: Add shared R-Car
> Gen2 CMA reservation code") in Simon's tree.

Right, I recall adding that CMA reservation code.

> That patch doesn't look like a very good approach to me, and neither does this
> one :-) drivers/staging/board is a hack, and we're starting to abuse it.

At the time the Gen2 CMA reservation code was written there was no DT
memory reservation support available.

Regarding this patch, it is just a proof of concept to test allocation
from a high memory address without modifying any hardware description.

> I
> don't want to see board code coming back through the back door. What's wrong
> with just reserving memory in DT with the reserved-memory bindings and
> assigning it to the DU ?

Describing device-to-memory bank assignment in DT equals mixing
software policy with hardware description. I prefer to keep the
software policy in C and the hardware description in DT.

If you think there are better ways to reserve memory, why don't you
cook up a counter proposal and post it in a public space? =)

Thanks,

/ magnus

Re: [PATCH/RFC] ARM: shmobile: LPAE memory bank CMA assignment prototype

[Laurent Pinchart]

Hi Magnus,

On Monday 28 December 2015 12:17:15 Magnus Damm wrote:
> On Sun, Dec 27, 2015 at 5:51 PM, Laurent Pinchart wrote:
> > On Saturday 19 December 2015 10:40:05 Magnus Damm wrote:
> >> From: Magnus Damm <damm+renesas@opensource.se>
> >> 
> >> This prototype patch extends the kernel to also reserve CMA memory
> >> in the top memory bank on R-Car Gen2 boards and ties this larger
> >> CMA area to the DU device for testing purpose.
> >> 
> >> This top portion of the memory requires 40-bits addressing support
> >> in bus master devices including LPAE for the ARM CPU cores.
> >> 
> >> The patch assigns a 512 MiB CMA area to the DU device that may be
> >> used with the IPMMU hardware to perform 40-bit bus master access.
> >> Without IPMMU the DU hardware only supports 32-bit addresses.
> >> 
> >> Tested on r8a7791 Koelsch HDMI output using the modetest utility:
> >> # modetest -M rcar-du -s 33:1024x768@AR24
> >> 
> >> Not for upstream merge.
> > 
> > I tried to understand where the setup-rcar-gen2.c code you're patching
> > came from. Due to a rebase the commit message of 83850b04ae77 ("ARM:
> > shmobile: rcar-gen2: Update for of_get_flat_dt_prop() update") is
> > incorrect, but I've traced the original commit to ae8bf91c80b0 ("ARM:
> > shmobile: Add shared R-Car Gen2 CMA reservation code") in Simon's tree.
> 
> Right, I recall adding that CMA reservation code.
> 
> > That patch doesn't look like a very good approach to me, and neither does
> > this one :-) drivers/staging/board is a hack, and we're starting to abuse
> > it.
>
> At the time the Gen2 CMA reservation code was written there was no DT
> memory reservation support available.

Good point.

> Regarding this patch, it is just a proof of concept to test allocation
> from a high memory address without modifying any hardware description.
> 
> > I don't want to see board code coming back through the back door. What's
> > wrong with just reserving memory in DT with the reserved-memory bindings
> > and assigning it to the DU ?
> 
> Describing device-to-memory bank assignment in DT equals mixing
> software policy with hardware description. I prefer to keep the
> software policy in C and the hardware description in DT.
> 
> If you think there are better ways to reserve memory, why don't you
> cook up a counter proposal and post it in a public space? =)

Doing it in DT is the better way in my opinion :-) There are established DT 
bindings for that purpose, and that's what upstream is using.

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC] ARM: shmobile: LPAE memory bank CMA assignment prototype

[Geert Uytterhoeven]

On Mon, Dec 28, 2015 at 11:10 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> Regarding this patch, it is just a proof of concept to test allocation
>> from a high memory address without modifying any hardware description.
>>
>> > I don't want to see board code coming back through the back door. What's
>> > wrong with just reserving memory in DT with the reserved-memory bindings
>> > and assigning it to the DU ?
>>
>> Describing device-to-memory bank assignment in DT equals mixing
>> software policy with hardware description. I prefer to keep the
>> software policy in C and the hardware description in DT.
>>
>> If you think there are better ways to reserve memory, why don't you
>> cook up a counter proposal and post it in a public space? =)
>
> Doing it in DT is the better way in my opinion :-) There are established DT
> bindings for that purpose, and that's what upstream is using.

I'm a bit sceptical about describing this in DT, too, as this is a software
policy, not a hardware description, but it's indeed described in
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH/RFC] ARM: shmobile: LPAE memory bank CMA assignment prototype

[Laurent Pinchart]

Hi Geert,

On Monday 28 December 2015 11:21:08 Geert Uytterhoeven wrote:
> On Mon, Dec 28, 2015 at 11:10 AM, Laurent Pinchart wrote:
> >> Regarding this patch, it is just a proof of concept to test allocation
> >> from a high memory address without modifying any hardware description.
> >> 
> >> > I don't want to see board code coming back through the back door.
> >> > What's wrong with just reserving memory in DT with the reserved-memory
> >> > bindings and assigning it to the DU ?
> >> 
> >> Describing device-to-memory bank assignment in DT equals mixing
> >> software policy with hardware description. I prefer to keep the
> >> software policy in C and the hardware description in DT.
> >> 
> >> If you think there are better ways to reserve memory, why don't you
> >> cook up a counter proposal and post it in a public space? =)
> > 
> > Doing it in DT is the better way in my opinion :-) There are established
> > DT bindings for that purpose, and that's what upstream is using.
> 
> I'm a bit sceptical about describing this in DT, too, as this is a software
> policy, not a hardware description, but it's indeed described in
> Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

The idea of a DT-like kernel configuration file has been toyed with in the 
past but as far as I know it got abandoned. It was certainly an interesting 
idea, and could possibly have led to a better solution, but today what we have 
is DT. Board files are not the way to go to implement such a feature.

-- 
Regards,

Laurent Pinchart