
Hi Simon,
On 03/08/23 19:32, Simon Glass wrote:
+Bin Meng
Hi Devarsh,
On Tue, 1 Aug 2023 at 08:04, Devarsh Thakkar devarsht@ti.com wrote:
When passing framebuffer address using bloblist, check that passed address is overlapping with current relocation address, if so move the relocation address after the framebuffer region to avoid overlap.
Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot") Signed-off-by: Devarsh Thakkar devarsht@ti.com
common/board_f.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 7d2c380e91..20fa17207a 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -419,7 +419,10 @@ static int reserve_video(void) if (!ho) return log_msg_ret("blf", -ENOENT); video_reserve_from_bloblist(ho);
gd->relocaddr = ho->fb;
/* Relocate after framebuffer area to avoid overlap */
if (gd->relocaddr > (unsigned long)ho->fb)
gd->relocaddr = ho->fb;
} else if (CONFIG_IS_ENABLED(VIDEO)) { ulong addr; int ret;
-- 2.34.1
Yes this happens to work with qemu-x86_64. However it depends on the SPL frame buffer being below the current allocation area. Why would it be below, in general? This seems like a land mine for others to trip up on.
Basically to avoid overlap, the thing we are enforcing here is that relocaddr should always be below framebuffer so that further areas reserved do not overlap with framebuffer since we decrement relocaddr to resereve them. If relocaddr is already below framebuffer then there is no need to update relocaddr as in your case otherwise we have to do it, hence working for both scenarios.
The way I look at it is at u-boot proper is getting a blob from SPL with video handoff structure and before carrying out further reservations u-boot proper being agnostic to the framebuffer address set by SPL should have some check to make sure there is no overlap with current relocaddr.
I think this is similar to suggestion from Tom and you regarding moving the ram_top if framebuffer is reserved near the ram_top, albeit instead of moving ram_top I am moving relocaddr which is derived from ram_top.
Kindly let me know if any queries.
Regards Devarsh
Anyway I am OK with this for now since it fixes my problems. I would prefer something positive like the Kconfig I provided, since I still think it is very weird to interrupt the U-Boot reservation process like this.> Reviewed-by: Simon Glass sjg@chromium.org
Regards, Simon