Re: [PATCH v2 1/5] arm: mach-k3: common: Reserve video memory from end of the RAM

Simon Glass
2023-11-02T22:46:30Z

Hi Devarsh,

On Tue, 31 Oct 2023 at 13:12, Devarsh Thakkar <devarsht@ti.com> wrote:
>
> Add function spl_reserve_video which is a wrapper
> around video_reserve to setup video memory and update
> the relocation address pointer.
>
> Setup video memory before page table reservation so that
> framebuffer memory gets reserved from the end of RAM.
>
> This is as per the new policy being discussed for passing
> blobs where each of the reserved areas for bloblists
> to be passed need to be reserved at the end of RAM.
>
> This is done to enable the next stage to directly skip
> the pre-reserved area from previous stage right from the end of RAM
> without having to make any gaps/holes to accommodate those
> regions which was the case before as previous stage
> reserved region not from the end of RAM.
>
> Suggested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
> ---
> V2: Make a generic function "spl_reserve_video" under
> common/spl which can be re-used by other platforms too
> for reserving video memory from spl.
> ---
> arch/arm/mach-k3/common.c | 2 ++
> common/spl/spl.c | 19 +++++++++++++++++++
> include/spl.h | 4 ++++
> 3 files changed, 25 insertions(+)

>
> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
> index c3006ba387..03e3b46282 100644
> --- a/arch/arm/mach-k3/common.c
> +++ b/arch/arm/mach-k3/common.c
> @@ -537,6 +537,8 @@ void spl_enable_dcache(void)
> if (ram_top >= 0x100000000)
> ram_top = (phys_addr_t) 0x100000000;
>
> + gd->relocaddr = ram_top;
> + spl_reserve_video();

Need to check error here

> gd->arch.tlb_addr = ram_top - gd->arch.tlb_size;
> gd->arch.tlb_addr &= ~(0x10000 - 1);
> debug("TLB table from %08lx to %08lx\n", gd->arch.tlb_addr,
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 732d90d39e..89172f2ebf 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -41,6 +41,7 @@
> #include <fdt_support.h>
> #include <bootcount.h>
> #include <wdt.h>
> +#include <video.h>
>
> DECLARE_GLOBAL_DATA_PTR;
> DECLARE_BINMAN_MAGIC_SYM;
> @@ -151,6 +152,24 @@ void spl_fixup_fdt(void *fdt_blob)
> #endif
> }
>
> +int spl_reserve_video(void)
> +{
> + if (CONFIG_IS_ENABLED(VIDEO)) {
> + ulong addr;
> + int ret;
> +
> + addr = gd->relocaddr;
> + ret = video_reserve(&addr);
> + if (ret)
> + return ret;
> + debug("Reserving %luk for video at: %08lx\n",
> + ((unsigned long)gd->relocaddr - addr) >> 10, addr);
> + gd->relocaddr = addr;
> + }
> +
> + return 0;
> +}
> +
> ulong spl_get_image_pos(void)
> {
> if (!CONFIG_IS_ENABLED(BINMAN_UBOOT_SYMBOLS))
> diff --git a/include/spl.h b/include/spl.h
> index 0d49e4a454..9682e51fc1 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -825,6 +825,10 @@ int spl_usb_load(struct spl_image_info *spl_image,
>
> int spl_ymodem_load_image(struct spl_image_info *spl_image,
> struct spl_boot_device *bootdev);
> +/**
> + * spl_reserve_video() - Reserve video and update relocation address

This needs more detail about:
- gd->relocaddr
- where the video memory is allocated
- where the allocation is stored (in the video-device plat?)
- return value

> + */
> +int spl_reserve_video(void);
>
> /**
> * spl_invoke_atf - boot using an ARM trusted firmware image
> --
> 2.34.1
>

Regards,
Simon