BeagleBadge prerelease add-ons - #10333
Conversation
Add the generic SSD16xx DRM driver, GDEY042T81 binding, BeagleBadge overlay, and vendor kernel configuration. Developed with assistance from OpenAI Codex. Signed-off-by: Andrei Aldea <andrei1998@gmail.com>
Add the generic SSD16xx video driver and the BeagleBadge splash, boot menu, recovery actions, USB storage, and guarded OSPI updater. Developed with assistance from OpenAI Codex. Signed-off-by: Andrei Aldea <andrei1998@gmail.com>
Enable the BeagleBadge e-paper overlay without replacing other overlays, and request the TI badge-launcher package by default. Developed with assistance from OpenAI Codex. Signed-off-by: Andrei Aldea <andrei1998@gmail.com>
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds Linux and U-Boot SSD16xx support for the BeagleBadge GDEY042T81 E-Ink panel. It adds device-tree integration, boot-menu and recovery actions, overlay activation, and synchronized splash rendering. ChangesLinux display integration
U-Boot integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant BeagleBadge
participant UBoot
participant SSD16xx
participant BootMenu
participant Linux
BeagleBadge->>UBoot: Initialize display and boot menu
UBoot->>SSD16xx: Clear panel and render splash
SSD16xx-->>UBoot: Complete refresh
UBoot->>BootMenu: Render configurable menu
BootMenu->>Linux: Select Linux entry
Linux->>SSD16xx: Register panel and submit framebuffer refresh
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
patch/u-boot/u-boot-k3/board_beaglebadge/0001-video-add-SSD16xx-e-paper-display-support.patch (1)
471-487: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an unsigned type for the column index.
row_bytesis asize_tandxis anint. The comparisonx < row_bytespromotesxto unsigned and triggers a sign-compare warning under-Wextra. Declarexandyas unsigned, or castrow_bytestoint.♻️ Proposed change
size_t row_bytes = ssd16xx_row_bytes(panel); - int x, y; + size_t x, y;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patch/u-boot/u-boot-k3/board_beaglebadge/0001-video-add-SSD16xx-e-paper-display-support.patch` around lines 471 - 487, Update the loop index declaration in the framebuffer conversion loop to use an unsigned type compatible with the size_t row_bytes comparison, eliminating the signed/unsigned warning while preserving the existing y and x iteration behavior.patch/kernel/archive/k3-6.18/0002-dt-bindings-display-add-Good-Display-GDEY042T81.patch (1)
65-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
spi-max-frequencyis required here but optional in the driver.The Linux driver in patch 0001 treats a missing
spi-max-frequencyas valid. It warns and falls back toSSD16XX_SPI_SPEED_DEFAULT. Removespi-max-frequencyfromrequired, or drop the fallback from the driver so both layers state the same contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patch/kernel/archive/k3-6.18/0002-dt-bindings-display-add-Good-Display-GDEY042T81.patch` around lines 65 - 71, Align the device-tree binding with the driver’s optional handling of spi-max-frequency: remove spi-max-frequency from the required list in the Good Display GDEY042T81 binding, preserving the other required properties and the driver fallback contract.patch/kernel/archive/k3-6.18/0001-PENDING-drm-tiny-add-SSD16xx-e-paper-display-support.patch (1)
478-479: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe fast-refresh branch is unreachable, and no full refresh ever runs again.
ssd16xx_hw_initsetspanel->partial_mode_ready = truebefore any frame is drawn.ssd16xx_fb_dirtytherefore always takes the partial-refresh branch, and theelsebranch that writes the RED RAM baseline is dead code. Every later frame uses mode0xFF, so ghosting accumulates for the whole session.The matching U-Boot driver in this stack solves this with
partial_refresh_limit = 5. Add the same counter here: force a full refresh after a fixed number of partial refreshes.Also applies to: 620-640
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patch/kernel/archive/k3-6.18/0001-PENDING-drm-tiny-add-SSD16xx-e-paper-display-support.patch` around lines 478 - 479, Update ssd16xx_hw_init and ssd16xx_fb_dirty to track partial refreshes with a partial_refresh_limit of 5, rather than marking partial_mode_ready permanently before the first frame. Increment the counter for partial refreshes and force the existing full-refresh/RED RAM baseline path when the limit is reached, then reset the counter after a full refresh.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@patch/kernel/archive/k3-6.18/0001-PENDING-drm-tiny-add-SSD16xx-e-paper-display-support.patch`:
- Around line 253-278: Change ssd16xx_wait_for_panel to return an error on
timeout and success otherwise, then propagate and check that result in
ssd16xx_hw_init, ssd16xx_display_update, and ssd16xx_clear_display. Ensure
callers stop their operation and return the failure instead of marking the panel
initialized or reporting a successful update when the busy wait times out.
- Around line 578-601: Update ssd16xx_fb_dirty to validate the result of
drm_fb_dma_get_gem_obj(fb, 0) and its vaddr CPU mapping before calling
iosys_map_set_vaddr or ssd16xx_convert_fb_to_1bpp. Abort the update cleanly when
either the GEM object or dma_obj->vaddr is unavailable, while preserving the
existing conversion flow for valid mappings.
In
`@patch/kernel/archive/k3-6.18/0003-arm64-dts-ti-add-BeagleBadge-GDEY042T81-overlay.patch`:
- Around line 49-97: Add
patch/kernel/archive/k3-6.18/0003-arm64-dts-ti-add-BeagleBadge-GDEY042T81-overlay.patch
to the k3-6.18 kernel patch list using the existing patch-list entry format,
ensuring it is applied with the other archive patches.
In
`@patch/u-boot/u-boot-k3/board_beaglebadge/0001-video-add-SSD16xx-e-paper-display-support.patch`:
- Around line 726-746: Restore the CYCLIC throttle check to after
ops->video_sync in video_sync so the driver callback always executes for every
sync request. Preserve the existing cache flush and return handling, and do not
throttle video_sync at the uclass level.
In
`@patch/u-boot/u-boot-k3/board_beaglebadge/0003-board-ti-am62lx-add-BeagleBadge-recovery-actions.patch`:
- Around line 75-113: Add image validation checks for trusted digest, signature,
or platform image format in each of the badge_load_tiboot3, badge_load_tispl,
and badge_load_uboot functions. After confirming the file loads and fits the
partition size limit, insert validation logic before returning success. If
validation fails, set the error state using itest 0 == 1 instead of itest 0 ==
0, so corrupt or improperly signed images do not proceed to flashing with sf
probe.
---
Nitpick comments:
In
`@patch/kernel/archive/k3-6.18/0001-PENDING-drm-tiny-add-SSD16xx-e-paper-display-support.patch`:
- Around line 478-479: Update ssd16xx_hw_init and ssd16xx_fb_dirty to track
partial refreshes with a partial_refresh_limit of 5, rather than marking
partial_mode_ready permanently before the first frame. Increment the counter for
partial refreshes and force the existing full-refresh/RED RAM baseline path when
the limit is reached, then reset the counter after a full refresh.
In
`@patch/kernel/archive/k3-6.18/0002-dt-bindings-display-add-Good-Display-GDEY042T81.patch`:
- Around line 65-71: Align the device-tree binding with the driver’s optional
handling of spi-max-frequency: remove spi-max-frequency from the required list
in the Good Display GDEY042T81 binding, preserving the other required properties
and the driver fallback contract.
In
`@patch/u-boot/u-boot-k3/board_beaglebadge/0001-video-add-SSD16xx-e-paper-display-support.patch`:
- Around line 471-487: Update the loop index declaration in the framebuffer
conversion loop to use an unsigned type compatible with the size_t row_bytes
comparison, eliminating the signed/unsigned warning while preserving the
existing y and x iteration behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 59f2941a-09e1-4c0d-8702-06747df6ba99
📒 Files selected for processing (9)
config/boards/beaglebadge.confconfig/kernel/linux-k3-vendor.configpatch/kernel/archive/k3-6.18/0001-PENDING-drm-tiny-add-SSD16xx-e-paper-display-support.patchpatch/kernel/archive/k3-6.18/0002-dt-bindings-display-add-Good-Display-GDEY042T81.patchpatch/kernel/archive/k3-6.18/0003-arm64-dts-ti-add-BeagleBadge-GDEY042T81-overlay.patchpatch/u-boot/u-boot-k3/board_beaglebadge/0001-video-add-SSD16xx-e-paper-display-support.patchpatch/u-boot/u-boot-k3/board_beaglebadge/0002-board-ti-am62lx-enable-BeagleBadge-display-and-menu.patchpatch/u-boot/u-boot-k3/board_beaglebadge/0003-board-ti-am62lx-add-BeagleBadge-recovery-actions.patchpatch/u-boot/u-boot-k3/board_beaglebadge/0004-video-add-BeagleBadge-e-paper-splash.patch
Propagate Linux panel-ready and SPI failures, reject unmapped framebuffers, and periodically run a full anti-ghosting refresh. Keep U-Boot video synchronization unconditional by default, opt SSD16xx into rate limiting, and cover forced synchronization in the sandbox test. Rebase the splash patch over the new video operations. Developed with assistance from OpenAI Codex. Signed-off-by: Andrei Aldea <andrei1998@gmail.com>
The BeagleBadge device tree routes its debug console through UART0, which Linux exposes as ttyS0. UART2 is disabled, so enabling a ttyS2 getty only adds a device timeout during boot. Developed with assistance from OpenAI Codex. Signed-off-by: Andrei Aldea <andrei1998@gmail.com>
|
✅ This PR has been reviewed and approved — all set for merge! |
Will look into when possible. Now in bug fixing mode :) |
Description
Add BeagleBadge specific changes -
@igorpecovnik the U-boot menu thing may be something to use on other boards that support U-boot, it's a nice recovery mechanism.
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes