* [PATCH 0/2] Update reserved memory for simple framebuffer
@ 2020-02-25 5:10 ` Michael Trimarchi
0 siblings, 0 replies; 13+ messages in thread
From: Michael Trimarchi @ 2020-02-25 5:10 UTC (permalink / raw)
To: u-boot
Reserved memory for simple frame buffer should be created in a different
way:
+ aliases {
+ display0 = &lcdif;
+ };
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ display_reserved: framebuffer at 86fa2000 {
+ reg = <0x86fa2000 0x80000>;
+ no-map;
+ };
+
+ };
We add a function to change the loaded dts and inject those information.
I have added another patch for meson. Right now I'm testing on tinker-s
board.
Michael Trimarchi (2):
common: fdt: Add a function for reserving memory without kernel linear
mapping
video: meson: Use reserving memory function without kernel linear
mapping
common/fdt_support.c | 40 +++++++++++++++++++++++++++++++++
drivers/video/meson/meson_vpu.c | 6 ++---
include/fdt_support.h | 11 +++++++++
3 files changed, 54 insertions(+), 3 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/2] Update reserved memory for simple framebuffer
@ 2020-02-25 5:10 ` Michael Trimarchi
0 siblings, 0 replies; 13+ messages in thread
From: Michael Trimarchi @ 2020-02-25 5:10 UTC (permalink / raw)
To: Simon Glass, Anatolij Gustschin, Neil Armstrong; +Cc: u-boot, u-boot-amlogic
Reserved memory for simple frame buffer should be created in a different
way:
+ aliases {
+ display0 = &lcdif;
+ };
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ display_reserved: framebuffer@86fa2000 {
+ reg = <0x86fa2000 0x80000>;
+ no-map;
+ };
+
+ };
We add a function to change the loaded dts and inject those information.
I have added another patch for meson. Right now I'm testing on tinker-s
board.
Michael Trimarchi (2):
common: fdt: Add a function for reserving memory without kernel linear
mapping
video: meson: Use reserving memory function without kernel linear
mapping
common/fdt_support.c | 40 +++++++++++++++++++++++++++++++++
drivers/video/meson/meson_vpu.c | 6 ++---
include/fdt_support.h | 11 +++++++++
3 files changed, 54 insertions(+), 3 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] common: fdt: Add a function for reserving memory without kernel linear mapping
2020-02-25 5:10 ` Michael Trimarchi
@ 2020-02-25 5:10 ` Michael Trimarchi
-1 siblings, 0 replies; 13+ messages in thread
From: Michael Trimarchi @ 2020-02-25 5:10 UTC (permalink / raw)
To: u-boot
The intent is to reserve memory _and_ prevent it from being included
in the kernel's linear map. For thos reason it is also necessary to include the
'no-map' property for this reserved-mem node.
From Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt:
no-map (optional) - empty property
- Indicates the operating system must not create a virtual mapping
of the region as part of its standard mapping of system memory,
nor permit speculative access to it under any circumstances other
than under the control of the device driver using the region.
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
Changes: RFC->v1
- Add a better commit message
---
common/fdt_support.c | 40 ++++++++++++++++++++++++++++++++++++++++
include/fdt_support.h | 11 +++++++++++
2 files changed, 51 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 02cf5c6241..a3662f4358 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -410,6 +410,46 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
return p - (char *)buf;
}
+int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[])
+{
+ int offs, len;
+ const char *subpath;
+ const char *path = "/reserved-memory";
+ fdt32_t address_cells = cpu_to_fdt32(fdt_address_cells(blob, 0));
+ fdt32_t size_cells = cpu_to_fdt32(fdt_size_cells(blob, 0));
+ u8 temp[16]; /* Up to 64-bit address + 64-bit size */
+
+ offs = fdt_path_offset(blob, path);
+ if (offs < 0) {
+ debug("Node %s not found\n", path);
+ path = "/";
+ subpath = "reserved-memory";
+ offs = fdt_path_offset(blob, path);
+ offs = fdt_add_subnode(blob, offs, subpath);
+ if (offs < 0) {
+ printf("Could not create %s%s node.\n", path, subpath);
+ return -1;
+ }
+ path = "/reserved-memory";
+ offs = fdt_path_offset(blob, path);
+
+ fdt_setprop(blob, offs, "#address-cells", &address_cells, sizeof(address_cells));
+ fdt_setprop(blob, offs, "#size-cells", &size_cells, sizeof(size_cells));
+ fdt_setprop(blob, offs, "ranges", NULL, 0);
+ }
+
+ offs = fdt_add_subnode(blob, offs, area ? : "private");
+ if (offs < 0) {
+ printf("Could not create %s%s node.\n", path, subpath);
+ return -1;
+ }
+
+ fdt_setprop(blob, offs, "no-map", NULL, 0);
+ len = fdt_pack_reg(blob, temp, start, size, 1);
+ fdt_setprop(blob, offs, "reg", temp, len);
+ return 0;
+}
+
#if CONFIG_NR_DRAM_BANKS > 4
#define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
#else
diff --git a/include/fdt_support.h b/include/fdt_support.h
index ba14acd7f6..7c8a280f53 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -93,6 +93,17 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
*/
int fdt_fixup_memory(void *blob, u64 start, u64 size);
+/**
+ * Setup the memory reserved node in the DT. Creates one if none was existing before.
+ *
+ * @param blob FDT blob to update
+ * @param area Reserved area name
+ * @param start Begin of DRAM mapping in physical memory
+ * @param size Size of the single memory bank
+ * @return 0 if ok, or -1 or -FDT_ERR_... on error
+ */
+int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[]);
+
/**
* Fill the DT memory node with multiple memory banks.
* Creates the node if none was existing before.
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 1/2] common: fdt: Add a function for reserving memory without kernel linear mapping
@ 2020-02-25 5:10 ` Michael Trimarchi
0 siblings, 0 replies; 13+ messages in thread
From: Michael Trimarchi @ 2020-02-25 5:10 UTC (permalink / raw)
To: Simon Glass, Anatolij Gustschin, Neil Armstrong; +Cc: u-boot, u-boot-amlogic
The intent is to reserve memory _and_ prevent it from being included
in the kernel's linear map. For thos reason it is also necessary to include the
'no-map' property for this reserved-mem node.
From Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt:
no-map (optional) - empty property
- Indicates the operating system must not create a virtual mapping
of the region as part of its standard mapping of system memory,
nor permit speculative access to it under any circumstances other
than under the control of the device driver using the region.
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
Changes: RFC->v1
- Add a better commit message
---
common/fdt_support.c | 40 ++++++++++++++++++++++++++++++++++++++++
include/fdt_support.h | 11 +++++++++++
2 files changed, 51 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 02cf5c6241..a3662f4358 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -410,6 +410,46 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
return p - (char *)buf;
}
+int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[])
+{
+ int offs, len;
+ const char *subpath;
+ const char *path = "/reserved-memory";
+ fdt32_t address_cells = cpu_to_fdt32(fdt_address_cells(blob, 0));
+ fdt32_t size_cells = cpu_to_fdt32(fdt_size_cells(blob, 0));
+ u8 temp[16]; /* Up to 64-bit address + 64-bit size */
+
+ offs = fdt_path_offset(blob, path);
+ if (offs < 0) {
+ debug("Node %s not found\n", path);
+ path = "/";
+ subpath = "reserved-memory";
+ offs = fdt_path_offset(blob, path);
+ offs = fdt_add_subnode(blob, offs, subpath);
+ if (offs < 0) {
+ printf("Could not create %s%s node.\n", path, subpath);
+ return -1;
+ }
+ path = "/reserved-memory";
+ offs = fdt_path_offset(blob, path);
+
+ fdt_setprop(blob, offs, "#address-cells", &address_cells, sizeof(address_cells));
+ fdt_setprop(blob, offs, "#size-cells", &size_cells, sizeof(size_cells));
+ fdt_setprop(blob, offs, "ranges", NULL, 0);
+ }
+
+ offs = fdt_add_subnode(blob, offs, area ? : "private");
+ if (offs < 0) {
+ printf("Could not create %s%s node.\n", path, subpath);
+ return -1;
+ }
+
+ fdt_setprop(blob, offs, "no-map", NULL, 0);
+ len = fdt_pack_reg(blob, temp, start, size, 1);
+ fdt_setprop(blob, offs, "reg", temp, len);
+ return 0;
+}
+
#if CONFIG_NR_DRAM_BANKS > 4
#define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
#else
diff --git a/include/fdt_support.h b/include/fdt_support.h
index ba14acd7f6..7c8a280f53 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -93,6 +93,17 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
*/
int fdt_fixup_memory(void *blob, u64 start, u64 size);
+/**
+ * Setup the memory reserved node in the DT. Creates one if none was existing before.
+ *
+ * @param blob FDT blob to update
+ * @param area Reserved area name
+ * @param start Begin of DRAM mapping in physical memory
+ * @param size Size of the single memory bank
+ * @return 0 if ok, or -1 or -FDT_ERR_... on error
+ */
+int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[]);
+
/**
* Fill the DT memory node with multiple memory banks.
* Creates the node if none was existing before.
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] video: meson: Use reserving memory function without kernel linear mapping
2020-02-25 5:10 ` Michael Trimarchi
@ 2020-02-25 5:10 ` Michael Trimarchi
-1 siblings, 0 replies; 13+ messages in thread
From: Michael Trimarchi @ 2020-02-25 5:10 UTC (permalink / raw)
To: u-boot
Memory reserved for the simple framebuffer should not be used
and part of memory linear mapping. See
https://patchwork.kernel.org/patch/10486131/ for more detailed
background information and discussion.
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
Changes RFC -> v1:
- Fix compilation issue on RFC
- change node name from display_reserved to display-reserved
---
drivers/video/meson/meson_vpu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/meson/meson_vpu.c b/drivers/video/meson/meson_vpu.c
index 4eb66398d0..5bfad05d75 100644
--- a/drivers/video/meson/meson_vpu.c
+++ b/drivers/video/meson/meson_vpu.c
@@ -173,9 +173,9 @@ static void meson_vpu_setup_simplefb(void *fdt)
* at the end of the RAM and we strip this portion from the kernel
* allowed region
*/
- mem_start = gd->bd->bi_dram[0].start;
- mem_size = gd->bd->bi_dram[0].size - meson_fb.fb_size;
- ret = fdt_fixup_memory_banks(fdt, &mem_start, &mem_size, 1);
+ mem_start = meson_fb.base;
+ mem_size = meson_fb.fb_size;
+ ret = fdt_fixup_reserved_memory(fdt, "display-reserved", &mem_start, &mem_size);
if (ret) {
eprintf("Cannot setup simplefb: Error reserving memory\n");
return;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] video: meson: Use reserving memory function without kernel linear mapping
@ 2020-02-25 5:10 ` Michael Trimarchi
0 siblings, 0 replies; 13+ messages in thread
From: Michael Trimarchi @ 2020-02-25 5:10 UTC (permalink / raw)
To: Simon Glass, Anatolij Gustschin, Neil Armstrong; +Cc: u-boot, u-boot-amlogic
Memory reserved for the simple framebuffer should not be used
and part of memory linear mapping. See
https://patchwork.kernel.org/patch/10486131/ for more detailed
background information and discussion.
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
Changes RFC -> v1:
- Fix compilation issue on RFC
- change node name from display_reserved to display-reserved
---
drivers/video/meson/meson_vpu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/meson/meson_vpu.c b/drivers/video/meson/meson_vpu.c
index 4eb66398d0..5bfad05d75 100644
--- a/drivers/video/meson/meson_vpu.c
+++ b/drivers/video/meson/meson_vpu.c
@@ -173,9 +173,9 @@ static void meson_vpu_setup_simplefb(void *fdt)
* at the end of the RAM and we strip this portion from the kernel
* allowed region
*/
- mem_start = gd->bd->bi_dram[0].start;
- mem_size = gd->bd->bi_dram[0].size - meson_fb.fb_size;
- ret = fdt_fixup_memory_banks(fdt, &mem_start, &mem_size, 1);
+ mem_start = meson_fb.base;
+ mem_size = meson_fb.fb_size;
+ ret = fdt_fixup_reserved_memory(fdt, "display-reserved", &mem_start, &mem_size);
if (ret) {
eprintf("Cannot setup simplefb: Error reserving memory\n");
return;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 1/2] common: fdt: Add a function for reserving memory without kernel linear mapping
2020-02-25 5:10 ` Michael Trimarchi
@ 2020-02-26 15:33 ` Simon Glass
-1 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2020-02-26 15:33 UTC (permalink / raw)
To: u-boot
Hi Michael,
On Mon, 24 Feb 2020 at 22:10, Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> The intent is to reserve memory _and_ prevent it from being included
> in the kernel's linear map. For thos reason it is also necessary to include the
> 'no-map' property for this reserved-mem node.
>
> From Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt:
>
> no-map (optional) - empty property
> - Indicates the operating system must not create a virtual mapping
> of the region as part of its standard mapping of system memory,
> nor permit speculative access to it under any circumstances other
> than under the control of the device driver using the region.
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
> Changes: RFC->v1
> - Add a better commit message
> ---
> common/fdt_support.c | 40 ++++++++++++++++++++++++++++++++++++++++
> include/fdt_support.h | 11 +++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 02cf5c6241..a3662f4358 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -410,6 +410,46 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
> return p - (char *)buf;
> }
>
> +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[])
> +{
> + int offs, len;
> + const char *subpath;
> + const char *path = "/reserved-memory";
> + fdt32_t address_cells = cpu_to_fdt32(fdt_address_cells(blob, 0));
> + fdt32_t size_cells = cpu_to_fdt32(fdt_size_cells(blob, 0));
> + u8 temp[16]; /* Up to 64-bit address + 64-bit size */
> +
> + offs = fdt_path_offset(blob, path);
> + if (offs < 0) {
> + debug("Node %s not found\n", path);
> + path = "/";
> + subpath = "reserved-memory";
> + offs = fdt_path_offset(blob, path);
Error check
> + offs = fdt_add_subnode(blob, offs, subpath);
> + if (offs < 0) {
> + printf("Could not create %s%s node.\n", path, subpath);
> + return -1;
> + }
> + path = "/reserved-memory";
> + offs = fdt_path_offset(blob, path);
> +
> + fdt_setprop(blob, offs, "#address-cells", &address_cells, sizeof(address_cells));
> + fdt_setprop(blob, offs, "#size-cells", &size_cells, sizeof(size_cells));
> + fdt_setprop(blob, offs, "ranges", NULL, 0);
Need error-checking on these three
> + }
> +
> + offs = fdt_add_subnode(blob, offs, area ? : "private");
Is this in a binding file somewhere?
> + if (offs < 0) {
> + printf("Could not create %s%s node.\n", path, subpath);
> + return -1;
return offs
> + }
> +
> + fdt_setprop(blob, offs, "no-map", NULL, 0);
and this?
Also needs error check
> + len = fdt_pack_reg(blob, temp, start, size, 1);
> + fdt_setprop(blob, offs, "reg", temp, len);
blank line before return
> + return 0;
> +}
> +
> #if CONFIG_NR_DRAM_BANKS > 4
> #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
> #else
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index ba14acd7f6..7c8a280f53 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -93,6 +93,17 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
> */
> int fdt_fixup_memory(void *blob, u64 start, u64 size);
>
> +/**
> + * Setup the memory reserved node in the DT. Creates one if none was existing before.
> + *
> + * @param blob FDT blob to update
> + * @param area Reserved area name
> + * @param start Begin of DRAM mapping in physical memory
> + * @param size Size of the single memory bank
> + * @return 0 if ok, or -1 or -FDT_ERR_... on error
Really we should return an FDT_ERR always. -1 is not a good idea and
in fact is an FDT_ERR
> + */
> +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[]);
> +
> /**
> * Fill the DT memory node with multiple memory banks.
> * Creates the node if none was existing before.
> --
> 2.17.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] common: fdt: Add a function for reserving memory without kernel linear mapping
@ 2020-02-26 15:33 ` Simon Glass
0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2020-02-26 15:33 UTC (permalink / raw)
To: Michael Trimarchi
Cc: Anatolij Gustschin, Neil Armstrong, U-Boot Mailing List, u-boot-amlogic
Hi Michael,
On Mon, 24 Feb 2020 at 22:10, Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> The intent is to reserve memory _and_ prevent it from being included
> in the kernel's linear map. For thos reason it is also necessary to include the
> 'no-map' property for this reserved-mem node.
>
> From Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt:
>
> no-map (optional) - empty property
> - Indicates the operating system must not create a virtual mapping
> of the region as part of its standard mapping of system memory,
> nor permit speculative access to it under any circumstances other
> than under the control of the device driver using the region.
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
> Changes: RFC->v1
> - Add a better commit message
> ---
> common/fdt_support.c | 40 ++++++++++++++++++++++++++++++++++++++++
> include/fdt_support.h | 11 +++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 02cf5c6241..a3662f4358 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -410,6 +410,46 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
> return p - (char *)buf;
> }
>
> +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[])
> +{
> + int offs, len;
> + const char *subpath;
> + const char *path = "/reserved-memory";
> + fdt32_t address_cells = cpu_to_fdt32(fdt_address_cells(blob, 0));
> + fdt32_t size_cells = cpu_to_fdt32(fdt_size_cells(blob, 0));
> + u8 temp[16]; /* Up to 64-bit address + 64-bit size */
> +
> + offs = fdt_path_offset(blob, path);
> + if (offs < 0) {
> + debug("Node %s not found\n", path);
> + path = "/";
> + subpath = "reserved-memory";
> + offs = fdt_path_offset(blob, path);
Error check
> + offs = fdt_add_subnode(blob, offs, subpath);
> + if (offs < 0) {
> + printf("Could not create %s%s node.\n", path, subpath);
> + return -1;
> + }
> + path = "/reserved-memory";
> + offs = fdt_path_offset(blob, path);
> +
> + fdt_setprop(blob, offs, "#address-cells", &address_cells, sizeof(address_cells));
> + fdt_setprop(blob, offs, "#size-cells", &size_cells, sizeof(size_cells));
> + fdt_setprop(blob, offs, "ranges", NULL, 0);
Need error-checking on these three
> + }
> +
> + offs = fdt_add_subnode(blob, offs, area ? : "private");
Is this in a binding file somewhere?
> + if (offs < 0) {
> + printf("Could not create %s%s node.\n", path, subpath);
> + return -1;
return offs
> + }
> +
> + fdt_setprop(blob, offs, "no-map", NULL, 0);
and this?
Also needs error check
> + len = fdt_pack_reg(blob, temp, start, size, 1);
> + fdt_setprop(blob, offs, "reg", temp, len);
blank line before return
> + return 0;
> +}
> +
> #if CONFIG_NR_DRAM_BANKS > 4
> #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
> #else
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index ba14acd7f6..7c8a280f53 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -93,6 +93,17 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
> */
> int fdt_fixup_memory(void *blob, u64 start, u64 size);
>
> +/**
> + * Setup the memory reserved node in the DT. Creates one if none was existing before.
> + *
> + * @param blob FDT blob to update
> + * @param area Reserved area name
> + * @param start Begin of DRAM mapping in physical memory
> + * @param size Size of the single memory bank
> + * @return 0 if ok, or -1 or -FDT_ERR_... on error
Really we should return an FDT_ERR always. -1 is not a good idea and
in fact is an FDT_ERR
> + */
> +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[]);
> +
> /**
> * Fill the DT memory node with multiple memory banks.
> * Creates the node if none was existing before.
> --
> 2.17.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] video: meson: Use reserving memory function without kernel linear mapping
2020-02-25 5:10 ` Michael Trimarchi
@ 2020-02-26 15:33 ` Simon Glass
-1 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2020-02-26 15:33 UTC (permalink / raw)
To: u-boot
Hi Michae,
On Mon, 24 Feb 2020 at 22:10, Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Memory reserved for the simple framebuffer should not be used
> and part of memory linear mapping. See
> https://patchwork.kernel.org/patch/10486131/ for more detailed
> background information and discussion.
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
> Changes RFC -> v1:
> - Fix compilation issue on RFC
> - change node name from display_reserved to display-reserved
> ---
> drivers/video/meson/meson_vpu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Is the binding file for this in U-Boot? If not can you please add it?
>
> diff --git a/drivers/video/meson/meson_vpu.c b/drivers/video/meson/meson_vpu.c
> index 4eb66398d0..5bfad05d75 100644
> --- a/drivers/video/meson/meson_vpu.c
> +++ b/drivers/video/meson/meson_vpu.c
> @@ -173,9 +173,9 @@ static void meson_vpu_setup_simplefb(void *fdt)
> * at the end of the RAM and we strip this portion from the kernel
> * allowed region
> */
> - mem_start = gd->bd->bi_dram[0].start;
> - mem_size = gd->bd->bi_dram[0].size - meson_fb.fb_size;
> - ret = fdt_fixup_memory_banks(fdt, &mem_start, &mem_size, 1);
> + mem_start = meson_fb.base;
> + mem_size = meson_fb.fb_size;
> + ret = fdt_fixup_reserved_memory(fdt, "display-reserved", &mem_start, &mem_size);
> if (ret) {
> eprintf("Cannot setup simplefb: Error reserving memory\n");
> return;
Needs to return an error code which needs to be checked by caller.
> --
> 2.17.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] video: meson: Use reserving memory function without kernel linear mapping
@ 2020-02-26 15:33 ` Simon Glass
0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2020-02-26 15:33 UTC (permalink / raw)
To: Michael Trimarchi
Cc: Anatolij Gustschin, Neil Armstrong, U-Boot Mailing List, u-boot-amlogic
Hi Michae,
On Mon, 24 Feb 2020 at 22:10, Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Memory reserved for the simple framebuffer should not be used
> and part of memory linear mapping. See
> https://patchwork.kernel.org/patch/10486131/ for more detailed
> background information and discussion.
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
> Changes RFC -> v1:
> - Fix compilation issue on RFC
> - change node name from display_reserved to display-reserved
> ---
> drivers/video/meson/meson_vpu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Is the binding file for this in U-Boot? If not can you please add it?
>
> diff --git a/drivers/video/meson/meson_vpu.c b/drivers/video/meson/meson_vpu.c
> index 4eb66398d0..5bfad05d75 100644
> --- a/drivers/video/meson/meson_vpu.c
> +++ b/drivers/video/meson/meson_vpu.c
> @@ -173,9 +173,9 @@ static void meson_vpu_setup_simplefb(void *fdt)
> * at the end of the RAM and we strip this portion from the kernel
> * allowed region
> */
> - mem_start = gd->bd->bi_dram[0].start;
> - mem_size = gd->bd->bi_dram[0].size - meson_fb.fb_size;
> - ret = fdt_fixup_memory_banks(fdt, &mem_start, &mem_size, 1);
> + mem_start = meson_fb.base;
> + mem_size = meson_fb.fb_size;
> + ret = fdt_fixup_reserved_memory(fdt, "display-reserved", &mem_start, &mem_size);
> if (ret) {
> eprintf("Cannot setup simplefb: Error reserving memory\n");
> return;
Needs to return an error code which needs to be checked by caller.
> --
> 2.17.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] common: fdt: Add a function for reserving memory without kernel linear mapping
2020-02-26 15:33 ` Simon Glass
@ 2020-02-29 10:44 ` Michael Nazzareno Trimarchi
-1 siblings, 0 replies; 13+ messages in thread
From: Michael Nazzareno Trimarchi @ 2020-02-29 10:44 UTC (permalink / raw)
To: u-boot
Hi
On Wed, Feb 26, 2020 at 4:33 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Michael,
>
> On Mon, 24 Feb 2020 at 22:10, Michael Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > The intent is to reserve memory _and_ prevent it from being included
> > in the kernel's linear map. For thos reason it is also necessary to include the
> > 'no-map' property for this reserved-mem node.
> >
> > From Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt:
> >
> > no-map (optional) - empty property
> > - Indicates the operating system must not create a virtual mapping
> > of the region as part of its standard mapping of system memory,
> > nor permit speculative access to it under any circumstances other
> > than under the control of the device driver using the region.
> >
> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > ---
> > Changes: RFC->v1
> > - Add a better commit message
> > ---
> > common/fdt_support.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > include/fdt_support.h | 11 +++++++++++
> > 2 files changed, 51 insertions(+)
> >
> > diff --git a/common/fdt_support.c b/common/fdt_support.c
> > index 02cf5c6241..a3662f4358 100644
> > --- a/common/fdt_support.c
> > +++ b/common/fdt_support.c
> > @@ -410,6 +410,46 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
> > return p - (char *)buf;
> > }
> >
> > +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[])
> > +{
> > + int offs, len;
> > + const char *subpath;
> > + const char *path = "/reserved-memory";
> > + fdt32_t address_cells = cpu_to_fdt32(fdt_address_cells(blob, 0));
> > + fdt32_t size_cells = cpu_to_fdt32(fdt_size_cells(blob, 0));
> > + u8 temp[16]; /* Up to 64-bit address + 64-bit size */
> > +
> > + offs = fdt_path_offset(blob, path);
> > + if (offs < 0) {
> > + debug("Node %s not found\n", path);
> > + path = "/";
> > + subpath = "reserved-memory";
> > + offs = fdt_path_offset(blob, path);
>
> Error check
>
Ok
> > + offs = fdt_add_subnode(blob, offs, subpath);
> > + if (offs < 0) {
> > + printf("Could not create %s%s node.\n", path, subpath);
> > + return -1;
> > + }
> > + path = "/reserved-memory";
> > + offs = fdt_path_offset(blob, path);
> > +
> > + fdt_setprop(blob, offs, "#address-cells", &address_cells, sizeof(address_cells));
> > + fdt_setprop(blob, offs, "#size-cells", &size_cells, sizeof(size_cells));
> > + fdt_setprop(blob, offs, "ranges", NULL, 0);
>
> Need error-checking on these three
>
ok
> > + }
> > +
> > + offs = fdt_add_subnode(blob, offs, area ? : "private");
>
> Is this in a binding file somewhere?
>
The reserved memory is needed to avoid the linux kernel to use it and
to get a persistent framebuffer. On some SoC I have implemented the
reuse on the memory on their graphics driver. I need to check how this
is
documented in Linux. The name of the reserved memory it's not mandatory.
> > + if (offs < 0) {
> > + printf("Could not create %s%s node.\n", path, subpath);
> > + return -1;
>
> return offs
>
ok
> > + }
> > +
> > + fdt_setprop(blob, offs, "no-map", NULL, 0);
>
> and this?
>
ok
> Also needs error check
>
> > + len = fdt_pack_reg(blob, temp, start, size, 1);
> > + fdt_setprop(blob, offs, "reg", temp, len);
>
> blank line before return
>
done
> > + return 0;
> > +}
> > +
> > #if CONFIG_NR_DRAM_BANKS > 4
> > #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
> > #else
> > diff --git a/include/fdt_support.h b/include/fdt_support.h
> > index ba14acd7f6..7c8a280f53 100644
> > --- a/include/fdt_support.h
> > +++ b/include/fdt_support.h
> > @@ -93,6 +93,17 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
> > */
> > int fdt_fixup_memory(void *blob, u64 start, u64 size);
> >
> > +/**
> > + * Setup the memory reserved node in the DT. Creates one if none was existing before.
> > + *
> > + * @param blob FDT blob to update
> > + * @param area Reserved area name
> > + * @param start Begin of DRAM mapping in physical memory
> > + * @param size Size of the single memory bank
> > + * @return 0 if ok, or -1 or -FDT_ERR_... on error
>
> Really we should return an FDT_ERR always. -1 is not a good idea and
> in fact is an FDT_ERR
>
Now with the change FDT_ERR only is returned.
> > + */
> > +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[]);
> > +
> > /**
> > * Fill the DT memory node with multiple memory banks.
> > * Creates the node if none was existing before.
> > --
> > 2.17.1
> >
>
> Regards,
> Simon
I will post a new one
Michael
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] common: fdt: Add a function for reserving memory without kernel linear mapping
@ 2020-02-29 10:44 ` Michael Nazzareno Trimarchi
0 siblings, 0 replies; 13+ messages in thread
From: Michael Nazzareno Trimarchi @ 2020-02-29 10:44 UTC (permalink / raw)
To: Simon Glass
Cc: Anatolij Gustschin, Neil Armstrong, U-Boot Mailing List, u-boot-amlogic
Hi
On Wed, Feb 26, 2020 at 4:33 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Michael,
>
> On Mon, 24 Feb 2020 at 22:10, Michael Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > The intent is to reserve memory _and_ prevent it from being included
> > in the kernel's linear map. For thos reason it is also necessary to include the
> > 'no-map' property for this reserved-mem node.
> >
> > From Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt:
> >
> > no-map (optional) - empty property
> > - Indicates the operating system must not create a virtual mapping
> > of the region as part of its standard mapping of system memory,
> > nor permit speculative access to it under any circumstances other
> > than under the control of the device driver using the region.
> >
> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > ---
> > Changes: RFC->v1
> > - Add a better commit message
> > ---
> > common/fdt_support.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > include/fdt_support.h | 11 +++++++++++
> > 2 files changed, 51 insertions(+)
> >
> > diff --git a/common/fdt_support.c b/common/fdt_support.c
> > index 02cf5c6241..a3662f4358 100644
> > --- a/common/fdt_support.c
> > +++ b/common/fdt_support.c
> > @@ -410,6 +410,46 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
> > return p - (char *)buf;
> > }
> >
> > +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[])
> > +{
> > + int offs, len;
> > + const char *subpath;
> > + const char *path = "/reserved-memory";
> > + fdt32_t address_cells = cpu_to_fdt32(fdt_address_cells(blob, 0));
> > + fdt32_t size_cells = cpu_to_fdt32(fdt_size_cells(blob, 0));
> > + u8 temp[16]; /* Up to 64-bit address + 64-bit size */
> > +
> > + offs = fdt_path_offset(blob, path);
> > + if (offs < 0) {
> > + debug("Node %s not found\n", path);
> > + path = "/";
> > + subpath = "reserved-memory";
> > + offs = fdt_path_offset(blob, path);
>
> Error check
>
Ok
> > + offs = fdt_add_subnode(blob, offs, subpath);
> > + if (offs < 0) {
> > + printf("Could not create %s%s node.\n", path, subpath);
> > + return -1;
> > + }
> > + path = "/reserved-memory";
> > + offs = fdt_path_offset(blob, path);
> > +
> > + fdt_setprop(blob, offs, "#address-cells", &address_cells, sizeof(address_cells));
> > + fdt_setprop(blob, offs, "#size-cells", &size_cells, sizeof(size_cells));
> > + fdt_setprop(blob, offs, "ranges", NULL, 0);
>
> Need error-checking on these three
>
ok
> > + }
> > +
> > + offs = fdt_add_subnode(blob, offs, area ? : "private");
>
> Is this in a binding file somewhere?
>
The reserved memory is needed to avoid the linux kernel to use it and
to get a persistent framebuffer. On some SoC I have implemented the
reuse on the memory on their graphics driver. I need to check how this
is
documented in Linux. The name of the reserved memory it's not mandatory.
> > + if (offs < 0) {
> > + printf("Could not create %s%s node.\n", path, subpath);
> > + return -1;
>
> return offs
>
ok
> > + }
> > +
> > + fdt_setprop(blob, offs, "no-map", NULL, 0);
>
> and this?
>
ok
> Also needs error check
>
> > + len = fdt_pack_reg(blob, temp, start, size, 1);
> > + fdt_setprop(blob, offs, "reg", temp, len);
>
> blank line before return
>
done
> > + return 0;
> > +}
> > +
> > #if CONFIG_NR_DRAM_BANKS > 4
> > #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
> > #else
> > diff --git a/include/fdt_support.h b/include/fdt_support.h
> > index ba14acd7f6..7c8a280f53 100644
> > --- a/include/fdt_support.h
> > +++ b/include/fdt_support.h
> > @@ -93,6 +93,17 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
> > */
> > int fdt_fixup_memory(void *blob, u64 start, u64 size);
> >
> > +/**
> > + * Setup the memory reserved node in the DT. Creates one if none was existing before.
> > + *
> > + * @param blob FDT blob to update
> > + * @param area Reserved area name
> > + * @param start Begin of DRAM mapping in physical memory
> > + * @param size Size of the single memory bank
> > + * @return 0 if ok, or -1 or -FDT_ERR_... on error
>
> Really we should return an FDT_ERR always. -1 is not a good idea and
> in fact is an FDT_ERR
>
Now with the change FDT_ERR only is returned.
> > + */
> > +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[]);
> > +
> > /**
> > * Fill the DT memory node with multiple memory banks.
> > * Creates the node if none was existing before.
> > --
> > 2.17.1
> >
>
> Regards,
> Simon
I will post a new one
Michael
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] common: fdt: Add a function for reserving memory without kernel linear mapping
2020-02-25 5:10 ` Michael Trimarchi
(?)
(?)
@ 2022-04-14 16:17 ` Michael Nazzareno Trimarchi
-1 siblings, 0 replies; 13+ messages in thread
From: Michael Nazzareno Trimarchi @ 2022-04-14 16:17 UTC (permalink / raw)
To: Simon Glass, Anatolij Gustschin, Neil Armstrong, Tommaso Merciai
Cc: u-boot, u-boot-amlogic
HI Tommaso,
Thank you to have time on this
Michael
On Tue, Feb 25, 2020 at 6:10 AM Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> The intent is to reserve memory _and_ prevent it from being included
> in the kernel's linear map. For thos reason it is also necessary to include the
> 'no-map' property for this reserved-mem node.
>
> From Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt:
>
> no-map (optional) - empty property
> - Indicates the operating system must not create a virtual mapping
> of the region as part of its standard mapping of system memory,
> nor permit speculative access to it under any circumstances other
> than under the control of the device driver using the region.
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
> Changes: RFC->v1
> - Add a better commit message
> ---
> common/fdt_support.c | 40 ++++++++++++++++++++++++++++++++++++++++
> include/fdt_support.h | 11 +++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 02cf5c6241..a3662f4358 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -410,6 +410,46 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
> return p - (char *)buf;
> }
>
> +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[])
> +{
> + int offs, len;
> + const char *subpath;
> + const char *path = "/reserved-memory";
> + fdt32_t address_cells = cpu_to_fdt32(fdt_address_cells(blob, 0));
> + fdt32_t size_cells = cpu_to_fdt32(fdt_size_cells(blob, 0));
> + u8 temp[16]; /* Up to 64-bit address + 64-bit size */
> +
> + offs = fdt_path_offset(blob, path);
> + if (offs < 0) {
> + debug("Node %s not found\n", path);
> + path = "/";
> + subpath = "reserved-memory";
> + offs = fdt_path_offset(blob, path);
> + offs = fdt_add_subnode(blob, offs, subpath);
> + if (offs < 0) {
> + printf("Could not create %s%s node.\n", path, subpath);
> + return -1;
> + }
> + path = "/reserved-memory";
> + offs = fdt_path_offset(blob, path);
> +
> + fdt_setprop(blob, offs, "#address-cells", &address_cells, sizeof(address_cells));
> + fdt_setprop(blob, offs, "#size-cells", &size_cells, sizeof(size_cells));
> + fdt_setprop(blob, offs, "ranges", NULL, 0);
> + }
> +
> + offs = fdt_add_subnode(blob, offs, area ? : "private");
> + if (offs < 0) {
> + printf("Could not create %s%s node.\n", path, subpath);
> + return -1;
> + }
> +
> + fdt_setprop(blob, offs, "no-map", NULL, 0);
> + len = fdt_pack_reg(blob, temp, start, size, 1);
> + fdt_setprop(blob, offs, "reg", temp, len);
> + return 0;
> +}
> +
> #if CONFIG_NR_DRAM_BANKS > 4
> #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
> #else
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index ba14acd7f6..7c8a280f53 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -93,6 +93,17 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
> */
> int fdt_fixup_memory(void *blob, u64 start, u64 size);
>
> +/**
> + * Setup the memory reserved node in the DT. Creates one if none was existing before.
> + *
> + * @param blob FDT blob to update
> + * @param area Reserved area name
> + * @param start Begin of DRAM mapping in physical memory
> + * @param size Size of the single memory bank
> + * @return 0 if ok, or -1 or -FDT_ERR_... on error
> + */
> +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 size[]);
> +
> /**
> * Fill the DT memory node with multiple memory banks.
> * Creates the node if none was existing before.
> --
> 2.17.1
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-04-14 16:17 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25 5:10 [PATCH 0/2] Update reserved memory for simple framebuffer Michael Trimarchi
2020-02-25 5:10 ` Michael Trimarchi
2020-02-25 5:10 ` [PATCH 1/2] common: fdt: Add a function for reserving memory without kernel linear mapping Michael Trimarchi
2020-02-25 5:10 ` Michael Trimarchi
2020-02-26 15:33 ` Simon Glass
2020-02-26 15:33 ` Simon Glass
2020-02-29 10:44 ` Michael Nazzareno Trimarchi
2020-02-29 10:44 ` Michael Nazzareno Trimarchi
2022-04-14 16:17 ` Michael Nazzareno Trimarchi
2020-02-25 5:10 ` [PATCH 2/2] video: meson: Use reserving memory function " Michael Trimarchi
2020-02-25 5:10 ` Michael Trimarchi
2020-02-26 15:33 ` Simon Glass
2020-02-26 15:33 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.