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

[Simon Glass]

Hi Devarsh,

On Fri, 10 Nov 2023 at 08:29, Devarsh Thakkar <devarsht at 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 at chromium.org>
> Signed-off-by: Devarsh Thakkar <devarsht at 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.
>
> V3:
> - Change spl_reserve_video to spl_reserve_video_from_ram_top
>   which enforce framebuffer reservation from end of RAM
> - Use gd->ram_top instead of local ram_top and update
>   gd->reloc_addr after each reservation
> - Print error message on framebuffer reservation
> ---
>  arch/arm/mach-k3/common.c | 17 ++++++++++++-----

Please split that off into its own patch, so that the generic code
stands on its own.

>  common/spl/spl.c          | 27 +++++++++++++++++++++++++++
>  include/spl.h             |  6 ++++++
>  3 files changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
> index c3006ba387..6590045140 100644
> --- a/arch/arm/mach-k3/common.c
> +++ b/arch/arm/mach-k3/common.c
> @@ -525,19 +525,26 @@ void remove_fwl_configs(struct fwl_data *fwl_data, size_t fwl_data_size)
>  void spl_enable_dcache(void)
>  {
>  #if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
> -       phys_addr_t ram_top = CFG_SYS_SDRAM_BASE;
> +       gd->ram_top = CFG_SYS_SDRAM_BASE;
> +       int ret = 0;
>
>         dram_init();
>
>         /* reserve TLB table */
>         gd->arch.tlb_size = PGTABLE_SIZE;
>
> -       ram_top += get_effective_memsize();
> +       gd->ram_top += get_effective_memsize();
>         /* keep ram_top in the 32-bit address space */
> -       if (ram_top >= 0x100000000)
> -               ram_top = (phys_addr_t) 0x100000000;
> +       if (gd->ram_top >= 0x100000000)
> +               gd->ram_top = (phys_addr_t)0x100000000;
>
> -       gd->arch.tlb_addr = ram_top - gd->arch.tlb_size;
> +       gd->relocaddr = gd->ram_top;
> +
> +       ret = spl_reserve_video_from_ram_top();
> +       if (ret)
> +               pr_err("ERROR: Failed to framebuffer memory in SPL\n");
> +
> +       gd->arch.tlb_addr = gd->relocaddr - gd->arch.tlb_size;
>         gd->arch.tlb_addr &= ~(0x10000 - 1);
>         debug("TLB table from %08lx to %08lx\n", gd->arch.tlb_addr,
>               gd->arch.tlb_addr + gd->arch.tlb_size);
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 732d90d39e..452bf303de 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,32 @@ void spl_fixup_fdt(void *fdt_blob)
>  #endif
>  }
>
> +/*
> + * Reserve video memory for SPL splash screen from
> + * end of RAM
> + *
> + * RETURN
> + * 0 : On success
> + * Non-zero : On failure
> + */

Drop that comment as you have one in the header

> +int spl_reserve_video_from_ram_top(void)
> +{
> +       if (CONFIG_IS_ENABLED(VIDEO)) {
> +               ulong addr;
> +               int ret;
> +
> +               addr = gd->ram_top;
> +               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..39f2a7581d 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -825,6 +825,12 @@ 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_from_ram_top() - Reserve framebuffer memory from end of RAM
> + *

This needs more detail

> + * Return: 0 on success, otherwise error code
> + */
> +int spl_reserve_video_from_ram_top(void);
>
>  /**
>   * spl_invoke_atf - boot using an ARM trusted firmware image
> --
> 2.34.1
>

Regards,
Simon