Hi Devarsh,
On Mon, 31 Jul 2023 at 05:10, Devarsh Thakkar <devar...@ti.com> wrote:
>
> Hi Simon,
>
> Thanks for the patch.
>
> On 30/07/23 22:46, Simon Glass wrote:
> > When the video framebuffer comes from the bloblist, we should not change
> > relocaddr to this address, since it interfers with the normal memory
> > 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 v2:
> > - Add a Kconfig as the suggested conditional did not work
>
> Overall this approach looks fine too but just curious to know, what is the
> ho->fb and gd->relocaddr in your case, so that we could understand your
> scenario better and maybe devise a more generic condition as a later-on patch
> since you mentioned that shared condition did not work for you.
Well, bear in mind that you are not filling in the video handoff
struct fully, so I need to disable it for qemu-x86_64.
Here is the trace. You can try it yourself with qemu-x86_64.
qemu-system-x86_64 -m 8G -smp 4 -bios /tmp/b/qemu-x86_64/u-boot.rom
-drive id=fdisk,file=root.img,if=virtio,driver=raw -serial mon:stdio
U-Boot SPL 2023.10-rc1-00119-g53cf4476d95-dirty (Jul 31 2023 - 09:51:11 -0600)
Video: 1024x768x32
Trying to boot from SPI
Jumping to 64-bit U-Boot: Note many features are missing
initcall: 0000000001133cdd
initcall: 000000000118c88a
initcall: 000000000113c550
initcall: 000000000113e006
initcall_run_list() initcall: 000000000113441a
initcall_run_list() initcall: 000000000113c75a
initcall_run_list() initcall: 000000000113ac52
initcall_run_list() initcall: 0000000001133cfe
initcall_run_list() initcall: 00000000011112d8
initcall_run_list() initcall: 0000000001133d01
initcall_run_list() initcall: 0000000001134444
initcall_run_list() initcall: 000000000118f81e
initcall_run_list() initcall: 000000000115c528
initcall_run_list() initcall: 00000000011337ff
initcall_run_list() initcall: 000000000114f33b
initcall_run_list() initcall: 000000000118d8b7
U-Boot 2023.10-rc1-00119-g53cf4476d95-dirty (Jul 31 2023 - 09:51:11 -0600)
initcall_run_list() initcall: 000000000113382b
display_text_info() U-Boot code: 01110000 -> 011C87B0 BSS: -> 011D2524
initcall_run_list() initcall: 0000000001111d3c
initcall_run_list() initcall: 0000000001133872
initcall_run_list() initcall: 0000000001133943
CPU: QEMU Virtual CPU version 2.5+
initcall_run_list() initcall: 0000000001134480
initcall_run_list() initcall: 0000000001133a01
DRAM: initcall_run_list() initcall: 0000000001111332
initcall_run_list() initcall: 0000000001133d07
setup_dest_addr() Monitor len: 000C2524
setup_dest_addr() Ram size: 200000000
setup_dest_addr() Ram top: C0000000
initcall_run_list() initcall: 0000000001133deb
initcall_run_list() initcall: 0000000001133e00
initcall_run_list() initcall: 0000000001133e03
gd->relocaddr=c0000000, ho->fb=d0000000
### ERROR ### Please RESET the board ###
QEMU: Terminated
The tree for this is basically u-boot-dm/bryd-working, just before this patch:
53cf4476d95 (HEAD) Revert "x86: Switch QEMU over to use the bochs driver"
5f78655f29d x86: Run QEMU machine setup in SPL
2952bc2f091 video: Tidy up Makefile rule for video
cd48d152138 x86: spl: Drop unwanted debug()
a4ce5665d55 Revert "efi debug"
530d87910dc efi debug
a36d59ba99a (x86/master, x86-public/master) Merge tag
'efi-2023-10-rc2' of
https://source.denx.de/u-boot/custodians/u-boot-efi
>
> >
> > common/board_f.c | 3 ++-
> > drivers/video/Kconfig | 8 ++++++++
> > 2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/board_f.c b/common/board_f.c
> > index 7d2c380e91e2..5173d0a0c2d5 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;
>
> Ok, so as I understand in your case you have enabled CONFIG_SPL_VIDEO but not
> calling reserve_video in SPL stage ?
Yes, see below.
>
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index b41dc60cec59..e0e07ed0cda5 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -1106,6 +1106,14 @@ 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 video being allocated to U-Boot, thus having some
> {some minor suggestion for correction on above text}
> avoids the memory used for framebuffer from being allocated to u-boot, thus
> preventing any further memory reservations done by u-boot from overwriting the
> framebuffer ?
>
> I think as mentioned earlier, overall this approach looks ok to me too, maybe
> later on we can add some conditions to check validity of passed framebuffer
> address and move the relocaddr pointer more intelligently.
The problem is really that by pre-allocating the display in SPL we
need to excluded it from the reservations done by U-Boot proper. It
should not be done again. For that to work, we may need to tell U-Boot
proper that it should start its reservations at a certain address,
e.g. the bottom of the framebuffer.
Also. could you send a patch to fully fill in the video handoff? At
present it is missing some fields.
Regards,
Simon
