rpi5 board with GPU support #4335
Conversation
📝 WalkthroughWalkthroughAdds a new Raspberry Pi 5 64-bit GPU board target with boot config, kernel AMDGPU enablement (including a TTM patch), Buildroot defconfig, image hooks, and RAUC tryboot management scripts. Includes cmdline/config files, board metadata, kernel config fragment, Linux patch, rootfs overlay helpers, and build integration. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as RAUC/Caller
participant RTS as rpi-tryboot.sh
participant FS as /boot (FAT)
participant CL as cmdline.sh helpers
Note over User,RTS: set-primary <slot>
User->>RTS: set-primary A|B
RTS->>FS: Read config.txt, cmdline.txt
RTS->>CL: get_value(rauc.slot, cmdline)
CL-->>RTS: current slot
RTS->>CL: set_value(root=PARTUUID..., rauc.slot=<slot>, cmdline)
CL-->>RTS: updated cmdline
RTS->>FS: Write cmdline.txt and cmdline-tryboot.txt
RTS->>FS: Write tryboot.txt (enable tryboot)
RTS-->>User: ok (reboot pending)
Note over User,RTS: set-state <slot> good|bad (commit path)
User->>RTS: set-state <slot> good
RTS->>FS: Verify cmdline-tryboot.txt and tryboot.txt present
RTS->>FS: Confirm tryboot slot == <slot>
RTS->>FS: Mark slot as good (create .good)
RTS->>FS: Update config/cmdline to commit primary
RTS-->>User: committed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
buildroot-external/board/raspberrypi/rpi5-64-gpu/hassos-hook.sh (1)
4-18: LGTM with optional suggestion.The boot artifact preparation logic is correct. The use of
|| truefor optional files (dtbo, README) is appropriate.Consider adding validation for required files before copying:
function hassos_pre_image() { local BOOT_DATA="$(path_boot_dir)" + + # Validate required files exist + if [ ! -f "${BINARIES_DIR}/Image" ]; then + echo "ERROR: Kernel Image not found in ${BINARIES_DIR}" + return 1 + fi mkdir -p "${BOOT_DATA}/slot-A/"This provides early failure rather than creating an incomplete boot partition.
buildroot-external/board/raspberrypi/rpi5-64-gpu/rootfs-overlay/usr/lib/rauc/rpi-tryboot.sh (2)
39-45: Emit an explicit error for invalid slot names.Right now an unexpected slot just exits 1 without any diagnostics, which makes RAUC logs opaque when someone misconfigures the slot name. Please emit a clear error before returning.
- else - exit 1 + else + echo "tryboot set-primary ${slot_bootname}: unknown slot" >&2 + exit 1
6-7: Fix the comment typos.Nit: “firmwaree” and “usesd” look like typos—worth tidying while we’re here.
-# RAUC hook script for Raspberry Pi firmwaree tryboot -# Meant to be usesd as a RAUC bootloader-custom-backend script. +# RAUC hook script for Raspberry Pi firmware tryboot +# Meant to be used as a RAUC bootloader-custom-backend script.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
buildroot-external/board/raspberrypi/rpi5-64-gpu/cmdline.txt(1 hunks)buildroot-external/board/raspberrypi/rpi5-64-gpu/config.txt(1 hunks)buildroot-external/board/raspberrypi/rpi5-64-gpu/hassos-hook.sh(1 hunks)buildroot-external/board/raspberrypi/rpi5-64-gpu/kernel-amd.config(1 hunks)buildroot-external/board/raspberrypi/rpi5-64-gpu/meta(1 hunks)buildroot-external/board/raspberrypi/rpi5-64-gpu/patches/linux/0001-amdgpu.patch(1 hunks)buildroot-external/board/raspberrypi/rpi5-64-gpu/rootfs-overlay/usr/lib/rauc/cmdline.sh(1 hunks)buildroot-external/board/raspberrypi/rpi5-64-gpu/rootfs-overlay/usr/lib/rauc/rpi-tryboot.sh(1 hunks)buildroot-external/configs/rpi5_64_gpu_defconfig(1 hunks)
🔇 Additional comments (8)
buildroot-external/board/raspberrypi/rpi5-64-gpu/kernel-amd.config (1)
1-17: LGTM!The AMDGPU kernel configuration options are appropriate and correctly formatted. The comment on line 2 documenting the diff methodology is helpful for maintainability.
buildroot-external/board/raspberrypi/rpi5-64-gpu/hassos-hook.sh (1)
21-23: LGTM!The post-image hook correctly delegates to the disk image conversion function.
buildroot-external/board/raspberrypi/rpi5-64-gpu/rootfs-overlay/usr/lib/rauc/cmdline.sh (1)
5-11: LGTM!The get_value function correctly extracts key-value pairs using anchored grep (
^$key=) to avoid partial matches.buildroot-external/board/raspberrypi/rpi5-64-gpu/config.txt (1)
1-60: LGTM with note about board-specific setting.The boot configuration is comprehensive and appropriate for Raspberry Pi 5 with GPU support. The RAUC A/B slot configuration (lines 45-46) correctly integrates with the tryboot boot management.
Note: Line 11 (
dtparam=ant2) enables the external antenna, which the PR author mentions is specific to their Sentinel Core board. Users with standard Raspberry Pi 5 boards should comment this out if they experience Wi-Fi issues.buildroot-external/board/raspberrypi/rpi5-64-gpu/patches/linux/0001-amdgpu.patch (2)
1-11: Verify patch provenance and upstream status
- Date set to “Wed, 28 May 2025” – typo (should this be 2024)?
- No matching upstream submission under “Yang Bo” in DRM/TTM lists – has this patch been sent to the maintainers?
- ARM64 cache-coherency TTM fixes have been discussed (2019–2023); is there a public bug report or thread for this exact change?
- Is this a temporary local workaround or intended as a permanent upstream fork?
13-45: Review comment incorrect: this patch is a board-specific overlay applied only to the Raspberry Pi 5 64-bit GPU build and does not affect other architectures.Likely an incorrect or invalid review comment.
buildroot-external/board/raspberrypi/rpi5-64-gpu/meta (1)
1-11: Reuse of BOARD_ID and SUPERVISOR_MACHINE is intentional and consistent.
BOARD_ID and SUPERVISOR_MACHINE for the GPU variant match the base board per existing conventions.buildroot-external/configs/rpi5_64_gpu_defconfig (1)
1-165: HASSIO_MACHINE identifier is consistentThe
BR2_PACKAGE_HASSIO_MACHINE="raspberrypi5-64"value matches the base rpi5_64 config and aligns with all other variants; no change required.
| @@ -0,0 +1 @@ | |||
| zram.enabled=1 zram.num_devices=3 rootwait cgroup_enable=memory fsck.repair=yes console=tty0 root=PARTUUID=8d3d53e3-6d49-4c38-8349-aff6859e82fd ro rauc.slot=A numa=fake=0 numa=off amdgpu.runpm=0 | |||
There was a problem hiding this comment.
🧩 Analysis chain
Hardcoded PARTUUID will break on other installations.
The PARTUUID 8d3d53e3-6d49-4c38-8349-aff6859e82fd is hardcoded and specific to your installation. This cmdline.txt serves as a template copied during image preparation (see hassos-hook.sh line 17), so it must work generically across all installations.
Typical solutions in boot management:
- Use a placeholder like
@@RAUC_PARTUUID@@and have the image build or first-boot script substitute the actual partition UUID - Reference by filesystem label (
root=LABEL=hassos-system) if the partition labeling scheme supports it - Generate cmdline.txt dynamically during image creation with the correct PARTUUID
Check how the existing rpi5-64 board handles this, or verify whether the build system is expected to template-substitute this value.
🏁 Script executed:
#!/bin/bash
# Check if other Raspberry Pi board cmdline.txt files use hardcoded PARTUUIDs or placeholders
rg -n 'PARTUUID=' buildroot-external/board/raspberrypi/*/cmdline.txtLength of output: 475
🏁 Script executed:
#!/bin/bash
# Locate and inspect the first 200 lines of hassos-hook.sh
file=$(find buildroot-external -type f -name 'hassos-hook.sh' | head -n1)
echo "Inspecting: $file"
sed -n '1,200p' "$file"Length of output: 975
Template PARTUUID in cmdline.txt instead of hardcoding
Both buildroot-external/board/raspberrypi/rpi5-64/cmdline.txt and …/rpi5-64-gpu/cmdline.txt hardcode root=PARTUUID=… and are copied unmodified by hassos-hook.sh, which will break on any installation whose partition UUID differs. Replace with a placeholder (e.g. @@RAUC_PARTUUID@@) or use root=LABEL=… and inject the real UUID at build/first-boot.
🤖 Prompt for AI Agents
In buildroot-external/board/raspberrypi/rpi5-64-gpu/cmdline.txt around line 1,
the cmdline hardcodes root=PARTUUID=8d3d53e3-6d49-4c38-8349-aff6859e82fd which
will break installs with different partition UUIDs; update the file to use a
template placeholder (for example @@RAUC_PARTUUID@@) or an alternative stable
identifier like root=LABEL=<label> and ensure hassos-hook.sh (or the
build/first-boot step) injects the actual PARTUUID or resolves the LABEL to the
real UUID at build or first boot time so the deployed cmdline contains the
correct root identifier for the target system.
| set_value() { | ||
| key="$1" | ||
| new_value="$2" | ||
| cmdline_string="$3" | ||
|
|
||
| # Use sed to replace the value of the key if it exists, or add a new key-value pair | ||
| if echo "$cmdline_string" | grep -q "$key="; then | ||
| echo "$cmdline_string" | sed "s/$key=[^ ]*/$key=$new_value/" | ||
| else | ||
| echo "$cmdline_string $key=$new_value" | ||
| fi | ||
| } |
There was a problem hiding this comment.
Fix regex to avoid partial matches and handle special characters.
The set_value function has two issues:
- Line 20:
grep "$key="matches partial keys (e.g., key="foo" matches "foobar=x") - Line 21:
sed "s/$key=[^ ]*/$key=$new_value/"is not anchored and doesn't escape special regex characters in key or value
Apply this diff:
set_value() {
key="$1"
new_value="$2"
cmdline_string="$3"
+
+ # Escape special characters for use in regex
+ escaped_key=$(printf '%s\n' "$key" | sed 's/[[\.*^$()+?{|]/\\&/g')
+ escaped_value=$(printf '%s\n' "$new_value" | sed 's/[&/\]/\\&/g')
# Use sed to replace the value of the key if it exists, or add a new key-value pair
- if echo "$cmdline_string" | grep -q "$key="; then
- echo "$cmdline_string" | sed "s/$key=[^ ]*/$key=$new_value/"
+ if echo "$cmdline_string" | grep -q "\(^\| \)$escaped_key="; then
+ echo "$cmdline_string" | sed "s/\(^\| \)$escaped_key=[^ ]*/\1$escaped_key=$escaped_value/"
else
echo "$cmdline_string $key=$new_value"
fi
}This ensures:
- Grep checks for word boundaries (start of line or space before key)
- Special regex characters are escaped
- Sed replacement is anchored to avoid partial matches
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| set_value() { | |
| key="$1" | |
| new_value="$2" | |
| cmdline_string="$3" | |
| # Use sed to replace the value of the key if it exists, or add a new key-value pair | |
| if echo "$cmdline_string" | grep -q "$key="; then | |
| echo "$cmdline_string" | sed "s/$key=[^ ]*/$key=$new_value/" | |
| else | |
| echo "$cmdline_string $key=$new_value" | |
| fi | |
| } | |
| set_value() { | |
| key="$1" | |
| new_value="$2" | |
| cmdline_string="$3" | |
| # Escape special characters for use in regex | |
| escaped_key=$(printf '%s\n' "$key" | sed 's/[[\.*^$()+?{|]/\\&/g') | |
| escaped_value=$(printf '%s\n' "$new_value" | sed 's/[&/\]/\\&/g') | |
| # Use sed to replace the value of the key if it exists, or add a new key-value pair | |
| if echo "$cmdline_string" | grep -q "\(^\| \)$escaped_key="; then | |
| echo "$cmdline_string" | sed "s/\(^\| \)$escaped_key=[^ ]*/\1$escaped_key=$escaped_value/" | |
| else | |
| echo "$cmdline_string $key=$new_value" | |
| fi | |
| } |
Hi it's me again with a simplified and board-scoped version of #4159 that is hopefully more reasonable to upstream.
I've split this PR into two commits: one straight up copy of the rpi5-64 board, and a separate commit to add all the GPU changes. This should make it easier to see what was changed and rebase the changes in the future.
The breakthrough that I hope makes this a reasonable PR now is this new patch that is only a couple of lines: geerlingguy/raspberry-pi-pcie-devices#756 It even opens the door to support other GPU vendors in the future.
So essentially what this is is a copy of rpi5 with
There is one other change that is specific to our Sentinel Core board: selecting the external wifi antenna. I'm open to suggestions to make this as useful as possible for anyone experimenting with GPUs, weather they use our board or a concoction of hats and eGPU cases.
The kernel patch applies cleanly on a wide range of kernels from the current 6.12 to the latest 6.17, and the rpi5-64 directory was last changed 8 months ago, so it seems like the maintenance burden of this PR would be extremely low. Getting this merged would relieve us of a great maintenance burden and make it a lot easier for our users to update their OS.
I'm not an expert on buildroot, so feedback welcome.
Summary by CodeRabbit