[PATCH 0/7] rockchip: veyron: Synchronize changes for veyron boards
There have been bug fixes and new features for some veyron-based boards which are applicable to others because they share most of the underlying hardware. This series tries to synchronize changes across all veyron boards. I couldn't personally test these myself as I don't have any of the supported boards, and couldn't spent time to port the veyron-mighty I have yet. Sending so others can test and for the first two patches I think should go in v2023.07, but take with a grain of salt. Alper Nebi Yasak (7): rockchip: veyron: Enable RESET driver rockchip: veyron: Enable building SPI ROM images rockchip: veyron: Unify u-boot.dtsi bootph-all fragments rockchip: veyron: Add serial, logging, silent console support rockchip: veyron: Use TrueType fonts rockchip: chromebook_jerry: Re-enable MAX98090 codec driver rockchip: chromebook_speedy: Enable sound arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi | 30 +-- arch/arm/dts/rk3288-veyron-u-boot.dtsi| 4 +++ arch/arm/mach-rockchip/rk3288/Kconfig | 6 configs/chromebit_mickey_defconfig| 6 configs/chromebook_jerry_defconfig| 1 + configs/chromebook_minnie_defconfig | 6 +++- configs/chromebook_speedy_defconfig | 7 + 7 files changed, 43 insertions(+), 17 deletions(-) -- 2.40.1
[PATCH 1/7] rockchip: veyron: Enable RESET driver
Commit 70e351bdfeee ("rockchip: jerry: Enable RESET driver") enables
DM_RESET for chromebook_jerry to fix its display as required by changes
to the Rockchip video drivers. Enable it for other veyron boards as
well.
Fixes: cd529f7ad62 ("rockchip: video: edp: Add missing reset support")
Fixes: 9749d2ea29e ("rockchip: video: vop: Add reset support")
Signed-off-by: Alper Nebi Yasak
---
configs/chromebit_mickey_defconfig | 1 +
configs/chromebook_minnie_defconfig | 1 +
configs/chromebook_speedy_defconfig | 1 +
3 files changed, 3 insertions(+)
diff --git a/configs/chromebit_mickey_defconfig
b/configs/chromebit_mickey_defconfig
index d4302353c5df..a7c6213a9892 100644
--- a/configs/chromebit_mickey_defconfig
+++ b/configs/chromebit_mickey_defconfig
@@ -10,6 +10,7 @@ CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x10
CONFIG_SF_DEFAULT_SPEED=2000
CONFIG_DEFAULT_DEVICE_TREE="rk3288-veyron-mickey"
CONFIG_SPL_TEXT_BASE=0xff704000
+CONFIG_DM_RESET=y
CONFIG_SYS_MONITOR_LEN=614400
CONFIG_ROCKCHIP_RK3288=y
# CONFIG_SPL_MMC is not set
diff --git a/configs/chromebook_minnie_defconfig
b/configs/chromebook_minnie_defconfig
index 73ab2f62af5e..8a4e1858c8bd 100644
--- a/configs/chromebook_minnie_defconfig
+++ b/configs/chromebook_minnie_defconfig
@@ -10,6 +10,7 @@ CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x10
CONFIG_SF_DEFAULT_SPEED=2000
CONFIG_DEFAULT_DEVICE_TREE="rk3288-veyron-minnie"
CONFIG_SPL_TEXT_BASE=0xff704000
+CONFIG_DM_RESET=y
CONFIG_SYS_MONITOR_LEN=614400
CONFIG_ROCKCHIP_RK3288=y
# CONFIG_SPL_MMC is not set
diff --git a/configs/chromebook_speedy_defconfig
b/configs/chromebook_speedy_defconfig
index 06437aae18d6..45c22f5b103a 100644
--- a/configs/chromebook_speedy_defconfig
+++ b/configs/chromebook_speedy_defconfig
@@ -10,6 +10,7 @@ CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x10
CONFIG_SF_DEFAULT_SPEED=2000
CONFIG_DEFAULT_DEVICE_TREE="rk3288-veyron-speedy"
CONFIG_SPL_TEXT_BASE=0xff704000
+CONFIG_DM_RESET=y
CONFIG_SYS_MONITOR_LEN=614400
CONFIG_ROCKCHIP_RK3288=y
# CONFIG_SPL_MMC is not set
--
2.40.1
[PATCH 2/7] rockchip: veyron: Enable building SPI ROM images
Commit 9b312e26fc77 ("rockchip: Enable building a SPI ROM image on
jerry") produces a u-boot.rom file for chromebook_jerry, intended to be
written to SPI flash. Build this file for other veyron boards as well,
especially because they are already configured only to boot from SPI.
Signed-off-by: Alper Nebi Yasak
---
arch/arm/mach-rockchip/rk3288/Kconfig | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3288/Kconfig
b/arch/arm/mach-rockchip/rk3288/Kconfig
index ea94ad114247..69a5614b449a 100644
--- a/arch/arm/mach-rockchip/rk3288/Kconfig
+++ b/arch/arm/mach-rockchip/rk3288/Kconfig
@@ -16,7 +16,9 @@ config TARGET_CHROMEBOOK_JERRY
config TARGET_CHROMEBIT_MICKEY
bool "Google/Rockchip Veyron-Mickey Chromebit"
+ select HAS_ROM
select BOARD_LATE_INIT
+ select ROCKCHIP_SPI_IMAGE
help
Mickey is a small RK3288-based device with one USB 3.0 port, HDMI
and WiFi. It has a separate power port and is designed to connect
@@ -26,7 +28,9 @@ config TARGET_CHROMEBIT_MICKEY
config TARGET_CHROMEBOOK_MINNIE
bool "Google/Rockchip Veyron-Minnie Chromebook"
+ select HAS_ROM
select BOARD_LATE_INIT
+ select ROCKCHIP_SPI_IMAGE
help
Minnie is a RK3288-based convertible clamshell device with 2 USB 3.0
ports, micro HDMI, a 10.1-inch 1280x800 EDP display, micro-SD card,
@@ -37,7 +41,9 @@ config TARGET_CHROMEBOOK_MINNIE
config TARGET_CHROMEBOOK_SPEEDY
bool "Google/Rockchip Veyron-Speedy Chromebook"
+ select HAS_ROM
select BOARD_LATE_INIT
+ select ROCKCHIP_SPI_IMAGE
help
Speedy is a RK3288-based clamshell device with 2 USB 2.0 ports,
micro HDMI, an 11.6 inch display, micro-SD card,
--
2.40.1
[PATCH 3/7] rockchip: veyron: Unify u-boot.dtsi bootph-all fragments
The rk3288-veyron-speedy-u-boot.dtsi file duplicates the bootphase dts
fragments from rk3288-veyron-u-boot.dtsi even though it #inclues that.
Deduplicate these into the latter file, which should also make the eMMC
available to the other veyron boards' SPL.
Signed-off-by: Alper Nebi Yasak
---
arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi | 16
arch/arm/dts/rk3288-veyron-u-boot.dtsi| 4
2 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
index 90ce9e1395de..2a4ba44e0bf9 100644
--- a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
+++ b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
@@ -15,19 +15,3 @@ &dmc {
0x0 0xc3 0x6 0x1>;
rockchip,sdram-params = <0x20D266A4 0x5B6 6 53300 6 13 0>;
};
-
-&sdmmc {
- bootph-all;
-};
-
-&emmc {
- bootph-all;
-};
-
-&uart2 {
- bootph-all;
-};
-
-&pinctrl {
- bootph-all;
-};
diff --git a/arch/arm/dts/rk3288-veyron-u-boot.dtsi
b/arch/arm/dts/rk3288-veyron-u-boot.dtsi
index ab564e73ed00..4f9c59c67573 100644
--- a/arch/arm/dts/rk3288-veyron-u-boot.dtsi
+++ b/arch/arm/dts/rk3288-veyron-u-boot.dtsi
@@ -31,6 +31,10 @@ &dmc {
>;
};
+&emmc {
+ bootph-all;
+};
+
&gpio3 {
bootph-all;
};
--
2.40.1
[PATCH 4/7] rockchip: veyron: Add serial, logging, silent console support
Commit eba768c54587 ("rockchip: jerry: Add serial support") enables
ROCKCHIP_SERIAL for chromebook_jerry to make the serial console work
correctly. Enable it also for other veyron boards.
Also enable logging and disable scrolling multiple lines at once as in
chromebook_jerry, and enable silent console as chromebook_minnie does.
Signed-off-by: Alper Nebi Yasak
---
configs/chromebit_mickey_defconfig | 3 +++
configs/chromebook_minnie_defconfig | 3 ++-
configs/chromebook_speedy_defconfig | 1 +
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/configs/chromebit_mickey_defconfig
b/configs/chromebit_mickey_defconfig
index a7c6213a9892..f45b14b9d3d1 100644
--- a/configs/chromebit_mickey_defconfig
+++ b/configs/chromebit_mickey_defconfig
@@ -26,6 +26,8 @@ CONFIG_SPL_PAYLOAD="u-boot.img"
CONFIG_DEBUG_UART=y
CONFIG_USE_PREBOOT=y
CONFIG_DEFAULT_FDT_FILE="rk3288-veyron-mickey.dtb"
+CONFIG_SILENT_CONSOLE=y
+CONFIG_LOG=y
# CONFIG_DISPLAY_CPUINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_BOARD_EARLY_INIT_R=y
@@ -96,6 +98,7 @@ CONFIG_RAM=y
CONFIG_SPL_RAM=y
CONFIG_DEBUG_UART_SHIFT=2
CONFIG_SYS_NS16550_MEM32=y
+CONFIG_ROCKCHIP_SERIAL=y
CONFIG_ROCKCHIP_SPI=y
CONFIG_SYSRESET=y
CONFIG_USB=y
diff --git a/configs/chromebook_minnie_defconfig
b/configs/chromebook_minnie_defconfig
index 8a4e1858c8bd..01964d13754e 100644
--- a/configs/chromebook_minnie_defconfig
+++ b/configs/chromebook_minnie_defconfig
@@ -27,6 +27,7 @@ CONFIG_DEBUG_UART=y
CONFIG_USE_PREBOOT=y
CONFIG_DEFAULT_FDT_FILE="rk3288-veyron-minnie.dtb"
CONFIG_SILENT_CONSOLE=y
+CONFIG_LOG=y
# CONFIG_DISPLAY_CPUINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_BOARD_EARLY_INIT_R=y
@@ -98,6 +99,7 @@ CONFIG_RAM=y
CONFIG_SPL_RAM=y
CONFIG_DEBUG_UART_SHIFT=2
CONFIG_SYS_NS16550_MEM32=y
+CONFIG_ROCKCHIP_SERIAL=y
CONFIG_SOUND=y
CONFIG_I2S=y
CONFIG_I2S_ROCKCHIP=y
@@ -114,7 +116,6 @@ CONFIG_DISPLAY=y
CONFIG_VIDEO_ROCKCHIP=y
CONFIG_DISPLAY_ROCKCHIP_EDP=y
CONFIG_DISPLAY_ROCKCHIP_HDMI=y
-CONFIG_CONSOLE_SCROLL_LINES=10
CONFIG_SPL_TINY_MEMSET=y
CONFIG_CMD_DHRYSTONE=y
CONFIG_ERRNO_STR=y
diff --git a/configs/chromebook_speedy_defconfig
b/configs/chromebook_speedy_defconfig
index 45c22f5b103a..f8f2a280f6cf 100644
--- a/configs/chromebook_speedy_defconfig
+++ b/configs/chromebook_speedy_defconfig
@@ -27,6 +27,7 @@ CONFIG_DEBUG_UART=y
CONFIG_USE_PREBOOT=y
CONFIG_DEFAULT_FDT_FILE="rk3288-veyron-speedy.dtb"
CONFIG_SILENT_CONSOLE=y
+CONFIG_LOG=y
# CONFIG_DISPLAY_CPUINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_BOARD_EARLY_INIT_R=y
--
2.40.1
[PATCH 5/7] rockchip: veyron: Use TrueType fonts
Commit 815ed79d8338 ("video: rockchip: Use TrueType fonts with jerry")
enables makes chromebook_jerry use TrueType fonts. Make other veyron
boards switch to it as well.
Signed-off-by: Alper Nebi Yasak
---
I have no idea why that commit disables USE_PRIVATE_LIBGCC, but I'm
following that change here.
configs/chromebit_mickey_defconfig | 2 ++
configs/chromebook_minnie_defconfig | 2 ++
2 files changed, 4 insertions(+)
diff --git a/configs/chromebit_mickey_defconfig
b/configs/chromebit_mickey_defconfig
index f45b14b9d3d1..81efa0fd4cc6 100644
--- a/configs/chromebit_mickey_defconfig
+++ b/configs/chromebit_mickey_defconfig
@@ -107,9 +107,11 @@ CONFIG_USB_DWC2=y
CONFIG_ROCKCHIP_USB2_PHY=y
CONFIG_VIDEO=y
# CONFIG_VIDEO_BPP8 is not set
+CONFIG_CONSOLE_TRUETYPE=y
CONFIG_DISPLAY=y
CONFIG_VIDEO_ROCKCHIP=y
CONFIG_DISPLAY_ROCKCHIP_HDMI=y
+# CONFIG_USE_PRIVATE_LIBGCC is not set
CONFIG_SPL_TINY_MEMSET=y
CONFIG_CMD_DHRYSTONE=y
CONFIG_ERRNO_STR=y
diff --git a/configs/chromebook_minnie_defconfig
b/configs/chromebook_minnie_defconfig
index 01964d13754e..e2302074e8b3 100644
--- a/configs/chromebook_minnie_defconfig
+++ b/configs/chromebook_minnie_defconfig
@@ -112,10 +112,12 @@ CONFIG_USB_DWC2=y
CONFIG_ROCKCHIP_USB2_PHY=y
CONFIG_VIDEO=y
# CONFIG_VIDEO_BPP8 is not set
+CONFIG_CONSOLE_TRUETYPE=y
CONFIG_DISPLAY=y
CONFIG_VIDEO_ROCKCHIP=y
CONFIG_DISPLAY_ROCKCHIP_EDP=y
CONFIG_DISPLAY_ROCKCHIP_HDMI=y
+# CONFIG_USE_PRIVATE_LIBGCC is not set
CONFIG_SPL_TINY_MEMSET=y
CONFIG_CMD_DHRYSTONE=y
CONFIG_ERRNO_STR=y
--
2.40.1
[PATCH 6/7] rockchip: chromebook_jerry: Re-enable MAX98090 codec driver
Sound support for chromebook_jerry needs the MAX98090 codec driver. This
was enabled in commit 2d0c01b8f0ad ("sound: rockchip: Add sound support
for jerry"), but apparently lost in commit 7ae2729401bb ("configs:
Resync with savedefconfig"). Enable it again.
Signed-off-by: Alper Nebi Yasak
---
configs/chromebook_jerry_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/configs/chromebook_jerry_defconfig
b/configs/chromebook_jerry_defconfig
index 1a54986d089e..5b601a0b5809 100644
--- a/configs/chromebook_jerry_defconfig
+++ b/configs/chromebook_jerry_defconfig
@@ -102,6 +102,7 @@ CONFIG_ROCKCHIP_SERIAL=y
CONFIG_SOUND=y
CONFIG_I2S=y
CONFIG_I2S_ROCKCHIP=y
+CONFIG_SOUND_MAX98090=y
CONFIG_ROCKCHIP_SPI=y
CONFIG_SYSRESET=y
CONFIG_USB=y
--
2.40.1
[PATCH 7/7] rockchip: chromebook_speedy: Enable sound
Commit ec107f04b619 ("rockchip: chromebook_minnie: Enable sound") and
commit 2d0c01b8f0ad ("sound: rockchip: Add sound support for jerry")
enable audio support for chromebook_minnie and chromebook_jerry. Enable
it for chromebook_speedy as well, but put the non-upstream sound node
in the board -u-boot.dtsi instead.
Signed-off-by: Alper Nebi Yasak
---
Not enabling these for chromebit_mickey, as that'd use HDMI in a way I
don't know how.
arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi | 14 ++
configs/chromebook_speedy_defconfig | 5 +
2 files changed, 19 insertions(+)
diff --git a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
index 2a4ba44e0bf9..6bfa84906e35 100644
--- a/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
+++ b/arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi
@@ -5,6 +5,20 @@
#include "rk3288-veyron-u-boot.dtsi"
+/ {
+ sound {
+ compatible = "rockchip,audio-max98090-jerry";
+
+ cpu {
+ sound-dai = <&i2s 0>;
+ };
+
+ codec {
+ sound-dai = <&max98090 0>;
+ };
+ };
+};
+
&dmc {
rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d
0x6 0x0 0x8 0x4 0x17 0x24 0xd 0x6
diff --git a/configs/chromebook_speedy_defconfig
b/configs/chromebook_speedy_defconfig
index f8f2a280f6cf..15b6e21095db 100644
--- a/configs/chromebook_speedy_defconfig
+++ b/configs/chromebook_speedy_defconfig
@@ -52,6 +52,7 @@ CONFIG_CMD_USB=y
# CONFIG_CMD_SETEXPR is not set
CONFIG_CMD_CACHE=y
CONFIG_CMD_TIME=y
+CONFIG_CMD_SOUND=y
CONFIG_CMD_PMIC=y
CONFIG_CMD_REGULATOR=y
# CONFIG_SPL_DOS_PARTITION is not set
@@ -99,6 +100,10 @@ CONFIG_SPL_RAM=y
CONFIG_DEBUG_UART_SHIFT=2
CONFIG_SYS_NS16550_MEM32=y
CONFIG_ROCKCHIP_SERIAL=y
+CONFIG_SOUND=y
+CONFIG_I2S=y
+CONFIG_I2S_ROCKCHIP=y
+CONFIG_SOUND_MAX98090=y
CONFIG_ROCKCHIP_SPI=y
CONFIG_SYSRESET=y
CONFIG_USB=y
--
2.40.1
[PATCH] efi_loader: Increase default variable store size to 32K
Debian's arm64 UEFI Secure Boot shim makes the EFI variable store run
out of space while mirroring its MOK database to variables. This can be
observed in QEMU like so:
$ tools/buildman/buildman -o build/qemu_arm64 --boards=qemu_arm64 -w
$ cd build/qemu_arm64
$ curl -L -o debian.iso \
https://cdimage.debian.org/debian-cd/current/arm64/iso-cd/debian-12.0.0-arm64-netinst.iso
$ qemu-system-aarch64 \
-nographic -bios u-boot.bin \
-machine virt -cpu cortex-a53 -m 1G -smp 2 \
-drive
if=virtio,file=debian.iso,index=0,format=raw,readonly=on,media=cdrom
[...]
=> # interrupt autoboot
=> env set -e -bs -nv -rt -guid 605dab50-e046-4300-abb6-3dd810dd8b23
SHIM_VERBOSE 1
=> boot
[...]
mok.c:296:mirror_one_esl() SetVariable("MokListXRT43", ... varsz=0x4C) = Out
of Resources
mok.c:452:mirror_mok_db() esd:0x7DB92D20 adj:0x30
Failed to set MokListXRT: Out of Resources
mok.c:767:mirror_one_mok_variable() mirror_mok_db("MokListXRT",
datasz=17328) returned Out of Resources
mok.c:812:mirror_one_mok_variable() returning Out of Resources
Could not create MokListXRT: Out of Resources
[...]
Welcome to GRUB!
This would normally be fine as shim would continue to run grubaa64.efi,
but shim's error handling code for this case has a bug [1] that causes a
synchronous abort on at least chromebook_kevin (but apparently not on
QEMU arm64).
Double the default variable store size so the variables fit. There is a
note about this value matching PcdFlashNvStorageVariableSize when
EFI_MM_COMM_TEE is enabled, so keep the old default in that case.
[1] https://github.com/rhboot/shim/pull/577
Signed-off-by: Alper Nebi Yasak
---
I'm not very familiar with EFI things, apologies if this default
should not be changed (consider this a bug report in that case).
lib/efi_loader/Kconfig | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index c5835e6ef61a..0660d1174902 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -96,7 +96,8 @@ endif
config EFI_VAR_BUF_SIZE
int "Memory size of the UEFI variable store"
- default 16384
+ default 16384 if EFI_MM_COMM_TEE
+ default 32768
range 4096 2147483647
help
This defines the size in bytes of the memory area reserved for keeping
@@ -106,7 +107,7 @@ config EFI_VAR_BUF_SIZE
match the value of PcdFlashNvStorageVariableSize used to compile the
StandAloneMM module.
- Minimum 4096, default 16384.
+ Minimum 4096, default 32768, or 16384 when using StandAloneMM.
config EFI_GET_TIME
bool "GetTime() runtime service"
--
2.40.1
[PATCH] efi_loader: Avoid underflow when calculating remaining var store size
The efi_var_mem_free() function calculates the available size for a new
EFI variable by subtracting the occupied buffer size and the overhead
for a new variable from the maximum buffer size set in Kconfig. This
is then returned as QueryVariableInfo()'s RemainingVariableStorageSize
output.
This can underflow as the calculation is done in and processed as
unsigned integer types. Check for underflow before doing the subtraction
and return zero if there's no space.
Signed-off-by: Alper Nebi Yasak
---
lib/efi_loader/efi_var_mem.c | 4
1 file changed, 4 insertions(+)
diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
index d6b65aed12ea..5fa7dcb8d3ed 100644
--- a/lib/efi_loader/efi_var_mem.c
+++ b/lib/efi_loader/efi_var_mem.c
@@ -177,6 +177,10 @@ efi_status_t __efi_runtime efi_var_mem_ins(
u64 __efi_runtime efi_var_mem_free(void)
{
+ if (efi_var_buf->length + sizeof(struct efi_var_entry) >=
+ EFI_VAR_BUF_SIZE)
+ return 0;
+
return EFI_VAR_BUF_SIZE - efi_var_buf->length -
sizeof(struct efi_var_entry);
}
--
2.40.1
Re: [PATCH 2/2] schemas: Add a schema for binman
Hi Simon,
I know I haven't been able to look at binman stuff for a long time, but
I've been occasionally thinking about it through the course of a year
and think binman severely needs a design-wise review before things get
entrenched, even in the most fundamental parts. I do see the
cross-project collaboration intention here, but still...
There's also the issue of binman having multiple different device-trees:
its input, the ones in fdtmaps per image, the ones injected to U-Boot
dtb files per image. I'd say each has different needs, and those
differences have to be figured out before specified upstream. I can only
guess this is about the one that'll be in a u-boot.dtb.
I want to go through binman more thoroughly, but a lot of changes
happened since I last looked at it and I'm a bit slow at writing things,
so won't exactly be soon. That being said, here's some ideas off the top
of my head, for inspiration on both this schema and binman itself.
On 2023-07-12 00:18 +03:00, Simon Glass wrote:
> I am unsure whether to add this with a generic name, such as 'layout',
> but for now am using /firmware/binman to avoid conflicts with any other
> firmware-layout schema that others might be working on.
>
> Signed-off-by: Simon Glass
> ---
I've been thinking of compatible = "data," for entries, so
proposing 'data' here. A big ask, but it might be the one schema to
unify them all if we look at things as "description of data" instead of
"firmware layout".
Also consider data layouts in-memory. For example it could be an
alternative to /chosen linux,initrd-start to specify initramfs location.
Or things like keys or logs received from previous stages / other parts?
Weak examples, but maybe might connect better into firmware handoff
things. (Sorry, I don't know much about those.)
I'm also thinking about things like JPEG/BMP files for ACPI BGRT-like
boot splash, unique firmware/configuration/calibration data for drivers,
but they don't exactly need to be in-memory I guess.
>
> dtschema/schemas/firmware/binman.yaml | 51 ++
> dtschema/schemas/firmware/binman/entry.yaml | 57 +
> 2 files changed, 108 insertions(+)
> create mode 100644 dtschema/schemas/firmware/binman.yaml
> create mode 100644 dtschema/schemas/firmware/binman/entry.yaml
>
> diff --git a/dtschema/schemas/firmware/binman.yaml
> b/dtschema/schemas/firmware/binman.yaml
> new file mode 100644
> index 000..4b1ecf6
> --- /dev/null
> +++ b/dtschema/schemas/firmware/binman.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Google LLC
> +
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/binman.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Binman firmware layout
> +
> +maintainers:
> + - Simon Glass
> +
> +description: |
> + The binman node provides a layout for firmware, used when packaging
> firmware
> + from multiple projects. For now it just supports a very simple set of
> + features, as a starting point for discussion.
> +
> + Documentation for Binman is available at:
> +
> + https://u-boot.readthedocs.io/en/latest/develop/package/binman.html
> +
> + with the current image-description format at:
> +
> +
> https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#image-description-format
> +
> +properties:
> + $nodename:
> +const: binman
I think having a single /firmware/binman node for a single image *and*
having a "multiple-images" mode under it is quite bad because the
single-image mode is multiple-images with one image. Multiple images
should be the only mode of operation
Furthermore, each image should be able to function independently of
others, so I think to help enforce that maybe there shouldn't be any
formal binman node. Merely instances of images that may happen to be
under one parent node that may happen to be called /firmware/binman. The
images can be compatible = "data,image" or something and we could search
by that.
Or, heh, a single image node that happens to be named /firmware/binman
because we are embedding it into a u-boot.dtb in that image. But perhaps
we shouldn't restrain things like that. A bit contrived, but why should
we be unable to start from an image in SPI and make it load something
from microSD?
Going even further, let me suggest putting the image nodes as children
of the devices whose layout the image describes. Then I imagine we can
have something like a "data driver" (both for U-Boot and Linux) that
reads the entry data from the parent data/block device and makes it
available for other drivers or in sysfs.
> +
> +required:
> + - compatible
> +
> +additionalProperties: false
Including the binman version or a version number for the spec might be
useful later on.
> +
> +examples:
> + - |
> + - |
> +firmware {
> + binman {
> +compatible = "u-boot,binman";
Consider including "image" or "namespace" in the compatible string to
disambi
[PATCH] rockchip: veyron: Enable Winbond SPI flash
Some veyron boards seem to have Winbond SPI flash chips instead of GigaDevice ones. At the very least, coreboot builds for veyron boards have them enabled [1]. Enable support for them here as well. [1] https://review.coreboot.org/c/coreboot/+/9719 Signed-off-by: Alper Nebi Yasak --- configs/chromebit_mickey_defconfig | 1 + configs/chromebook_jerry_defconfig | 1 + configs/chromebook_minnie_defconfig | 1 + configs/chromebook_speedy_defconfig | 1 + 4 files changed, 4 insertions(+) diff --git a/configs/chromebit_mickey_defconfig b/configs/chromebit_mickey_defconfig index d4302353c5df..253ef99f9939 100644 --- a/configs/chromebit_mickey_defconfig +++ b/configs/chromebit_mickey_defconfig @@ -81,6 +81,7 @@ CONFIG_MMC_DW_ROCKCHIP=y CONFIG_MTD=y CONFIG_SF_DEFAULT_BUS=2 CONFIG_SPI_FLASH_GIGADEVICE=y +CONFIG_SPI_FLASH_WINBOND=y CONFIG_PINCTRL=y CONFIG_PINCONF=y CONFIG_SPL_PINCTRL=y diff --git a/configs/chromebook_jerry_defconfig b/configs/chromebook_jerry_defconfig index 1a54986d089e..3172f04a2648 100644 --- a/configs/chromebook_jerry_defconfig +++ b/configs/chromebook_jerry_defconfig @@ -84,6 +84,7 @@ CONFIG_MMC_DW_ROCKCHIP=y CONFIG_MTD=y CONFIG_SF_DEFAULT_BUS=2 CONFIG_SPI_FLASH_GIGADEVICE=y +CONFIG_SPI_FLASH_WINBOND=y CONFIG_PINCTRL=y CONFIG_PINCONF=y CONFIG_SPL_PINCTRL=y diff --git a/configs/chromebook_minnie_defconfig b/configs/chromebook_minnie_defconfig index 73ab2f62af5e..25a56f45fe6c 100644 --- a/configs/chromebook_minnie_defconfig +++ b/configs/chromebook_minnie_defconfig @@ -83,6 +83,7 @@ CONFIG_MMC_DW_ROCKCHIP=y CONFIG_MTD=y CONFIG_SF_DEFAULT_BUS=2 CONFIG_SPI_FLASH_GIGADEVICE=y +CONFIG_SPI_FLASH_WINBOND=y CONFIG_PINCTRL=y CONFIG_PINCONF=y CONFIG_SPL_PINCTRL=y diff --git a/configs/chromebook_speedy_defconfig b/configs/chromebook_speedy_defconfig index 06437aae18d6..ff2a12b25c34 100644 --- a/configs/chromebook_speedy_defconfig +++ b/configs/chromebook_speedy_defconfig @@ -82,6 +82,7 @@ CONFIG_MMC_DW_ROCKCHIP=y CONFIG_MTD=y CONFIG_SF_DEFAULT_BUS=2 CONFIG_SPI_FLASH_GIGADEVICE=y +CONFIG_SPI_FLASH_WINBOND=y CONFIG_PINCTRL=y CONFIG_PINCONF=y CONFIG_SPL_PINCTRL=y -- 2.40.1
[PATCH v5 00/13] Add video damage tracking
ons to all vidconsole drivers video: Add damage notification on bmp display efi_loader: GOP: Add damage notification on BLT video: Only dcache flush damaged lines video: Use VIDEO_DAMAGE for VIDEO_COPY video: Always compile cache flushing code video: Enable VIDEO_DAMAGE for drivers that need it Alper Nebi Yasak (4): video: test: Split copy frame buffer check into a function video: test: Support checking copy frame buffer contents video: test: Test partial updates of hardware frame buffer video: test: Test video damage tracking via vidconsole arch/arm/mach-omap2/omap3/Kconfig | 1 + arch/arm/mach-sunxi/Kconfig | 1 + drivers/video/Kconfig | 26 +++ drivers/video/console_normal.c| 27 ++-- drivers/video/console_rotate.c| 94 +++ drivers/video/console_truetype.c | 37 +++-- drivers/video/exynos/Kconfig | 1 + drivers/video/imx/Kconfig | 1 + drivers/video/meson/Kconfig | 1 + drivers/video/rockchip/Kconfig| 1 + drivers/video/stm32/Kconfig | 1 + drivers/video/tegra20/Kconfig | 1 + drivers/video/tidss/Kconfig | 1 + drivers/video/vidconsole-uclass.c | 16 -- drivers/video/video-uclass.c | 190 -- drivers/video/video_bmp.c | 7 +- include/video.h | 59 +++ include/video_console.h | 52 -- lib/efi_loader/efi_gop.c | 7 + test/dm/video.c | 256 -- 20 files changed, 483 insertions(+), 297 deletions(-) base-commit: 3881c9fbb7fdd98f6eae5cd33f7e9abe9455a585 -- 2.40.1
[PATCH v5 01/13] video: test: Split copy frame buffer check into a function
While checking frame buffer contents, the video tests also check if the
copy frame buffer contents match the main frame buffer. To test if only
the modified regions are updated after a sync, we will need to create
situations where the two are mismatched. Split this check into another
function that we can skip calling, since we won't want it to error on
those mismatched cases.
Signed-off-by: Alper Nebi Yasak
---
Changes in v5:
- Add patch "video: test: Split copy frame buffer check into a function"
test/dm/video.c | 69 +
1 file changed, 58 insertions(+), 11 deletions(-)
diff --git a/test/dm/video.c b/test/dm/video.c
index d907f681600b..641a6250100a 100644
--- a/test/dm/video.c
+++ b/test/dm/video.c
@@ -55,9 +55,6 @@ DM_TEST(dm_test_video_base, UT_TESTF_SCAN_PDATA |
UT_TESTF_SCAN_FDT);
* size of the compressed data. This provides a pretty good level of
* certainty and the resulting tests need only check a single value.
*
- * If the copy framebuffer is enabled, this compares it to the main framebuffer
- * too.
- *
* @uts: Test state
* @dev: Video device
* Return: compressed size of the frame buffer, or -ve on error
@@ -66,7 +63,6 @@ static int compress_frame_buffer(struct unit_test_state *uts,
struct udevice *dev)
{
struct video_priv *priv = dev_get_uclass_priv(dev);
- struct video_priv *uc_priv = dev_get_uclass_priv(dev);
uint destlen;
void *dest;
int ret;
@@ -82,16 +78,34 @@ static int compress_frame_buffer(struct unit_test_state
*uts,
if (ret)
return ret;
- /* Check here that the copy frame buffer is working correctly */
- if (IS_ENABLED(CONFIG_VIDEO_COPY)) {
- ut_assertf(!memcmp(uc_priv->fb, uc_priv->copy_fb,
- uc_priv->fb_size),
- "Copy framebuffer does not match fb");
- }
-
return destlen;
}
+/**
+ * check_copy_frame_buffer() - Compare main frame buffer to copy
+ *
+ * If the copy frame buffer is enabled, this compares it to the main
+ * frame buffer. Normally they should have the same contents after a
+ * sync.
+ *
+ * @uts: Test state
+ * @dev: Video device
+ * Return: 0, or -ve on error
+ */
+static int check_copy_frame_buffer(struct unit_test_state *uts,
+ struct udevice *dev)
+{
+ struct video_priv *priv = dev_get_uclass_priv(dev);
+
+ if (!IS_ENABLED(CONFIG_VIDEO_COPY))
+ return 0;
+
+ ut_assertf(!memcmp(priv->fb, priv->copy_fb, priv->fb_size),
+ "Copy framebuffer does not match fb");
+
+ return 0;
+}
+
/*
* Call this function at any point to halt and show the current display. Be
* sure to run the test with the -l flag.
@@ -155,24 +169,30 @@ static int dm_test_video_text(struct unit_test_state *uts)
ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con));
ut_assertok(vidconsole_select_font(con, "8x16", 0));
ut_asserteq(46, compress_frame_buffer(uts, dev));
+ ut_assertok(check_copy_frame_buffer(uts, dev));
ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con));
vidconsole_putc_xy(con, 0, 0, 'a');
ut_asserteq(79, compress_frame_buffer(uts, dev));
+ ut_assertok(check_copy_frame_buffer(uts, dev));
vidconsole_putc_xy(con, 0, 0, ' ');
ut_asserteq(46, compress_frame_buffer(uts, dev));
+ ut_assertok(check_copy_frame_buffer(uts, dev));
for (i = 0; i < 20; i++)
vidconsole_putc_xy(con, VID_TO_POS(i * 8), 0, ' ' + i);
ut_asserteq(273, compress_frame_buffer(uts, dev));
+ ut_assertok(check_copy_frame_buffer(uts, dev));
vidconsole_set_row(con, 0, WHITE);
ut_asserteq(46, compress_frame_buffer(uts, dev));
+ ut_assertok(check_copy_frame_buffer(uts, dev));
for (i = 0; i < 20; i++)
vidconsole_putc_xy(con, VID_TO_POS(i * 8), 0, ' ' + i);
ut_asserteq(273, compress_frame_buffer(uts, dev));
+ ut_assertok(check_copy_frame_buffer(uts, dev));
return 0;
}
@@ -191,24 +211,30 @@ static int dm_test_video_text_12x22(struct
unit_test_state *uts)
ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con));
ut_assertok(vidconsole_select_font(con, "12x22", 0));
ut_asserteq(46, compress_frame_buffer(uts, dev));
+ ut_assertok(check_copy_frame_buffer(uts, dev));
ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con));
vidconsole_putc_xy(con, 0, 0, 'a');
ut_asserteq(89, compress_frame_buffer(uts, dev));
+ ut_assertok(check_copy_frame_buffer(uts, dev));
vidconsole_putc_xy(con, 0, 0, '
[PATCH v5 02/13] video: test: Support checking copy frame buffer contents
The video tests have a helper function to generate a pseudo-digest of
frame buffer contents, but it only does so for the main one. There is
another check that the copy frame buffer is the same as that. But
neither is enough to test if only the modified regions are copied to the
copy frame buffer, since we will want the two to be different in very
specific ways.
Add a boolean argument to the existing helper function to indicate which
frame buffer we want to inspect, and update the existing callers.
Signed-off-by: Alper Nebi Yasak
---
Changes in v5:
- Add patch "video: test: Support checking copy frame buffer contents"
test/dm/video.c | 76 ++---
1 file changed, 41 insertions(+), 35 deletions(-)
diff --git a/test/dm/video.c b/test/dm/video.c
index 641a6250100a..b9ff3da10c18 100644
--- a/test/dm/video.c
+++ b/test/dm/video.c
@@ -57,22 +57,28 @@ DM_TEST(dm_test_video_base, UT_TESTF_SCAN_PDATA |
UT_TESTF_SCAN_FDT);
*
* @uts: Test state
* @dev: Video device
+ * @use_copy: Use copy frame buffer if available
* Return: compressed size of the frame buffer, or -ve on error
*/
static int compress_frame_buffer(struct unit_test_state *uts,
-struct udevice *dev)
+struct udevice *dev,
+bool use_copy)
{
struct video_priv *priv = dev_get_uclass_priv(dev);
uint destlen;
void *dest;
int ret;
+ if (!IS_ENABLED(CONFIG_VIDEO_COPY))
+ use_copy = false;
+
destlen = priv->fb_size;
dest = malloc(priv->fb_size);
if (!dest)
return -ENOMEM;
ret = BZ2_bzBuffToBuffCompress(dest, &destlen,
- priv->fb, priv->fb_size,
+ use_copy ? priv->copy_fb : priv->fb,
+ priv->fb_size,
3, 0, 0);
free(dest);
if (ret)
@@ -168,30 +174,30 @@ static int dm_test_video_text(struct unit_test_state *uts)
ut_assertok(video_get_nologo(uts, &dev));
ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con));
ut_assertok(vidconsole_select_font(con, "8x16", 0));
- ut_asserteq(46, compress_frame_buffer(uts, dev));
+ ut_asserteq(46, compress_frame_buffer(uts, dev, false));
ut_assertok(check_copy_frame_buffer(uts, dev));
ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con));
vidconsole_putc_xy(con, 0, 0, 'a');
- ut_asserteq(79, compress_frame_buffer(uts, dev));
+ ut_asserteq(79, compress_frame_buffer(uts, dev, false));
ut_assertok(check_copy_frame_buffer(uts, dev));
vidconsole_putc_xy(con, 0, 0, ' ');
- ut_asserteq(46, compress_frame_buffer(uts, dev));
+ ut_asserteq(46, compress_frame_buffer(uts, dev, false));
ut_assertok(check_copy_frame_buffer(uts, dev));
for (i = 0; i < 20; i++)
vidconsole_putc_xy(con, VID_TO_POS(i * 8), 0, ' ' + i);
- ut_asserteq(273, compress_frame_buffer(uts, dev));
+ ut_asserteq(273, compress_frame_buffer(uts, dev, false));
ut_assertok(check_copy_frame_buffer(uts, dev));
vidconsole_set_row(con, 0, WHITE);
- ut_asserteq(46, compress_frame_buffer(uts, dev));
+ ut_asserteq(46, compress_frame_buffer(uts, dev, false));
ut_assertok(check_copy_frame_buffer(uts, dev));
for (i = 0; i < 20; i++)
vidconsole_putc_xy(con, VID_TO_POS(i * 8), 0, ' ' + i);
- ut_asserteq(273, compress_frame_buffer(uts, dev));
+ ut_asserteq(273, compress_frame_buffer(uts, dev, false));
ut_assertok(check_copy_frame_buffer(uts, dev));
return 0;
@@ -210,30 +216,30 @@ static int dm_test_video_text_12x22(struct
unit_test_state *uts)
ut_assertok(video_get_nologo(uts, &dev));
ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con));
ut_assertok(vidconsole_select_font(con, "12x22", 0));
- ut_asserteq(46, compress_frame_buffer(uts, dev));
+ ut_asserteq(46, compress_frame_buffer(uts, dev, false));
ut_assertok(check_copy_frame_buffer(uts, dev));
ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con));
vidconsole_putc_xy(con, 0, 0, 'a');
- ut_asserteq(89, compress_frame_buffer(uts, dev));
+ ut_asserteq(89, compress_frame_buffer(uts, dev, false));
ut_assertok(check_copy_frame_buffer(uts, dev));
vidconsole_putc_xy(con, 0, 0, ' ');
- ut_asserteq(46, compress_frame_buffer(uts, dev));
+ ut_asserteq(46, compress_frame_buffer(uts, dev, false));
ut_assertok(check_copy_frame_buffer(uts, dev));
for (i = 0; i < 20; i++)
[PATCH v5 03/13] video: test: Test partial updates of hardware frame buffer
With VIDEO_COPY enabled, only the modified parts of the frame buffer are
intended to be copied to the hardware. Add a test that checks this, by
overwriting contents we prepared without telling the video uclass and
then checking if the overwritten contents have been redrawn on the next
sync.
Signed-off-by: Alper Nebi Yasak
---
Changes in v5:
- Add patch "video: test: Test partial updates of hardware frame buffer"
test/dm/video.c | 54 +
1 file changed, 54 insertions(+)
diff --git a/test/dm/video.c b/test/dm/video.c
index b9ff3da10c18..e4bd27a6b76f 100644
--- a/test/dm/video.c
+++ b/test/dm/video.c
@@ -657,3 +657,57 @@ static int dm_test_video_truetype_bs(struct
unit_test_state *uts)
return 0;
}
DM_TEST(dm_test_video_truetype_bs, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+/* Test partial rendering onto hardware frame buffer */
+static int dm_test_video_copy(struct unit_test_state *uts)
+{
+ struct sandbox_sdl_plat *plat;
+ struct video_uc_plat *uc_plat;
+ struct udevice *dev, *con;
+ struct video_priv *priv;
+ const char *test_string = "\n\tCriticism may not be agreeable, but it
is necessary.\t";
+ ulong addr;
+
+ if (!IS_ENABLED(CONFIG_VIDEO_COPY))
+ return -EAGAIN;
+
+ ut_assertok(uclass_find_first_device(UCLASS_VIDEO, &dev));
+ ut_assertnonnull(dev);
+ uc_plat = dev_get_uclass_plat(dev);
+ uc_plat->hide_logo = true;
+ plat = dev_get_plat(dev);
+ plat->font_size = 32;
+ ut_assert(!device_active(dev));
+ ut_assertok(uclass_first_device_err(UCLASS_VIDEO, &dev));
+ ut_assertnonnull(dev);
+ priv = dev_get_uclass_priv(dev);
+
+ ut_assertok(read_file(uts, "tools/logos/denx.bmp", &addr));
+ ut_assertok(video_bmp_display(dev, addr, 0, 0, false));
+
+ ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con));
+ vidconsole_put_string(con, "\n\n\n\n\n");
+ vidconsole_put_string(con, test_string);
+ vidconsole_put_string(con, test_string);
+
+ ut_asserteq(6678, compress_frame_buffer(uts, dev, false));
+ ut_assertok(check_copy_frame_buffer(uts, dev));
+
+ /*
+* Secretly clear the hardware frame buffer, but in a different
+* color (black) to see which parts will be overwritten.
+*/
+ memset(priv->copy_fb, 0, priv->fb_size);
+
+ /*
+* We should have the full content on the main buffer, but only
+* the new content should have been copied to the copy buffer.
+*/
+ vidconsole_put_string(con, test_string);
+ vidconsole_put_string(con, test_string);
+ ut_asserteq(7589, compress_frame_buffer(uts, dev, false));
+ ut_asserteq(5278, compress_frame_buffer(uts, dev, true));
+
+ return 0;
+}
+DM_TEST(dm_test_video_copy, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.40.1
[PATCH v5 04/13] dm: video: Add damage tracking API
From: Alexander Graf
We are going to introduce image damage tracking to fasten up screen
refresh on large displays. This patch adds damage tracking for up to
one rectangle of the screen which is typically enough to hold blt or
text print updates. Callers into this API and a reduced dcache flush
code path will follow in later patches.
Signed-off-by: Alexander Graf
Reported-by: Da Xue
[Alper: Use xstart/yend, document new fields, return void from
video_damage(), declare priv, drop headers, use IS_ENABLED()]
Co-developed-by: Alper Nebi Yasak
Signed-off-by: Alper Nebi Yasak
---
Changes in v5:
- Use xstart, ystart, xend, yend as names for damage region
- Document damage struct and fields in struct video_priv comment
- Return void from video_damage()
- Fix undeclared priv error in video_sync()
- Drop unused headers from video-uclass.c
- Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
Changes in v4:
- Move damage clear to patch "dm: video: Add damage tracking API"
- Simplify first damage logic
- Remove VIDEO_DAMAGE default for ARM
Changes in v3:
- Adapt to always assume DM is used
Changes in v2:
- Remove ifdefs
drivers/video/Kconfig| 13
drivers/video/video-uclass.c | 41 +---
include/video.h | 32 ++--
3 files changed, 81 insertions(+), 5 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index fe43fbd7004a..97f494a1340b 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -92,6 +92,19 @@ config VIDEO_COPY
To use this, your video driver must set @copy_base in
struct video_uc_plat.
+config VIDEO_DAMAGE
+ bool "Enable damage tracking of frame buffer regions"
+ help
+ On some machines (most ARM), the display frame buffer resides in
+ RAM. To make the display controller pick up screen updates, we
+ have to flush frame buffer contents from CPU caches into RAM which
+ can be a slow operation.
+
+ This feature adds damage tracking to collect information about regions
+ that received updates. When we want to sync, we then only flush
+ regions of the frame buffer that were modified before, speeding up
+ screen refreshes significantly.
+
config BACKLIGHT_PWM
bool "Generic PWM based Backlight Driver"
depends on BACKLIGHT && DM_PWM
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 8f268fc4063f..447689581668 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -351,9 +351,39 @@ void video_set_default_colors(struct udevice *dev, bool
invert)
priv->colour_bg = video_index_to_colour(priv, back);
}
+/* Notify about changes in the frame buffer */
+void video_damage(struct udevice *vid, int x, int y, int width, int height)
+{
+ struct video_priv *priv = dev_get_uclass_priv(vid);
+ int xend = x + width;
+ int yend = y + height;
+
+ if (!IS_ENABLED(CONFIG_VIDEO_DAMAGE))
+ return;
+
+ if (x > priv->xsize)
+ return;
+
+ if (y > priv->ysize)
+ return;
+
+ if (xend > priv->xsize)
+ xend = priv->xsize;
+
+ if (yend > priv->ysize)
+ yend = priv->ysize;
+
+ /* Span a rectangle across all old and new damage */
+ priv->damage.xstart = min(x, priv->damage.xstart);
+ priv->damage.ystart = min(y, priv->damage.ystart);
+ priv->damage.xend = max(xend, priv->damage.xend);
+ priv->damage.yend = max(yend, priv->damage.yend);
+}
+
/* Flush video activity to the caches */
int video_sync(struct udevice *vid, bool force)
{
+ struct video_priv *priv = dev_get_uclass_priv(vid);
struct video_ops *ops = video_get_ops(vid);
int ret;
@@ -369,15 +399,12 @@ int video_sync(struct udevice *vid, bool force)
* out whether it exists? For now, ARM is safe.
*/
#if defined(CONFIG_ARM) && !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
- struct video_priv *priv = dev_get_uclass_priv(vid);
-
if (priv->flush_dcache) {
flush_dcache_range((ulong)priv->fb,
ALIGN((ulong)priv->fb + priv->fb_size,
CONFIG_SYS_CACHELINE_SIZE));
}
#elif defined(CONFIG_VIDEO_SANDBOX_SDL)
- struct video_priv *priv = dev_get_uclass_priv(vid);
static ulong last_sync;
if (force || get_timer(last_sync) > 100) {
@@ -385,6 +412,14 @@ int video_sync(struct udevice *vid, bool force)
last_sync = get_timer(0);
}
#endif
+
+ if (IS_ENABLED(CONFIG_VIDEO_DAMAGE)) {
+ priv->damage.xstart = priv->xsize;
+ priv->damage.ystart = priv->ysize;
+ priv->damage.xend = 0;
+
[PATCH v5 05/13] dm: video: Add damage notification on display fills
From: Alexander Graf Let's report the video damage when we fill parts of the screen. This way we can later lazily flush only relevant regions to hardware. Signed-off-by: Alexander Graf Reported-by: Da Xue [Alper: Call video_damage() in video_fill_part(), edit commit message] Signed-off-by: Alper Nebi Yasak --- Does video_fill_part() need a video_sync(dev, false) here? Changes in v5: - Call video_damage() also in video_fill_part() drivers/video/video-uclass.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index 447689581668..ebf409d839f0 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -203,6 +203,8 @@ int video_fill_part(struct udevice *dev, int xstart, int ystart, int xend, if (ret) return ret; + video_damage(dev, xstart, ystart, xend - xstart, yend - ystart); + return 0; } @@ -249,6 +251,8 @@ int video_fill(struct udevice *dev, u32 colour) if (ret) return ret; + video_damage(dev, 0, 0, priv->xsize, priv->ysize); + return video_sync(dev, false); } -- 2.40.1
[PATCH v5 06/13] vidconsole: Add damage notifications to all vidconsole drivers
From: Alexander Graf Now that we have a damage tracking API, let's populate damage done by vidconsole drivers. We try to declare as little memory as damaged as possible. Signed-off-by: Alexander Graf Reported-by: Da Xue [Alper: Rebase for met->baseline, fontdata->height/width, make rotated console_putc_xy() damages pass tests, edit patch message] Co-developed-by: Alper Nebi Yasak Signed-off-by: Alper Nebi Yasak --- Changes in v5: - Use met->baseline instead of priv->baseline - Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH - Update console_rotate.c video_damage() calls to pass video tests - Remove mention about not having minimal damage for console_rotate.c Changes in v2: - Fix ranges in truetype target - Limit rotate to necessary damage drivers/video/console_normal.c | 18 +++ drivers/video/console_rotate.c | 54 drivers/video/console_truetype.c | 21 + drivers/video/video-uclass.c | 1 + 4 files changed, 94 insertions(+) diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c index 413c7abee9e1..a19ce6a2bc11 100644 --- a/drivers/video/console_normal.c +++ b/drivers/video/console_normal.c @@ -39,6 +39,12 @@ static int console_set_row(struct udevice *dev, uint row, int clr) if (ret) return ret; + video_damage(dev->parent, +0, +fontdata->height * row, +vid_priv->xsize, +fontdata->height); + return 0; } @@ -60,6 +66,12 @@ static int console_move_rows(struct udevice *dev, uint rowdst, if (ret) return ret; + video_damage(dev->parent, +0, +fontdata->height * rowdst, +vid_priv->xsize, +fontdata->height * count); + return 0; } @@ -90,6 +102,12 @@ static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, char ch) if (ret) return ret; + video_damage(dev->parent, +x, +y, +fontdata->width, +fontdata->height); + ret = vidconsole_sync_copy(dev, start, line); if (ret) return ret; diff --git a/drivers/video/console_rotate.c b/drivers/video/console_rotate.c index 65358a1c6e74..6c3e7c1bb8dc 100644 --- a/drivers/video/console_rotate.c +++ b/drivers/video/console_rotate.c @@ -36,6 +36,12 @@ static int console_set_row_1(struct udevice *dev, uint row, int clr) if (ret) return ret; + video_damage(dev->parent, +vid_priv->xsize - ((row + 1) * fontdata->height), +0, +fontdata->height, +vid_priv->ysize); + return 0; } @@ -64,6 +70,12 @@ static int console_move_rows_1(struct udevice *dev, uint rowdst, uint rowsrc, dst += vid_priv->line_length; } + video_damage(dev->parent, +vid_priv->xsize - ((rowdst + count) * fontdata->height), +0, +count * fontdata->height, +vid_priv->ysize); + return 0; } @@ -96,6 +108,12 @@ static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, char ch) if (ret) return ret; + video_damage(dev->parent, +vid_priv->xsize - y - fontdata->height, +linenum - 1, +fontdata->height, +fontdata->width); + return VID_TO_POS(fontdata->width); } @@ -121,6 +139,12 @@ static int console_set_row_2(struct udevice *dev, uint row, int clr) if (ret) return ret; + video_damage(dev->parent, +0, +vid_priv->ysize - (row + 1) * fontdata->height, +vid_priv->xsize, +fontdata->height); + return 0; } @@ -142,6 +166,12 @@ static int console_move_rows_2(struct udevice *dev, uint rowdst, uint rowsrc, vidconsole_memmove(dev, dst, src, fontdata->height * vid_priv->line_length * count); + video_damage(dev->parent, +0, +vid_priv->ysize - (rowdst + count) * fontdata->height, +vid_priv->xsize, +count * fontdata->height); + return 0; } @@ -174,6 +204,12 @@ static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, char ch) if (ret) return ret; + video_damage(dev->parent, +x - fontdata->width + 1, +linenum - fontdata->height + 1, +fontdata->width, +
[PATCH v5 07/13] video: test: Test video damage tracking via vidconsole
With VIDEO_DAMAGE, the video uclass tracks updated regions of the frame
buffer in order to avoid unnecessary work during a video sync. Enable
the config in sandbox and add a test for it, by printing strings at a
few locations and checking the tracked region.
Signed-off-by: Alper Nebi Yasak
---
This is hard to test because most things issue video syncs that process
and reset the damaged region.
Changes in v5:
- Add patch "video: test: Test video damage tracking via vidconsole"
configs/sandbox_defconfig | 1 +
test/dm/video.c | 56 +++
2 files changed, 57 insertions(+)
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 259f31f26cee..51b820f13121 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -307,6 +307,7 @@ CONFIG_USB_ETH_CDC=y
CONFIG_VIDEO=y
CONFIG_VIDEO_FONT_SUN12X22=y
CONFIG_VIDEO_COPY=y
+CONFIG_VIDEO_DAMAGE=y
CONFIG_CONSOLE_ROTATION=y
CONFIG_CONSOLE_TRUETYPE=y
CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
diff --git a/test/dm/video.c b/test/dm/video.c
index e4bd27a6b76f..8c7d9800a42e 100644
--- a/test/dm/video.c
+++ b/test/dm/video.c
@@ -711,3 +711,59 @@ static int dm_test_video_copy(struct unit_test_state *uts)
return 0;
}
DM_TEST(dm_test_video_copy, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+/* Test video damage tracking */
+static int dm_test_video_damage(struct unit_test_state *uts)
+{
+ struct sandbox_sdl_plat *plat;
+ struct udevice *dev, *con;
+ struct video_priv *priv;
+ const char *test_string_1 = "Criticism may not be agreeable, ";
+ const char *test_string_2 = "but it is necessary.";
+ const char *test_string_3 = "It fulfils the same function as pain in
the human body.";
+
+ if (!IS_ENABLED(CONFIG_VIDEO_DAMAGE))
+ return -EAGAIN;
+
+ ut_assertok(uclass_find_device(UCLASS_VIDEO, 0, &dev));
+ ut_assert(!device_active(dev));
+ plat = dev_get_plat(dev);
+ plat->font_size = 32;
+
+ ut_assertok(video_get_nologo(uts, &dev));
+ ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con));
+ priv = dev_get_uclass_priv(dev);
+
+ vidconsole_position_cursor(con, 14, 10);
+ vidconsole_put_string(con, test_string_2);
+ ut_asserteq(449, priv->damage.xstart);
+ ut_asserteq(325, priv->damage.ystart);
+ ut_asserteq(661, priv->damage.xend);
+ ut_asserteq(350, priv->damage.yend);
+
+ vidconsole_position_cursor(con, 7, 5);
+ vidconsole_put_string(con, test_string_1);
+ ut_asserteq(225, priv->damage.xstart);
+ ut_asserteq(164, priv->damage.ystart);
+ ut_asserteq(661, priv->damage.xend);
+ ut_asserteq(350, priv->damage.yend);
+
+ vidconsole_position_cursor(con, 21, 15);
+ vidconsole_put_string(con, test_string_3);
+ ut_asserteq(225, priv->damage.xstart);
+ ut_asserteq(164, priv->damage.ystart);
+ ut_asserteq(1280, priv->damage.xend);
+ ut_asserteq(510, priv->damage.yend);
+
+ video_sync(dev, false);
+ ut_asserteq(priv->xsize, priv->damage.xstart);
+ ut_asserteq(priv->ysize, priv->damage.ystart);
+ ut_asserteq(0, priv->damage.xend);
+ ut_asserteq(0, priv->damage.yend);
+
+ ut_asserteq(7339, compress_frame_buffer(uts, dev, false));
+ ut_assertok(check_copy_frame_buffer(uts, dev));
+
+ return 0;
+}
+DM_TEST(dm_test_video_damage, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.40.1
[PATCH v5 08/13] video: Add damage notification on bmp display
From: Alexander Graf Let's report the video damage when we draw a bitmap on the screen. This way we can later lazily flush only relevant regions to hardware. Signed-off-by: Alexander Graf Reported-by: Da Xue Reviewed-by: Simon Glass Signed-off-by: Alper Nebi Yasak --- (no changes since v1) drivers/video/video_bmp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c index 45f003c8251a..10943b9ca19f 100644 --- a/drivers/video/video_bmp.c +++ b/drivers/video/video_bmp.c @@ -460,6 +460,8 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y, break; }; + video_damage(dev, x, y, width, height); + /* Find the position of the top left of the image in the framebuffer */ fb = (uchar *)(priv->fb + y * priv->line_length + x * bpix / 8); ret = video_sync_copy(dev, start, fb); -- 2.40.1
[PATCH v5 09/13] efi_loader: GOP: Add damage notification on BLT
From: Alexander Graf
Now that we have a damage tracking API, let's populate damage done by
UEFI payloads when they BLT data onto the screen.
Signed-off-by: Alexander Graf
Reported-by: Da Xue
Reviewed-by: Heinrich Schuchardt
[Alper: Add struct comment for new member]
Signed-off-by: Alper Nebi Yasak
---
Changes in v5:
- Document new vdev field in struct efi_gop_obj comment
Changes in v4:
- Skip damage on EfiBltVideoToBltBuffer
Changes in v3:
- Adapt to always assume DM is used
Changes in v2:
- Remove ifdefs from gop
lib/efi_loader/efi_gop.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
index 778b693f983a..db6535e080c4 100644
--- a/lib/efi_loader/efi_gop.c
+++ b/lib/efi_loader/efi_gop.c
@@ -24,6 +24,7 @@ static const efi_guid_t efi_gop_guid =
EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID;
* @ops: graphical output protocol interface
* @info: graphical output mode information
* @mode: graphical output mode
+ * @vdev: backing video device
* @bpix: bits per pixel
* @fb:frame buffer
*/
@@ -32,6 +33,7 @@ struct efi_gop_obj {
struct efi_gop ops;
struct efi_gop_mode_info info;
struct efi_gop_mode mode;
+ struct udevice *vdev;
/* Fields we only have access to during init */
u32 bpix;
void *fb;
@@ -120,6 +122,7 @@ static __always_inline efi_status_t gop_blt_int(struct
efi_gop *this,
u32 *fb32 = gopobj->fb;
u16 *fb16 = gopobj->fb;
struct efi_gop_pixel *buffer = __builtin_assume_aligned(bufferp, 4);
+ bool blt_to_video = (operation != EFI_BLT_VIDEO_TO_BLT_BUFFER);
if (delta) {
/* Check for 4 byte alignment */
@@ -243,6 +246,9 @@ static __always_inline efi_status_t gop_blt_int(struct
efi_gop *this,
dlineoff += dwidth;
}
+ if (blt_to_video)
+ video_damage(gopobj->vdev, dx, dy, width, height);
+
return EFI_SUCCESS;
}
@@ -548,6 +554,7 @@ efi_status_t efi_gop_register(void)
gopobj->info.pixels_per_scanline = col;
gopobj->bpix = bpix;
gopobj->fb = fb;
+ gopobj->vdev = vdev;
return EFI_SUCCESS;
}
--
2.40.1
[PATCH v5 10/13] video: Only dcache flush damaged lines
From: Alexander Graf
Now that we have a damage area tells us which parts of the frame buffer
actually need updating, let's only dcache flush those on video_sync()
calls. With this optimization in place, frame buffer updates - especially
on large screen such as 4k displays - speed up significantly.
Signed-off-by: Alexander Graf
Reported-by: Da Xue
[Alper: Use damage.xstart/yend, IS_ENABLED()]
Co-developed-by: Alper Nebi Yasak
Signed-off-by: Alper Nebi Yasak
---
Changes in v5:
- Use xstart, ystart, xend, yend as names for damage region
- Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
Changes in v2:
- Fix dcache range; we were flushing too much before
- Remove ifdefs
drivers/video/video-uclass.c | 41 +++-
1 file changed, 36 insertions(+), 5 deletions(-)
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 8bfcbc88dda7..a50220bcc684 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -385,6 +385,41 @@ void video_damage(struct udevice *vid, int x, int y, int
width, int height)
priv->damage.yend = max(yend, priv->damage.yend);
}
+#if defined(CONFIG_ARM) && !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
+static void video_flush_dcache(struct udevice *vid)
+{
+ struct video_priv *priv = dev_get_uclass_priv(vid);
+
+ if (!priv->flush_dcache)
+ return;
+
+ if (!IS_ENABLED(CONFIG_VIDEO_DAMAGE)) {
+ flush_dcache_range((ulong)priv->fb,
+ ALIGN((ulong)priv->fb + priv->fb_size,
+CONFIG_SYS_CACHELINE_SIZE));
+
+ return;
+ }
+
+ if (priv->damage.xend && priv->damage.yend) {
+ int lstart = priv->damage.xstart * VNBYTES(priv->bpix);
+ int lend = priv->damage.xend * VNBYTES(priv->bpix);
+ int y;
+
+ for (y = priv->damage.ystart; y < priv->damage.yend; y++) {
+ ulong fb = (ulong)priv->fb;
+ ulong start = fb + (y * priv->line_length) + lstart;
+ ulong end = start + lend - lstart;
+
+ start = ALIGN_DOWN(start, CONFIG_SYS_CACHELINE_SIZE);
+ end = ALIGN(end, CONFIG_SYS_CACHELINE_SIZE);
+
+ flush_dcache_range(start, end);
+ }
+ }
+}
+#endif
+
/* Flush video activity to the caches */
int video_sync(struct udevice *vid, bool force)
{
@@ -404,11 +439,7 @@ int video_sync(struct udevice *vid, bool force)
* out whether it exists? For now, ARM is safe.
*/
#if defined(CONFIG_ARM) && !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
- if (priv->flush_dcache) {
- flush_dcache_range((ulong)priv->fb,
- ALIGN((ulong)priv->fb + priv->fb_size,
-CONFIG_SYS_CACHELINE_SIZE));
- }
+ video_flush_dcache(vid);
#elif defined(CONFIG_VIDEO_SANDBOX_SDL)
static ulong last_sync;
--
2.40.1
[PATCH v5 11/13] video: Use VIDEO_DAMAGE for VIDEO_COPY
From: Alexander Graf CONFIG_VIDEO_COPY implemented a range-based copying mechanism: If we print a single character, it will always copy the full range of bytes from the top left corner of the character to the lower right onto the uncached frame buffer. This includes pretty much the full line contents of the printed character. Since we now have proper damage tracking, let's make use of that to reduce the amount of data we need to copy. With this patch applied, we will only copy the tiny rectangle surrounding characters when we print them, speeding up the video console. After this, changes to the main frame buffer are not immediately copied to the copy frame buffer, but postponed until the next video device sync. So issue an explicit sync before inspecting the copy frame buffer contents for the video tests. Signed-off-by: Alexander Graf [Alper: Rebase for fontdata->height/w, fill_part(), fix memmove(dev), drop from defconfig, use damage.xstart/yend, use IS_ENABLED(), call video_sync() before copy_fb check, update video_copy test] Co-developed-by: Alper Nebi Yasak Signed-off-by: Alper Nebi Yasak --- Changes in v5: - Remove video_sync_copy() also from video_fill(), video_fill_part() - Fix memmove() calls by removing the extra dev argument - Call video_sync() before checking copy_fb in video tests - Use xstart, ystart, xend, yend as names for damage region - Use met->baseline instead of priv->baseline - Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH - Use xstart, ystart, xend, yend as names for damage region - Use IS_ENABLED() instead of CONFIG_IS_ENABLED() - Drop VIDEO_DAMAGE from sandbox defconfig added in a new patch - Update dm_test_video_copy test added in a new patch Changes in v3: - Make VIDEO_COPY always select VIDEO_DAMAGE Changes in v2: - Add patch "video: Use VIDEO_DAMAGE for VIDEO_COPY" configs/sandbox_defconfig | 1 - drivers/video/Kconfig | 5 ++ drivers/video/console_normal.c| 13 + drivers/video/console_rotate.c| 44 +++--- drivers/video/console_truetype.c | 16 + drivers/video/vidconsole-uclass.c | 16 - drivers/video/video-uclass.c | 97 --- drivers/video/video_bmp.c | 7 --- include/video.h | 37 include/video_console.h | 52 - test/dm/video.c | 3 +- 11 files changed, 43 insertions(+), 248 deletions(-) diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 51b820f13121..259f31f26cee 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -307,7 +307,6 @@ CONFIG_USB_ETH_CDC=y CONFIG_VIDEO=y CONFIG_VIDEO_FONT_SUN12X22=y CONFIG_VIDEO_COPY=y -CONFIG_VIDEO_DAMAGE=y CONFIG_CONSOLE_ROTATION=y CONFIG_CONSOLE_TRUETYPE=y CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 97f494a1340b..b3fbd9d7d9ca 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -83,11 +83,14 @@ config VIDEO_PCI_DEFAULT_FB_SIZE config VIDEO_COPY bool "Enable copying the frame buffer to a hardware copy" + select VIDEO_DAMAGE help On some machines (e.g. x86), reading from the frame buffer is very slow because it is uncached. To improve performance, this feature allows the frame buffer to be kept in cached memory (allocated by U-Boot) and then copied to the hardware frame-buffer as needed. + It uses the VIDEO_DAMAGE feature to keep track of regions to copy + and will only copy actually touched regions. To use this, your video driver must set @copy_base in struct video_uc_plat. @@ -105,6 +108,8 @@ config VIDEO_DAMAGE regions of the frame buffer that were modified before, speeding up screen refreshes significantly. + It is also used by VIDEO_COPY to identify which regions changed. + config BACKLIGHT_PWM bool "Generic PWM based Backlight Driver" depends on BACKLIGHT && DM_PWM diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c index a19ce6a2bc11..c44aa09473a3 100644 --- a/drivers/video/console_normal.c +++ b/drivers/video/console_normal.c @@ -35,10 +35,6 @@ static int console_set_row(struct udevice *dev, uint row, int clr) fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes); end = dst; - ret = vidconsole_sync_copy(dev, line, end); - if (ret) - return ret; - video_damage(dev->parent, 0, fontdata->height * row, @@ -57,14 +53,11 @@ static int console_move_rows(struct udevice *dev, uint rowdst, void *dst; void *src; int size; - int ret; dst = vid_priv->fb + rowdst * fontdata->height * vid_priv->line_length; src = vid_priv->fb
[PATCH v5 12/13] video: Always compile cache flushing code
From: Alexander Graf
The dcache flushing code path was conditional on ARM && !DCACHE config
options. However, dcaches exist on other platforms as well and may need
clearing if their driver requires it.
Simplify the compile logic and always enable the dcache flush logic in
the video core. That way, drivers can always rely on it to call the arch
specific callbacks.
This will increase code size for non-ARM platforms with CONFIG_VIDEO=y
slightly.
Reported-by: Heinrich Schuchardt
Signed-off-by: Alexander Graf
Reviewed-by: Simon Glass
Signed-off-by: Alper Nebi Yasak
---
(no changes since v4)
Changes in v4:
- Add patch "video: Always compile cache flushing code"
drivers/video/video-uclass.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index c79499252a22..3f9ddaadd15d 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -377,11 +377,13 @@ void video_damage(struct udevice *vid, int x, int y, int
width, int height)
priv->damage.yend = max(yend, priv->damage.yend);
}
-#if defined(CONFIG_ARM) && !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
static void video_flush_dcache(struct udevice *vid)
{
struct video_priv *priv = dev_get_uclass_priv(vid);
+ if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF))
+ return;
+
if (!priv->flush_dcache)
return;
@@ -410,7 +412,6 @@ static void video_flush_dcache(struct udevice *vid)
}
}
}
-#endif
static void video_flush_copy(struct udevice *vid)
{
@@ -449,14 +450,9 @@ int video_sync(struct udevice *vid, bool force)
return ret;
}
- /*
-* flush_dcache_range() is declared in common.h but it seems that some
-* architectures do not actually implement it. Is there a way to find
-* out whether it exists? For now, ARM is safe.
-*/
-#if defined(CONFIG_ARM) && !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
video_flush_dcache(vid);
-#elif defined(CONFIG_VIDEO_SANDBOX_SDL)
+
+#if defined(CONFIG_VIDEO_SANDBOX_SDL)
static ulong last_sync;
if (force || get_timer(last_sync) > 100) {
--
2.40.1
[PATCH v5 13/13] video: Enable VIDEO_DAMAGE for drivers that need it
From: Alexander Graf Some drivers call video_set_flush_dcache() to indicate that they want to have the dcache flushed for the frame buffer. These drivers benefit from our new video damage control, because we can reduce the amount of memory that gets flushed significantly. This patch enables video damage control for all device drivers that call video_set_flush_dcache() to make sure they benefit from it. Signed-off-by: Alexander Graf [Alper: Add to VIDEO_TIDSS, imply instead of select] Co-developed-by: Alper Nebi Yasak Signed-off-by: Alper Nebi Yasak --- Changes in v5: - Imply VIDEO_DAMAGE for video drivers instead of selecting it - Imply VIDEO_DAMAGE also for VIDEO_TIDSS Changes in v4: - Add patch "video: Enable VIDEO_DAMAGE for drivers that need it" arch/arm/mach-omap2/omap3/Kconfig | 1 + arch/arm/mach-sunxi/Kconfig | 1 + drivers/video/Kconfig | 8 drivers/video/exynos/Kconfig | 1 + drivers/video/imx/Kconfig | 1 + drivers/video/meson/Kconfig | 1 + drivers/video/rockchip/Kconfig| 1 + drivers/video/stm32/Kconfig | 1 + drivers/video/tegra20/Kconfig | 1 + drivers/video/tidss/Kconfig | 1 + 10 files changed, 17 insertions(+) diff --git a/arch/arm/mach-omap2/omap3/Kconfig b/arch/arm/mach-omap2/omap3/Kconfig index 671e4791c67f..fd858f7b50f2 100644 --- a/arch/arm/mach-omap2/omap3/Kconfig +++ b/arch/arm/mach-omap2/omap3/Kconfig @@ -113,6 +113,7 @@ config TARGET_NOKIA_RX51 select CMDLINE_TAG select INITRD_TAG select REVISION_TAG + imply VIDEO_DAMAGE config TARGET_TAO3530 bool "TAO3530" diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig index 9d5df2c10273..fb4478ea32e8 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -813,6 +813,7 @@ config VIDEO_SUNXI depends on !SUN50I_GEN_H6 select VIDEO select DISPLAY + imply VIDEO_DAMAGE imply VIDEO_DT_SIMPLEFB default y ---help--- diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index b3fbd9d7d9ca..185dbb1f8390 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -499,6 +499,7 @@ config VIDEO_LCD_ANX9804 config ATMEL_LCD bool "Atmel LCD panel support" + imply VIDEO_DAMAGE depends on ARCH_AT91 config ATMEL_LCD_BGR555 @@ -508,6 +509,7 @@ config ATMEL_LCD_BGR555 config VIDEO_BCM2835 bool "Display support for BCM2835" + imply VIDEO_DAMAGE help The graphics processor already sets up the display so this driver simply checks the resolution and then sets up the frame buffer with @@ -654,6 +656,7 @@ source "drivers/video/meson/Kconfig" config VIDEO_MVEBU bool "Armada XP LCD controller" + imply VIDEO_DAMAGE ---help--- Support for the LCD controller integrated in the Marvell Armada XP SoC. @@ -688,6 +691,7 @@ config NXP_TDA19988 config ATMEL_HLCD bool "Enable ATMEL video support using HLCDC" + imply VIDEO_DAMAGE help HLCDC supports video output to an attached LCD panel. @@ -764,6 +768,7 @@ source "drivers/video/tidss/Kconfig" config VIDEO_TEGRA124 bool "Enable video support on Tegra124" + imply VIDEO_DAMAGE help Tegra124 supports many video output options including eDP and HDMI. At present only eDP is supported by U-Boot. This option @@ -778,6 +783,7 @@ source "drivers/video/imx/Kconfig" config VIDEO_MXS bool "Enable video support on i.MX28/i.MX6UL/i.MX7 SoCs" + imply VIDEO_DAMAGE help Enable framebuffer driver for i.MX28/i.MX6UL/i.MX7 processors @@ -840,6 +846,7 @@ config VIDEO_DW_MIPI_DSI config VIDEO_SIMPLE bool "Simple display driver for preconfigured display" + imply VIDEO_DAMAGE help Enables a simple generic display driver which utilizes the simple-framebuffer devicetree bindings. @@ -858,6 +865,7 @@ config VIDEO_DT_SIMPLEFB config VIDEO_MCDE_SIMPLE bool "Simple driver for ST-Ericsson MCDE with preconfigured display" + imply VIDEO_DAMAGE help Enables a simple display driver for ST-Ericsson MCDE (Multichannel Display Engine), which reads the configuration from diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig index 599d19d5ecc2..a2cf752aac03 100644 --- a/drivers/video/exynos/Kconfig +++ b/drivers/video/exynos/Kconfig @@ -12,6 +12,7 @@ config EXYNOS_DP config EXYNOS_FB bool "Exynos FIMD support" + imply VIDEO_DAMAGE config EXYNOS_MIPI_DSIM bool "Exynos MIPI DSI support" diff --git a/drivers/video/imx/Kconfig b/drivers/video/imx/Kconfig index 34e8b640595b..5db3e5c0499e 100644 --- a/drivers/video/imx/Kconfig ++
[PATCH 0/5] sandbox: video: Refactor out of uclass, try partial screen updates
While working on the video damage tracking series [1], I noticed the 10Hz limitation on the sandbox screen refresh rate, and thought maybe applying the damage tracking idea to SDL would make it reasonable to increase the rate or even remove the limit completely. Experimenting on that idea resulted in a bit of a refactoring to pull sandbox out of the video uclass, so I'm sending it as a series in case some of it could be useful. As for the partial update idea itself, I found that having the delay around 1/refresh-rate appears to be the most visually smooth (about 7ms for 144Hz, 16ms for 60Hz etc.), but couldn't exactly evaluate CPU usage for which the original 10Hz rate limit was set. In any case, the visual appeal of sandbox doesn't really matter, so I don't exactly care. Of course, this applies onto the video damage tracking series that I recently updated and sent [1], but only the "partial updates" patch is functionally dependent on it. [1] https://lore.kernel.org/u-boot/20230821135111.3558478-1-alpernebiya...@gmail.com/ Alper Nebi Yasak (5): sandbox: video: Display copy framebuffer if enabled video: Allow deferring and retrying driver-specific video sync sandbox: video: Move sync rate limit into SDL code sandbox: video: Move sandbox video sync to a driver operation RFC: sandbox: video: Use partial updates for SDL display arch/sandbox/cpu/sdl.c | 34 ++ arch/sandbox/include/asm/sdl.h | 13 ++--- drivers/video/sandbox_sdl.c| 30 ++ drivers/video/video-uclass.c | 14 ++ include/video.h| 3 ++- test/dm/video.c| 2 +- 6 files changed, 71 insertions(+), 25 deletions(-) base-commit: 3881c9fbb7fdd98f6eae5cd33f7e9abe9455a585 prerequisite-patch-id: aad206b8942d0e8654ba1b11f28104950baf1518 prerequisite-patch-id: ee3d21bb91062ebbcf0c3a75342d7fa37d5630ce prerequisite-patch-id: 7fcaec0bb8b5bbb5b3813bda896034c9b98b67b9 prerequisite-patch-id: aa4173ebedc3f6d0f04f72bf49bed642614f42a7 prerequisite-patch-id: 06676907c4d262880f7904fed33de11057e886f0 prerequisite-patch-id: 63bbfd7808bddea8b39120d50bb61ac1c4b3eeb8 prerequisite-patch-id: 8941f56b57f7ece319eed67ce8584e7769a36a3b prerequisite-patch-id: b89b375a3221ac1f89f3eb15fa20a2140ea5687c prerequisite-patch-id: 950508eaecbbc01560a2f24778786dd50e7c20e8 prerequisite-patch-id: c30b2a60881bc813e4d8a7e1a99fd99c368957d2 prerequisite-patch-id: 963770b5a43c750bac620788562ce549ac48dacd prerequisite-patch-id: 500af25e869687516da147ce7c819c6b32a9bc92 prerequisite-patch-id: b064cd6b7db9cd122f10c2ae2573328ac945b3be -- 2.40.1
[PATCH 1/5] sandbox: video: Display copy framebuffer if enabled
When VIDEO_COPY is enabled, the "main" framebuffer is a cached work area
in U-Boot allocated memory and the "copy" framebuffer is the hardware
frame buffer displayed on the screen. The sandbox SDL video driver sets
copy_base to indicate support for this, but it displays the work area
instead. Change it to display the copy buffer if enabled.
Signed-off-by: Alper Nebi Yasak
---
drivers/video/video-uclass.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 3f9ddaadd15d..7cec6362570f 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -454,9 +454,13 @@ int video_sync(struct udevice *vid, bool force)
#if defined(CONFIG_VIDEO_SANDBOX_SDL)
static ulong last_sync;
+ void *fb = priv->fb;
+
+ if (IS_ENABLED(CONFIG_VIDEO_COPY))
+ fb = priv->copy_fb;
if (force || get_timer(last_sync) > 100) {
- sandbox_sdl_sync(priv->fb);
+ sandbox_sdl_sync(fb);
last_sync = get_timer(0);
}
#endif
--
2.40.1
[PATCH 2/5] video: Allow deferring and retrying driver-specific video sync
The sandbox SDL driver has some code in the video uclass to rate limit
video syncs by postponing them, and forcing a sync nonetheless with a
"force" argument.
Add infrastructure for doing this through driver ops, where the driver
can request to defer a sync with -EAGAIN, and the uclass can force it by
calling the op again it until it does something.
Signed-off-by: Alper Nebi Yasak
---
Alternatively, add "force" argument into the driver ops->video_sync().
But I think it should go away instead. The problem is video_sync() being
called irregularly and too frequently, maybe we can call it only from
cyclic at the hardware refresh rate?
drivers/video/video-uclass.c | 2 ++
include/video.h | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 7cec6362570f..accf486509cb 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -446,6 +446,8 @@ int video_sync(struct udevice *vid, bool force)
if (ops && ops->video_sync) {
ret = ops->video_sync(vid);
+ while (force && ret == -EAGAIN)
+ ret = ops->video_sync(vid);
if (ret)
return ret;
}
diff --git a/include/video.h b/include/video.h
index 42e57b44188d..5c4327d4e455 100644
--- a/include/video.h
+++ b/include/video.h
@@ -137,7 +137,8 @@ struct video_priv {
* displays needs synchronization when data in an FB is available.
* For these devices implement video_sync hook to call a sync
* function. vid is pointer to video device udevice. Function
- * should return 0 on success video_sync and error code otherwise
+ * should return 0 on success video_sync, -EAGAIN if it was
+ * deferred and should be tried again, and error code otherwise
*/
struct video_ops {
int (*video_sync)(struct udevice *vid);
--
2.40.1
[PATCH 3/5] sandbox: video: Move sync rate limit into SDL code
As a first step of removing sandbox-specific code from the video uclass,
move the sync rate limiter into the SDL code. We lose the ability to
immediately force a screen refresh, but can retry until it does.
Signed-off-by: Alper Nebi Yasak
---
arch/sandbox/cpu/sdl.c | 9 +
arch/sandbox/include/asm/sdl.h | 4 +++-
drivers/video/video-uclass.c | 8 +++-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c
index 590e406517bf..48fae20b4c2d 100644
--- a/arch/sandbox/cpu/sdl.c
+++ b/arch/sandbox/cpu/sdl.c
@@ -48,6 +48,7 @@ struct buf_info {
* @screen: SDL window to use
* @src_depth: Number of bits per pixel in the source frame buffer (that we
read
* from and render to SDL)
+ * @last_sync: Timestamp of last display update in milliseconds
*/
static struct sdl_info {
int width;
@@ -67,6 +68,7 @@ static struct sdl_info {
SDL_Renderer *renderer;
SDL_Window *screen;
int src_depth;
+ int last_sync;
} sdl;
static void sandbox_sdl_poll_events(void)
@@ -148,6 +150,7 @@ int sandbox_sdl_init_display(int width, int height, int
log2_bpp,
sdl.vis_width = sdl.width;
sdl.vis_height = sdl.height;
}
+ sdl.last_sync = 0;
if (!SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "1"))
printf("Unable to init hinting: %s", SDL_GetError());
@@ -245,6 +248,10 @@ int sandbox_sdl_sync(void *lcd_base)
if (!sdl.texture)
return 0;
+
+ if (SDL_GetTicks() - sdl.last_sync < 100)
+ return -EAGAIN;
+
SDL_RenderClear(sdl.renderer);
ret = copy_to_texture(lcd_base);
if (ret) {
@@ -268,6 +275,8 @@ int sandbox_sdl_sync(void *lcd_base)
SDL_RenderDrawRect(sdl.renderer, &rect);
SDL_RenderPresent(sdl.renderer);
+ sdl.last_sync = SDL_GetTicks();
+
sandbox_sdl_poll_events();
return 0;
diff --git a/arch/sandbox/include/asm/sdl.h b/arch/sandbox/include/asm/sdl.h
index ee4991f7c24a..1ace7d1a1217 100644
--- a/arch/sandbox/include/asm/sdl.h
+++ b/arch/sandbox/include/asm/sdl.h
@@ -37,10 +37,12 @@ int sandbox_sdl_remove_display(void);
* sandbox_sdl_sync() - Sync current U-Boot LCD frame buffer to SDL
*
* This must be called periodically to update the screen for SDL so that the
- * user can see it.
+ * user can see it. It is internally rate limited to 10 Hz to avoid using
+ * system resources too much.
*
* @lcd_base: Base of frame buffer
* Return: 0 if screen was updated, -ENODEV is there is no screen.
+ *-EAGAIN if screen was not updated due to sync rate limit.
*/
int sandbox_sdl_sync(void *lcd_base);
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index accf486509cb..d867185da539 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -455,16 +455,14 @@ int video_sync(struct udevice *vid, bool force)
video_flush_dcache(vid);
#if defined(CONFIG_VIDEO_SANDBOX_SDL)
- static ulong last_sync;
void *fb = priv->fb;
if (IS_ENABLED(CONFIG_VIDEO_COPY))
fb = priv->copy_fb;
- if (force || get_timer(last_sync) > 100) {
- sandbox_sdl_sync(fb);
- last_sync = get_timer(0);
- }
+ ret = sandbox_sdl_sync(fb);
+ while (force && ret == -EAGAIN)
+ ret = sandbox_sdl_sync(fb);
#endif
if (IS_ENABLED(CONFIG_VIDEO_DAMAGE)) {
--
2.40.1
[PATCH 4/5] sandbox: video: Move sandbox video sync to a driver operation
The sandbox SDL video sync is handled in the uclass because there has
been a sync rate limiter and a way to bypass that. Previous patches
move the rate limit code into SDL-specific files, and provide a generic
way to defer and force video syncs.
Sandbox code shouldn't be in the uclasses if possible. Move the
remaining sandbox sync call into the driver ops. Now that sandbox video
sync attempts can defer without resetting video damage, force a video
sync before checking that video syncs reset video damage.
Signed-off-by: Alper Nebi Yasak
---
drivers/video/sandbox_sdl.c | 16
drivers/video/video-uclass.c | 14 --
test/dm/video.c | 2 +-
3 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/video/sandbox_sdl.c b/drivers/video/sandbox_sdl.c
index 9081c7da62e4..7dc2787a5d25 100644
--- a/drivers/video/sandbox_sdl.c
+++ b/drivers/video/sandbox_sdl.c
@@ -99,6 +99,17 @@ int sandbox_sdl_set_bpp(struct udevice *dev, enum
video_log2_bpp l2bpp)
return 0;
}
+static int sandbox_sdl_video_sync(struct udevice *dev)
+{
+ struct video_priv *priv = dev_get_uclass_priv(dev);
+ void *fb = priv->fb;
+
+ if (IS_ENABLED(CONFIG_VIDEO_COPY))
+ fb = priv->copy_fb;
+
+ return sandbox_sdl_sync(fb);
+}
+
static int sandbox_sdl_remove(struct udevice *dev)
{
/*
@@ -133,6 +144,10 @@ static const struct udevice_id sandbox_sdl_ids[] = {
{ }
};
+static struct video_ops sandbox_sdl_ops = {
+ .video_sync = sandbox_sdl_video_sync,
+};
+
U_BOOT_DRIVER(sandbox_lcd_sdl) = {
.name = "sandbox_lcd_sdl",
.id = UCLASS_VIDEO,
@@ -141,4 +156,5 @@ U_BOOT_DRIVER(sandbox_lcd_sdl) = {
.probe = sandbox_sdl_probe,
.remove = sandbox_sdl_remove,
.plat_auto = sizeof(struct sandbox_sdl_plat),
+ .ops= &sandbox_sdl_ops,
};
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index d867185da539..2632216c05ae 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -23,9 +23,6 @@
#include
#include
#include
-#ifdef CONFIG_SANDBOX
-#include
-#endif
/*
* Theory of operation:
@@ -454,17 +451,6 @@ int video_sync(struct udevice *vid, bool force)
video_flush_dcache(vid);
-#if defined(CONFIG_VIDEO_SANDBOX_SDL)
- void *fb = priv->fb;
-
- if (IS_ENABLED(CONFIG_VIDEO_COPY))
- fb = priv->copy_fb;
-
- ret = sandbox_sdl_sync(fb);
- while (force && ret == -EAGAIN)
- ret = sandbox_sdl_sync(fb);
-#endif
-
if (IS_ENABLED(CONFIG_VIDEO_DAMAGE)) {
priv->damage.xstart = priv->xsize;
priv->damage.ystart = priv->ysize;
diff --git a/test/dm/video.c b/test/dm/video.c
index 4c3bcd26e94f..487e4d4a73a2 100644
--- a/test/dm/video.c
+++ b/test/dm/video.c
@@ -756,7 +756,7 @@ static int dm_test_video_damage(struct unit_test_state *uts)
ut_asserteq(1280, priv->damage.xend);
ut_asserteq(510, priv->damage.yend);
- video_sync(dev, false);
+ video_sync(dev, true);
ut_asserteq(priv->xsize, priv->damage.xstart);
ut_asserteq(priv->ysize, priv->damage.ystart);
ut_asserteq(0, priv->damage.xend);
--
2.40.1
[RFC PATCH 5/5] sandbox: video: Use partial updates for SDL display
Now that we have video damage tracking, try to reduce the SDL display
work by copying only the updated regions onto the SDL texture instead of
the entire framebuffer. We still have to do RenderClear and RenderCopy
the whole texture onto the renderer, but that allegedly happens in the
GPU.
Signed-off-by: Alper Nebi Yasak
---
The second half of copy_to_texture is untested.
arch/sandbox/cpu/sdl.c | 25 +
arch/sandbox/include/asm/sdl.h | 9 +++--
drivers/video/sandbox_sdl.c| 16 +++-
3 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c
index 48fae20b4c2d..3a3221d89066 100644
--- a/arch/sandbox/cpu/sdl.c
+++ b/arch/sandbox/cpu/sdl.c
@@ -192,8 +192,10 @@ int sandbox_sdl_init_display(int width, int height, int
log2_bpp,
return 0;
}
-static int copy_to_texture(void *lcd_base)
+static int copy_to_texture(void *lcd_base, int xstart, int ystart,
+ int xend, int yend)
{
+ struct SDL_Rect rect;
char *dest;
int pitch, x, y;
int src_pitch;
@@ -201,8 +203,15 @@ static int copy_to_texture(void *lcd_base)
char *src;
int ret;
+ rect.x = xstart;
+ rect.y = ystart;
+ rect.w = xend - xstart + 1;
+ rect.h = yend - ystart + 1;
+
if (sdl.src_depth == sdl.depth) {
- SDL_UpdateTexture(sdl.texture, NULL, lcd_base, sdl.pitch);
+ src_pitch = sdl.width * sdl.src_depth / 8;
+ src = lcd_base + src_pitch * rect.y + rect.x * sdl.src_depth /
8;
+ SDL_UpdateTexture(sdl.texture, &rect, src, src_pitch);
return 0;
}
@@ -215,7 +224,7 @@ static int copy_to_texture(void *lcd_base)
return -EINVAL;
}
- ret = SDL_LockTexture(sdl.texture, NULL, &pixels, &pitch);
+ ret = SDL_LockTexture(sdl.texture, &rect, &pixels, &pitch);
if (ret) {
printf("SDL lock %d: %s\n", ret, SDL_GetError());
return ret;
@@ -223,12 +232,12 @@ static int copy_to_texture(void *lcd_base)
/* Copy the pixels one by one */
src_pitch = sdl.width * sdl.src_depth / 8;
- for (y = 0; y < sdl.height; y++) {
+ for (y = 0; y < rect.h; y++) {
char val;
dest = pixels + y * pitch;
- src = lcd_base + src_pitch * y;
- for (x = 0; x < sdl.width; x++, dest += 4) {
+ src = lcd_base + src_pitch * (ystart + y) + xstart;
+ for (x = 0; x < rect.w; x++, dest += 4) {
val = *src++;
dest[0] = val;
dest[1] = val;
@@ -241,7 +250,7 @@ static int copy_to_texture(void *lcd_base)
return 0;
}
-int sandbox_sdl_sync(void *lcd_base)
+int sandbox_sdl_sync(void *lcd_base, int xstart, int ystart, int xend, int
yend)
{
struct SDL_Rect rect;
int ret;
@@ -253,7 +262,7 @@ int sandbox_sdl_sync(void *lcd_base)
return -EAGAIN;
SDL_RenderClear(sdl.renderer);
- ret = copy_to_texture(lcd_base);
+ ret = copy_to_texture(lcd_base, xstart, ystart, xend, yend);
if (ret) {
printf("copy_to_texture: %d: %s\n", ret, SDL_GetError());
return -EIO;
diff --git a/arch/sandbox/include/asm/sdl.h b/arch/sandbox/include/asm/sdl.h
index 1ace7d1a1217..c7c73ef3a3e6 100644
--- a/arch/sandbox/include/asm/sdl.h
+++ b/arch/sandbox/include/asm/sdl.h
@@ -41,10 +41,14 @@ int sandbox_sdl_remove_display(void);
* system resources too much.
*
* @lcd_base: Base of frame buffer
+ * @xstart: X start position of updated region in pixels from the left
+ * @ystart: Y start position of updated region in pixels from the top
+ * @xend: X end position of updated region in pixels from the left
+ * @yend: Y end position of updated region in pixels from the top
* Return: 0 if screen was updated, -ENODEV is there is no screen.
*-EAGAIN if screen was not updated due to sync rate limit.
*/
-int sandbox_sdl_sync(void *lcd_base);
+int sandbox_sdl_sync(void *lcd_base, int xstart, int ystart, int xend, int
yend);
/**
* sandbox_sdl_scan_keys() - scan for pressed keys
@@ -118,7 +122,8 @@ static inline int sandbox_sdl_remove_display(void)
return -ENODEV;
}
-static inline int sandbox_sdl_sync(void *lcd_base)
+static inline int sandbox_sdl_sync(void *lcd_base, int xstart, int ystart,
+ int xend, int yend)
{
return -ENODEV;
}
diff --git a/drivers/video/sandbox_sdl.c b/drivers/video/sandbox_sdl.c
index 7dc2787a5d25..eb424072b6fe 100644
--- a/drivers/video/sandbox_sdl.c
+++ b/drivers/video/sandbox_sdl.c
@@ -103,11 +103,25 @@ static int sandbox_sdl_video_sync(struct udevice *dev)
{
struct video_priv *priv = dev_get_uclass_priv(dev);
void *fb =
[PATCH v2 0/7] Add support for QEMU's ramfb display
This is a rebase of Alexander Graf's QEMU ramfb support series with some changes of mine, in part to address reviews and to accommodate other changes enabling Bochs video driver. The original cover letter is as follows: > QEMU implements multiple ways to expose graphics output to the virt > machine, but most of them are incompatible with hardware virtualization. > > The one that does work reliably is ramfb. It's a very simple mechanism > in which the guest reserves a memory region for the frame buffer and then > notifies the host about its location and properties. The host then just > displays the contents of the frame buffer on screen. > > This patch set adds support to drive the ramfb device in our QEMU arm > targets. Theoretically, it should work as is with any of the other > architectures as well though. > > With this driver, we have a very simple, KVM compatible way to expose > GOP via UEFI to payloads and thus enable development and testing of > graphical OS functionality with QEMU's -M virt. Inspired by the "video: bochs: Remove the x86 limitation" series [1] that was recently merged, I have also sent a series adding Bochs video device support for ARM [2], and assuming that would get merged before this I've rebased this on that to avoid conflicts. [1] https://lore.kernel.org/u-boot/20230723044041.1089804-1-bm...@tinylab.org/ [2] https://lore.kernel.org/u-boot/20230814173944.288356-1-alpernebiya...@gmail.com/ Changes in v2: - Remove extra "depends on DM_VIDEO" already in "if VIDEO" - Drop drivers/video/MAINTAINERS file - Decouple framebuffer allocation from EFI_LOADER - Add .bind() method for ramfb driver - Make resolution configurable with kconfig - Move struct to qfw.h and add comments for members - Use RAMFB_* definitions instead of DEFAULT_* - Use if (IS_ENABLED(CONFIG_VIDEO_RAMFB)) instead of #ifdef - Add patch "qfw: Add flag to allow probing before relocation" - Rebase on "qemu: arm: Enable Bochs, console buffering, USB keyboard" - Drop env changes from ARM (necessary but in prerequisite series) - Drop imply VIDEO, SYS_CONSOLE_IN_ENV changes from ARM (in prereq.) - Probe QFW in ARM QEMU board_early_init_f to bind ramfb pre-reloc - Add IS_ENABLED(CONFIG_QFW) check and error handling to ARM QEMU - Drop imply VIDEO, env changes from RISC-V patch (in u-boot/next) - Drop imply SYS_CONSOLE_IS_IN_ENV from RISC-V patch (def. y if VIDEO) - Probe QFW in RISC-V QEMU board_early_init_f to bind ramfb pre-reloc - Add IS_ENABLED(CONFIG_QFW) check and error handling to RISC-V QEMU - Add patch "x86: qemu: Enable ramfb by default" v1: https://lore.kernel.org/u-boot/20220227144043.37359-1-ag...@csgraf.de/ Alexander Graf (5): qfw: Add WRITE definition ramfb: Add driver for ramfb display qfw: Spawn ramfb device if its file is present arm: qemu: Enable ramfb by default riscv: qemu: Enable ramfb by default Alper Nebi Yasak (2): qfw: Add flag to allow probing before relocation x86: qemu: Enable ramfb by default arch/arm/Kconfig| 3 + arch/x86/cpu/qemu/Kconfig | 4 + board/emulation/qemu-arm/qemu-arm.c | 41 +++ board/emulation/qemu-riscv/Kconfig | 5 ++ board/emulation/qemu-riscv/qemu-riscv.c | 41 +++ board/emulation/qemu-x86/qemu-x86.c | 47 configs/qemu-x86_64_defconfig | 4 +- configs/qemu-x86_defconfig | 1 - drivers/misc/qfw.c | 25 +++ drivers/misc/qfw_mmio.c | 1 + drivers/misc/qfw_pio.c | 1 + drivers/misc/qfw_sandbox.c | 1 + drivers/video/Kconfig | 30 drivers/video/Makefile | 1 + drivers/video/ramfb.c | 97 + include/qfw.h | 11 +++ 16 files changed, 309 insertions(+), 4 deletions(-) create mode 100644 drivers/video/ramfb.c base-commit: 832148f675e427060be074c276956962fa9b5cb6 prerequisite-patch-id: 2c8662d18f6f1e2d4e014bcd5d468e0396064f4f prerequisite-patch-id: 5f3e5e1d814187841ce299df0da16df16d0121c3 prerequisite-patch-id: adbd0fb953e726ea5ea0b248e00092dfc318fd46 prerequisite-patch-id: 7470d3af5e9ded16eb5a4de7c7264062dcb3ef7f -- 2.40.1
[PATCH v2 1/7] qfw: Add WRITE definition
From: Alexander Graf
The QEMU fw_cfg device supports writing entries as well. Add the constant
define for it so that we can leverage write functionality later.
Signed-off-by: Alexander Graf
Reviewed-by: Simon Glass
Signed-off-by: Alper Nebi Yasak
---
(no changes since v1)
include/qfw.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/qfw.h b/include/qfw.h
index 42798fea7db4..d3aaa4d54efc 100644
--- a/include/qfw.h
+++ b/include/qfw.h
@@ -70,6 +70,7 @@ enum {
#define FW_CFG_DMA_READ(1 << 1)
#define FW_CFG_DMA_SKIP(1 << 2)
#define FW_CFG_DMA_SELECT (1 << 3)
+#define FW_CFG_DMA_WRITE (1 << 4)
/* Bit set in FW_CFG_ID response to indicate DMA interface availability. */
#define FW_CFG_DMA_ENABLED (1 << 1)
--
2.40.1
[PATCH v2 2/7] ramfb: Add driver for ramfb display
From: Alexander Graf
QEMU implements multiple ways to expose graphics output to the virt
machine, but most of them are incompatible with hardware virtualization.
The one that does work reliably is ramfb. It's a very simple mechanism
in which the guest reserves a memory region for the frame buffer and then
notifies the host about its location and properties. The host then just
displays the contents of the frame buffer on screen.
This patch implements a trivial version of a ramfb driver - hard coded
to a single resolution set in Kconfig.
Signed-off-by: Alexander Graf
[Alper: Deduplicate depends on DM_VIDEO, drop MAINTAINERS, decouple from
EFI_LOADER, add .bind(), kconfigurable resolution, struct in .h]
Co-developed-by: Alper Nebi Yasak
Signed-off-by: Alper Nebi Yasak
---
Changes in v2:
- Remove extra "depends on DM_VIDEO" already in "if VIDEO"
- Drop drivers/video/MAINTAINERS file
- Decouple framebuffer allocation from EFI_LOADER
- Add .bind() method for ramfb driver
- Make resolution configurable with kconfig
- Move struct to qfw.h and add comments for members
- Use RAMFB_* definitions instead of DEFAULT_*
drivers/video/Kconfig | 30 +
drivers/video/Makefile | 1 +
drivers/video/ramfb.c | 97 ++
include/qfw.h | 10 +
4 files changed, 138 insertions(+)
create mode 100644 drivers/video/ramfb.c
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 69f4809cf4a6..6ab46107483e 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -849,6 +849,36 @@ config VIDEO_MCDE_SIMPLE
before u-boot starts, and u-boot will simply render to the pre-
allocated frame buffer surface.
+config VIDEO_RAMFB
+ bool "QEMU ramfb display driver for in-RAM display"
+ depends on QFW
+ help
+ Enables a RAM based simple frame buffer driver which uses qfw to
+ notify the hypervisor about the location of a Frame Buffer allocated
+ in guest RAM as well as its properties.
+
+if VIDEO_RAMFB
+
+config VIDEO_RAMFB_SIZE_X
+ int "Width of ramfb display (X resolution)"
+ default 1280
+ help
+ Sets the width of the ramfb display.
+
+ These two options control the size of the display set up by QEMU.
+ Typical sizes are 1024 x 768 or 1280 x 1024.
+
+config VIDEO_RAMFB_SIZE_Y
+ int "Height of ramfb display (Y resolution)"
+ default 1024
+ help
+ Sets the height of the ramfb display.
+
+ These two options control the size of the display set up by QEMU.
+ Typical sizes are 1024 x 768 or 1280 x 1024.
+
+endif
+
config OSD
bool "Enable OSD support"
depends on DM
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index d13af9f3b19b..775b4e46e600 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -75,6 +75,7 @@ obj-$(CONFIG_VIDEO_SANDBOX_SDL) += sandbox_sdl.o
obj-$(CONFIG_VIDEO_SIMPLE) += simplefb.o
obj-$(CONFIG_VIDEO_VESA) += vesa.o
obj-$(CONFIG_VIDEO_SEPS525) += seps525.o
+obj-$(CONFIG_VIDEO_RAMFB) += ramfb.o
obj-$(CONFIG_VIDEO_ZYNQMP_DPSUB) += zynqmp/
obj-y += bridge/
diff --git a/drivers/video/ramfb.c b/drivers/video/ramfb.c
new file mode 100644
index ..ba17cac312b6
--- /dev/null
+++ b/drivers/video/ramfb.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2022 Alexander Graf
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define fourcc_code(a, b, c, d) ((u32)(a) | ((u32)(b) << 8) | \
+((u32)(c) << 16) | ((u32)(d) << 24))
+#define DRM_FORMAT_XRGB fourcc_code('X', 'R', '2', '4')
+
+#define RAMFB_WIDTHCONFIG_VIDEO_RAMFB_SIZE_X
+#define RAMFB_HEIGHT CONFIG_VIDEO_RAMFB_SIZE_Y
+#define RAMFB_BPIX VIDEO_BPP32
+#define RAMFB_FORMAT VIDEO_X8R8G8B8
+
+static int ramfb_probe(struct udevice *dev)
+{
+ struct video_uc_plat *plat = dev_get_uclass_plat(dev);
+ struct video_priv *uc_priv = dev_get_uclass_priv(dev);
+ u32 selector;
+ int ret;
+ struct fw_file *file;
+ struct udevice *qfw;
+ struct dm_qfw_ops *ops;
+ struct qfw_dma dma = {};
+ struct ramfb_cfg cfg = {
+ .addr = cpu_to_be64(plat->base),
+ .fourcc = cpu_to_be32(DRM_FORMAT_XRGB),
+ .flags = 0,
+ .width = cpu_to_be32(RAMFB_WIDTH),
+ .height = cpu_to_be32(RAMFB_HEIGHT),
+ .stride = 0,
+ };
+
+ if (plat->base == 0)
+ return -EPROBE_DEFER;
+
+ debug("%s: Frame buffer base %lx\n", __func__, plat->base);
+
+ ret = qfw_get_dev(&qfw);
+ if (ret)
+ return -EPROBE_DEFER;
+
+ ops = dm_qfw_get_ops(qfw);
+ if (!ops)
+ ret
[PATCH v2 3/7] qfw: Spawn ramfb device if its file is present
From: Alexander Graf
Now that we have a ramfb device driver, let's add the necessary glueing
magic to also spawn it when we find its qfw file node.
Signed-off-by: Alexander Graf
[Alper: Use if IS_ENABLED() instead of #ifdef]
Signed-off-by: Alper Nebi Yasak
---
Changes in v2:
- Use if (IS_ENABLED(CONFIG_VIDEO_RAMFB)) instead of #ifdef
drivers/misc/qfw.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c
index 7c01bf23d53b..4e4260982cce 100644
--- a/drivers/misc/qfw.c
+++ b/drivers/misc/qfw.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -307,6 +308,27 @@ void qfw_read_entry(struct udevice *dev, u16 entry, u32
size, void *address)
qfw_read_entry_io(qdev, entry, size, address);
}
+static void qfw_bind_ramfb(struct udevice *dev)
+{
+ struct fw_file *file;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_VIDEO_RAMFB))
+ return;
+
+ ret = qfw_read_firmware_list(dev);
+ if (ret)
+ return;
+
+ file = qfw_find_file(dev, "etc/ramfb");
+ if (!file) {
+ /* No ramfb available. */
+ return;
+ }
+
+ device_bind_driver(dev, "ramfb", "qfw-ramfb", NULL);
+}
+
int qfw_register(struct udevice *dev)
{
struct qfw_dev *qdev = dev_get_uclass_priv(dev);
@@ -323,6 +345,8 @@ int qfw_register(struct udevice *dev)
if (dma_enabled & FW_CFG_DMA_ENABLED)
qdev->dma_present = true;
+ qfw_bind_ramfb(dev);
+
return 0;
}
--
2.40.1
[PATCH v2 4/7] qfw: Add flag to allow probing before relocation
QEMU firmware config drivers need to be probed to bind the ramfb device.
The ramfb driver needs to be bound before relocation to properly reserve
video memory for it, otherwise it cannot be probed after relocation. Add
the flag to probe QEMU firmware config drivers before relocation so that
ramfb can work as an initial vidconsole.
Signed-off-by: Alper Nebi Yasak
---
Alternatively, I guess I could default VIDEO_PCI_DEFAULT_FB_SIZE to a
higher size with "if VIDEO_RAMFB". But it exists because "PCI drivers
cannot be bound before relocation unless they are mentioned in the
devicetree" and qfw is in the QEMU-generated devicetree unlike those, so
I assumed this would be the preferred way.
Changes in v2:
- Add patch "qfw: Add flag to allow probing before relocation"
drivers/misc/qfw.c | 1 +
drivers/misc/qfw_mmio.c| 1 +
drivers/misc/qfw_pio.c | 1 +
drivers/misc/qfw_sandbox.c | 1 +
4 files changed, 4 insertions(+)
diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c
index 4e4260982cce..265f45290011 100644
--- a/drivers/misc/qfw.c
+++ b/drivers/misc/qfw.c
@@ -414,6 +414,7 @@ UCLASS_DRIVER(qfw) = {
.name = "qfw",
.post_bind = qfw_post_bind,
.per_device_auto= sizeof(struct qfw_dev),
+ .flags = DM_FLAG_PRE_RELOC,
};
struct bootdev_ops qfw_bootdev_ops = {
diff --git a/drivers/misc/qfw_mmio.c b/drivers/misc/qfw_mmio.c
index f397384054a6..a5274982711c 100644
--- a/drivers/misc/qfw_mmio.c
+++ b/drivers/misc/qfw_mmio.c
@@ -116,4 +116,5 @@ U_BOOT_DRIVER(qfw_mmio) = {
.of_to_plat = qfw_mmio_of_to_plat,
.probe = qfw_mmio_probe,
.ops= &qfw_mmio_ops,
+ .flags = DM_FLAG_PRE_RELOC,
};
diff --git a/drivers/misc/qfw_pio.c b/drivers/misc/qfw_pio.c
index e2f628d33830..dae809bd3fea 100644
--- a/drivers/misc/qfw_pio.c
+++ b/drivers/misc/qfw_pio.c
@@ -66,4 +66,5 @@ U_BOOT_DRIVER(qfw_pio) = {
.id = UCLASS_QFW,
.probe = qfw_pio_probe,
.ops= &qfw_pio_ops,
+ .flags = DM_FLAG_PRE_RELOC,
};
diff --git a/drivers/misc/qfw_sandbox.c b/drivers/misc/qfw_sandbox.c
index 1002df75339e..0bd36acd1a09 100644
--- a/drivers/misc/qfw_sandbox.c
+++ b/drivers/misc/qfw_sandbox.c
@@ -120,6 +120,7 @@ U_BOOT_DRIVER(qfw_sandbox) = {
.plat_auto = sizeof(struct qfw_sandbox_plat),
.probe = qfw_sandbox_probe,
.ops= &qfw_sandbox_ops,
+ .flags = DM_FLAG_PRE_RELOC,
};
U_BOOT_DRVINFO(qfw_sandbox) = {
--
2.40.1
[PATCH v2 5/7] arm: qemu: Enable ramfb by default
From: Alexander Graf
Now that we have everything in place to support ramfb, let's wire it up
by default in the ARM QEMU targets. That way, you can easily use a
graphical console by just passing -device ramfb to the QEMU command line.
Signed-off-by: Alexander Graf
[Alper: Rebase on bochs changes, add pre-reloc init, error handling]
Co-developed-by: Alper Nebi Yasak
Signed-off-by: Alper Nebi Yasak
---
Changes in v2:
- Rebase on "qemu: arm: Enable Bochs, console buffering, USB keyboard"
- Drop env changes from ARM (necessary but in prerequisite series)
- Drop imply VIDEO, SYS_CONSOLE_IN_ENV changes from ARM (in prereq.)
- Probe QFW in ARM QEMU board_early_init_f to bind ramfb pre-reloc
- Add IS_ENABLED(CONFIG_QFW) check and error handling to ARM QEMU
arch/arm/Kconfig| 3 +++
board/emulation/qemu-arm/qemu-arm.c | 41 +
2 files changed, 44 insertions(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1fd3ccd1607f..7afe26ac804f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1046,6 +1046,9 @@ config ARCH_QEMU
imply USB_XHCI_PCI
imply USB_KEYBOARD
imply CMD_USB
+ imply VIDEO_RAMFB
+ imply BOARD_EARLY_INIT_F
+ imply BOARD_EARLY_INIT_R
config ARCH_RMOBILE
bool "Renesas ARM SoCs"
diff --git a/board/emulation/qemu-arm/qemu-arm.c
b/board/emulation/qemu-arm/qemu-arm.c
index 942f1fff5717..23ef31cb7feb 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -102,6 +103,46 @@ static struct mm_region qemu_arm64_mem_map[] = {
struct mm_region *mem_map = qemu_arm64_mem_map;
#endif
+int board_early_init_f(void)
+{
+ struct udevice *dev;
+ int ret;
+
+ /*
+* Make sure we enumerate the QEMU Firmware device to bind ramfb
+* so video_reserve() can reserve memory for it.
+*/
+ if (IS_ENABLED(CONFIG_QFW)) {
+ ret = qfw_get_dev(&dev);
+ if (ret) {
+ log_err("Failed to get QEMU FW device: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+int board_early_init_r(void)
+{
+ struct udevice *dev;
+ int ret;
+
+ /*
+* Make sure we enumerate the QEMU Firmware device to find ramfb
+* before console_init.
+*/
+ if (IS_ENABLED(CONFIG_QFW)) {
+ ret = qfw_get_dev(&dev);
+ if (ret) {
+ log_err("Failed to get QEMU FW device: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
int board_init(void)
{
return 0;
--
2.40.1
[PATCH v2 6/7] riscv: qemu: Enable ramfb by default
From: Alexander Graf
Now that we have everything in place to support ramfb, let's wire it up
by default in the RISC-V QEMU targets. That way, you can easily use a
graphical console by just passing -device ramfb to the QEMU command line.
Signed-off-by: Alexander Graf
[Alper: Rebase on bochs changes, add pre-reloc init and error handling]
Co-developed-by: Alper Nebi Yasak
Signed-off-by: Alper Nebi Yasak
---
Changes in v2:
- Drop imply VIDEO, env changes from RISC-V patch (in u-boot/next)
- Drop imply SYS_CONSOLE_IS_IN_ENV from RISC-V patch (def. y if VIDEO)
- Probe QFW in RISC-V QEMU board_early_init_f to bind ramfb pre-reloc
- Add IS_ENABLED(CONFIG_QFW) check and error handling to RISC-V QEMU
board/emulation/qemu-riscv/Kconfig | 5 +++
board/emulation/qemu-riscv/qemu-riscv.c | 41 +
2 files changed, 46 insertions(+)
diff --git a/board/emulation/qemu-riscv/Kconfig
b/board/emulation/qemu-riscv/Kconfig
index d56b4b5bc1ed..b5c72982e565 100644
--- a/board/emulation/qemu-riscv/Kconfig
+++ b/board/emulation/qemu-riscv/Kconfig
@@ -82,5 +82,10 @@ config BOARD_SPECIFIC_OPTIONS # dummy
imply USB_XHCI_PCI
imply USB_KEYBOARD
imply CMD_USB
+ imply BOARD_EARLY_INIT_F
+ imply BOARD_EARLY_INIT_R
+ imply VIDEO_RAMFB
+ imply CMD_QFW
+ imply QFW_MMIO
endif
diff --git a/board/emulation/qemu-riscv/qemu-riscv.c
b/board/emulation/qemu-riscv/qemu-riscv.c
index 181abbbf97d8..1041e936e9d1 100644
--- a/board/emulation/qemu-riscv/qemu-riscv.c
+++ b/board/emulation/qemu-riscv/qemu-riscv.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -29,6 +30,46 @@ int is_flash_available(void)
}
#endif
+int board_early_init_f(void)
+{
+ struct udevice *dev;
+ int ret;
+
+ /*
+* Make sure we enumerate the QEMU Firmware device to bind ramfb
+* so video_reserve() can reserve memory for it.
+*/
+ if (IS_ENABLED(CONFIG_QFW)) {
+ ret = qfw_get_dev(&dev);
+ if (ret) {
+ log_err("Failed to get QEMU FW device: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+int board_early_init_r(void)
+{
+ struct udevice *dev;
+ int ret;
+
+ /*
+* Make sure we enumerate the QEMU Firmware device to find ramfb
+* before console_init.
+*/
+ if (IS_ENABLED(CONFIG_QFW)) {
+ ret = qfw_get_dev(&dev);
+ if (ret) {
+ log_err("Failed to get QEMU FW device: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
int board_init(void)
{
/*
--
2.40.1
[PATCH v2 7/7] x86: qemu: Enable ramfb by default
Now that we have everything in place to support ramfb, let's wire it up
by default in the x86 QEMU targets. That way, we can use ramfb graphical
console instead of the default by passing -vga none -device ramfb to the
QEMU command line.
Also increase SYS_MALLOC_F_LEN for QEMU x86_64 to be the same as its SPL
counterpart, because we're running out of alloc space in pre-reloc stage
with ramfb enabled.
Signed-off-by: Alper Nebi Yasak
---
This also suffers from the same issue with distros as the Bochs display
driver [1], where it results in a hang after GRUB menu selection before
the kernel can display anything. Couldn't reproduce on arm*/riscv*.
But just having it enabled doesn't seem to cause problems unless you run
QEMU with -device ramfb, so this (unlike the Bochs video driver) can
actually be co-enabled with VIDEO_VESA.
[1] https://lore.kernel.org/u-boot/20230724145210.304917-4-...@chromium.org/
Changes in v2:
- Add patch "x86: qemu: Enable ramfb by default"
arch/x86/cpu/qemu/Kconfig | 4 +++
board/emulation/qemu-x86/qemu-x86.c | 47 +
configs/qemu-x86_64_defconfig | 4 +--
configs/qemu-x86_defconfig | 1 -
4 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/arch/x86/cpu/qemu/Kconfig b/arch/x86/cpu/qemu/Kconfig
index f8f2f6473088..e0a57ac2d687 100644
--- a/arch/x86/cpu/qemu/Kconfig
+++ b/arch/x86/cpu/qemu/Kconfig
@@ -13,6 +13,10 @@ config QEMU
imply USB
imply USB_EHCI_HCD
imply VIDEO_VESA
+ imply VIDEO_RAMFB
+ imply BOARD_EARLY_INIT_F
+ imply BOARD_EARLY_INIT_R
+ imply CMD_QFW
if QEMU
diff --git a/board/emulation/qemu-x86/qemu-x86.c
b/board/emulation/qemu-x86/qemu-x86.c
index e69de29bb2d1..3a8a580cc591 100644
--- a/board/emulation/qemu-x86/qemu-x86.c
+++ b/board/emulation/qemu-x86/qemu-x86.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include
+#include
+#include
+#include
+#include
+
+int board_early_init_f(void)
+{
+ struct udevice *dev;
+ int ret;
+
+ /*
+* Make sure we enumerate the QEMU Firmware device to bind ramfb
+* so video_reserve() can reserve memory for it.
+*/
+ if (IS_ENABLED(CONFIG_QFW)) {
+ ret = qfw_get_dev(&dev);
+ if (ret) {
+ log_err("Failed to get QEMU FW device: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+int board_early_init_r(void)
+{
+ struct udevice *dev;
+ int ret;
+
+ /*
+* Make sure we enumerate the QEMU Firmware device to find ramfb
+* before console_init.
+*/
+ if (IS_ENABLED(CONFIG_QFW)) {
+ ret = qfw_get_dev(&dev);
+ if (ret) {
+ log_err("Failed to get QEMU FW device: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig
index c6f30674a8fc..c9ad6eebd35c 100644
--- a/configs/qemu-x86_64_defconfig
+++ b/configs/qemu-x86_64_defconfig
@@ -1,13 +1,12 @@
CONFIG_X86=y
CONFIG_TEXT_BASE=0x111
-CONFIG_SYS_MALLOC_F_LEN=0x1000
+CONFIG_SYS_MALLOC_F_LEN=0x2000
CONFIG_NR_DRAM_BANKS=8
CONFIG_ENV_SIZE=0x4
CONFIG_MAX_CPUS=2
CONFIG_SPL_DM_SPI=y
CONFIG_DEFAULT_DEVICE_TREE="qemu-x86_i440fx"
CONFIG_SPL_TEXT_BASE=0xfffd8000
-CONFIG_SPL_SYS_MALLOC_F_LEN=0x2000
CONFIG_DEBUG_UART_BASE=0x3f8
CONFIG_DEBUG_UART_CLOCK=1843200
CONFIG_X86_RUN_64BIT=y
@@ -59,7 +58,6 @@ CONFIG_CMD_USB=y
CONFIG_BOOTP_BOOTFILESIZE=y
CONFIG_CMD_EFIDEBUG=y
CONFIG_CMD_TIME=y
-CONFIG_CMD_QFW=y
CONFIG_CMD_BOOTSTAGE=y
CONFIG_CMD_EXT4_WRITE=y
CONFIG_ENV_OVERWRITE=y
diff --git a/configs/qemu-x86_defconfig b/configs/qemu-x86_defconfig
index 24682a5387df..8a2311720f02 100644
--- a/configs/qemu-x86_defconfig
+++ b/configs/qemu-x86_defconfig
@@ -37,7 +37,6 @@ CONFIG_CMD_USB=y
CONFIG_BOOTP_BOOTFILESIZE=y
CONFIG_CMD_EFIDEBUG=y
CONFIG_CMD_TIME=y
-CONFIG_CMD_QFW=y
CONFIG_CMD_BOOTSTAGE=y
CONFIG_CMD_EXT4_WRITE=y
CONFIG_MAC_PARTITION=y
--
2.40.1
Re: [PATCH] efi: Correct handling of frame buffer
On 2023-08-25 22:28 +03:00, Simon Glass wrote:
> The efi_gop driver uses private fields from the video uclass to obtain a
> pointer to the frame buffer. Use the platform data instead.
>
> Check the VIDEO_COPY setting to determine which frame buffer to use. Once
> the next stage is running (and making use of U-Boot's EFI boot services)
> U-Boot does not handle copying from priv->fb to the hardware framebuffer,
So far this copying seems to be done case-by-case like calling
vidconsole_sync_copy() any time vidconsole does something, but the video
damage tracking series moves that to video_sync(), so this change is
incompatible with that (it will keep overwriting copy_fb with fb).
> so we must allow EFI to write directly to the hardware framebuffer.
If you want a fix independent of that series, I think the proper
approach here is having EFI draw to fb as it already does, then copying
from that to copy_fb at the end of gop_blt_int().
> Signed-off-by: Simon Glass
> ---
>
> lib/efi_loader/efi_gop.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
> index 778b693f983..a09db31eb46 100644
> --- a/lib/efi_loader/efi_gop.c
> +++ b/lib/efi_loader/efi_gop.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -467,10 +468,10 @@ efi_status_t efi_gop_register(void)
> struct efi_gop_obj *gopobj;
> u32 bpix, format, col, row;
> u64 fb_base, fb_size;
> - void *fb;
> efi_status_t ret;
> struct udevice *vdev;
> struct video_priv *priv;
> + struct video_uc_plat *plat;
>
> /* We only support a single video output device for now */
> if (uclass_first_device_err(UCLASS_VIDEO, &vdev)) {
> @@ -483,9 +484,10 @@ efi_status_t efi_gop_register(void)
> format = priv->format;
> col = video_get_xsize(vdev);
> row = video_get_ysize(vdev);
> - fb_base = (uintptr_t)priv->fb;
> - fb_size = priv->fb_size;
> - fb = priv->fb;
> +
> + plat = dev_get_uclass_plat(vdev);
> + fb_base = IS_ENABLED(CONFIG_VIDEO_COPY) ? plat->copy_base : plat->base;
> + fb_size = plat->size;
>
> switch (bpix) {
> case VIDEO_BPP16:
> @@ -547,7 +549,7 @@ efi_status_t efi_gop_register(void)
> }
> gopobj->info.pixels_per_scanline = col;
> gopobj->bpix = bpix;
> - gopobj->fb = fb;
> + gopobj->fb = map_sysmem(fb_base, fb_size);
>
> return EFI_SUCCESS;
> }
Re: [PATCH v2 4/4] doc: qemu: arm: Add a section on booting Linux distros
On 2023-08-15 01:43 +03:00, Simon Glass wrote: > Hi Alper, > > On Mon, 14 Aug 2023 at 11:40, Alper Nebi Yasak > wrote: >> I actually want to put the root.img device first so that the VM can boot >> into the installed system when it reboots, but U-Boot can't find the >> bootflow on the second drive. I tried e.g. `bootdev list -p; bootflow >> scan -lab`, but it seems to only ever search the first virtio-blk. >> However, `eficonfig; bootefi bootmgr` makes it boot somehow. > > Yes, this is annoying. > > The problem is that the scanning is getting so complicated. The > boot_targets var lists things which can either be a uclass, or a > uclass with a device number. The currently implementation sees > "virtio" and moves on to the next thing in boot_targets once it has > processed one virtio device. That is obviously not correct, but... > > Would it be possible to just drop the 'boot_targets' var? That is what > is stuffing it up, but we don't actually need it now. The defaults end > up doing the same thing, apart (perhaps) from the strangeness of > virtio which can be both a network and a blk device. > > I believe it is possible to do the right thing, but I'll need to > create a better test mechanism to handle all the cases. I think some kind of a "priority score" could help -- iterate over boot_targets first just to generate bootdev priorities, then carry them over as bootflows are discovered (adjusted for bootmeth order), then use those scores as the order to boot / to show a sorted menu / to test?
Re: [PATCH] efi: Correct handling of frame buffer
On 2023-08-27 19:32 +03:00, Heinrich Schuchardt wrote: > Am 27. August 2023 16:56:51 MESZ schrieb Alper Nebi Yasak > : >> On 2023-08-25 22:28 +03:00, Simon Glass wrote: >>> so we must allow EFI to write directly to the hardware framebuffer. >> >> If you want a fix independent of that series, I think the proper >> approach here is having EFI draw to fb as it already does, then copying >> from that to copy_fb at the end of gop_blt_int(). > > An EFI app can write directly to the framebuffer and mix that with API calls. Ack. Can EFI apps write to framebuffer without informing U-Boot at all? What I meant is, copying whenever EFI asks us to modify something or tells us it modified something. If it can write without telling us, how are cache flushes and hardware syncs handled? Asking these because I'm doing some weird experiments with video damage and cyclic video_sync() calls and VIDEO_COPY... > blt functions causing a full copy would be much slower than what you get when > eliminating the copy fb. When we expose copy_fb to EFI, don't we need to keep fb up to date, incurring the same cost? The VIDEO_COPY description mentions "reading from the frame buffer is slow", so whatever code is "reading" would use fb and would get bad data otherwise. But I can't exactly point to any examples other than the video damage series copying fb onto copy_fb.
Re: [PATCH 0/6] Attempt to enforce standard extensions for build output
On 2023-08-24 06:02 +03:00, Simon Glass wrote: > In this early stage of using binman to produce output files, we are mostly > seeing people using common extensions such as '.bin' and '.rom' > > But unusual extensions appear in some places. > > We would like 'buildman -k' to keep the build outputs, but this is hard if > there is no consistency as to the extension used. > > This series adjusts binman to enforce just 4 extensions for output images: > >.bin >.rom >.itb >.img > > Other extensions will produce an error. With this rule observed, buildman > can keep the required files. I dislike this limitation. We know what files we will generate, they are listed in binman dtb, so we can add something like `binman build --ls` to print their names/paths for buildman to preserve them. Regarding the output directory suggestion, I think the binman outputs (not temporary/intermediate files) should be in the same directory as make outputs, and the Makefile should default to O=build to achieve the "output dir". I'm not sure if that's going to happen.
Re: [PATCH v2 3/7] qfw: Spawn ramfb device if its file is present
On 2023-08-22 21:56 +03:00, Simon Glass wrote:
> Hi Alper,
>
> On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak
> wrote:
>>
>> From: Alexander Graf
>>
>> Now that we have a ramfb device driver, let's add the necessary glueing
>> magic to also spawn it when we find its qfw file node.
>
> So then how do we select which video driver is used? I think we should
> have this in the DT so there is some control.
I think you mean choosing between multiple potential video devices for a
video console, but just in case: AFAIK only the ramfb driver can be used
with -device ramfb, unlike how either vesa or bochs can be used for
-device VGA.
This series is less about being able to use multiple video devices, but
more about supporting by default different configurations in which QEMU
can be launched: with "-device VGA" or "-vga none -device ramfb".
That being said, I had some success with multiple video console
attempts. For "-device VGA -device ramfb", we need to make `*addrp -=
CONFIG_VAL(VIDEO_PCI_DEFAULT_FB_SIZE);` unconditional in video_reserve()
to correctly allocate for both vesa and ramfb. Then we can run `env set
serial,vidconsole,vidconsole1` which sets up both video/vidconsole
devices with same text content.
Regardless of this series, with VIDEO_PCI_DEFAULT_FB_SIZE=0x300 and
"-device VGA -device VGA -device VGA" we get two broken and one empty
display, and `env set serial,vidconsole,vidconsole1,vidconsole2` puts a
console to the third display. I think it's a bug the first two are
broken, and it's initially using the first broken display as vidconsole.
Anyway, my opinion is: if stdout/stderr has "vidconsole", we should
enable a vidconsole on all consoles, but if it has "vidconsole#" we
should only enable the one on that video# device. Then device-tree could
have /aliases { video0 = "..."; video1 = &...; } or something to have a
stable order?
Or, you might mean adding a ramfb node to the device-tree, in which case
that's going to reopen a can of OF_BOARD worms... (Same as what applies
to putting bootph,* into qfw device-tree node, so I'll reply there.)
>> [...]
>>
>> +static void qfw_bind_ramfb(struct udevice *dev)
>> +{
>> + struct fw_file *file;
>> + int ret;
>> +
>> + if (!IS_ENABLED(CONFIG_VIDEO_RAMFB))
>> + return;
>
> Needs an error-code return
>
>> +
>> + ret = qfw_read_firmware_list(dev);
>> + if (ret)
>> + return;
>> +
>> + file = qfw_find_file(dev, "etc/ramfb");
>> + if (!file) {
>> + /* No ramfb available. */
>> + return;
>> + }
>> +
>> + device_bind_driver(dev, "ramfb", "qfw-ramfb", NULL);
>
> check error
>
Will try to fix both, thanks.
Re: [PATCH v2 4/7] qfw: Add flag to allow probing before relocation
On 2023-08-22 21:56 +03:00, Simon Glass wrote:
> Hi Alper,
>
> On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak
> wrote:
>>
>> QEMU firmware config drivers need to be probed to bind the ramfb device.
>> The ramfb driver needs to be bound before relocation to properly reserve
>> video memory for it, otherwise it cannot be probed after relocation. Add
>> the flag to probe QEMU firmware config drivers before relocation so that
>> ramfb can work as an initial vidconsole.
>>
>> Signed-off-by: Alper Nebi Yasak
>> ---
>> Alternatively, I guess I could default VIDEO_PCI_DEFAULT_FB_SIZE to a
>> higher size with "if VIDEO_RAMFB". But it exists because "PCI drivers
>> cannot be bound before relocation unless they are mentioned in the
>> devicetree" and qfw is in the QEMU-generated devicetree unlike those, so
>> I assumed this would be the preferred way.
>>
>> Changes in v2:
>> - Add patch "qfw: Add flag to allow probing before relocation"
>>
>> drivers/misc/qfw.c | 1 +
>> drivers/misc/qfw_mmio.c| 1 +
>> drivers/misc/qfw_pio.c | 1 +
>> drivers/misc/qfw_sandbox.c | 1 +
>> 4 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c
>> index 4e4260982cce..265f45290011 100644
>> --- a/drivers/misc/qfw.c
>> +++ b/drivers/misc/qfw.c
>> @@ -414,6 +414,7 @@ UCLASS_DRIVER(qfw) = {
>> .name = "qfw",
>> .post_bind = qfw_post_bind,
>> .per_device_auto= sizeof(struct qfw_dev),
>> + .flags = DM_FLAG_PRE_RELOC,
>> };
>
> Should we add this to the DT instead?
I've experimented a bit, and got something that works nice on x86
because we have the device-trees in-tree. So I can add something to
arch/x86/dts/qemu-*.dts like:
/ {
fw-cfg {
compatible = "qemu,fw-cfg-pio";
bootph-some-ram;
qfw-ramfb {
compatible = "qemu,qfw-ramfb";
bootph-some-ram;
};
};
};
Then add the compatibles as .of_match to qfw_pio and ramfb drivers, call
dm_scan_fdt_dev(dev) in qfw_post_bind(), merge qfw_bind_ramfb() into the
ramfb driver probe, and it will work without the board_early_init_[fr]
hooks or these flags. That all seems to be fine.
The problem is other arches where QEMU generates a device-tree at
runtime and passes it to U-Boot with OF_BOARD. It has, on arm64 (and
with @1010 instead on riscv64):
fw-cfg@902 {
dma-coherent;
reg = <0x00 0x902 0x00 0x18>;
compatible = "qemu,fw-cfg-mmio";
};
As a proof of concept, I used `qemu-system-* -M dumpdtb=file.dtb` and
`dtc -I dtb -O dts` to replace the empty qemu dts files, then set
OF_BOARD=n and OF_OMIT_DTB=n, and added similar modifications to
qemu-*-u-boot.dtsi. And everything seems fine like that as well.
I think it can be automated in Makefile, but I don't think disabling
OF_BOARD is right here. I see there's OF_BOARD_FIXUP, so I guess I'll
try finding the fw-cfg node there and adding the bootph props and ramfb
nodes... Is this going the right way?
(And the pio/ramfb compatibles don't exist, do we have to use u-boot,*?)
> In the case where it isn't present it can return -EPERM.
>
> Regards,
> Simon
Re: [PATCH v2 7/7] x86: qemu: Enable ramfb by default
On 2023-08-22 21:56 +03:00, Simon Glass wrote: > Hi Alper, > > On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak > wrote: >> >> Now that we have everything in place to support ramfb, let's wire it up >> by default in the x86 QEMU targets. That way, we can use ramfb graphical >> console instead of the default by passing -vga none -device ramfb to the >> QEMU command line. >> >> Also increase SYS_MALLOC_F_LEN for QEMU x86_64 to be the same as its SPL >> counterpart, because we're running out of alloc space in pre-reloc stage >> with ramfb enabled. >> >> Signed-off-by: Alper Nebi Yasak >> --- >> This also suffers from the same issue with distros as the Bochs display >> driver [1], where it results in a hang after GRUB menu selection before >> the kernel can display anything. Couldn't reproduce on arm*/riscv*. > > Yes I see that problem too. I wonder how we can debug it? No idea, and I couldn't find a good commit to bisect from, tried as far back as v2021.10. >> But just having it enabled doesn't seem to cause problems unless you run >> QEMU with -device ramfb, so this (unlike the Bochs video driver) can >> actually be co-enabled with VIDEO_VESA. > > Indeed...which makes me wonder if we can do something similar with > Bochs, so that (from the cmdline) it is possible to chose ramfb, bochs > or vesa? (Tried to answer video choice and DT concerns in another mail) >> [1] https://lore.kernel.org/u-boot/20230724145210.304917-4-...@chromium.org/ >> >> Changes in v2: >> - Add patch "x86: qemu: Enable ramfb by default" >> >> arch/x86/cpu/qemu/Kconfig | 4 +++ >> board/emulation/qemu-x86/qemu-x86.c | 47 + >> configs/qemu-x86_64_defconfig | 4 +-- >> configs/qemu-x86_defconfig | 1 - >> 4 files changed, 52 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/cpu/qemu/Kconfig b/arch/x86/cpu/qemu/Kconfig >> index f8f2f6473088..e0a57ac2d687 100644 >> --- a/arch/x86/cpu/qemu/Kconfig >> +++ b/arch/x86/cpu/qemu/Kconfig >> @@ -13,6 +13,10 @@ config QEMU >> imply USB >> imply USB_EHCI_HCD >> imply VIDEO_VESA >> + imply VIDEO_RAMFB >> + imply BOARD_EARLY_INIT_F >> + imply BOARD_EARLY_INIT_R >> + imply CMD_QFW >> >> if QEMU >> >> [...] >> >> @@ -59,7 +58,6 @@ CONFIG_CMD_USB=y >> CONFIG_BOOTP_BOOTFILESIZE=y >> CONFIG_CMD_EFIDEBUG=y >> CONFIG_CMD_TIME=y >> -CONFIG_CMD_QFW=y > > What is happening here? Why disable it? I used `imply CMD_QFW` above to be consistent with previous patches, so it's no longer necessary in defconfigs. Should I drop the imply and keep these in defconfig?
Re: [PATCH v2 5/7] arm: qemu: Enable ramfb by default
On 2023-08-22 21:56 +03:00, Simon Glass wrote:
> Hi Alper,
>
> On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak
> wrote:
>>
>> From: Alexander Graf
>>
>> Now that we have everything in place to support ramfb, let's wire it up
>> by default in the ARM QEMU targets. That way, you can easily use a
>> graphical console by just passing -device ramfb to the QEMU command line.
>>
>> Signed-off-by: Alexander Graf
>> [Alper: Rebase on bochs changes, add pre-reloc init, error handling]
>> Co-developed-by: Alper Nebi Yasak
>> Signed-off-by: Alper Nebi Yasak
>> ---
>>
>> Changes in v2:
>> - Rebase on "qemu: arm: Enable Bochs, console buffering, USB keyboard"
>> - Drop env changes from ARM (necessary but in prerequisite series)
>> - Drop imply VIDEO, SYS_CONSOLE_IN_ENV changes from ARM (in prereq.)
>> - Probe QFW in ARM QEMU board_early_init_f to bind ramfb pre-reloc
>> - Add IS_ENABLED(CONFIG_QFW) check and error handling to ARM QEMU
>>
>> arch/arm/Kconfig| 3 +++
>> board/emulation/qemu-arm/qemu-arm.c | 41 +
>> 2 files changed, 44 insertions(+)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 1fd3ccd1607f..7afe26ac804f 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1046,6 +1046,9 @@ config ARCH_QEMU
>> imply USB_XHCI_PCI
>> imply USB_KEYBOARD
>> imply CMD_USB
>> + imply VIDEO_RAMFB
>> + imply BOARD_EARLY_INIT_F
>> + imply BOARD_EARLY_INIT_R
>>
>> config ARCH_RMOBILE
>> bool "Renesas ARM SoCs"
>> diff --git a/board/emulation/qemu-arm/qemu-arm.c
>> b/board/emulation/qemu-arm/qemu-arm.c
>> index 942f1fff5717..23ef31cb7feb 100644
>> --- a/board/emulation/qemu-arm/qemu-arm.c
>> +++ b/board/emulation/qemu-arm/qemu-arm.c
>> @@ -11,6 +11,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -102,6 +103,46 @@ static struct mm_region qemu_arm64_mem_map[] = {
>> struct mm_region *mem_map = qemu_arm64_mem_map;
>> #endif
>>
>> +int board_early_init_f(void)
>> +{
>> + struct udevice *dev;
>> + int ret;
>> +
>> + /*
>> +* Make sure we enumerate the QEMU Firmware device to bind ramfb
>> +* so video_reserve() can reserve memory for it.
>> +*/
>> + if (IS_ENABLED(CONFIG_QFW)) {
>> + ret = qfw_get_dev(&dev);
>> + if (ret) {
>> + log_err("Failed to get QEMU FW device: %d\n", ret);
>
> We should only present an error if the device is present but
> failed...so if the user doesn't provide the flag, all should be well.
I don't understand what you mean by "user doesn't provide the flag". But
I assume this should ignore the error (unless what?) so that we can
continue to boot. Would that apply for board_early_init_r below as well?
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int board_early_init_r(void)
>> +{
>> + struct udevice *dev;
>> + int ret;
>> +
>> + /*
>> +* Make sure we enumerate the QEMU Firmware device to find ramfb
>> +* before console_init.
>> +*/
>> + if (IS_ENABLED(CONFIG_QFW)) {
>> + ret = qfw_get_dev(&dev);
>> + if (ret) {
>> + log_err("Failed to get QEMU FW device: %d\n", ret);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int board_init(void)
>> {
>> return 0;
>> --
>> 2.40.1
>>
>
> The glue here feels like a bit of a hack...we should rely on normal DT
> mechanisms here.
>
> Regards,
> Simon
Re: [PATCH v2 5/7] arm: qemu: Enable ramfb by default
On 2023-08-28 20:54 +03:00, Simon Glass wrote:
> Hi Alper,
> On Mon, 28 Aug 2023 at 09:46, Alper Nebi Yasak
> wrote:
>>
>> On 2023-08-22 21:56 +03:00, Simon Glass wrote:
>>> Hi Alper,
>>>
>>> On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak
>>> wrote:
>>>>
>>>> From: Alexander Graf
>>>>
>>>> Now that we have everything in place to support ramfb, let's wire it up
>>>> by default in the ARM QEMU targets. That way, you can easily use a
>>>> graphical console by just passing -device ramfb to the QEMU command line.
>>>>
>>>> Signed-off-by: Alexander Graf
>>>> [Alper: Rebase on bochs changes, add pre-reloc init, error handling]
>>>> Co-developed-by: Alper Nebi Yasak
>>>> Signed-off-by: Alper Nebi Yasak
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - Rebase on "qemu: arm: Enable Bochs, console buffering, USB keyboard"
>>>> - Drop env changes from ARM (necessary but in prerequisite series)
>>>> - Drop imply VIDEO, SYS_CONSOLE_IN_ENV changes from ARM (in prereq.)
>>>> - Probe QFW in ARM QEMU board_early_init_f to bind ramfb pre-reloc
>>>> - Add IS_ENABLED(CONFIG_QFW) check and error handling to ARM QEMU
>>>>
>>>> arch/arm/Kconfig| 3 +++
>>>> board/emulation/qemu-arm/qemu-arm.c | 41 +
>>>> 2 files changed, 44 insertions(+)
>>>>
>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>>> index 1fd3ccd1607f..7afe26ac804f 100644
>>>> --- a/arch/arm/Kconfig
>>>> +++ b/arch/arm/Kconfig
>>>> @@ -1046,6 +1046,9 @@ config ARCH_QEMU
>>>> imply USB_XHCI_PCI
>>>> imply USB_KEYBOARD
>>>> imply CMD_USB
>>>> + imply VIDEO_RAMFB
>>>> + imply BOARD_EARLY_INIT_F
>>>> + imply BOARD_EARLY_INIT_R
>>>>
>>>> config ARCH_RMOBILE
>>>> bool "Renesas ARM SoCs"
>>>> diff --git a/board/emulation/qemu-arm/qemu-arm.c
>>>> b/board/emulation/qemu-arm/qemu-arm.c
>>>> index 942f1fff5717..23ef31cb7feb 100644
>>>> --- a/board/emulation/qemu-arm/qemu-arm.c
>>>> +++ b/board/emulation/qemu-arm/qemu-arm.c
>>>> @@ -11,6 +11,7 @@
>>>> #include
>>>> #include
>>>> #include
>>>> +#include
>>>> #include
>>>> #include
>>>> #include
>>>> @@ -102,6 +103,46 @@ static struct mm_region qemu_arm64_mem_map[] = {
>>>> struct mm_region *mem_map = qemu_arm64_mem_map;
>>>> #endif
>>>>
>>>> +int board_early_init_f(void)
>>>> +{
>>>> + struct udevice *dev;
>>>> + int ret;
>>>> +
>>>> + /*
>>>> +* Make sure we enumerate the QEMU Firmware device to bind ramfb
>>>> +* so video_reserve() can reserve memory for it.
>>>> +*/
>>>> + if (IS_ENABLED(CONFIG_QFW)) {
>>>> + ret = qfw_get_dev(&dev);
>>>> + if (ret) {
>>>> + log_err("Failed to get QEMU FW device: %d\n", ret);
>>>
>>> We should only present an error if the device is present but
>>> failed...so if the user doesn't provide the flag, all should be well.
>>
>> I don't understand what you mean by "user doesn't provide the flag". But
>> I assume this should ignore the error (unless what?) so that we can
>> continue to boot. Would that apply for board_early_init_r below as well?
>
> I thought you said there was a qemu flag to enable ramfb? So add it to
> the DT and then the device can (in its bind routine) decide not to be
> bound by returning -ENODEV
Ah. I'm not used to calling them flags, especially those that have
values as in "-device ramfb". My mind jumped to DM_FLAG_PRE_RELOC /
bootph-some-ram.
But note that what this code tries to probe here is not the ramfb
device, it's the bus/parent of that. If this fails, we can't know if the
user specified "-device ramfb" or not.
Is the parent node's device probed by the time .bind() is called? Or,
can I have it probed by calling qfw_get_dev() (which is just
uclass_first_device_err(UCLASS_QFW, ...))?
Re: [PATCH v5 00/13] Add video damage tracking
ms for both on sandbox on my PC.
>>>>> I fail to see how invalidating the frame buffer for the screen every
>>>>> 10ms is any better than doing flush+invalidate operations only on screen
>>>>> areas that changed? It's more fragile, more difficult to understand and
>>>>> also significantly more expensive given that most of the time very
>>>>> little on the screen actually changes.
>>>>>>>> I am not suggesting it is better, at all. I am just trying to explain
>>>> that the U-Boot EFI implementation should not be calling video_sync()
>>>> after every character, before or after this series.
>>>
>>> Ah, it doesn't :). It just calls the normal U-Boot "write character on
>>> screen" function. What that does is up to U-Boot internals - and those
>>> basically today dictate that something needs to call video_sync() to
>>> make FB modifications actually pop up on screen.
>>>
>> Hmmm, so what function is it calling, then? I think we are getting
>> closer to the 'fix' I am trying to tease out.
>
> It literally calls vidconsole_puts():
>
> https://github.com/u-boot/u-boot/blob/master/lib/efi_loader/efi_console.c#L185
I'd think "sync after a whole string is printed" is an OK heuristic, and
GRUB is abusing it... But maybe GRUB is doing these things as an ad-hoc
double buffering implementation with forced syncs at the expense of
performance to avoid buggy firmware causing graphical artifacts.
In any case, apologies but I'll be unable to work on U-Boot things until
late September, may also be unable to respond. (Going to DebConf soon)From 5144e9479c6b31cac33e98b5ae00a6d103f19462 Mon Sep 17 00:00:00 2001
From: Alper Nebi Yasak
Date: Tue, 22 Aug 2023 18:05:29 +0300
Subject: [PATCH] WIP: video: Replace all video_sync calls with a cyclic
function
---
boot/expo.c | 2 --
cmd/video.c | 2 --
drivers/serial/sandbox.c | 2 --
drivers/video/Kconfig | 1 +
drivers/video/vidconsole-uclass.c | 35 +++
drivers/video/video-uclass.c | 28 -
drivers/video/video_bmp.c | 2 +-
include/video.h | 2 ++
lib/efi_loader/efi_gop.c | 2 --
9 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/boot/expo.c b/boot/expo.c
index 139d684f8e6e..03e858c03c80 100644
--- a/boot/expo.c
+++ b/boot/expo.c
@@ -208,8 +208,6 @@ int expo_render(struct expo *exp)
return log_msg_ret("ren", ret);
}
- video_sync(dev, true);
-
return scn ? 0 : -ECHILD;
}
diff --git a/cmd/video.c b/cmd/video.c
index 942f81c16336..98de2c9f1b8d 100644
--- a/cmd/video.c
+++ b/cmd/video.c
@@ -42,8 +42,6 @@ static int do_video_puts(struct cmd_tbl *cmdtp, int flag, int argc,
if (uclass_first_device_err(UCLASS_VIDEO_CONSOLE, &dev))
return CMD_RET_FAILURE;
ret = vidconsole_put_string(dev, argv[1]);
- if (!ret)
- ret = video_sync(dev->parent, false);
return ret ? CMD_RET_FAILURE : 0;
}
diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c
index f4003811ee75..586e1154a327 100644
--- a/drivers/serial/sandbox.c
+++ b/drivers/serial/sandbox.c
@@ -139,8 +139,6 @@ static int sandbox_serial_pending(struct udevice *dev, bool input)
return 0;
os_usleep(100);
- if (IS_ENABLED(CONFIG_VIDEO) && !IS_ENABLED(CONFIG_SPL_BUILD))
- video_sync_all();
avail = membuff_putraw(&priv->buf, 100, false, &data);
if (!avail)
return 1; /* buffer full */
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 09f2cb1a7321..530485102c78 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -7,6 +7,7 @@ menu "Graphics support"
config VIDEO
bool "Enable driver model support for LCD/video"
depends on DM
+ select CYCLIC
help
This enables driver model for LCD and video devices. These support
a bitmap display of various sizes and depths which can be drawn on
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c
index b5b3b6625902..876e80f9ebe5 100644
--- a/drivers/video/vidconsole-uclass.c
+++ b/drivers/video/vidconsole-uclass.c
@@ -77,7 +77,7 @@ static int vidconsole_back(struct udevice *dev)
if (priv->ycur < 0)
priv->ycur = 0;
}
- return video_sync(dev->parent, false);
+ return 0;
}
/* Move to a newline, scrolling the display if necessary */
@@ -87,7 +87,7 @@ static void vidconsole_newline(struct udevice *dev)
struct udevice *vid_dev = dev->parent;
struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev);
const int rows = CONFIG_VAL(CONSOLE_SCROLL_LINES);
- int i, ret;
+ int i;
priv->xcur_frac = priv->xstart_frac;
priv->ycur += priv->y_charsize;
@@ -101,13 +101,6 @@ static void vidconsol
Re: [PATCH v5 11/13] video: Use VIDEO_DAMAGE for VIDEO_COPY
On 2023-08-21 23:06 +03:00, Alexander Graf wrote:
>
> On 21.08.23 21:11, Simon Glass wrote:
>> Hi Alper,
>>
>> On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak
>> wrote:
>>> From: Alexander Graf
>>>
>>> CONFIG_VIDEO_COPY implemented a range-based copying mechanism: If we
>>> print a single character, it will always copy the full range of bytes
>>> from the top left corner of the character to the lower right onto the
>>> uncached frame buffer. This includes pretty much the full line contents
>>> of the printed character.
>>>
>>> Since we now have proper damage tracking, let's make use of that to reduce
>>> the amount of data we need to copy. With this patch applied, we will only
>>> copy the tiny rectangle surrounding characters when we print them,
>>> speeding up the video console.
>> I suppose for rotated consoles it copies whole lines, but otherwise it
>> does a lot of small copies?
>
>
> I tried to keep the code as simple as possible and only track an "upper
> left" and "lower right" corner of modifications. So sync will always
> copy/flush a single rectangle.
Yep, see patch 06/13 for size of the regions. E.g. for putc_xy()s it's
fontdata->height * fontdata->width, for rows it's like fontdata->height
* vid_priv->xsize * count...
>>
>>> After this, changes to the main frame buffer are not immediately copied
>>> to the copy frame buffer, but postponed until the next video device
>>> sync. So issue an explicit sync before inspecting the copy frame buffer
>>> contents for the video tests.
>> So how does the sync get done in this case?
>
> It gets called as part of video_sync():
>
> +static void video_flush_copy(struct udevice *vid)
> +{
> + struct video_priv *priv = dev_get_uclass_priv(vid);
> +
> + if (!priv->copy_fb)
> + return;
> +
> + if (priv->damage.xend && priv->damage.yend) {
> + int lstart = priv->damage.xstart * VNBYTES(priv->bpix);
> + int lend = priv->damage.xend * VNBYTES(priv->bpix);
> + int y;
> +
> + for (y = priv->damage.ystart; y < priv->damage.yend; y++) {
> + ulong offset = (y * priv->line_length) + lstart;
> + ulong len = lend - lstart;
> +
> + memcpy(priv->copy_fb + offset, priv->fb + offset, len);
> + }
> + }
> +}
I think Simon was asking how and when video_sync() is called outside the
tests. The tests use lower-level functions that are ops->putc_xy() in
each console, and normally vidconsole calls higher on the call-chain
also maybe do a video_sync() when they think it's worth updating the
display.
>>
>>> Signed-off-by: Alexander Graf
>>> [Alper: Rebase for fontdata->height/w, fill_part(), fix memmove(dev),
>>> drop from defconfig, use damage.xstart/yend, use IS_ENABLED(),
>>> call video_sync() before copy_fb check, update video_copy test]
>>> Co-developed-by: Alper Nebi Yasak
>>> Signed-off-by: Alper Nebi Yasak
>>> ---
>>>
>>> Changes in v5:
>>> - Remove video_sync_copy() also from video_fill(), video_fill_part()
>>> - Fix memmove() calls by removing the extra dev argument
>>> - Call video_sync() before checking copy_fb in video tests
>>> - Use xstart, ystart, xend, yend as names for damage region
>>> - Use met->baseline instead of priv->baseline
>>> - Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH
>>> - Use xstart, ystart, xend, yend as names for damage region
>>> - Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
>>> - Drop VIDEO_DAMAGE from sandbox defconfig added in a new patch
>>> - Update dm_test_video_copy test added in a new patch
>>>
>>> Changes in v3:
>>> - Make VIDEO_COPY always select VIDEO_DAMAGE
>>>
>>> Changes in v2:
>>> - Add patch "video: Use VIDEO_DAMAGE for VIDEO_COPY"
>>>
>>> configs/sandbox_defconfig | 1 -
>>> drivers/video/Kconfig | 5 ++
>>> drivers/video/console_normal.c| 13 +
>>> drivers/video/console_rotate.c| 44 +++---
>>> drivers/video/console_truetype.c | 16 +
>>> drivers/video/vidconsole-uclass.c | 16 -
>>> drivers/video/video-uclass.c | 97 ---
>>> drivers/video/video_bmp.c | 7 ---
>>> include/video.h | 37
&
Re: [PATCH v5 10/13] video: Only dcache flush damaged lines
On 2023-08-22 02:03 +03:00, Simon Glass wrote: > Hi Alex, > > On Mon, 21 Aug 2023 at 16:44, Alexander Graf wrote: >> >> >> On 22.08.23 00:10, Simon Glass wrote: >>> Hi Alex, >>> >>> On Mon, 21 Aug 2023 at 13:59, Alexander Graf wrote: >>>> >>>> On 21.08.23 21:11, Simon Glass wrote: >>>>> Hi Alper, >>>>> >>>>> On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak >>>>> wrote: >>>>>> From: Alexander Graf >>>>>> >>>>>> Now that we have a damage area tells us which parts of the frame buffer >>>>>> actually need updating, let's only dcache flush those on video_sync() >>>>>> calls. With this optimization in place, frame buffer updates - especially >>>>>> on large screen such as 4k displays - speed up significantly. >>>>>> >>>>>> Signed-off-by: Alexander Graf >>>>>> Reported-by: Da Xue >>>>>> [Alper: Use damage.xstart/yend, IS_ENABLED()] >>>>>> Co-developed-by: Alper Nebi Yasak >>>>>> Signed-off-by: Alper Nebi Yasak >>>>>> --- >>>>>> >>>>>> Changes in v5: >>>>>> - Use xstart, ystart, xend, yend as names for damage region >>>>>> - Use IS_ENABLED() instead of CONFIG_IS_ENABLED() >>>>>> >>>>>> Changes in v2: >>>>>> - Fix dcache range; we were flushing too much before >>>>>> - Remove ifdefs >>>>>> >>>>>>drivers/video/video-uclass.c | 41 +++- >>>>>>1 file changed, 36 insertions(+), 5 deletions(-) >>>>> This is a little strange, since flushing the whole cache will only >>>>> actually write out data that was actually written (to the display). Is >>>>> there a benefit to this patch, in terms of performance? >>>> >>>> I'm happy to see you go through the same thought process I went through >>>> when writing these: "This surely can't be the problem, can it?". The >>>> answer is "simple" in hindsight: >>>> >>>> Have a look at the ARMv8 cache flush function. It does the only "safe" >>>> thing you can expect it to do: Clean+Invalidate to POC because we use it >>>> for multiple things, clearing modified code among others: >>>> >>>> ENTRY(__asm_flush_dcache_range) >>>> mrs x3, ctr_el0 >>>> ubfxx3, x3, #16, #4 >>>> mov x2, #4 >>>> lsl x2, x2, x3 /* cache line size */ >>>> >>>> /* x2 <- minimal cache line size in cache system */ >>>> sub x3, x2, #1 >>>> bic x0, x0, x3 >>>> 1: dc civac, x0 /* clean & invalidate data or unified >>>> cache */ >>>> add x0, x0, x2 >>>> cmp x0, x1 >>>> b.lo1b >>>> dsb sy >>>> ret >>>> ENDPROC(__asm_flush_dcache_range) >>>> >>>> >>>> Looking at the "dc civac" call, we find this documentation page from >>>> ARM: >>>> https://developer.arm.com/documentation/ddi0601/2022-03/AArch64-Instructions/DC-CIVAC--Data-or-unified-Cache-line-Clean-and-Invalidate-by-VA-to-PoC >>>> >>>> This says we're writing any dirtyness of this cache line up to the POC >>>> and then invalidate (remove the cache line) also up to POC. That means >>>> when you look at a typical SBC, this will either be L2 or system level >>>> cache. Every read afterwards needs to go and pull it all the way back to >>>> L1 to modify it (or not) on the next character write and then flush it >>>> again. >>>> >>>> Even worse: Because of the invalidate, we may even evict it from caches >>>> that the display controller uses to read the frame buffer. So depending >>>> on the SoC's cache topology and implementation, we may force the display >>>> controller to refetch the full FB content on its next screen refresh cycle. >>>> >>>> I faintly remember that I tried to experiment with CVAC instead to only >>>> flush without invalidating. I don't fully remember the results anymore >>>> though. I believe
Re: [PATCH v5 04/13] dm: video: Add damage tracking API
On 2023-08-21 22:11 +03:00, Simon Glass wrote: > On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak > wrote: >> >> From: Alexander Graf >> >> We are going to introduce image damage tracking to fasten up screen >> refresh on large displays. This patch adds damage tracking for up to >> one rectangle of the screen which is typically enough to hold blt or >> text print updates. Callers into this API and a reduced dcache flush >> code path will follow in later patches. >> >> Signed-off-by: Alexander Graf >> Reported-by: Da Xue >> [Alper: Use xstart/yend, document new fields, return void from >> video_damage(), declare priv, drop headers, use IS_ENABLED()] >> Co-developed-by: Alper Nebi Yasak >> Signed-off-by: Alper Nebi Yasak >> --- >> >> Changes in v5: >> - Use xstart, ystart, xend, yend as names for damage region >> - Document damage struct and fields in struct video_priv comment >> - Return void from video_damage() >> - Fix undeclared priv error in video_sync() >> - Drop unused headers from video-uclass.c >> - Use IS_ENABLED() instead of CONFIG_IS_ENABLED() >> >> Changes in v4: >> - Move damage clear to patch "dm: video: Add damage tracking API" >> - Simplify first damage logic >> - Remove VIDEO_DAMAGE default for ARM >> >> Changes in v3: >> - Adapt to always assume DM is used >> >> Changes in v2: >> - Remove ifdefs >> >> drivers/video/Kconfig| 13 >> drivers/video/video-uclass.c | 41 +--- >> include/video.h | 32 ++-- >> 3 files changed, 81 insertions(+), 5 deletions(-) >> > > Reviewed-by: Simon Glass > > But I suggest an empty static inline in the case where the feature is > disabled? You mean with something like #ifdef CONFIG_VIDEO_DAMAGE, right?
Re: [PATCH v2 4/4] doc: qemu: arm: Add a section on booting Linux distros
On 2023-08-30 10:33 +03:00, Ilias Apalodimas wrote: > Hi Alper, > > On Sun, 27 Aug 2023 at 18:49, Alper Nebi Yasak > wrote: >> >> On 2023-08-15 01:43 +03:00, Simon Glass wrote: >>> Hi Alper, >>> >>> On Mon, 14 Aug 2023 at 11:40, Alper Nebi Yasak >>> wrote: >>>> I actually want to put the root.img device first so that the VM can boot >>>> into the installed system when it reboots, but U-Boot can't find the >>>> bootflow on the second drive. I tried e.g. `bootdev list -p; bootflow >>>> scan -lab`, but it seems to only ever search the first virtio-blk. >>>> However, `eficonfig; bootefi bootmgr` makes it boot somehow. > > eficonfig, apart from displaying the boot options on a menu scans all > media that have a simple filesystem protocol installed and configures > the efi boot options. That's why bootefi bootmgr boots properly > afterwards. You can probably boot even without eficonfig, but only > bootXXX.efi files will be accounted for and no boot options will be > added. > > The EFI spec describes the bootmgr functionality in detail. Back when > the bootmeth patches were added, I suggested we shouldn't deal with > EFI at all simply because it all already works and is backed by a > spec. Instead, we should just have a boot option in the methods that > spells "EFI" and let the bootmanager deal with the details. But > honestly, I've lost track of those patches. I think bootflow and EFI should be different views on top of a single "what can we do now that we are in U-Boot" model. IIUIC it's possible for e.g. UEFI Shell to be built-in boot entries, so we could expose bootflow entries (and more) similar to those? And EFI-based secondary bootloaders like rEFInd could let us choose them in a graphical menu, efibootmgr could change their order or reboot into a non-EFI flow...
Re: [PATCH v2 4/4] doc: qemu: arm: Add a section on booting Linux distros
On 2023-08-28 20:54 +03:00, Simon Glass wrote: > Hi Alper, > > On Sun, 27 Aug 2023 at 09:49, Alper Nebi Yasak > wrote: >> >> On 2023-08-15 01:43 +03:00, Simon Glass wrote: >>> Hi Alper, >>> >>> On Mon, 14 Aug 2023 at 11:40, Alper Nebi Yasak >>> wrote: >>>> I actually want to put the root.img device first so that the VM can boot >>>> into the installed system when it reboots, but U-Boot can't find the >>>> bootflow on the second drive. I tried e.g. `bootdev list -p; bootflow >>>> scan -lab`, but it seems to only ever search the first virtio-blk. >>>> However, `eficonfig; bootefi bootmgr` makes it boot somehow. >>> >>> Yes, this is annoying. >>> >>> The problem is that the scanning is getting so complicated. The >>> boot_targets var lists things which can either be a uclass, or a >>> uclass with a device number. The currently implementation sees >>> "virtio" and moves on to the next thing in boot_targets once it has >>> processed one virtio device. That is obviously not correct, but... >>> >>> Would it be possible to just drop the 'boot_targets' var? That is what >>> is stuffing it up, but we don't actually need it now. The defaults end >>> up doing the same thing, apart (perhaps) from the strangeness of >>> virtio which can be both a network and a blk device. >>> >>> I believe it is possible to do the right thing, but I'll need to >>> create a better test mechanism to handle all the cases. >> >> I think some kind of a "priority score" could help -- iterate over >> boot_targets first just to generate bootdev priorities, then carry them >> over as bootflows are discovered (adjusted for bootmeth order), then use >> those scores as the order to boot / to show a sorted menu / to test? > > We sort-of have that, in that bootdevs have a priority. We could add > something to struct bootflow which takes that but adds more > fine-grained information, e.g. based on some sort of filter function > that can decide? > > Bear in mind that we normally want to boot the first thing we find, > and we also want to use lazy init (so no USB unless we need it). But > yes for the case where we display a menu I think we could make more of > the priority feature. What are you thinking of? For the priorities, I'm thinking of ChromiumOS boot flow where there is an in-method order that's different from the order we encounter things in. And I'm also thinking of customizing boot order for same-uclass device (I guess boot_targets="usb1 usb0" works?). And a bit worried about if we would want to mix and match orders. If we say methA > methB and dev0 > dev1, do we say methA-on-dev1 > methB-on-dev0 or the other way? Not sure if I make sense here at all, need to think more on it. In general, what I want is a graphical boot menu like rEFInd [1], but more specifically with a better theme more like [2]. Or, at least to support rEFInd better. (More bugs here, upstream builds don't start at all with U-Boot, Debian builds boot on arm64 but not x86). Regarding lazy init. If we don't init USB, rEFInd can't search USB drives to boot things from it. Not sure if that's a fundamental problem or there's some way for EFI apps to tell firmware to init USB etc. [1] https://www.rodsbooks.com/refind/ [2] https://github.com/bobafetthotmail/refind-theme-regular > BTW, even for the menu, my original vision was that the menu would > display immediately, with new bootflows appearing as they are > discovered. Shifting list elements around while people are choosing from the list is definitely not good, only way that could be OK is if the list was append-only and not centered-in-append-direction. We should be displaying a priority-sorted list, I'm not sure we can guarantee we will add things in priority order, and I would like things centered as personal preference...
Re: [PATCH 0/6] Attempt to enforce standard extensions for build output
On 2023-08-28 20:54 +03:00, Simon Glass wrote: > Hi Alper, > > On Sun, 27 Aug 2023 at 13:17, Alper Nebi Yasak > wrote: >> >> On 2023-08-24 06:02 +03:00, Simon Glass wrote: >>> In this early stage of using binman to produce output files, we are mostly >>> seeing people using common extensions such as '.bin' and '.rom' >>> >>> But unusual extensions appear in some places. >>> >>> We would like 'buildman -k' to keep the build outputs, but this is hard if >>> there is no consistency as to the extension used. >>> >>> This series adjusts binman to enforce just 4 extensions for output images: >>> >>>.bin >>>.rom >>>.itb >>>.img >>> >>> Other extensions will produce an error. With this rule observed, buildman >>> can keep the required files. >> >> I dislike this limitation. We know what files we will generate, they are >> listed in binman dtb, so we can add something like `binman build --ls` >> to print their names/paths for buildman to preserve them. > > Yes, it would be good to have that... > > But why do you dislike the limitation? Do you think extensions provide > useful information? I suppose one problem is that *.bin might pick up > private blobs that happen to be in the source directory? I guess my strongest argument is that people already/will have workflows that depend on certain filenames or extensions, and shouldn't have to modify binman code (consider that people may be using the PyPI wheels) to accommodate those when we are already putting the name in the dtb. I do want some standardization to the U-Boot build outputs though, but more like a global binman.dts with like u-boot.rom, u-boot-sdmmc.img images with well-defined TPL, SPL, U-Boot sections that arch-dtsi files can fill in. >> Regarding the output directory suggestion, I think the binman outputs >> (not temporary/intermediate files) should be in the same directory as >> make outputs > > Agreed > >> , and the Makefile should default to O=build to achieve the >> "output dir". I'm not sure if that's going to happen. > > I would quite like the 'non-output' file (i.e. things that are not a > binman image) to appear in a 'binman-work' subdir of the output dir. > What do you think? I'd prefer them to go in a tempfile.TemporaryDirectory() and discarded at the end of a binman run, and a "--tmpdir " option to use a given directory instead and preserve things for debugging.
Re: [PATCH] Adjust gitignore for /build
On 2023-09-04 22:18 +05:30, Simon Glass wrote: > Hi Rong, > > On Sun, 3 Sept 2023 at 20:50, Rong Tao wrote: >> >> From: Rong Tao >> >> /build-* can't ignore /build. > > How does that directory get created? Most likely `make O=build` as an attempt to do out-of-tree builds? FWIW, I manually create and use build/$board/ directories for build outputs, but adding "build" to .git/info/exclude is an easy enough alternative. >> Signed-off-by: Rong Tao >> --- >> .gitignore | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/.gitignore b/.gitignore >> index 002f95de4feb..9697f0088f80 100644 >> --- a/.gitignore >> +++ b/.gitignore >> @@ -35,7 +35,7 @@ >> *.tab.[ch] >> >> # Build tree >> -/build-* >> +/build* >> >> # >> # Top-level generic files >> -- >> 2.41.0 >> > > Regards, > Simon
Re: [PATCHv9 01/15] submodule: add lwIP as git submodule
On 2023-09-21 18:39 +03:00, Tom Rini wrote: > On Wed, Sep 20, 2023 at 07:03:07PM -0600, Simon Glass wrote: >> Hi Maxim, >> >> On Thu, 14 Sept 2023 at 10:20, Maxim Uvarov wrote: >>> >>> add external lwIP library as a git submodule. If we add submodules, they should point to mirrors on source.denx.de, in case upstream repositories vanish. >> >> Oh dear...what is the motivation for using a submodule? > > That we don't have a better alternative. Using "git subtree" has it's > own problems and won't make things overall better. I need to raise a technical concern here, buildman uses "git worktree" to save disk space (provides git functionality in .bm-work source copies without copying .git into N such dirs), and there's this warning in the git-worktree(1) manual page: Multiple checkout in general is still experimental, and the support for submodules is incomplete. It is NOT recommended to make multiple checkouts of a superproject. It's been there for a while, and I don't know what the current status is. But if we decide to add submodules we'll most likely need to rework some stuff in buildman to support that. Besides that, I've been running into git submodule problems in other projects and don't like it at all. It doesn't seem to play nice with even fundamental git features/commands like git pull and git checkout. >> Our current stack is nicely integrated into U-Boot. This would make >> moving between development branches much more painful. > > It really shouldn't matter in that case. Unless you're trying out a new > lwip upstream commit, nothing changes in there. It _may_ mean that if > we want to update lwip it's not something that can be staged first in > the -next branch but instead something to pull in just after release, > but I'd need to see how smart or not git is today about things like > that. > >> I would much prefer that we bring in the necessary code, and that you >> send a patch every 3 months or so to deal with updates, making sure >> there are no code-size regressions. > > And I much prefer something that will make sure that we don't start > making changes in upstream code and diverging. I don't think there's a > mechanic short of submodule that will do that for us. Coreboot clones some payload repositories in Makefiles (although they use submodules a lot as well). Another alternative is having buildman manage external sources like how binman can download sources and build the tools it needs. Or maybe just assume the source already exists at ../lwip/ or $LWIP_SOURCE and raise an error if we can't find it there. I admit these might sound worse than git submodules, but at least any problems with them will be our fault and would be solvable in U-Boot instead of requiring a trip to upstream git.
Re: [PATCH] dt-bindings: mtd: Add a schema for binman
On 2023-09-21 21:45 +03:00, Simon Glass wrote: > Binman[1] is a tool for creating firmware images. It allows you to > combine various binaries and place them in an output file. > > Binman uses a DT schema to describe an image, in enough detail that > it can be automatically built from component parts, disassembled, > replaced, listed, etc. > > Images are typically stored in flash, which is why this binding is > targeted at mtd. Previous discussion is at [2] [3]. > > [1] https://u-boot.readthedocs.io/en/stable/develop/package/binman.html > [2] https://lore.kernel.org/u-boot/20230821180220.2724080-3-...@chromium.org/ > [3] https://www.spinics.net/lists/devicetree/msg626149.html > > Signed-off-by: Simon Glass > --- > > .../bindings/mtd/partitions/binman.yaml | 50 +++ > .../bindings/mtd/partitions/binman/entry.yaml | 61 +++ > .../bindings/mtd/partitions/partitions.yaml | 1 + > MAINTAINERS | 5 ++ > 4 files changed, 117 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman.yaml > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman/entry.yaml This doesn't match the schema in [2], but seems more like v1 of that. Is that intentional?
Re: R: Pull request: u-boot-rockchip-20230728
Hi Kever, On 2023-07-31 04:27 +03:00, Kever Yang wrote: > I have send the review tag for these patches, but the patchwork does not > able to get my review mail, and not able to update the patch status to > reviewed, > > and that's why I missing this patch set. > > I will apply these patch manually in next PR. Please consider my other rk3288-veyron config changes [1] as well. [1] https://patchwork.ozlabs.org/project/uboot/list/?series=362920 Thanks, Alper
Re: [PATCH 2/2] schemas: Add a schema for binman
(I think I've strayed far away from schema/dtb side of things towards
binman-the-program side, so I'm dropping Rob and devicetree@ from Cc).
On 2023-07-20 22:55 +03:00, Simon Glass wrote:
> On Thu, 20 Jul 2023 at 09:23, Alper Nebi Yasak
> wrote:
>> There's also the issue of binman having multiple different device-trees:
>> its input, the ones in fdtmaps per image, the ones injected to U-Boot
>> dtb files per image. I'd say each has different needs, and those
>> differences have to be figured out before specified upstream. I can only
>> guess this is about the one that'll be in a u-boot.dtb.
>
> Well, there is really only one. The fdtmap is actually the same
> schema, except that it mentions only the image that it is embedded in,
> i.e. if the fdtmap is for the SPI image, then the fdtmap in SPI flash
> only contains the definition for the SPI image, not the MMC image
> which is in a different device. The input is the same schema, albeit
> that things like 'offset' may be filled in by Binman automatically
> when it packs things.
I was thinking of things like generator nodes and templates and
"filename"s in blob entries that (IMO) only make sense as binman inputs
and never should be in a fdtmap. Trying to highlight a difference
between "how to build this image" and "what this image contains".
But I guess it's OK to call them the same schema unless something has
two conflicting meanings in input-domain and output-domain, and I can't
think of any counter-examples now.
>> I want to go through binman more thoroughly, but a lot of changes
>> happened since I last looked at it and I'm a bit slow at writing things,
>> so won't exactly be soon. That being said, here's some ideas off the top
>> of my head, for inspiration on both this schema and binman itself.
>
> Do you mean the code? There are definitely some abstractions that are
> stretching a bit, but it is mostly holding together for now.
I mean both the implementation and the design. There's a lot more on my
mind, some more examples:
- Precise structures for data instead of thinking of them as black boxes
- Heuristically parsing arbitrary images/data for e.g. binman extract
- Deconstructing input files to reuse their pieces for building images
- Explicit dependency tracking and resolution for data and layout
- Making things act more like native Python objects
- In fact, making it entirely operable with just Python code
- Decoupling internals from the control dtbs ("entry._node" etc.)
- Ensuring reproducible builds and testing for it
- Fuzzing as a replacement for most tests
- ...
I think it has the beginnings of a very nice declarative-style tool, but
has to embrace that style to get there. (And I guess I'll have to do the
work to properly express myself on some points...)
> [...]
>> Or maybe even better, I think we should make it like FIT: allow a "data"
>> property that has it inside the dtb, or a pair of "data-offset/position"
>> and "data-size" properties if it's outside.
>
> Eh? The point of the entry is to declare the position of actual data.
> If the data is elsewhere, then the entry will be too.
Sorry, I'm jumping into a different contexts here without explaining it.
I'm seeing a similarity between images that start with a fdtmap and the
images built by "mkimage -T fit -E". Then I'm extrapolating to the
non-"-E" case. Then extending to make it possible for a "fdtmap + data"
binman description to result in something almost backwards-compatible
with FIT, which could replace most uses of a fit etype.
(Of course, backwards compatible only if you add config nodes, and
flatten or don't nest entries, and whatnot. And fit etype would still stay.)
>> Inlining data inside the dtb
>> could help us do nice things in binman, like hashes/signatures as entry
>> types instead of special-casing them.
>
> We already do that though, right? See testHash() for example. This is
> a powerful feature of a firmware-layout description / Binman, IMO,
> since all the metadata you need is right there.
Having that functionality in state.py and having to work around it for
mkimage/fit etypes (because we want to embed them into the data instead
of the binman dtb) is what I mean by special-casing.
If we had them as etypes, and could embed arbitrary entries as "data" in
binman dtb, I think we would have the best of both worlds -- avoid
calling mkimage just for hash/signature embedding, have it still
possible to put those in binman metadata, and enable a foundation for
other sign-verify flows (leaning towards replacing custom signing
tools/scripts with binman).
>> In fact it could be possible to turn binman
Re: [PATCH v2 2/2] schemas: Add a schema for binman
On 2023-07-20 22:56 +03:00, Simon Glass wrote:
> With this version I have done with a generic name, in this case 'data',
> as suggested by Alper Nebi Yasak. This may be controversial, but we may
> as well have the dicussion now. I assume that there are no other
> ongoing attempts to define the layout of firmware in devicetree.
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v2:
> - Reworked significantly based on Alper's comments
>
> dtschema/schemas/firmware/binman/entry.yaml | 80 +
> dtschema/schemas/firmware/image.yaml| 77
> 2 files changed, 157 insertions(+)
> create mode 100644 dtschema/schemas/firmware/binman/entry.yaml
> create mode 100644 dtschema/schemas/firmware/image.yaml
>
> diff --git a/dtschema/schemas/firmware/binman/entry.yaml
> b/dtschema/schemas/firmware/binman/entry.yaml
> new file mode 100644
> index 000..d50f96d
> --- /dev/null
> +++ b/dtschema/schemas/firmware/binman/entry.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Google LLC
> +
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/image/entry.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Image entry
> +
> +maintainers:
> + - Simon Glass
> +
> +description: |
> + The entry node specifies a single entry in the firmware image.
> +
> + Entries have a specific type, such as "u-boot" or "atf-bl31". This is
> provided
> + using compatible = "data,".
> +
> + Note: This definition is intended to be hierarchical, so that entries can
> + appear in other entries. Schema for that is TBD.
> +
> +properties:
> + $nodename:
> +pattern: "^[-a-z]+(-[0-9]+)?$"
> +
> + compatible:
> +$ref: /schemas/types.yaml#/definitions/string
> +
> + offset:
> +$ref: /schemas/types.yaml#/definitions/uint32
> +description: |
> + Provides the offset of this entry from the start of its parent section.
> +
> + This may be omitted in the description provided by Binman, in which
> case
> + the value is calculated as part of image packing.
> +
> + size:
> +$ref: /schemas/types.yaml#/definitions/uint32
> +description: |
> + Provides the size of this entry in bytes.
> +
> + This may be omitted in the description provided by Binman, in which
> case
> + the value is calculated as part of image packing.
So AFAIU, binman will take none/one/both of "offset" and "size" as
inputs and will pass them to the output unmodified, instead adding a
"reg" pair of their calculated final values?
Is there a schema-computational way to ensure that "reg" has to contain
the same values as "offset" and "size"? Or is that not a restriction at
all and "reg" overrides the others?
> +
> + reg:
> +description: |
> + Defines the offset and size of this entry, with reference to its parent
> + image / section.
> +
> + Note This is typically omitted in the description provided to Binman,
> + since the value is calculated as part of image packing. Separate
> + properties are provided for the size and offset of an entry, so that
> it is
> + easy to specify none, one or both. The `reg` property is the only one
> that
> + needs to be looked at once the image has been built.
> +
Do we not need a $ref for "reg"? Is there anything applicable?
BTW, I'm not that familiar with device-tree interpretation, I
occasionally saw 'reg' being used as an pair, and was
mostly just asking if it's appropriate.
(Also TIL "ranges", and I'm imagining ranges = <0 $size 4G-$size 4G>; as
an end-at-4gb replacement/generalization :P, but I know, later, later.)
> +required:
> + - compatible
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +firmware {
> + image {
> +compatible = "data,image";
> +#address-cells = <1>;
> +$size-cells = <1>;
> +
> +u-boot@0 {
> + compatible = "data,u-boot";
> + reg = <0 0xa>;
> +};
> +
> +atf-bl31@0x10 {
> + compatible = "data,atf-bl31";
> + reg = <0x10 0x2>;
> +};
> + };
> +};
> diff --git a/dtschema/schemas/firmware/image.yaml
> b/dtschema/schemas/firmware/image.yaml
> new file mode 100644
> index 000..949b067
> --- /dev/null
> +++ b/dtschema/schemas/firmware/image.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-Licens
Re: [PATCH 00/16] bootstd: Improve ChromiumOS support
Hi Simon, On 2023-07-30 20:16 +03:00, Simon Glass wrote: > The ChromiumOS bootmeth is fairly basic at present. It is able to boot > only x86 kernels and contains quite a few hard-coded offsets. > > This series tidies it up by bringing in some vboot structures and adding > support for ARM. > > It adds a few more features to bootstd, including display of x86 setup > information. Can't do a detailed review or test for a while, but had a cursory look now and wanted to reply. I'm not sure if you heard of it, but I maintain an alternate implementation of ChromiumOS verified boot userspace called depthcharge-tools [1]. Its main purpose is booting ordinary distros on Chromebooks, but it would be nice if your U-Boot implementation was also compatible with images produced from that. I guess it boils down to: - Enable booting from any chromeos_kernel partition (not just 2 & 4) - Fixup the ramdisk addr/size in x86 setup info after reading the kernel - Replace %U with chromeos_kernel PARTUUID (you probably already do?) - Prepend cros_secure to kernel cmdline There's a ready made debian-installer image if you want to test [2]. Has a partitioning bug defaulting to MBR, but otherwise can install for CrOS verified boot as well. (It's what took my last year away from U-Boot). [1] depthcharge-tools -- Tools to manage the Chrome OS bootloader https://github.com/alpernebbi/depthcharge-tools [2] Debian Installer netboot image for amd64 Chromebooks https://d-i.debian.org/daily-images/amd64/daily/netboot/depthcharge/ > So far this does not actually boot correctly on any ARM Chromebook: > >jerry - hangs when booting kernel >bob - Bad Linux ARM64 Image magic! with lz4-compressed kernel > > Further work can address these issues. I haven't been able to boot recent kernels with extlinux.conf on my pre-bootstd kevin U-Boot builds which I remember were working with older kernels. And I've heard people not being boot with extlinux.conf on veyrons as well. Might be the same, maybe try an older kernel for jerry? Again, there's a debian-installer arm64 image if you want to test [3], which could work fine on bob (not with depthcharge's size limit though), but kernel support is missing for anything else. Oh, and postmarketOS also supports CrOS verified boot [4] via depthcharge-tools, but they don't have pre-built images and it's a bit of a hassle. [3] Debian Installer netboot image for arm64 Chromebooks https://d-i.debian.org/daily-images/arm64/daily/netboot/depthcharge/ [4] postmarketOS Wiki - Chrome OS devices https://wiki.postmarketos.org/wiki/Chrome_OS_devices Anyway, I'm glad you're working on this, looking forward to testing it myself when I get the chance! > Simon Glass (16): > bootstd: cros: Correct reporting of I/O errors > bootstd: cros: Move partition reading into a function > bootstd: cros: Bring in some ChromiumOS structures > bootstd: cros: Support a kernel on either partition > bootstd: cros: Decode some kernel preamble fields > bootstd: cros: Simplify setup and cmdline expressions > bootstd: Move common zimage functions to bootm.h > bootstd: cros: Add docs for the kernel layout > bootstd: cros: Add private info for ChromiumOS > bootstd: Add private bootmeth data to the bootflow > bootstd: cros: Add a function to read info from partition > bootstd: cros: Add a function to read a kernel > bootstd: cros: Split up reading info and kernel > bootstd: Allow display of the x86 setup information > bootstd: Add a command to read all files for a bootflow > bootstd: cros: Add ARM support > > arch/x86/include/asm/zimage.h | 37 > arch/x86/lib/zimage.c | 8 +- > boot/Kconfig | 4 +- > boot/bootflow.c | 15 ++ > boot/bootm.c | 37 > boot/bootmeth-uclass.c| 10 + > boot/bootmeth_cros.c | 363 -- > boot/bootmeth_cros.h | 197 ++ > cmd/bootflow.c| 47 - > doc/usage/cmd/bootflow.rst| 139 - > include/bootflow.h| 15 +- > include/bootm.h | 47 + > include/bootmeth.h| 23 +++ > 13 files changed, 836 insertions(+), 106 deletions(-) > create mode 100644 boot/bootmeth_cros.h >
Re: [PATCH 00/16] bootstd: Improve ChromiumOS support
(Note that I'm not objecting to merging the patchset as is, just detailing future work you/I might want to do for non-CrOS userspace support for this bootmeth.) On 2023-08-04 06:02 +03:00, Simon Glass wrote: > On Thu, 3 Aug 2023 at 15:31, Alper Nebi Yasak > wrote: >> I guess it boils down to: >> >> - Enable booting from any chromeos_kernel partition (not just 2 & 4) > > How do you know which ones hold kernels, and which root disks > correspond to each kernel? You should search all those with FE3A2A5D-4F32-41A7-B725-ACCC3285A309 as their GPT partition type GUID. They are ordered wrt/ the type-specific Successful, Priority, Tries attribute flags (bits 56, 55-52, 51-48). The firmware is also supposed to modify these (decrement Tries on each boot attempt, set Priority = 0 if Tries == 0). All these are stock depthcharge behaviour, see ChromiumOS docs [5]. The kernels don't necessarily correspond to a specific root, that's a per-OS concept. For example on Debian, I have two or three of these partitions for the same rootfs used in a cyclic fashion to have A/B-style automatic fallback for kernel/initramfs changes. And there may not be a rootfs at all, like how Debian Installer runs entirely from its initramfs. [5] ChromiumOS Docs - Disk Format - Selecting the kernel https://chromium.googlesource.com/chromiumos/docs/+/HEAD/disk_format.md#selecting-the-kernel >> - Fixup the ramdisk addr/size in x86 setup info after reading the kernel > > But ChromeOS doesn't use ramdisk, right? Not in this way, AFAIK it embeds one into vmlinuz during compile-time? I guess the ramdisk addr/size in x86 setup is 0/0 for ChromeOS indicating there isn't (an external) one. But depthcharge is so minimalistic that it loads everything at a fixed address and passes x86 setup unchanged as prepared beforehand, so I can append a ramdisk to vmlinuz and tell Linux where that will end up in memory behind depthcharge's back. (A bit more complicated than that.) So the thing is, when bootmeth_cros reads the "kernel" data into memory somewhere other than 0x10, the x86 setup from my images will be pointing to an unrelated location as the ramdisk, which Linux will fail to interpret as a ramdisk and panic. >> - Prepend cros_secure to kernel cmdline > > Does it have to be at the start of the cmdline? Also, the cmdline > comes from cros at present, so does include that normally. I think > the best way is for you to give it a try and see what is needed. Doesn't have to be at the start, but Depthcharge unconditionally prepends it, I do see multiple "cros_secure"s on ChromeOS /proc/cmdline. I'd like to have that here too as a way to identify the boot method, since non-CrOS userspaces will not have configured cmdline to have that. In Debian Installer, I do `grep cros_secure /proc/cmdline` to see if we should set up with depthcharge-tools or something else like GRUB. (Yeah, I should be testing this, but stretched myself too thin.) >> There's a ready made debian-installer image if you want to test [2]. Has >> a partitioning bug defaulting to MBR, but otherwise can install for CrOS >> verified boot as well. (It's what took my last year away from U-Boot). >> >> [1] depthcharge-tools -- Tools to manage the Chrome OS bootloader >> https://github.com/alpernebbi/depthcharge-tools >> >> [2] Debian Installer netboot image for amd64 Chromebooks >> https://d-i.debian.org/daily-images/amd64/daily/netboot/depthcharge/ > > I see the installer, but how do I actually run it on bob, say? I hoped the names were self-evident, but I know I should be writing docs on them, just postponing until they are in better shape. Basically `gzip -d
Re: [PATCH] efi_loader: Increase default variable store size to 32K
Hi Ilias,
On 2023-07-28 14:15 +03:00, Ilias Apalodimas wrote:
> On Sat, 8 Jul 2023 at 18:21, Alper Nebi Yasak
> wrote:
>>
>> Debian's arm64 UEFI Secure Boot shim makes the EFI variable store run
>> out of space while mirroring its MOK database to variables. This can be
>> observed in QEMU like so:
>>
>> $ tools/buildman/buildman -o build/qemu_arm64 --boards=qemu_arm64 -w
>> $ cd build/qemu_arm64
>> $ curl -L -o debian.iso \
>>
>> https://cdimage.debian.org/debian-cd/current/arm64/iso-cd/debian-12.0.0-arm64-netinst.iso
>> $ qemu-system-aarch64 \
>> -nographic -bios u-boot.bin \
>> -machine virt -cpu cortex-a53 -m 1G -smp 2 \
>> -drive
>> if=virtio,file=debian.iso,index=0,format=raw,readonly=on,media=cdrom
>> [...]
>> => # interrupt autoboot
>> => env set -e -bs -nv -rt -guid 605dab50-e046-4300-abb6-3dd810dd8b23
>> SHIM_VERBOSE 1
>> => boot
>> [...]
>> mok.c:296:mirror_one_esl() SetVariable("MokListXRT43", ... varsz=0x4C) =
>> Out of Resources
>> mok.c:452:mirror_mok_db() esd:0x7DB92D20 adj:0x30
>> Failed to set MokListXRT: Out of Resources
>> mok.c:767:mirror_one_mok_variable() mirror_mok_db("MokListXRT",
>> datasz=17328) returned Out of Resources
>> mok.c:812:mirror_one_mok_variable() returning Out of Resources
>> Could not create MokListXRT: Out of Resources
>> [...]
>> Welcome to GRUB!
>>
>> This would normally be fine as shim would continue to run grubaa64.efi,
>> but shim's error handling code for this case has a bug [1] that causes a
>> synchronous abort on at least chromebook_kevin (but apparently not on
>> QEMU arm64).
>>
>> Double the default variable store size so the variables fit. There is a
>> note about this value matching PcdFlashNvStorageVariableSize when
>> EFI_MM_COMM_TEE is enabled, so keep the old default in that case.
>
> Thanks for the report. That EFI_MM_COMM_TEE basically means that the
> variables will be stored in an RPMB partition of an eMMC device. This
> has a couple of advantages compared to storing it in a file (mostly
> security related), but I can change that in the future.
I've read your article on it, but haven't really explored this stuff
because one-time-programmable things make me a bit afraid.
Mind that this happens even without any persistence at all, i.e.
ENV_IS_NOWHERE and EFI_VARIABLE_NO_STORE.
> When you use 32kb how much space do you have left after MoK etc have
> been written?
I can press "c" at the GRUB menu and run "exit" to get back to the
U-Boot shell, then I have:
=> efidebug query -bs -rt -nv
Max storage size 65512
Remaining storage size 54968
Max variable size 65480
=> env print -e -n
SecureBoot:
8be4df61-93ca-11d2-aa0d-00e098032b8c (EFI_GLOBAL_VARIABLE_GUID)
BS|RT|RO, DataSize = 0x1
SetupMode:
8be4df61-93ca-11d2-aa0d-00e098032b8c (EFI_GLOBAL_VARIABLE_GUID)
BS|RT|RO, DataSize = 0x1
AuditMode:
8be4df61-93ca-11d2-aa0d-00e098032b8c (EFI_GLOBAL_VARIABLE_GUID)
BS|RT|RO, DataSize = 0x1
DeployedMode:
8be4df61-93ca-11d2-aa0d-00e098032b8c (EFI_GLOBAL_VARIABLE_GUID)
BS|RT|RO, DataSize = 0x1
VendorKeys:
8be4df61-93ca-11d2-aa0d-00e098032b8c (EFI_GLOBAL_VARIABLE_GUID)
BS|RT|RO, DataSize = 0x1
PlatformLangCodes:
8be4df61-93ca-11d2-aa0d-00e098032b8c (EFI_GLOBAL_VARIABLE_GUID)
BS|RT|RO, DataSize = 0x6
PlatformLang:
8be4df61-93ca-11d2-aa0d-00e098032b8c (EFI_GLOBAL_VARIABLE_GUID)
NV|BS|RT, DataSize = 0x6
OsIndicationsSupported:
8be4df61-93ca-11d2-aa0d-00e098032b8c (EFI_GLOBAL_VARIABLE_GUID)
BS|RT|RO, DataSize = 0x8
SbatLevel:
605dab50-e046-4300-abb6-3dd810dd8b23
(605dab50-e046-4300-abb6-3dd810dd8b23)
NV|BS, DataSize = 0x19
MokListRT:
605dab50-e046-4300-abb6-3dd810dd8b23
(605dab50-e046-4300-abb6-3dd810dd8b23)
BS|RT, DataSize = 0x3ce
MokListXRT:
605dab50-e046-4300-abb6-3dd810dd8b23
(605dab50-e046-4300-abb6-3dd810dd8b23)
BS|RT, DataSize = 0x21d8
SbatLevelRT:
605dab50-e046-4300-abb6-3dd810dd8b23
(605dab50-e046-4300-abb6-3dd810dd8b23)
BS|RT, DataSize = 0x19
MokListTrustedRT:
605dab50-e046-4300-abb6-3dd810dd8b23
(605dab50-e046-4300-abb6-3dd810dd8b23)
BS|RT, DataSize = 0x1
It's weird seeing 65512 - 54968 == 10544 < 16K. (Heinrich bumped it to
64K as he applied the patch, based on Windows docs.) But I'm noticing
how MokListXRT DataSize 0x21d8 * 2 == 17328 matches the datasz value in
the error.
If I retry with 16K, I see 43 extra values each with 0x4c size in
addition to the above.
=> efidebug query -bs -rt -nv
Max storage size 16360
Remaining storage size 0
Max variable size 16328
=> env print -e -n
[...]
MokListXRT1:
605dab50-e046-4300-abb6-3dd810dd8b23
(605dab50-e046-4300-abb6-3dd810dd8b23)
BS|RT, DataSize = 0x4c
[..]
MokListXRT43:
605dab50-e046-4300-abb6-3dd810dd8b23
(605dab50-e046-4300-abb6-3dd810dd8b23)
BS|RT, DataSize = 0x4c
Hope that helps, tell me if there's other commands you want me to run.
[PATCH 0/3] arm: qemu: Enable Bochs, console buffering, USB keyboard
Now that the driver for the Bochs VGA card emulated by QEMU is no longer limited to x86 architectures [1], this series enables it on arm and arm64 virtual machines to provide a graphical interface. In line with that series this also enables console buffering and USB keyboard. Tested with the Debian 12 installer using GRUB EFI: $ tools/buildman/buildman -o build/qemu_arm64 --boards=qemu_arm64 -w $ cd build/qemu_arm64 $ curl -L -o debian.img \ https://cdimage.debian.org/debian-cd/current/arm64/iso-cd/debian-12.0.0-arm64-netinst.iso $ qemu-system-aarch64 \ -machine virt -cpu cortex-a53 -m 1G -smp 2 \ -bios u-boot.bin \ -serial stdio -device VGA \ -device qemu-xhci,id=xhci -device usb-kbd,bus=xhci.0 -device usb-mouse,bus=xhci.0 \ -drive if=virtio,file=debian.img,index=0,format=raw,readonly=on,media=cdrom And with one using extlinux.conf: $ [...] $ curl -L -o head.img.gz \ https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/SD-card-images/gtk/firmware.none.img.gz $ curl -L -o partition.img.gz \ https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/SD-card-images/gtk/partition.img.gz $ zcat head.img.gz partition.img.gz >debian.img $ [...] Both can get to a graphical installer just fine, in addition to U-Boot video console showing up in a GTK window. [1] video: bochs: Remove the x86 limitation https://lore.kernel.org/u-boot/20230723044041.1089804-1-bm...@tinylab.org/ Alper Nebi Yasak (3): arm: qemu: Enable Bochs video support arm: qemu: Enable PRE_CONSOLE_BUFFER arm: qemu: Enable usb keyboard as an input device arch/arm/Kconfig | 10 ++ board/emulation/qemu-arm/Kconfig | 4 board/emulation/qemu-arm/qemu-arm.c | 5 + board/emulation/qemu-arm/qemu-arm.env | 3 +++ configs/qemu_arm64_defconfig | 2 -- configs/qemu_arm_defconfig| 2 -- doc/board/emulation/qemu-arm.rst | 8 7 files changed, 30 insertions(+), 4 deletions(-) base-commit: a169438411f9277cc689c14078151aa1d1caae3c -- 2.40.1
[PATCH 1/3] arm: qemu: Enable Bochs video support
Commit 716161663ec49 ("riscv: qemu: Enable Bochs video support") enables
a video console for QEMU RISC-V virtual machines using an emulated Bochs
VGA card. Similarly, enable it for ARM virtual machines as well.
Signed-off-by: Alper Nebi Yasak
---
arch/arm/Kconfig | 4
board/emulation/qemu-arm/qemu-arm.env | 3 +++
doc/board/emulation/qemu-arm.rst | 4
3 files changed, 11 insertions(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 97c25b4f146d..0d4654fb9dfc 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1036,6 +1036,10 @@ config ARCH_QEMU
imply DM_RTC
imply RTC_PL031
imply OF_HAS_PRIOR_STAGE
+ imply VIDEO
+ imply VIDEO_BOCHS
+ imply SYS_WHITE_ON_BLACK
+ imply SYS_CONSOLE_IS_IN_ENV
config ARCH_RMOBILE
bool "Renesas ARM SoCs"
diff --git a/board/emulation/qemu-arm/qemu-arm.env
b/board/emulation/qemu-arm/qemu-arm.env
index e658d5ee7d63..86a99a2e8713 100644
--- a/board/emulation/qemu-arm/qemu-arm.env
+++ b/board/emulation/qemu-arm/qemu-arm.env
@@ -2,6 +2,9 @@
/* environment for qemu-arm and qemu-arm64 */
+stdin=serial
+stdout=serial,vidconsole
+stderr=serial,vidconsole
fdt_high=0x
initrd_high=0x
fdt_addr=0x4000
diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
index b42d924cc66a..1108fe5f8184 100644
--- a/doc/board/emulation/qemu-arm.rst
+++ b/doc/board/emulation/qemu-arm.rst
@@ -67,6 +67,10 @@ Additional persistent U-Boot environment support can be
added as follows:
Additional peripherals that have been tested to work in both U-Boot and Linux
can be enabled with the following command line parameters:
+- To add a video console, remove "-nographic" and add e.g.::
+
+-serial stdio -device VGA
+
- To add a Serial ATA disk via an Intel ICH9 AHCI controller, pass e.g.::
-drive if=none,file=disk.img,format=raw,id=mydisk \
--
2.40.1
[PATCH 2/3] arm: qemu: Enable PRE_CONSOLE_BUFFER
Commit 608b80b5b855 ("riscv: qemu: Enable PRE_CONSOLE_BUFFER") enables
buffering console messages for QEMU RISC-V virtual machines so those
printed before the video console is available will still show up on the
display. Similarly, enable it for ARM virtual machines as well.
Signed-off-by: Alper Nebi Yasak
---
Not sure about the address, here are select values from bdinfo and env:
DRAM bank = 0x
-> start= 0x4000
-> size = 0x0800
relocaddr = 0x476d6000
fdt_blob= 0x46595d90
lmb_dump_all:
memory.cnt = 0x1 / max = 0x10
memory[0] [0x4000-0x47ff], 0x0800 bytes flags: 0
reserved.cnt = 0x2 / max = 0x10
reserved[0][0x45591000-0x47ff], 0x02a6f000 bytes flags: 0
reserved[1][0x46591760-0x47ff], 0x01a6e8a0 bytes flags: 0
TLB addr= 0x47fe
irq_sp = 0x46595d80
sp start= 0x46595d80
fdt_addr= 0x4000
scriptaddr = 0x4020
loadaddr= 0x4020
pxefile_addr_r = 0x4030
kernel_addr_r = 0x4040
ramdisk_addr_r = 0x4400
fdtcontroladdr = 465e5ea0
arch/arm/Kconfig | 1 +
board/emulation/qemu-arm/Kconfig | 4
2 files changed, 5 insertions(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0d4654fb9dfc..89b978797720 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1040,6 +1040,7 @@ config ARCH_QEMU
imply VIDEO_BOCHS
imply SYS_WHITE_ON_BLACK
imply SYS_CONSOLE_IS_IN_ENV
+ imply PRE_CONSOLE_BUFFER
config ARCH_RMOBILE
bool "Renesas ARM SoCs"
diff --git a/board/emulation/qemu-arm/Kconfig b/board/emulation/qemu-arm/Kconfig
index ed9949651c4b..09c95413a541 100644
--- a/board/emulation/qemu-arm/Kconfig
+++ b/board/emulation/qemu-arm/Kconfig
@@ -12,6 +12,10 @@ config BOARD_SPECIFIC_OPTIONS # dummy
imply VIRTIO_NET
imply VIRTIO_BLK
+config PRE_CON_BUF_ADDR
+ hex
+ default 0x4010
+
endif
if TARGET_QEMU_ARM_64BIT && !TFABOOT
--
2.40.1
[PATCH 3/3] arm: qemu: Enable usb keyboard as an input device
Commit 02be57caf730 ("riscv: qemu: Enable usb keyboard as an input
device") adds PCI xHCI support to QEMU RISC-V virtual machines and
enables using a USB keyboard as one of the input devices. Similarly,
enable those for ARM virtual machines as well.
Signed-off-by: Alper Nebi Yasak
---
arch/arm/Kconfig | 5 +
board/emulation/qemu-arm/qemu-arm.c | 5 +
board/emulation/qemu-arm/qemu-arm.env | 2 +-
configs/qemu_arm64_defconfig | 2 --
configs/qemu_arm_defconfig| 2 --
doc/board/emulation/qemu-arm.rst | 4
6 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89b978797720..1fd3ccd1607f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1041,6 +1041,11 @@ config ARCH_QEMU
imply SYS_WHITE_ON_BLACK
imply SYS_CONSOLE_IS_IN_ENV
imply PRE_CONSOLE_BUFFER
+ imply USB
+ imply USB_XHCI_HCD
+ imply USB_XHCI_PCI
+ imply USB_KEYBOARD
+ imply CMD_USB
config ARCH_RMOBILE
bool "Renesas ARM SoCs"
diff --git a/board/emulation/qemu-arm/qemu-arm.c
b/board/emulation/qemu-arm/qemu-arm.c
index dfea0d92a3c8..942f1fff5717 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
@@ -114,6 +115,10 @@ int board_late_init(void)
*/
virtio_init();
+ /* start usb so that usb keyboard can be used as input device */
+ if (CONFIG_IS_ENABLED(USB_KEYBOARD))
+ usb_init();
+
return 0;
}
diff --git a/board/emulation/qemu-arm/qemu-arm.env
b/board/emulation/qemu-arm/qemu-arm.env
index 86a99a2e8713..fb4adef281ed 100644
--- a/board/emulation/qemu-arm/qemu-arm.env
+++ b/board/emulation/qemu-arm/qemu-arm.env
@@ -2,7 +2,7 @@
/* environment for qemu-arm and qemu-arm64 */
-stdin=serial
+stdin=serial,usbkbd
stdout=serial,vidconsole
stderr=serial,vidconsole
fdt_high=0x
diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
index 94bd96678443..f6b8ae530a4a 100644
--- a/configs/qemu_arm64_defconfig
+++ b/configs/qemu_arm64_defconfig
@@ -35,7 +35,6 @@ CONFIG_CMD_NVEDIT_EFI=y
CONFIG_CMD_DFU=y
CONFIG_CMD_MTD=y
CONFIG_CMD_PCI=y
-CONFIG_CMD_USB=y
CONFIG_CMD_TPM=y
CONFIG_CMD_MTDPARTS=y
CONFIG_ENV_IS_IN_FLASH=y
@@ -68,7 +67,6 @@ CONFIG_SYSRESET=y
CONFIG_SYSRESET_CMD_POWEROFF=y
CONFIG_SYSRESET_PSCI=y
CONFIG_TPM2_MMIO=y
-CONFIG_USB=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_PCI=y
CONFIG_TPM=y
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
index 7cb1e9f037ff..1347b86f34b1 100644
--- a/configs/qemu_arm_defconfig
+++ b/configs/qemu_arm_defconfig
@@ -36,7 +36,6 @@ CONFIG_CMD_NVEDIT_EFI=y
CONFIG_CMD_DFU=y
CONFIG_CMD_MTD=y
CONFIG_CMD_PCI=y
-CONFIG_CMD_USB=y
CONFIG_CMD_TPM=y
CONFIG_CMD_MTDPARTS=y
CONFIG_ENV_IS_IN_FLASH=y
@@ -69,7 +68,6 @@ CONFIG_SYSRESET=y
CONFIG_SYSRESET_CMD_POWEROFF=y
CONFIG_SYSRESET_PSCI=y
CONFIG_TPM2_MMIO=y
-CONFIG_USB=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_PCI=y
CONFIG_TPM=y
diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
index 1108fe5f8184..8ec5349fc9ea 100644
--- a/doc/board/emulation/qemu-arm.rst
+++ b/doc/board/emulation/qemu-arm.rst
@@ -84,6 +84,10 @@ can be enabled with the following command line parameters:
-device usb-ehci,id=ehci
+- To add a USB keyboard attached to an emulated xHCI controller, pass e.g.::
+
+-device qemu-xhci,id=xhci -device usb-kbd,bus=xhci.0
+
- To add an NVMe disk, pass e.g.::
-drive if=none,file=disk.img,id=mydisk -device nvme,drive=mydisk,serial=foo
--
2.40.1
[PATCH v2 0/4] arm: qemu: Enable Bochs, console buffering, USB keyboard
Now that the driver for the Bochs VGA card emulated by QEMU is no longer limited to x86 architectures [1], this series enables it on arm and arm64 virtual machines to provide a graphical interface. In line with that series this also enables console buffering and USB keyboard. Tested with the Debian 12 installer using GRUB EFI: $ tools/buildman/buildman -o build/qemu_arm64 --boards=qemu_arm64 -w $ cd build/qemu_arm64 $ curl -L -o debian.img \ https://cdimage.debian.org/debian-cd/current/arm64/iso-cd/debian-12.0.0-arm64-netinst.iso $ qemu-system-aarch64 \ -machine virt -cpu cortex-a53 -m 4G -smp 4 \ -bios u-boot.bin \ -serial stdio -device VGA \ -nic user,model=virtio-net-pci \ -device virtio-rng-pci \ -device qemu-xhci,id=xhci -device usb-kbd -device usb-tablet \ -drive if=virtio,file=debian.img,format=raw,readonly=on,media=cdrom And with one using extlinux.conf: $ [...] $ curl -L -o head.img.gz \ https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/SD-card-images/gtk/firmware.none.img.gz $ curl -L -o partition.img.gz \ https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/SD-card-images/gtk/partition.img.gz $ zcat head.img.gz partition.img.gz >debian.img $ [...] Both can get to a graphical installer just fine, in addition to U-Boot video console showing up in a GTK window. [1] video: bochs: Remove the x86 limitation https://lore.kernel.org/u-boot/20230723044041.1089804-1-bm...@tinylab.org/ Changes in v2: - Add new patch for doc section on booting Linux distros - Improve and simplify the qemu command line used for testing v1: https://lore.kernel.org/u-boot/20230808191901.1268429-1-alpernebiya...@gmail.com/ Alper Nebi Yasak (4): arm: qemu: Enable Bochs video support arm: qemu: Enable PRE_CONSOLE_BUFFER arm: qemu: Enable usb keyboard as an input device doc: qemu: arm: Add a section on booting Linux distros arch/arm/Kconfig | 10 board/emulation/qemu-arm/Kconfig | 4 ++ board/emulation/qemu-arm/qemu-arm.c | 5 ++ board/emulation/qemu-arm/qemu-arm.env | 3 ++ configs/qemu_arm64_defconfig | 2 - configs/qemu_arm_defconfig| 2 - doc/board/emulation/qemu-arm.rst | 76 +++ 7 files changed, 98 insertions(+), 4 deletions(-) base-commit: 832148f675e427060be074c276956962fa9b5cb6 -- 2.40.1
[PATCH v2 1/4] arm: qemu: Enable Bochs video support
Commit 716161663ec49 ("riscv: qemu: Enable Bochs video support") enables
a video console for QEMU RISC-V virtual machines using an emulated Bochs
VGA card. Similarly, enable it for ARM virtual machines as well.
Signed-off-by: Alper Nebi Yasak
Reviewed-by: Bin Meng
---
Changes in v2:
- Add tag: "Reviewed-by: Bin Meng "
arch/arm/Kconfig | 4
board/emulation/qemu-arm/qemu-arm.env | 3 +++
doc/board/emulation/qemu-arm.rst | 4
3 files changed, 11 insertions(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 97c25b4f146d..0d4654fb9dfc 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1036,6 +1036,10 @@ config ARCH_QEMU
imply DM_RTC
imply RTC_PL031
imply OF_HAS_PRIOR_STAGE
+ imply VIDEO
+ imply VIDEO_BOCHS
+ imply SYS_WHITE_ON_BLACK
+ imply SYS_CONSOLE_IS_IN_ENV
config ARCH_RMOBILE
bool "Renesas ARM SoCs"
diff --git a/board/emulation/qemu-arm/qemu-arm.env
b/board/emulation/qemu-arm/qemu-arm.env
index e658d5ee7d63..86a99a2e8713 100644
--- a/board/emulation/qemu-arm/qemu-arm.env
+++ b/board/emulation/qemu-arm/qemu-arm.env
@@ -2,6 +2,9 @@
/* environment for qemu-arm and qemu-arm64 */
+stdin=serial
+stdout=serial,vidconsole
+stderr=serial,vidconsole
fdt_high=0x
initrd_high=0x
fdt_addr=0x4000
diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
index b42d924cc66a..1108fe5f8184 100644
--- a/doc/board/emulation/qemu-arm.rst
+++ b/doc/board/emulation/qemu-arm.rst
@@ -67,6 +67,10 @@ Additional persistent U-Boot environment support can be
added as follows:
Additional peripherals that have been tested to work in both U-Boot and Linux
can be enabled with the following command line parameters:
+- To add a video console, remove "-nographic" and add e.g.::
+
+-serial stdio -device VGA
+
- To add a Serial ATA disk via an Intel ICH9 AHCI controller, pass e.g.::
-drive if=none,file=disk.img,format=raw,id=mydisk \
--
2.40.1
[PATCH v2 2/4] arm: qemu: Enable PRE_CONSOLE_BUFFER
Commit 608b80b5b855 ("riscv: qemu: Enable PRE_CONSOLE_BUFFER") enables
buffering console messages for QEMU RISC-V virtual machines so those
printed before the video console is available will still show up on the
display. Similarly, enable it for ARM virtual machines as well.
Signed-off-by: Alper Nebi Yasak
Reviewed-by: Simon Glass
Reviewed-by: Bin Meng
---
Here are select values from bdinfo and env:
DRAM bank = 0x
-> start= 0x4000
-> size = 0x0800
relocaddr = 0x476d6000
fdt_blob= 0x46595d90
lmb_dump_all:
memory.cnt = 0x1 / max = 0x10
memory[0] [0x4000-0x47ff], 0x0800 bytes flags: 0
reserved.cnt = 0x2 / max = 0x10
reserved[0][0x45591000-0x47ff], 0x02a6f000 bytes flags: 0
reserved[1][0x46591760-0x47ff], 0x01a6e8a0 bytes flags: 0
TLB addr= 0x47fe
irq_sp = 0x46595d80
sp start= 0x46595d80
fdt_addr= 0x4000
scriptaddr = 0x4020
loadaddr= 0x4020
pxefile_addr_r = 0x4030
kernel_addr_r = 0x4040
ramdisk_addr_r = 0x4400
fdtcontroladdr = 465e5ea0
Changes in v2:
- Add tag: "Reviewed-by: Simon Glass "
- Add tag: "Reviewed-by: Bin Meng "
arch/arm/Kconfig | 1 +
board/emulation/qemu-arm/Kconfig | 4
2 files changed, 5 insertions(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0d4654fb9dfc..89b978797720 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1040,6 +1040,7 @@ config ARCH_QEMU
imply VIDEO_BOCHS
imply SYS_WHITE_ON_BLACK
imply SYS_CONSOLE_IS_IN_ENV
+ imply PRE_CONSOLE_BUFFER
config ARCH_RMOBILE
bool "Renesas ARM SoCs"
diff --git a/board/emulation/qemu-arm/Kconfig b/board/emulation/qemu-arm/Kconfig
index ed9949651c4b..09c95413a541 100644
--- a/board/emulation/qemu-arm/Kconfig
+++ b/board/emulation/qemu-arm/Kconfig
@@ -12,6 +12,10 @@ config BOARD_SPECIFIC_OPTIONS # dummy
imply VIRTIO_NET
imply VIRTIO_BLK
+config PRE_CON_BUF_ADDR
+ hex
+ default 0x4010
+
endif
if TARGET_QEMU_ARM_64BIT && !TFABOOT
--
2.40.1
[PATCH v2 3/4] arm: qemu: Enable usb keyboard as an input device
Commit 02be57caf730 ("riscv: qemu: Enable usb keyboard as an input
device") adds PCI xHCI support to QEMU RISC-V virtual machines and
enables using a USB keyboard as one of the input devices. Similarly,
enable those for ARM virtual machines as well.
Signed-off-by: Alper Nebi Yasak
Reviewed-by: Simon Glass
Reviewed-by: Bin Meng
---
Changes in v2:
- Add tag: "Reviewed-by: Simon Glass "
- Add tag: "Reviewed-by: Bin Meng "
arch/arm/Kconfig | 5 +
board/emulation/qemu-arm/qemu-arm.c | 5 +
board/emulation/qemu-arm/qemu-arm.env | 2 +-
configs/qemu_arm64_defconfig | 2 --
configs/qemu_arm_defconfig| 2 --
doc/board/emulation/qemu-arm.rst | 4
6 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89b978797720..1fd3ccd1607f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1041,6 +1041,11 @@ config ARCH_QEMU
imply SYS_WHITE_ON_BLACK
imply SYS_CONSOLE_IS_IN_ENV
imply PRE_CONSOLE_BUFFER
+ imply USB
+ imply USB_XHCI_HCD
+ imply USB_XHCI_PCI
+ imply USB_KEYBOARD
+ imply CMD_USB
config ARCH_RMOBILE
bool "Renesas ARM SoCs"
diff --git a/board/emulation/qemu-arm/qemu-arm.c
b/board/emulation/qemu-arm/qemu-arm.c
index dfea0d92a3c8..942f1fff5717 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
@@ -114,6 +115,10 @@ int board_late_init(void)
*/
virtio_init();
+ /* start usb so that usb keyboard can be used as input device */
+ if (CONFIG_IS_ENABLED(USB_KEYBOARD))
+ usb_init();
+
return 0;
}
diff --git a/board/emulation/qemu-arm/qemu-arm.env
b/board/emulation/qemu-arm/qemu-arm.env
index 86a99a2e8713..fb4adef281ed 100644
--- a/board/emulation/qemu-arm/qemu-arm.env
+++ b/board/emulation/qemu-arm/qemu-arm.env
@@ -2,7 +2,7 @@
/* environment for qemu-arm and qemu-arm64 */
-stdin=serial
+stdin=serial,usbkbd
stdout=serial,vidconsole
stderr=serial,vidconsole
fdt_high=0x
diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
index 94bd96678443..f6b8ae530a4a 100644
--- a/configs/qemu_arm64_defconfig
+++ b/configs/qemu_arm64_defconfig
@@ -35,7 +35,6 @@ CONFIG_CMD_NVEDIT_EFI=y
CONFIG_CMD_DFU=y
CONFIG_CMD_MTD=y
CONFIG_CMD_PCI=y
-CONFIG_CMD_USB=y
CONFIG_CMD_TPM=y
CONFIG_CMD_MTDPARTS=y
CONFIG_ENV_IS_IN_FLASH=y
@@ -68,7 +67,6 @@ CONFIG_SYSRESET=y
CONFIG_SYSRESET_CMD_POWEROFF=y
CONFIG_SYSRESET_PSCI=y
CONFIG_TPM2_MMIO=y
-CONFIG_USB=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_PCI=y
CONFIG_TPM=y
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
index 7cb1e9f037ff..1347b86f34b1 100644
--- a/configs/qemu_arm_defconfig
+++ b/configs/qemu_arm_defconfig
@@ -36,7 +36,6 @@ CONFIG_CMD_NVEDIT_EFI=y
CONFIG_CMD_DFU=y
CONFIG_CMD_MTD=y
CONFIG_CMD_PCI=y
-CONFIG_CMD_USB=y
CONFIG_CMD_TPM=y
CONFIG_CMD_MTDPARTS=y
CONFIG_ENV_IS_IN_FLASH=y
@@ -69,7 +68,6 @@ CONFIG_SYSRESET=y
CONFIG_SYSRESET_CMD_POWEROFF=y
CONFIG_SYSRESET_PSCI=y
CONFIG_TPM2_MMIO=y
-CONFIG_USB=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_PCI=y
CONFIG_TPM=y
diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
index 1108fe5f8184..8ec5349fc9ea 100644
--- a/doc/board/emulation/qemu-arm.rst
+++ b/doc/board/emulation/qemu-arm.rst
@@ -84,6 +84,10 @@ can be enabled with the following command line parameters:
-device usb-ehci,id=ehci
+- To add a USB keyboard attached to an emulated xHCI controller, pass e.g.::
+
+-device qemu-xhci,id=xhci -device usb-kbd,bus=xhci.0
+
- To add an NVMe disk, pass e.g.::
-drive if=none,file=disk.img,id=mydisk -device nvme,drive=mydisk,serial=foo
--
2.40.1
[PATCH v2 4/4] doc: qemu: arm: Add a section on booting Linux distros
Add an example qemu-system-aarch64 command that can make U-Boot on QEMU boot into the Debian Installer, along with resulting console messages from U-Boot, based on the existing documentation section for the x86 version. Signed-off-by: Alper Nebi Yasak --- I actually want to put the root.img device first so that the VM can boot into the installed system when it reboots, but U-Boot can't find the bootflow on the second drive. I tried e.g. `bootdev list -p; bootflow scan -lab`, but it seems to only ever search the first virtio-blk. However, `eficonfig; bootefi bootmgr` makes it boot somehow. Changes in v2: - Add new patch for doc section on booting Linux distros doc/board/emulation/qemu-arm.rst | 68 1 file changed, 68 insertions(+) diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst index 8ec5349fc9ea..78bcc3ee44c0 100644 --- a/doc/board/emulation/qemu-arm.rst +++ b/doc/board/emulation/qemu-arm.rst @@ -98,6 +98,74 @@ can be enabled with the following command line parameters: These have been tested in QEMU 2.9.0 but should work in at least 2.5.0 as well. +Booting distros +--- + +It is possible to install and boot a standard Linux distribution using +qemu_arm64 by setting up a root disk:: + +qemu-img create root.img 20G + +then using the installer to install. For example, with Debian 12:: + +qemu-system-aarch64 \ + -machine virt -cpu cortex-a53 -m 4G -smp 4 \ + -bios u-boot.bin \ + -serial stdio -device VGA \ + -nic user,model=virtio-net-pci \ + -device virtio-rng-pci \ + -device qemu-xhci,id=xhci \ + -device usb-kbd -device usb-tablet \ + -drive if=virtio,file=debian-12.0.0-arm64-netinst.iso,format=raw,readonly=on,media=cdrom \ + -drive if=virtio,file=root.img,format=raw,media=disk + +The output will be something like this:: + +U-Boot 2023.10-rc2-00075-gbe8fbe718e35 (Aug 11 2023 - 08:38:49 +) + +DRAM: 4 GiB +Core: 51 devices, 14 uclasses, devicetree: board +Flash: 64 MiB +Loading Environment from Flash... *** Warning - bad CRC, using default environment + +In:serial,usbkbd +Out: serial,vidconsole +Err: serial,vidconsole +Bus xhci_pci: Register 8001040 NbrPorts 8 +Starting the controller +USB XHCI 1.00 +scanning bus xhci_pci for devices... 3 USB Device(s) found +Net: eth0: virtio-net#32 +Hit any key to stop autoboot: 0 +Scanning for bootflows in all bootdevs +Seq Method State UclassPart Name Filename +--- --- -- +Scanning global bootmeth 'efi_mgr': +Scanning bootdev 'fw-cfg@902.bootdev': +fatal: no kernel available +scanning bus for devices... +Scanning bootdev 'virtio-blk#34.bootdev': + 0 efi ready virtio 2 virtio-blk#34.bootdev.par efi/boot/bootaa64.efi +** Booting bootflow 'virtio-blk#34.bootdev.part_2' with efi +Using prior-stage device tree +Failed to load EFI variables +Error: writing contents +** Unable to write file ubootefi.var ** +Failed to persist EFI variables +Missing TPMv2 device for EFI_TCG_PROTOCOL +Booting /efi\boot\bootaa64.efi +Error: writing contents +** Unable to write file ubootefi.var ** +Failed to persist EFI variables +Welcome to GRUB! + +Standard boot looks through various available devices and finds the virtio +disks, then boots from the first one. After a second or so the grub menu appears +and you can work through the installer flow normally. + +After the installation, you can boot into the installed system by running QEMU +again without the drive argument corresponding to the installer CD image. + Enabling TPMv2 support -- -- 2.40.1
Re: Boot failure triggered by USB on rockpro64-rk3399 and pinebook-pro-rk3399
On 21/01/2021 03:08, Vagrant Cascadian wrote: > It seems rockpro64-rk3399 and pinebook-pro-rk3399 fail to boot when usb > is started. It hangs indefinitely at: > > ## Flattened Device Tree blob at 01f0 > Booting using the fdt blob at 0x1f0 > > I have observed this also using 2020.10 on rockpro64-rk3399, though on > pinebook-pro-rk3399 usb does not work and so it basically avoids > triggering the issue. > > Setting CONFIG_USE_PREBOOT=n in the config works around the problem, > though obviously by breaking usb keyboard support or booting from USB > devices. This might be the same as [1] where running "usb stop" would hang, but disabling CONFIG_USB_OHCI_HCD and CONFIG_USB_OHCI_GENERIC gets the board to boot (still breaks the keyboard). Might help narrow things down. [1] https://lists.denx.de/pipermail/u-boot/2020-November/432931.html On 21/01/2021 06:37, Kever Yang wrote: > Â Â Â Do you know which version is the last version that works in this case? The email I linked above has some versions they've tested, in case this is the same issue.
[PATCH 0/3] binman: Make FIT image subentries respect offset, alignment and padding
I've been automating the process in doc/README.chromium-chainload and
while experimenting with whether a "kernel" image with u-boot-spl and
u-boot would work, noticed I couldn't align/offset/pad the two parts.
E.g. in something like the following, binman doesn't add the necessary
padding to place the "u-boot" to the correct offset within the
"kernel-1" data:
fit {
description = "example";
images {
kernel-1 {
description = "U-Boot with SPL";
type = "kernel";
arch = "arm64";
os = "linux";
compression = "none";
u-boot-spl {
};
u-boot {
offset = ;
};
};
};
};
Not sure if that'll ever be really necessary besides my experiment, but
it doesn't seem like skipping the padding was a deliberate choice, so
here are some fixes I wrote for that.
Alper Nebi Yasak (3):
binman: Ignore hash*, signature* nodes in sections
binman: Respect pad-before property of section subentries
binman: Build FIT image subentries with the section etype
tools/binman/etype/fit.py | 22 +++
tools/binman/etype/section.py | 4 +-
tools/binman/ftest.py | 32 +++
tools/binman/test/165_pad_in_sections.dts | 26 +
.../test/166_fit_image_subentry_alignment.dts | 57 +++
5 files changed, 129 insertions(+), 12 deletions(-)
create mode 100644 tools/binman/test/165_pad_in_sections.dts
create mode 100644 tools/binman/test/166_fit_image_subentry_alignment.dts
--
2.28.0
[PATCH 1/3] binman: Ignore hash*, signature* nodes in sections
Switch to str.startswith for matching like the FIT etype does since the
current version doesn't ignore 'hash-1', 'hash-2', etc.
Signed-off-by: Alper Nebi Yasak
---
tools/binman/etype/section.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 73c5553c81..c5166a5b57 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -83,7 +83,7 @@ class Entry_section(Entry):
def _ReadEntries(self):
for node in self._node.subnodes:
-if node.name == 'hash':
+if node.name.startswith('hash') or
node.name.startswith('signature'):
continue
entry = Entry.Create(self, node)
entry.ReadNode()
--
2.28.0
[PATCH 2/3] binman: Respect pad-before property of section subentries
Other relevant properties (pad-after, offset, size, align, align-size,
align-end) already work since Pack() sets correct ranges for subentries'
data (.offset, .size variables), but some padding here is necessary to
align the data within this range to match the pad-before property.
Signed-off-by: Alper Nebi Yasak
---
tools/binman/etype/section.py | 2 +-
tools/binman/ftest.py | 8 +++
tools/binman/test/165_pad_in_sections.dts | 26 +++
3 files changed, 35 insertions(+), 1 deletion(-)
create mode 100644 tools/binman/test/165_pad_in_sections.dts
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index c5166a5b57..72600b1ef3 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -152,7 +152,7 @@ class Entry_section(Entry):
for entry in self._entries.values():
data = entry.GetData()
base = self.pad_before + (entry.offset or 0) - self._skip_at_start
-pad = base - len(section_data)
+pad = base - len(section_data) + (entry.pad_before or 0)
if pad > 0:
section_data += tools.GetBytes(self._pad_byte, pad)
section_data += data
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 5f650b5f94..8edf85c89f 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1269,6 +1269,14 @@ class TestFunctional(unittest.TestCase):
U_BOOT_DATA + tools.GetBytes(ord('&'), 4))
self.assertEqual(expected, data)
+def testPadInSections(self):
+"""Test pad-before, pad-after for entries in sections"""
+data = self._DoReadFile('165_pad_in_sections.dts')
+expected = (U_BOOT_DATA + tools.GetBytes(ord('!'), 12) +
+U_BOOT_DATA + tools.GetBytes(ord('!'), 6) +
+U_BOOT_DATA)
+self.assertEqual(expected, data)
+
def testMap(self):
"""Tests outputting a map of the images"""
_, _, map_data, _ = self._DoReadFileDtb('055_sections.dts', map=True)
diff --git a/tools/binman/test/165_pad_in_sections.dts
b/tools/binman/test/165_pad_in_sections.dts
new file mode 100644
index 00..f2b327ff9f
--- /dev/null
+++ b/tools/binman/test/165_pad_in_sections.dts
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ pad-byte = <0x26>;
+ section {
+ pad-byte = <0x21>;
+
+ before {
+ type = "u-boot";
+ };
+ u-boot {
+ pad-before = <12>;
+ pad-after = <6>;
+ };
+ after {
+ type = "u-boot";
+ };
+ };
+ };
+};
--
2.28.0
[PATCH 3/3] binman: Build FIT image subentries with the section etype
When reading subentries of each image, the FIT entry type directly
concatenates their contents without padding them according to their
offset, size, align, align-size, align-end, pad-before, pad-after
properties.
This patch makes sure these properties are respected by offloading this
image-data building to the section etype, where each subnode of the
"images" node is processed as a section. Alignments and offsets are
respective to the beginning of each image. For example, the following
fragment can end up having "u-boot-spl" start at 0x88 within the final
FIT binary, while "u-boot" would then end up starting at e.g. 0x20088.
fit {
description = "example";
images {
kernel-1 {
description = "U-Boot with SPL";
type = "kernel";
arch = "arm64";
os = "linux";
compression = "none";
u-boot-spl {
};
u-boot {
align = <0x1>;
};
};
};
}
Signed-off-by: Alper Nebi Yasak
---
tools/binman/etype/fit.py | 22 +++
tools/binman/ftest.py | 24
.../test/166_fit_image_subentry_alignment.dts | 57 +++
3 files changed, 93 insertions(+), 10 deletions(-)
create mode 100644 tools/binman/test/166_fit_image_subentry_alignment.dts
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index 75712f4409..f136a2c254 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -62,7 +62,7 @@ class Entry_fit(Entry):
"""
super().__init__(section, etype, node)
self._fit = None
-self._fit_content = defaultdict(list)
+self._fit_images = {}
self._fit_props = {}
def ReadNode(self):
@@ -91,15 +91,18 @@ class Entry_fit(Entry):
rel_path = node.path[len(base_node.path):]
has_images = depth == 2 and rel_path.startswith('/images/')
+if has_images:
+entry = Entry.Create(self.section, node, etype='section')
+entry.ReadNode()
+self._fit_images[rel_path] = entry
+
for subnode in node.subnodes:
if has_images and not (subnode.name.startswith('hash') or
subnode.name.startswith('signature')):
# This is a content node. We collect all of these together
# and put them in the 'data' property. They do not appear
# in the FIT.
-entry = Entry.Create(self.section, subnode)
-entry.ReadNode()
-self._fit_content[rel_path].append(entry)
+pass
else:
with fsw.add_node(subnode.name):
_AddNode(base_node, depth + 1, subnode)
@@ -150,13 +153,12 @@ class Entry_fit(Entry):
Returns:
New fdt contents (bytes)
"""
-for path, entries in self._fit_content.items():
+for path, image in self._fit_images.items():
node = fdt.GetNode(path)
-data = b''
-for entry in entries:
-if not entry.ObtainContents():
-return False
-data += entry.GetData()
+if not image.ObtainContents():
+return False
+image.Pack(0)
+data = image.GetData()
node.AddData('data', data)
fdt.Sync(auto_resize=True)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 8edf85c89f..ed573d8991 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -3485,5 +3485,29 @@ class TestFunctional(unittest.TestCase):
fnode = dtb.GetNode('/images/kernel')
self.assertNotIn('data', fnode.props)
+def testFitImageSubentryAlignment(self):
+"""Test relative alignability of FIT image subentries"""
+entry_args = {
+'test-id': TEXT_DATA,
+}
+data, _, _, _ =
self._DoReadFileDtb('166_fit_image_subentry_alignment.dts',
+entry_args=entry_args)
+dtb = fdt.Fdt.FromData(data)
+dtb.Scan()
+
+node = dtb.GetNode('/images/kernel')
+data = dtb.GetProps(node)["data"].bytes
+align_pad = 0x10 - (len(U_BOOT_SPL_DATA) % 0x10)
+expected = (t
Re: [PATCH 3/3] binman: Build FIT image subentries with the section etype
On 30/08/2020 00:20, Simon Glass wrote:
> On Tue, 25 Aug 2020 at 12:01, Alper Nebi Yasak
> wrote:
>> super().__init__(section, etype, node)
>> self._fit = None
>> -self._fit_content = defaultdict(list)
>> +self._fit_images = {}
>
> Can you add a Properties comment above for this?
There's already a Members comment that I forgot to adjust for the
_fit_content variable I renamed, changing it would be like this:
def __init__(self, section, etype, node):
"""
Members:
_fit: FIT file being built
-_fit_content: dict:
+_fit_sections: dict:
key: relative path to entry Node (from the base of the FIT)
-value: List of Entry objects comprising the contents of this
+value: Entry_section object comprising the contents of this
node
"""
super().__init__(section, etype, node)
self._fit = None
-self._fit_content = defaultdict(list)
+self._fit_sections = {}
self._fit_props = {
Would that be enough? Putting that in the Properties sounds weird to
me since it isn't the same kind of thing as "fit,external-offset", but
section.py documents its _allow_missing in its Properties as well.
OTOH, nothing except fit.py has an __init__ docstring, so I think I can
move the entire thing into the class docstring. Either into the
Properties, or next to the Properties still under a Members heading.
What would you prefer?
>> for subnode in node.subnodes:
>> if has_images and not (subnode.name.startswith('hash') or
>>
>> subnode.name.startswith('signature')):
>> # This is a content node. We collect all of these
>> together
>> # and put them in the 'data' property. They do not
>> appear
>> # in the FIT.
>
> This comment should move along with the code you moved. Perhaps here
> you can mention that it is not a content node.
I tried to clarify and elaborate the comments a bit, because it sounded
ambiguous to me when I just moved it. How about:
has_images = depth == 2 and rel_path.startswith('/images/')
if has_images:
# This node is a FIT subimage node (e.g. "/images/kernel")
# containing content nodes. We collect the subimage nodes and
# section entries for them here to merge the content subnodes
# together and put the merged contents in the subimage node's
# 'data' property later.
entry = Entry.Create(self.section, node, etype='section')
entry.ReadNode()
self._fit_sections[rel_path] = entry
for subnode in node.subnodes:
if has_images and not (subnode.name.startswith('hash') or
subnode.name.startswith('signature')):
# This subnode is a content node not meant to appear in the
# FIT (e.g. "/images/kernel/u-boot"), so don't call
# fsw.add_node() or _AddNode() for it.
pass
else:
with fsw.add_node(subnode.name):
_AddNode(base_node, depth + 1, subnode)
>> -for path, entries in self._fit_content.items():
>> +for path, image in self._fit_images.items():
>
> I think section is a better name than image. In binman, 'image' means
> the final output file, containing a collection of binaries. The FIT
> itself therefore ends up in an image, so calling the components of the
> FIT 'images' is confusing.
I'm changing it to section and also _fit_images to _fit_sections, in
order to match that.
> Please also do check code coverage: binman test -T
Entry_section.ObtainContents() never returns False, so I'm removing the
checks for that, which contained the statements the test didn't cover.
Re: [PATCH 3/3] binman: Build FIT image subentries with the section etype
On 30/08/2020 23:37, Simon Glass wrote: > On Sun, 30 Aug 2020 at 12:17, Alper Nebi Yasak > wrote: >> Entry_section.ObtainContents() never returns False, so I'm removing the >> checks for that, which contained the statements the test didn't cover. > > If you put something in the FIT that depends on something else, it > will return False on the first pass. So you can't remove that code. Section etype already does three passes over its subentries and either returns True or raises an exception. Maybe it should return False instead, but that breaks the 057_unknown_contents.dts test. > Instead, use the _testing etype with a 'return-contents-later' > property. to ensure the path is testing. See 162_fit_external.dts for > how this works. Since section does multiple passes, this appears to only add some more data to wherever it's inserted, with no change in coverage. I can get the exception with 'return-unknown-contents'. If I replace the raise with 'return False', it fails in section etype's GetData(). If I fix that (section_data += data or b''), the FIT entry returns b'' as its entire data due to those checks and only then I can cover them with a new test. More trouble than it's worth?
[PATCH v2 0/3] binman: Make FIT image subentries respect offset, alignment and padding
I've been automating the process in doc/README.chromium-chainload and
while experimenting with whether a "kernel" image with u-boot-spl and
u-boot would work, noticed I couldn't align/offset/pad the two parts.
E.g. in something like the following, binman doesn't add the necessary
padding to place the "u-boot" to the correct offset within the
"kernel-1" data:
fit {
description = "example";
images {
kernel-1 {
description = "U-Boot with SPL";
type = "kernel";
arch = "arm64";
os = "linux";
compression = "none";
u-boot-spl {
};
u-boot {
offset = ;
};
};
};
};
Not sure if that'll ever be really necessary besides my experiment, but
it doesn't seem like skipping the padding was a deliberate choice, so
here are some fixes I wrote for that.
Changes in v2:
- Add test to check that sections ignore hash*, signature* nodes
- Move section padding test to the end of file
- Renumber tests to accommodate for the first patch's new test
- Use 'section' instead of 'image' for FIT subimage sections
- Also rename 'Members:' comment for the renamed _fit_content variable
- Clarify comments around FIT subimage section/content processing
- Don't check for section.ObtainContents() returning False (never does)
v1: https://patchwork.ozlabs.org/project/uboot/list/?series=197693
Alper Nebi Yasak (3):
binman: Ignore hash*, signature* nodes in sections
binman: Respect pad-before property of section subentries
binman: Build FIT image subentries with the section etype
tools/binman/etype/fit.py | 41 +++--
tools/binman/etype/section.py | 4 +-
tools/binman/ftest.py | 37
.../165_section_ignore_hash_signature.dts | 40 +
tools/binman/test/166_pad_in_sections.dts | 26 +
.../test/167_fit_image_subentry_alignment.dts | 57 +++
6 files changed, 186 insertions(+), 19 deletions(-)
create mode 100644 tools/binman/test/165_section_ignore_hash_signature.dts
create mode 100644 tools/binman/test/166_pad_in_sections.dts
create mode 100644 tools/binman/test/167_fit_image_subentry_alignment.dts
--
2.28.0
[PATCH v2 1/3] binman: Ignore hash*, signature* nodes in sections
Switch to str.startswith for matching like the FIT etype does since the
current version doesn't ignore 'hash-1', 'hash-2', etc.
Signed-off-by: Alper Nebi Yasak
---
Changes in v2:
- Add test to check that sections ignore hash*, signature* nodes
tools/binman/etype/section.py | 2 +-
tools/binman/ftest.py | 6 +++
.../165_section_ignore_hash_signature.dts | 40 +++
3 files changed, 47 insertions(+), 1 deletion(-)
create mode 100644 tools/binman/test/165_section_ignore_hash_signature.dts
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 73c5553c81..c5166a5b57 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -83,7 +83,7 @@ class Entry_section(Entry):
def _ReadEntries(self):
for node in self._node.subnodes:
-if node.name == 'hash':
+if node.name.startswith('hash') or
node.name.startswith('signature'):
continue
entry = Entry.Create(self, node)
entry.ReadNode()
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 5f650b5f94..ab88ee96ab 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -3477,5 +3477,11 @@ class TestFunctional(unittest.TestCase):
fnode = dtb.GetNode('/images/kernel')
self.assertNotIn('data', fnode.props)
+def testSectionIgnoreHashSignature(self):
+"""Test that sections ignore hash, signature nodes for its data"""
+data = self._DoReadFile('165_section_ignore_hash_signature.dts')
+expected = (U_BOOT_DATA + U_BOOT_DATA)
+self.assertEqual(expected, data)
+
if __name__ == "__main__":
unittest.main()
diff --git a/tools/binman/test/165_section_ignore_hash_signature.dts
b/tools/binman/test/165_section_ignore_hash_signature.dts
new file mode 100644
index 00..8adbe25512
--- /dev/null
+++ b/tools/binman/test/165_section_ignore_hash_signature.dts
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ section@0 {
+ u-boot {
+ };
+ hash {
+ algo = "sha256";
+ };
+ signature {
+ algo = "sha256,rsa2048";
+ key-name-hint = "dev";
+ };
+ };
+ section@1 {
+ u-boot {
+ };
+ hash-1 {
+ algo = "sha1";
+ };
+ hash-2 {
+ algo = "sha256";
+ };
+ signature-1 {
+ algo = "sha1,rsa2048";
+ key-name-hint = "dev";
+ };
+ signature-2 {
+ algo = "sha256,rsa2048";
+ key-name-hint = "dev";
+ };
+ };
+ };
+};
--
2.28.0
[PATCH v2 2/3] binman: Respect pad-before property of section subentries
Other relevant properties (pad-after, offset, size, align, align-size,
align-end) already work since Pack() sets correct ranges for subentries'
data (.offset, .size variables), but some padding here is necessary to
align the data within this range to match the pad-before property.
Signed-off-by: Alper Nebi Yasak
---
Changes in v2:
- Move section padding test to the end of file
- Renumber test to accommodate for the first patch's new test
tools/binman/etype/section.py | 2 +-
tools/binman/ftest.py | 8 +++
tools/binman/test/166_pad_in_sections.dts | 26 +++
3 files changed, 35 insertions(+), 1 deletion(-)
create mode 100644 tools/binman/test/166_pad_in_sections.dts
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index c5166a5b57..72600b1ef3 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -152,7 +152,7 @@ class Entry_section(Entry):
for entry in self._entries.values():
data = entry.GetData()
base = self.pad_before + (entry.offset or 0) - self._skip_at_start
-pad = base - len(section_data)
+pad = base - len(section_data) + (entry.pad_before or 0)
if pad > 0:
section_data += tools.GetBytes(self._pad_byte, pad)
section_data += data
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index ab88ee96ab..53da709d51 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -3483,5 +3483,13 @@ class TestFunctional(unittest.TestCase):
expected = (U_BOOT_DATA + U_BOOT_DATA)
self.assertEqual(expected, data)
+def testPadInSections(self):
+"""Test pad-before, pad-after for entries in sections"""
+data = self._DoReadFile('166_pad_in_sections.dts')
+expected = (U_BOOT_DATA + tools.GetBytes(ord('!'), 12) +
+U_BOOT_DATA + tools.GetBytes(ord('!'), 6) +
+U_BOOT_DATA)
+self.assertEqual(expected, data)
+
if __name__ == "__main__":
unittest.main()
diff --git a/tools/binman/test/166_pad_in_sections.dts
b/tools/binman/test/166_pad_in_sections.dts
new file mode 100644
index 00..f2b327ff9f
--- /dev/null
+++ b/tools/binman/test/166_pad_in_sections.dts
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ pad-byte = <0x26>;
+ section {
+ pad-byte = <0x21>;
+
+ before {
+ type = "u-boot";
+ };
+ u-boot {
+ pad-before = <12>;
+ pad-after = <6>;
+ };
+ after {
+ type = "u-boot";
+ };
+ };
+ };
+};
--
2.28.0
[PATCH v2 3/3] binman: Build FIT image subentries with the section etype
When reading subentries of each image, the FIT entry type directly
concatenates their contents without padding them according to their
offset, size, align, align-size, align-end, pad-before, pad-after
properties.
This patch makes sure these properties are respected by offloading this
image-data building to the section etype, where each subnode of the
"images" node is processed as a section. Alignments and offsets are
respective to the beginning of each image. For example, the following
fragment can end up having "u-boot-spl" start at 0x88 within the final
FIT binary, while "u-boot" would then end up starting at e.g. 0x20088.
fit {
description = "example";
images {
kernel-1 {
description = "U-Boot with SPL";
type = "kernel";
arch = "arm64";
os = "linux";
compression = "none";
u-boot-spl {
};
u-boot {
align = <0x1>;
};
};
};
}
Signed-off-by: Alper Nebi Yasak
---
Changes in v2:
- Renumber test to accommodate for the first patch's new test
- Use 'section' instead of 'image' for FIT subimage sections
- Also rename 'Members:' comment for the renamed _fit_content variable
- Clarify comments around FIT subimage section/content processing
- Don't check for section.ObtainContents() returning False (never does)
tools/binman/etype/fit.py | 41 +++--
tools/binman/ftest.py | 23
.../test/167_fit_image_subentry_alignment.dts | 57 +++
3 files changed, 104 insertions(+), 17 deletions(-)
create mode 100644 tools/binman/test/167_fit_image_subentry_alignment.dts
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index 75712f4409..acba53aa92 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -55,14 +55,14 @@ class Entry_fit(Entry):
"""
Members:
_fit: FIT file being built
-_fit_content: dict:
+_fit_sections: dict:
key: relative path to entry Node (from the base of the FIT)
-value: List of Entry objects comprising the contents of this
+value: Entry_section object comprising the contents of this
node
"""
super().__init__(section, etype, node)
self._fit = None
-self._fit_content = defaultdict(list)
+self._fit_sections = {}
self._fit_props = {}
def ReadNode(self):
@@ -91,15 +91,23 @@ class Entry_fit(Entry):
rel_path = node.path[len(base_node.path):]
has_images = depth == 2 and rel_path.startswith('/images/')
+if has_images:
+# This node is a FIT subimage node (e.g. "/images/kernel")
+# containing content nodes. We collect the subimage nodes and
+# section entries for them here to merge the content subnodes
+# together and put the merged contents in the subimage node's
+# 'data' property later.
+entry = Entry.Create(self.section, node, etype='section')
+entry.ReadNode()
+self._fit_sections[rel_path] = entry
+
for subnode in node.subnodes:
if has_images and not (subnode.name.startswith('hash') or
subnode.name.startswith('signature')):
-# This is a content node. We collect all of these together
-# and put them in the 'data' property. They do not appear
-# in the FIT.
-entry = Entry.Create(self.section, subnode)
-entry.ReadNode()
-self._fit_content[rel_path].append(entry)
+# This subnode is a content node not meant to appear in
+# the FIT (e.g. "/images/kernel/u-boot"), so don't call
+# fsw.add_node() or _AddNode() for it.
+pass
else:
with fsw.add_node(subnode.name):
_AddNode(base_node, depth + 1, subnode)
@@ -123,9 +131,8 @@ class Entry_fit(Entry):
This adds the 'data' properties to the input ITB (Image-tree Binary)
then runs mkimage to process it.
"""
+# self._BuildInput() either returns by
Re: [PATCH] binman: Allow FIT binaries to have missing external blobs
On 01/09/2020 13:36, Simon Glass wrote:
> Hi Alper,
>
> I found that evb-rk3288 fails to build with the final patch from your
> series. See u-boot-dm/testing for the tree.
I had seen your patch and assumed that nothing was already using the
allow-missing functionality in its FITs (since you're adding it now).
Looks like the version in your tree (aa3f219e) succeeds in the pipeline,
so I think there's nothing for me to do now. Thanks for fixing it.
> At present if a FIT references a missing external blob, binman reports an
> error, even if the image is supposed to allow this.
>
> Propagate this setting down to the child sections of the FIT, so that the
> behaviour is consistent.
>
> This is a fix-up patch to:
>
>binman: Build FIT image subentries with the section etype
>
> and will be squashed into that.
>
> Signed-off-by: Simon Glass
> ---
>
> tools/binman/etype/fit.py | 15
> tools/binman/ftest.py | 8 +
> tools/binman/test/168_fit_missing_blob.dts | 41 ++
> 3 files changed, 64 insertions(+)
> create mode 100644 tools/binman/test/168_fit_missing_blob.dts
>
> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> index acba53aa92c..615b2fd8778 100644
> --- a/tools/binman/etype/fit.py
> +++ b/tools/binman/etype/fit.py
> @@ -169,3 +169,18 @@ class Entry_fit(Entry):
> fdt.Sync(auto_resize=True)
> data = fdt.GetContents()
> return data
> +
> +def CheckMissing(self, missing_list):
> +"""Check if any entries in this FIT have missing external blobs
> +
> +If there are missing blobs, the entries are added to the list
> +
> +Args:
> +missing_list: List of Entry objects to be added to
> +"""
> +for path, section in self._fit_sections.items():
> +section.CheckMissing(missing_list)
> +
> +def SetAllowMissing(self, allow_missing):
> +for section in self._fit_sections.values():
> +section.SetAllowMissing(allow_missing)
I saw GetAllowMissing() in section.py, doesn't this also need it?
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 762535b5a74..e672967dbaa 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -3515,5 +3515,13 @@ class TestFunctional(unittest.TestCase):
> U_BOOT_DTB_DATA)
> self.assertEqual(expected, data)
>
> +def testFitExtblobMissingOk(self):
> +"""Test a FIT with a missing external blob that is allowed"""
> +with test_util.capture_sys_output() as (stdout, stderr):
> +self._DoTestFile('168_fit_missing_blob.dts',
> + allow_missing=True)
> +err = stderr.getvalue()
> +self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")
> +
> if __name__ == "__main__":
> unittest.main()
> diff --git a/tools/binman/test/168_fit_missing_blob.dts
> b/tools/binman/test/168_fit_missing_blob.dts
> new file mode 100644
> index 000..e007aa41d8a
> --- /dev/null
> +++ b/tools/binman/test/168_fit_missing_blob.dts
> @@ -0,0 +1,41 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + binman {
> + u-boot {
> + };
> + fit {
> + description = "test-desc";
> + #address-cells = <1>;
> + fit,fdt-list = "of-list";
> +
> + images {
> + kernel {
> + description = "ATF BL31";
> + type = "kernel";
> + arch = "ppc";
> + os = "linux";
> + compression = "gzip";
> + load = <>;
> + entry = <>;
> + hash-1 {
> + algo = "crc32";
> + };
> + hash-2 {
> + algo = "sha1";
> + };
> + blob-ext {
> + filename = "missing";
> + };
> + };
> + };
> + };
> + u-boot-nodtb {
> + };
> + };
> +};
Anyway, feel free to add if you need to:
Reviewed-by: Alper Nebi Yasak
(or Acked-by, or neither)
[PATCH] buildman: Use git worktrees instead of git clones
This patch makes buildman create linked working trees instead of clones
of the source repository, but keeps updating the older clones of the
repository that might already exist. These worktrees share "everything
except working directory specific files such as HEAD, index, etc." with
the source repository. See the git-worktree(1) manual page for more
information.
Signed-off-by: Alper Nebi Yasak
---
The git-worktree is available since about 2015 (git v2.5), but its
manual page mentions this in its BUGS section:
Multiple checkout in general is still experimental, and the
support for submodules is incomplete. It is NOT recommended to
make multiple checkouts of a superproject.
I think it'll be fine since U-Boot doesn't use submodules, but still
wanted to let you know.
tools/buildman/builder.py | 36
tools/buildman/func_test.py | 2 ++
tools/patman/gitutil.py | 28
3 files changed, 54 insertions(+), 12 deletions(-)
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index dbb75b35c1..9cb65c15a8 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -1540,31 +1540,38 @@ class Builder:
def _PrepareThread(self, thread_num, setup_git):
"""Prepare the working directory for a thread.
-This clones or fetches the repo into the thread's work directory.
+This adds a git worktree for the repo into the thread's work
+directory or fetches the repo into a clone that might already
+exist there.
Args:
thread_num: Thread number (0, 1, ...)
-setup_git: True to set up a git repo clone
+setup_git: True to set up a git worktree
"""
thread_dir = self.GetThreadDir(thread_num)
builderthread.Mkdir(thread_dir)
git_dir = os.path.join(thread_dir, '.git')
-# Clone the repo if it doesn't already exist
-# TODO(sjg@chromium): Perhaps some git hackery to symlink instead, so
-# we have a private index but uses the origin repo's contents?
+# Create a worktree for this thread if it (or a git repo clone)
+# doesn't already exist
if setup_git and self.git_dir:
src_dir = os.path.abspath(self.git_dir)
-if os.path.exists(git_dir):
+if not os.path.exists(git_dir):
+Print('\rChecking out worktree for thread %d' % thread_num,
+ newline=False)
+gitutil.AddWorktree(src_dir, thread_dir)
+terminal.PrintClear()
+elif os.path.isdir(git_dir):
+# Older versions cloned the src_dir repo, we can keep using
+# the clones but need to fetch from src_dir.
Print('\rFetching repo for thread %d' % thread_num,
newline=False)
gitutil.Fetch(git_dir, thread_dir)
terminal.PrintClear()
-else:
-Print('\rCloning repo for thread %d' % thread_num,
- newline=False)
-gitutil.Clone(src_dir, thread_dir)
-terminal.PrintClear()
+elif os.path.isfile(git_dir):
+# This is a worktree of the src_dir repo, we don't need to
+# create it again.
+pass
def _PrepareWorkingSpace(self, max_threads, setup_git):
"""Prepare the working directory for use.
@@ -1573,9 +1580,14 @@ class Builder:
Args:
max_threads: Maximum number of threads we expect to need.
-setup_git: True to set up a git repo clone
+setup_git: True to set up a git worktree
"""
builderthread.Mkdir(self._working_dir)
+if setup_git and self.git_dir:
+# If we previously added a worktree but the directory for it
+# got deleted, we need to prune its files from the repo so
+# that we can check out another in its place.
+gitutil.PruneWorktrees(os.path.abspath(self.git_dir))
for thread in range(max_threads):
self._PrepareThread(thread, setup_git)
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 418677f9cc..3dd2e6ee5b 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -319,6 +319,8 @@ class TestFunctional(unittest.TestCase):
return command.CommandResult(return_code=0)
elif sub_cmd == 'checkout':
return command.CommandResult(return_code=0)
+elif sub_cmd == 'worktree':
+return command.CommandResult(return_code=0)
# Not handled, so abort
print('git', git_args, sub_cmd, args)
diff --git a/tools/patman/gitutil.py b/tools/pat
Re: [PATCH] buildman: Use git worktrees instead of git clones
On 02/09/2020 20:07, Simon Glass wrote: > It looks like that version of git was not in Ubuntu 14.04 but appeared > in 16.04. I've just done some patches to make patman work in 14.04, so > I'd prefer to keep buildman working for that also. I'll keep that in mind. Looks like Ubuntu 14.04 has python v3.4 and git v1.9. I'm used to more recent versions so maybe I'll take the time to set up a VM to test things in. > So I think we should have an option to keep the old behaviour, or > perhaps detect when 'git worktree' is not available and fall back. I can check whether 'git worktree list' succeeds, it should have no effect on the repo but would give a 'not a git command' error if things wouldn't work; in which case buildman could fall back to cloning. I'll implement and send a v2 with something like that.
[PATCH v2] buildman: Use git worktrees instead of git clones when possible
This patch makes buildman create linked working trees instead of clones
of the source repository, but keeps updating the older clones of the
repository that might already exist. These worktrees share "everything
except working directory specific files such as HEAD, index, etc." with
the source repository. See the git-worktree(1) manual page for more
information.
If git-worktree isn't available, silently falls back to cloning the
repository.
Signed-off-by: Alper Nebi Yasak
---
I used 'git worktree list's return code, since 'git worktree' returns
non-zero (prints usage) even if it's available.
This does fall back to git clone on my Ubuntu 14.04 amd64 VM, but all
the builds instantly give an error about the Makefile even before this
patch. The buildman parts are working as far as I can tell.
Changes in v2:
- Fall back to cloning if git-worktree isn't available
- Add a gitutil.CheckWorktreeIsAvailable(git_dir) function
- Refactor the _PrepareThread changes
- Make _PrepeareThread's setup_git argument accept 'clone' or 'worktree'
- Some comment and docstring changes
v1: https://patchwork.ozlabs.org/project/uboot/list/?series=199060
tools/buildman/builder.py | 48 ++---
tools/buildman/func_test.py | 2 ++
tools/patman/gitutil.py | 42
3 files changed, 84 insertions(+), 8 deletions(-)
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index dbb75b35c1..c93946842a 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -1541,41 +1541,73 @@ class Builder:
"""Prepare the working directory for a thread.
This clones or fetches the repo into the thread's work directory.
+Optionally, it can create a linked working tree of the repo in the
+thread's work directory instead.
Args:
thread_num: Thread number (0, 1, ...)
-setup_git: True to set up a git repo clone
+setup_git:
+ 'clone' to set up a git clone
+ 'worktree' to set up a git worktree
"""
thread_dir = self.GetThreadDir(thread_num)
builderthread.Mkdir(thread_dir)
git_dir = os.path.join(thread_dir, '.git')
-# Clone the repo if it doesn't already exist
-# TODO(sjg@chromium): Perhaps some git hackery to symlink instead, so
-# we have a private index but uses the origin repo's contents?
+# Create a worktree or a git repo clone for this thread if it
+# doesn't already exist
if setup_git and self.git_dir:
src_dir = os.path.abspath(self.git_dir)
-if os.path.exists(git_dir):
+if os.path.isdir(git_dir):
+# This is a clone of the src_dir repo, we can keep using
+# it but need to fetch from src_dir.
Print('\rFetching repo for thread %d' % thread_num,
newline=False)
gitutil.Fetch(git_dir, thread_dir)
terminal.PrintClear()
-else:
+elif os.path.isfile(git_dir):
+# This is a worktree of the src_dir repo, we don't need to
+# create it again or update it in any way.
+pass
+elif os.path.exists(git_dir):
+# Don't know what could trigger this, but we probably
+# can't create a git worktree/clone here.
+raise ValueError('Git dir %s exists, but is not a file '
+ 'or a directory.' % git_dir)
+elif setup_git == 'worktree':
+Print('\rChecking out worktree for thread %d' % thread_num,
+ newline=False)
+gitutil.AddWorktree(src_dir, thread_dir)
+terminal.PrintClear()
+elif setup_git == 'clone' or setup_git == True:
Print('\rCloning repo for thread %d' % thread_num,
newline=False)
gitutil.Clone(src_dir, thread_dir)
terminal.PrintClear()
+else:
+raise ValueError("Can't setup git repo with %s." % setup_git)
def _PrepareWorkingSpace(self, max_threads, setup_git):
"""Prepare the working directory for use.
-Set up the git repo for each thread.
+Set up the git repo for each thread. Creates a linked working tree
+if git-worktree is available, or clones the repo if it isn't.
Args:
max_threads: Maximum number of threads we expect to need.
-setup_git: True to set up a git repo clone
+setup_git: True to set up a git worktree or a git clone
[PATCH 2/3] binman: Use target-specific tools when cross-compiling
Currently, binman always runs the compile tools like cc, objcopy, strip,
etc. using their literal name. Instead, this patch makes it use the
target-specific versions by default, derived from the tool-specific
environment variables (CC, OBJCOPY, STRIP, etc.) or from the
CROSS_COMPILE environment variable.
For example, the u-boot-elf etype directly uses 'strip'. Trying to run
the tests with 'CROSS_COMPILE=i686-linux-gnu- binman test' on an arm64
host results in the '097_elf_strip.dts' test to fail as the arm64
version of 'strip' can't understand the format of the x86 ELF file.
This also adjusts some command.Output() calls that caused test errors or
failures to use the target versions of the tools they call. After this,
patch, an arm64 host can run all tests with no errors or failures using
a correct CROSS_COMPILE value.
Signed-off-by: Alper Nebi Yasak
---
tools/binman/elf.py | 6 +++--
tools/binman/elf_test.py | 4 ++-
tools/dtoc/fdt_util.py | 9 ---
tools/patman/tools.py| 58
4 files changed, 70 insertions(+), 7 deletions(-)
diff --git a/tools/binman/elf.py b/tools/binman/elf.py
index f88031c2bf..5e566e56cb 100644
--- a/tools/binman/elf.py
+++ b/tools/binman/elf.py
@@ -234,8 +234,10 @@ SECTIONS
# text section at the start
# -m32: Build for 32-bit x86
# -T...: Specifies the link script, which sets the start address
-stdout = command.Output('cc', '-static', '-nostdlib',
'-Wl,--build-id=none',
-'-m32','-T', lds_file, '-o', elf_fname, s_file)
+cc, args = tools.GetTargetCompileTool('cc')
+args += ['-static', '-nostdlib', '-Wl,--build-id=none', '-m32', '-T',
+lds_file, '-o', elf_fname, s_file]
+stdout = command.Output(cc, *args)
shutil.rmtree(outdir)
def DecodeElf(data, location):
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py
index 37e1b423cf..e3d218a89e 100644
--- a/tools/binman/elf_test.py
+++ b/tools/binman/elf_test.py
@@ -186,7 +186,9 @@ class TestElf(unittest.TestCase):
# Make an Elf file and then convert it to a fkat binary file. This
# should produce the original data.
elf.MakeElf(elf_fname, expected_text, expected_data)
-stdout = command.Output('objcopy', '-O', 'binary', elf_fname,
bin_fname)
+objcopy, args = tools.GetTargetCompileTool('objcopy')
+args += ['-O', 'binary', elf_fname, bin_fname]
+stdout = command.Output(objcopy, *args)
with open(bin_fname, 'rb') as fd:
data = fd.read()
self.assertEqual(expected_text + expected_data, data)
diff --git a/tools/dtoc/fdt_util.py b/tools/dtoc/fdt_util.py
index b040793772..37e96b9864 100644
--- a/tools/dtoc/fdt_util.py
+++ b/tools/dtoc/fdt_util.py
@@ -68,22 +68,23 @@ def EnsureCompiled(fname, tmpdir=None,
capture_stderr=False):
search_paths = [os.path.join(os.getcwd(), 'include')]
root, _ = os.path.splitext(fname)
-args = ['-E', '-P', '-x', 'assembler-with-cpp', '-D__ASSEMBLY__']
+cc, args = tools.GetTargetCompileTool('cc')
+args += ['-E', '-P', '-x', 'assembler-with-cpp', '-D__ASSEMBLY__']
args += ['-Ulinux']
for path in search_paths:
args.extend(['-I', path])
args += ['-o', dts_input, fname]
-command.Run('cc', *args)
+command.Run(cc, *args)
# If we don't have a directory, put it in the tools tempdir
search_list = []
for path in search_paths:
search_list.extend(['-i', path])
-args = ['-I', 'dts', '-o', dtb_output, '-O', 'dtb',
+dtc, args = tools.GetTargetCompileTool('dtc')
+args += ['-I', 'dts', '-o', dtb_output, '-O', 'dtb',
'-W', 'no-unit_address_vs_reg']
args.extend(search_list)
args.append(dts_input)
-dtc = os.environ.get('DTC') or 'dtc'
command.Run(dtc, *args, capture_stderr=capture_stderr)
return dtb_output
diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index d41115a22c..ee8b70d0cc 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -188,6 +188,58 @@ def PathHasFile(path_spec, fname):
return True
return False
+def GetTargetCompileTool(name, cross_compile=None):
+"""Get the target-specific version for a compile tool
+
+This first checks the environment variables that specify which
+version of the tool should be used (e,g, $
[PATCH 1/3] binman: Support cross-compiling test files to x86
These test files are currently "intended for use on x86 hosts", but most of the tests using them can still pass when cross-compiled to x86 on an arm64 host. This patch enables non-x86 hosts to run the tests by specifying a cross-compiler via CROSS_COMPILE. The list of variables it sets is taken from the top-level Makefile. It would be possible to automatically set an x86 cross-compiler with a few blocks like: ifneq ($(shell i386-linux-gnu-gcc --version 2> /dev/null),) CROSS_COMPILE = i386-linux-gnu- endif But it wouldn't propagate to the binman process calling this Makefile, so it's better just raise an error and expect 'binman test' to be run with a correct CROSS_COMPILE. Signed-off-by: Alper Nebi Yasak --- tools/binman/test/Makefile | 28 +++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index e4fd97bb2e..e10a8625db 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -7,6 +7,32 @@ # SPDX-License-Identifier: GPL-2.0+ # +HOST_ARCH := $(shell uname -m | sed -e s/i.86/x86/ ) +ifeq ($(findstring $(HOSTARCH),"x86" "x86_64"),) +ifeq ($(findstring $(MAKECMDGOALS),"help" "clean"),) +ifndef CROSS_COMPILE +$(error Binman tests need to compile to x86, but the CPU arch of your \ + machine is $(HOST_ARCH). Set CROSS_COMPILE to a suitable cross compiler) +endif +endif +endif + +AS = $(CROSS_COMPILE)as +# Always use GNU ld +ifneq ($(shell $(CROSS_COMPILE)ld.bfd -v 2> /dev/null),) +LD = $(CROSS_COMPILE)ld.bfd +else +LD = $(CROSS_COMPILE)ld +endif +CC = $(CROSS_COMPILE)gcc +CPP= $(CC) -E +AR = $(CROSS_COMPILE)ar +NM = $(CROSS_COMPILE)nm +LDR= $(CROSS_COMPILE)ldr +STRIP = $(CROSS_COMPILE)strip +OBJCOPY= $(CROSS_COMPILE)objcopy +OBJDUMP= $(CROSS_COMPILE)objdump + VPATH := $(SRC) CFLAGS := -march=i386 -m32 -nostdlib -I $(SRC)../../../include \ -Wl,--no-dynamic-linker @@ -32,7 +58,7 @@ bss_data: CFLAGS += $(SRC)bss_data.lds bss_data: bss_data.c u_boot_binman_syms.bin: u_boot_binman_syms - objcopy -O binary $< -R .note.gnu.build-id $@ + $(OBJCOPY) -O binary $< -R .note.gnu.build-id $@ u_boot_binman_syms: CFLAGS += $(LDS_BINMAN) u_boot_binman_syms: u_boot_binman_syms.c -- 2.28.0
[PATCH 3/3] binman: Allow resolving host-specific tools from env vars
This patch lets tools.Run() use host-specific versions with the
for_host keyword argument, based on the host-specific environment
variables (HOSTCC, HOSTOBJCOPY, HOSTSTRIP, etc.).
Signed-off-by: Alper Nebi Yasak
---
Not sure if this patch will ever be useful, but it complements the
previous patch very well.
tools/patman/tools.py | 32 +++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index ee8b70d0cc..6d539fe594 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -188,6 +188,31 @@ def PathHasFile(path_spec, fname):
return True
return False
+def GetHostCompileTool(name):
+"""Get the host-specific version for a compile tool
+
+This checks the environment variables that specify which version of
+the tool should be used.
+
+Args:
+name: Command name to run
+
+Returns:
+host_name: Exact command name to run instead
+extra_args: List of extra arguments to pass
+"""
+host_name = None
+extra_args = []
+if name in ('as', 'ld', 'cc', 'cpp', 'ar', 'nm', 'ldr', 'strip',
+'objcopy', 'objdump', 'dtc'):
+host_name, *host_args = env.get('HOST' + name.upper(), '').split(' ')
+elif name == 'c++':
+host_name, *host_args = env.get('HOSTCXX', '').split(' ')
+
+if host_name:
+return host_name, extra_args
+return name, []
+
def GetTargetCompileTool(name, cross_compile=None):
"""Get the target-specific version for a compile tool
@@ -250,6 +275,7 @@ def Run(name, *args, **kwargs):
Args:
name: Command name to run
args: Arguments to the tool
+for_host: True to resolve the command to the version for the host
for_target: False to run the command as-is, without resolving it
to the version for the compile target
@@ -258,7 +284,8 @@ def Run(name, *args, **kwargs):
"""
try:
binary = kwargs.get('binary')
-for_target = kwargs.get('for_target', True)
+for_host = kwargs.get('for_host', False)
+for_target = kwargs.get('for_target', not for_host)
env = None
if tool_search_paths:
env = dict(os.environ)
@@ -266,6 +293,9 @@ def Run(name, *args, **kwargs):
if for_target:
name, extra_args = GetTargetCompileTool(name)
args = tuple(extra_args) + args
+elif for_host:
+name, extra_args = GetHostCompileTool(name)
+args = tuple(extra_args) + args
all_args = (name,) + args
result = command.RunPipe([all_args], capture=True, capture_stderr=True,
env=env, raise_on_error=False, binary=binary)
--
2.28.0
[PATCH 0/3] binman: Make tests work on non-x86 architectures via cross-compilation
Right now the 'binman test' command fails spectacularly on arm64 since it cannot even setup the test environments properly due to errors during setUpClass(). I can get a 100% coverage result with all tests passing if I cross-compile things to x86 and run the cross-compiling versions of some tools. This series tries to implement that solution in a general way. I think the thoroughly proper thing would be to make the tests and their files portable. I don't really know how. Another alternative is to split the test environments into multiple parts and skip the parts that can't be prepared for an architecture, but that affects the test coverage results. In any case, this series doesn't really prevent these other solutions from being implemented on top of it. Alper Nebi Yasak (3): binman: Support cross-compiling test files to x86 binman: Use target-specific tools when cross-compiling binman: Allow resolving host-specific tools from env vars tools/binman/elf.py| 6 ++- tools/binman/elf_test.py | 4 +- tools/binman/test/Makefile | 28 +++- tools/dtoc/fdt_util.py | 9 ++-- tools/patman/tools.py | 88 ++ 5 files changed, 127 insertions(+), 8 deletions(-) -- 2.28.0
Re: [PATCH 1/3] binman: Support cross-compiling test files to x86
On 05/09/2020 19:36, Simon Glass wrote: > For me this fails on x86_64, complaining for example: > > Exception: Error 2 running 'make -C /tmp/binmant.d17vfu3j/elftest -f > /scratch/sglass/cosarm/src/third_party/u-boot/files/tools/binman/test/Makefile > SRC=/scratch/sglass/cosarm/src/third_party/u-boot/files/tools/binman/test/': > /scratch/sglass/cosarm/src/third_party/u-boot/files/tools/binman/test/Makefile:14: > *** Binman tests need to compile to x86, but the CPU arch of your > machine is x86_64. Set CROSS_COMPILE to a suitable cross compiler. > Stop. > > Can you make it work on both i386 and x86_64 without complaining? It > looks like that is the intent. I messed up the variable names: defined HOST_ARCH, but used HOSTARCH in the check. Fixing that makes it work like it should. (I'll go with HOSTARCH since that's where I took the "uname -m | sed" call from). > Also I'm not sure we need to define vars for all the tools, so you > could perhaps drop those that are not needed. Looks like we don't need anything except CC and OBJCOPY, I'll drop the rest.
Re: [PATCH 2/3] binman: Use target-specific tools when cross-compiling
On 05/09/2020 19:37, Simon Glass wrote:
> This looks good, but it drops the use of DTC to specify the
> device-tree compiler. Can you add it back?
I think you're referring to this hunk:
> # If we don't have a directory, put it in the tools tempdir
> search_list = []
> for path in search_paths:
> search_list.extend(['-i', path])
> -args = ['-I', 'dts', '-o', dtb_output, '-O', 'dtb',
> +dtc, args = tools.GetTargetCompileTool('dtc')
> +args += ['-I', 'dts', '-o', dtb_output, '-O', 'dtb',
> '-W', 'no-unit_address_vs_reg']
> args.extend(search_list)
> args.append(dts_input)
> -dtc = os.environ.get('DTC') or 'dtc'
> command.Run(dtc, *args, capture_stderr=capture_stderr)
> return dtb_output
where I removed the os.environ.get('DTC'). Instead of that, I get the
command for dtc from GetTargetCompileTool('dtc'), which does check the
'DTC' environment variable:
> +def GetTargetCompileTool(name, cross_compile=None):
> [...]
> +env = dict(os.environ)
> +
> +target_name = None
> +extra_args = []
> +if name in ('as', 'ld', 'cc', 'cpp', 'ar', 'nm', 'ldr', 'strip',
> +'objcopy', 'objdump', 'dtc'):
> +target_name, *extra_args = env.get(name.upper(), '').split(' ')
> +
> +if target_name:
> +return target_name, extra_args
If that's not convincing enough: running 'DTC=false binman test' gets me
a lot of test errors that I don't get without the 'DTC=false'.
Re: [PATCH 3/3] binman: Allow resolving host-specific tools from env vars
On 05/09/2020 19:37, Simon Glass wrote: > Please see below. > > Also please add a mention of the CROSS_COMPILE thing in binman's README. > >> +def GetHostCompileTool(name): >> +"""Get the host-specific version for a compile tool >> + >> +This checks the environment variables that specify which version of >> +the tool should be used. > > Can you please expand the comment to mention the environment variables > it checks? OK, will send a v2 after doing these with the fixes for the first patch (unless you point out until then something new that I should fix).
[PATCH v2 0/4] binman: Make tests work on non-x86 architectures via cross-compilation
Right now the 'binman test' command fails spectacularly on arm64 since it cannot even setup the test environments properly due to errors during setUpClass(). I can get a 100% coverage result with all tests passing if I cross-compile things to x86 and run the cross-compiling versions of some tools. This series tries to implement that solution in a general way. I think the thoroughly proper thing would be to make the tests and their files portable. I don't really know how. Another alternative is to split the test environments into multiple parts and skip the parts that can't be prepared for an architecture, but that affects the test coverage results. In any case, this series doesn't really prevent these other solutions from being implemented on top of it. Changes in v2: - Fix detecting architecture (HOST_ARCH should've been HOSTARCH) - Remove Makefile variables for unused tools - Fix typo in an "e.g." (had used commas instead of dots) - Add a table of target-specific versions in the docstring - Add a table of host-specific versions in the docstring - Add patch to document CROSS_COMPILE, CC, HOSTCC etc. in README - Collect tags v1: https://patchwork.ozlabs.org/project/uboot/list/?series=199707 Alper Nebi Yasak (4): binman: Support cross-compiling test files to x86 binman: Use target-specific tools when cross-compiling binman: Allow resolving host-specific tools from env vars binman: Document how CROSS_COMPILE, CC, HOSTCC etc. are used in README tools/binman/README| 24 +++ tools/binman/elf.py| 6 +- tools/binman/elf_test.py | 4 +- tools/binman/test/Makefile | 15 - tools/dtoc/fdt_util.py | 9 +-- tools/patman/tools.py | 125 + 6 files changed, 175 insertions(+), 8 deletions(-) -- 2.28.0
[PATCH v2 1/4] binman: Support cross-compiling test files to x86
These test files are currently "intended for use on x86 hosts", but most of the tests using them can still pass when cross-compiled to x86 on an arm64 host. This patch enables non-x86 hosts to run the tests by specifying a cross-compiler via CROSS_COMPILE. The list of variables it sets is taken from the top-level Makefile. It would be possible to automatically set an x86 cross-compiler with a few blocks like: ifneq ($(shell i386-linux-gnu-gcc --version 2> /dev/null),) CROSS_COMPILE = i386-linux-gnu- endif But it wouldn't propagate to the binman process calling this Makefile, so it's better just raise an error and expect 'binman test' to be run with a correct CROSS_COMPILE. Signed-off-by: Alper Nebi Yasak --- Changes in v2: - Fix detecting architecture (HOST_ARCH should've been HOSTARCH) - Remove Makefile variables for unused tools tools/binman/test/Makefile | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index e4fd97bb2e..0b19b7d993 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -7,6 +7,19 @@ # SPDX-License-Identifier: GPL-2.0+ # +HOSTARCH := $(shell uname -m | sed -e s/i.86/x86/ ) +ifeq ($(findstring $(HOSTARCH),"x86" "x86_64"),) +ifeq ($(findstring $(MAKECMDGOALS),"help" "clean"),) +ifndef CROSS_COMPILE +$(error Binman tests need to compile to x86, but the CPU arch of your \ + machine is $(HOSTARCH). Set CROSS_COMPILE to a suitable cross compiler) +endif +endif +endif + +CC = $(CROSS_COMPILE)gcc +OBJCOPY= $(CROSS_COMPILE)objcopy + VPATH := $(SRC) CFLAGS := -march=i386 -m32 -nostdlib -I $(SRC)../../../include \ -Wl,--no-dynamic-linker @@ -32,7 +45,7 @@ bss_data: CFLAGS += $(SRC)bss_data.lds bss_data: bss_data.c u_boot_binman_syms.bin: u_boot_binman_syms - objcopy -O binary $< -R .note.gnu.build-id $@ + $(OBJCOPY) -O binary $< -R .note.gnu.build-id $@ u_boot_binman_syms: CFLAGS += $(LDS_BINMAN) u_boot_binman_syms: u_boot_binman_syms.c -- 2.28.0
[PATCH v2 2/4] binman: Use target-specific tools when cross-compiling
Currently, binman always runs the compile tools like cc, objcopy, strip,
etc. using their literal name. Instead, this patch makes it use the
target-specific versions by default, derived from the tool-specific
environment variables (CC, OBJCOPY, STRIP, etc.) or from the
CROSS_COMPILE environment variable.
For example, the u-boot-elf etype directly uses 'strip'. Trying to run
the tests with 'CROSS_COMPILE=i686-linux-gnu- binman test' on an arm64
host results in the '097_elf_strip.dts' test to fail as the arm64
version of 'strip' can't understand the format of the x86 ELF file.
This also adjusts some command.Output() calls that caused test errors or
failures to use the target versions of the tools they call. After this,
patch, an arm64 host can run all tests with no errors or failures using
a correct CROSS_COMPILE value.
Signed-off-by: Alper Nebi Yasak
Reviewed-by: Simon Glass
---
Changes in v2:
- Fix typo in an "e.g." (had used commas instead of dots)
- Add a table of target-specific versions in the docstring
- Add tag: "Reviewed-by: Simon Glass "
tools/binman/elf.py | 6 ++--
tools/binman/elf_test.py | 4 ++-
tools/dtoc/fdt_util.py | 9 ++---
tools/patman/tools.py| 77
4 files changed, 89 insertions(+), 7 deletions(-)
diff --git a/tools/binman/elf.py b/tools/binman/elf.py
index f88031c2bf..5e566e56cb 100644
--- a/tools/binman/elf.py
+++ b/tools/binman/elf.py
@@ -234,8 +234,10 @@ SECTIONS
# text section at the start
# -m32: Build for 32-bit x86
# -T...: Specifies the link script, which sets the start address
-stdout = command.Output('cc', '-static', '-nostdlib',
'-Wl,--build-id=none',
-'-m32','-T', lds_file, '-o', elf_fname, s_file)
+cc, args = tools.GetTargetCompileTool('cc')
+args += ['-static', '-nostdlib', '-Wl,--build-id=none', '-m32', '-T',
+lds_file, '-o', elf_fname, s_file]
+stdout = command.Output(cc, *args)
shutil.rmtree(outdir)
def DecodeElf(data, location):
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py
index 37e1b423cf..e3d218a89e 100644
--- a/tools/binman/elf_test.py
+++ b/tools/binman/elf_test.py
@@ -186,7 +186,9 @@ class TestElf(unittest.TestCase):
# Make an Elf file and then convert it to a fkat binary file. This
# should produce the original data.
elf.MakeElf(elf_fname, expected_text, expected_data)
-stdout = command.Output('objcopy', '-O', 'binary', elf_fname,
bin_fname)
+objcopy, args = tools.GetTargetCompileTool('objcopy')
+args += ['-O', 'binary', elf_fname, bin_fname]
+stdout = command.Output(objcopy, *args)
with open(bin_fname, 'rb') as fd:
data = fd.read()
self.assertEqual(expected_text + expected_data, data)
diff --git a/tools/dtoc/fdt_util.py b/tools/dtoc/fdt_util.py
index b040793772..37e96b9864 100644
--- a/tools/dtoc/fdt_util.py
+++ b/tools/dtoc/fdt_util.py
@@ -68,22 +68,23 @@ def EnsureCompiled(fname, tmpdir=None,
capture_stderr=False):
search_paths = [os.path.join(os.getcwd(), 'include')]
root, _ = os.path.splitext(fname)
-args = ['-E', '-P', '-x', 'assembler-with-cpp', '-D__ASSEMBLY__']
+cc, args = tools.GetTargetCompileTool('cc')
+args += ['-E', '-P', '-x', 'assembler-with-cpp', '-D__ASSEMBLY__']
args += ['-Ulinux']
for path in search_paths:
args.extend(['-I', path])
args += ['-o', dts_input, fname]
-command.Run('cc', *args)
+command.Run(cc, *args)
# If we don't have a directory, put it in the tools tempdir
search_list = []
for path in search_paths:
search_list.extend(['-i', path])
-args = ['-I', 'dts', '-o', dtb_output, '-O', 'dtb',
+dtc, args = tools.GetTargetCompileTool('dtc')
+args += ['-I', 'dts', '-o', dtb_output, '-O', 'dtb',
'-W', 'no-unit_address_vs_reg']
args.extend(search_list)
args.append(dts_input)
-dtc = os.environ.get('DTC') or 'dtc'
command.Run(dtc, *args, capture_stderr=capture_stderr)
return dtb_output
diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index d41115a22c..66f6ab7af0 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -188,6 +188,77 @@ def PathHasFile(path_spec, fname):
[PATCH v2 3/4] binman: Allow resolving host-specific tools from env vars
This patch lets tools.Run() use host-specific versions with the
for_host keyword argument, based on the host-specific environment
variables (HOSTCC, HOSTOBJCOPY, HOSTSTRIP, etc.).
Signed-off-by: Alper Nebi Yasak
Reviewed-by: Simon Glass
---
Changes in v2:
- Add a table of host-specific versions in the docstring
- Add tag: "Reviewed-by: Simon Glass "
tools/patman/tools.py | 50 ++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index 66f6ab7af0..bbb157da87 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -188,6 +188,49 @@ def PathHasFile(path_spec, fname):
return True
return False
+def GetHostCompileTool(name):
+"""Get the host-specific version for a compile tool
+
+This checks the environment variables that specify which version of
+the tool should be used (e.g. ${HOSTCC}).
+
+The following table lists the host-specific versions of the tools
+this function resolves to:
+
+Compile Tool | Host version
+--+
+as| ${HOSTAS}
+ld| ${HOSTLD}
+cc| ${HOSTCC}
+cpp | ${HOSTCPP}
+c++ | ${HOSTCXX}
+ar| ${HOSTAR}
+nm| ${HOSTNM}
+ldr | ${HOSTLDR}
+strip | ${HOSTSTRIP}
+objcopy | ${HOSTOBJCOPY}
+objdump | ${HOSTOBJDUMP}
+dtc | ${HOSTDTC}
+
+Args:
+name: Command name to run
+
+Returns:
+host_name: Exact command name to run instead
+extra_args: List of extra arguments to pass
+"""
+host_name = None
+extra_args = []
+if name in ('as', 'ld', 'cc', 'cpp', 'ar', 'nm', 'ldr', 'strip',
+'objcopy', 'objdump', 'dtc'):
+host_name, *host_args = env.get('HOST' + name.upper(), '').split(' ')
+elif name == 'c++':
+host_name, *host_args = env.get('HOSTCXX', '').split(' ')
+
+if host_name:
+return host_name, extra_args
+return name, []
+
def GetTargetCompileTool(name, cross_compile=None):
"""Get the target-specific version for a compile tool
@@ -269,6 +312,7 @@ def Run(name, *args, **kwargs):
Args:
name: Command name to run
args: Arguments to the tool
+for_host: True to resolve the command to the version for the host
for_target: False to run the command as-is, without resolving it
to the version for the compile target
@@ -277,7 +321,8 @@ def Run(name, *args, **kwargs):
"""
try:
binary = kwargs.get('binary')
-for_target = kwargs.get('for_target', True)
+for_host = kwargs.get('for_host', False)
+for_target = kwargs.get('for_target', not for_host)
env = None
if tool_search_paths:
env = dict(os.environ)
@@ -285,6 +330,9 @@ def Run(name, *args, **kwargs):
if for_target:
name, extra_args = GetTargetCompileTool(name)
args = tuple(extra_args) + args
+elif for_host:
+name, extra_args = GetHostCompileTool(name)
+args = tuple(extra_args) + args
all_args = (name,) + args
result = command.RunPipe([all_args], capture=True, capture_stderr=True,
env=env, raise_on_error=False, binary=binary)
--
2.28.0
