Hi Simon, Tom,

On 15/08/23 04:13, Simon Glass wrote:
Hi Devarsh, Nikhil, Tom,

On Wed, 9 Aug 2023 at 09:29, Bin Meng <bmeng...@gmail.com> wrote:

On Thu, Aug 3, 2023 at 7:03 PM Bin Meng <bmeng...@gmail.com> wrote:

On Thu, Aug 3, 2023 at 6:37 PM Bin Meng <bmeng...@gmail.com> wrote:

On Tue, Aug 1, 2023 at 12:00 AM Simon Glass <s...@chromium.org> wrote:

When the video framebuffer comes from the bloblist, we should not change
relocaddr to this address, since it interfers with the normal memory

typo: interferes

allocation.

This fixes a boot loop in qemu-x86_64

Signed-off-by: Simon Glass <s...@chromium.org>
Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to 
u-boot")
Suggested-by: Nikhil M Jain <n-ja...@ti.com>
---

Changes in v3:
- Reword the Kconfig help as suggested

Changes in v2:
- Add a Kconfig as the suggested conditional did not work

  common/board_f.c      | 3 ++-
  drivers/video/Kconfig | 9 +++++++++
  2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/common/board_f.c b/common/board_f.c
index 7d2c380e91e..5173d0a0c2d 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -419,7 +419,8 @@ static int reserve_video(void)
                 if (!ho)
                         return log_msg_ret("blf", -ENOENT);
                 video_reserve_from_bloblist(ho);
-               gd->relocaddr = ho->fb;
+               if (IS_ENABLED(CONFIG_VIDEO_RESERVE_SPL))
+                       gd->relocaddr = ho->fb;
         } else if (CONFIG_IS_ENABLED(VIDEO)) {
                 ulong addr;
                 int ret;
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index b41dc60cec5..f2e56204d52 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1106,6 +1106,15 @@ config SPL_VIDEO_REMOVE
           if this  option is enabled video driver will be removed at the end of
           SPL stage, beforeloading the next stage.

+config VIDEO_RESERVE_SPL
+       bool
+       help
+         This adjusts reserve_video() to redirect memory reservation when it
+         sees a video handoff blob (BLOBLISTT_U_BOOT_VIDEO). This avoids the
+         memory used for framebuffer from being allocated by U-Boot proper,
+         thus preventing any further memory reservations done by U-Boot proper
+         from overwriting the framebuffer.
+
  if SPL_SPLASH_SCREEN

  config SPL_SPLASH_SCREEN_ALIGN

Reviewed-by: Bin Meng <bmeng...@gmail.com>

applied to u-boot-x86, thanks!

Dropped this one from the x86 queue per the discussion.

I just wanted to come back to this discussion.

Do we have an agreed way forward? Who is waiting for who?


I was waiting on feedback on https://lore.kernel.org/all/3b1e8005-f161-8058-13e7-3de2316aa...@ti.com/ but per my opinion, I would prefer to go with "Approach 2" with a Kconfig as it looks simpler to me. It would look something like below :

if (gd->relocaddr > (unsigned long)ho->fb) {
     ulong fb_reloc_gap = gd->relocaddr - gd->ho->fb;

     /* Relocate after framebuffer area if nearing too close to it */
     if (fb_reloc_gap < CONFIG_BLOBLIST_FB_RELOC_MIN_GAP)
            gd->relocaddr = ho->fb;
}

Regarding CONFIG_BLOBLIST_FB_RELOC_MIN_GAP
-> This describes minimum gap to keep between framebuffer address and relocation address to avoid overlap when framebuffer address used by blob is below the current relocation address

-> It would be selected as default when CONFIG_BLOBLIST is selected with default value set to 100Mb

-> SoC specific Vendors can override this in their defconfigs to a custom value if they feel 100Mb is not enough

Also probably we can have some debug/error prints in the code to show overlap happened (or is going happen) so that users can fine tune this Kconfig if they got it wrong at first place.

I can re-spin updated patch if we are aligned on this,
Kindly let me know your opinion.

Regards
Devarsh

Regards,
Simon

Reply via email to