fix(tab5): place ST7121 display buffers in PSRAM

[vincentkoc]

Problem

The ST7121 display path requests DMA-capable LVGL buffers without explicitly
selecting PSRAM, unlike the sibling Tab5 panel path. On the current test board,
retained boot logs identify touch firmware revision 1, selecting ST7121.
A later node-role WebSocket startup failed while requesting an 8192-byte
internal/8-bit allocation. That observation motivates protecting internal memory;
it does not prove these display buffers caused the failure.

Change

• With flash encryption inactive, set the ST7121 display flags to
  buff_dma=false and buff_spiram=true, selecting PSRAM for the two draw
  buffers and PPA rotation buffer.
• With flash encryption active, retain the original true,false allocation
  flags. The pinned PPA SRM implementation rejects external buffers in this
  mode. Sample the public runtime encryption predicate once and declare its
  existing bootloader_support component as a private build dependency.
• Preserve the upstream August 2026 ST7123 hardware report while distinguishing
  this campaign's ST7121 panel and LVGL startup evidence.
• Keep buffer geometry, double buffering, rotation/cache handling, sibling
  panels, task stacks, SDK/dependency versions, and global allocation settings
  unchanged.

The three configured RGB565 buffer payloads total 172,800 bytes, excluding
allocation overhead. This calculation is not a measurement of recovered memory.
The encrypted fallback preserves the original allocation request; it does not
qualify encrypted camera, media, or voice. Public bundles still reject encrypted
builds.

Repair Validation

• The encrypted regression compiled and failed on the original source at the
  DMA allocation assertion. Both encrypted and unencrypted cases pass after
  the repair with ASan/UBSan, executing the production display configuration.
  They also retain geometry, buffering, handles, rotation and DSI settings.
• The two cases run in the existing host-test CI step. The native Tab5 build
  must validate the actual SDK header and component linkage.
• actionlint and git diff --check passed. Independent review and fresh scoped
  P2 autoreview found no actionable findings.
• Signed repair head: 0160b153db80069e19e18513a18330ce29fd4647.
  All five jobs passed in CI run 34504342813,
  including the host regressions and native Tab5 SDK/header/component linkage.
  The artifact evidence below predates this encryption-aware repair.
  No encrypted-device or voice proof is claimed.

Prior Build Evidence

• Six existing camera capture/transform tests passed with ASan/UBSan. These
  protect unchanged board-adapter behavior, not physical display placement.
• All five builds passed in CI run 34485144704, attempt 1
  for original head e166026c7616e1c7eaeea1c023585d6c2748c02e.
  The compiled test merge was d9bbea660bcfcd571df1bba81a9d121d594e97e9,
  with expected base/head parents and a tree matching that head.
• Tab5 artifact 10155641928 was verified: 2,898,637 bytes, API/archive SHA256
  8ba18ef61f6dd20a58c9683af4dbb91f93ab4058daa7c3da27e068da3a7f347d.
  All 11 internal checksums, 10 manifest file records, four image mappings,
  and non-overlap with protected partitions passed.
• That build used SDK 362a1776ec212788fda95f75b733bfdde3a0c394,
  v5.5.5-648-g362a1776ec2-dirty, with the existing tracked patches.
  SDK/component patch records, BSP, tools, generated configuration, and parsed
  dependency lock matched the verified 6be43e8 baseline. The 1024-byte
  threshold, 32768-byte reserve, and both 8192-byte node/transport stacks remain
  unchanged by this repair.

Stacked on #50. This is a
bounded board-specific allocation change, not a stack-size or global allocator
workaround. No release or complete media/voice qualification is claimed.

Maintainer Disposition

Accept the board-specific placement change with its encryption-aware fallback,
actual-owner regressions and exact-head native CI. The configured payload
calculation is not a guaranteed heap gain or a claim that WebSocket startup
is fixed. Encrypted-device media and voice qualification remain open.

Land after #50 with a merge
commit and exact cumulative-tree verification. Preserve the signed original
change and accepted encryption repair, along with earlier privacy, packaging
and qualified SDK-image changes. This disposition does not authorize a release.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 12, 2026, 12:13 PM ET / 16:13 UTC (Revision 5).

ClawSweeper review

What this changes

The branch moves unencrypted ST7121 display buffers into PSRAM while preserving encrypted-device allocation, and includes predecessor camera compatibility, geometry, allocation diagnostics, tests, and packaging changes.

Merge readiness

⛔ Blocked before merge - 2 items remain

The change remains necessary, and the earlier encryption finding is fixed. No new blocking code defect was found; the accepted stacked work still needs conflict resolution and cumulative-tree validation.

Priority: P2
Reviewed head: 0160b153db80069e19e18513a18330ce29fd4647

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	The repair is focused and supported by relevant regressions and native builds; integration conflicts remain a separate landing blocker.
Proof confidence	🌊 off-meta tidepool	Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Display-configuration regressions and native CI support the accepted change; predecessor hardware traces demonstrate diagnostics, not this head's physical buffer placement, heap benefit, or encrypted media operation.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Display-configuration regressions and native CI support the accepted change; predecessor hardware traces demonstrate diagnostics, not this head's physical buffer placement, heap benefit, or encrypted media operation.
Evidence reviewed	9 items	Current main still needs the display change: The fetched main revision still sets ST7121 buff_dma=true and buff_spiram=false. The reviewed head selects these flags from the runtime encryption predicate. Earlier encryption finding resolved: The current configuration samples esp_flash_encryption_enabled once and retains the original DMA flags when true. The display fixture checks both encryption states, dimensions, double buffering, rotation, and handles. GitHub source inspection of the earlier head confirmed its unconditional PSRAM selection. Pinned graphics dependency confirms fallback: Dependency inspection applies because the board calls the PPA-backed display path and explicitly bases its fallback on this SDK contract. The pinned SDK rejects external buffers for SRM operations when flash encryption is active.
Findings	None	None.
Security	None	None.

How this fits together

The Tab5 board adapter connects the room-node application to its display and camera hardware. It configures graphics buffers and camera transforms, while transport diagnostics and firmware packaging help operators inspect failures and identify the compiled SDK profile.

flowchart TD
  A[Tab5 room application] --> B[Board adapter]
  B --> C{Flash encryption active}
  C -->|Yes| D[Existing DMA buffers]
  C -->|No| E[PSRAM graphics buffers]
  D --> F[Rotated display]
  E --> F
  B --> G[Camera format and geometry]
  G --> H[JPEG response]

Loading

Before merge

• Resolve merge risk (P1) - The branch conflicts with current main, and the combined result has not been verified; earlier passing builds do not establish preservation of current-main changes after conflict resolution.
• Complete next step (P2) - Land chore(tab5): trace transport-start allocation failures #50 first, resolve conflicts while preserving signed history and accepted encryption, privacy, packaging, and SDK-image changes, then verify and build the cumulative merge tree.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Cumulative code delta	Production/build support +467/-19; tests +1505/-2; documentation and workflow excluded	Most growth belongs to the explicitly stacked camera compatibility and allocation-diagnostic work, rather than the display flag change alone.

Merge-risk options

Maintainer options:

1. Decide the mitigation before merge
   Retain the encryption-aware board allocation and accepted predecessor repairs in a verified cumulative tree, without treating configured buffer payload as measured heap recovery or release qualification.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Retain the encryption-aware board allocation and accepted predecessor repairs in a verified cumulative tree, without treating configured buffer payload as measured heap recovery or release qualification.

Do we have a high-confidence way to reproduce the issue?

Yes, source inspection establishes that current main requests DMA buffers without explicitly selecting PSRAM on ST7121. It does not establish that this allocation caused the reported WebSocket startup failure.

Is this the best way to solve the issue?

Yes, using the existing per-display allocation flags with the SDK-required encrypted fallback is a narrow solution; a global allocator or stack-size change is unnecessary.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 6b63951f0ac5.

Labels

Label justifications:

• P2: This is a bounded Tab5 memory-placement repair without evidence that it resolves the observed WebSocket startup failure.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Display-configuration regressions and native CI support the accepted change; predecessor hardware traces demonstrate diagnostics, not this head's physical buffer placement, heap benefit, or encrypted media operation.

Evidence

What I checked:

• Current main still needs the display change: The fetched main revision still sets ST7121 buff_dma=true and buff_spiram=false. The reviewed head selects these flags from the runtime encryption predicate. (examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c:253, 6b63951f0ac5)
• Earlier encryption finding resolved: The current configuration samples esp_flash_encryption_enabled once and retains the original DMA flags when true. The display fixture checks both encryption states, dimensions, double buffering, rotation, and handles. GitHub source inspection of the earlier head confirmed its unconditional PSRAM selection. (scripts/tests/fixtures/test_tab5_display_allocation.c:78, 0160b153db80)
• Pinned graphics dependency confirms fallback: Dependency inspection applies because the board calls the PPA-backed display path and explicitly bases its fallback on this SDK contract. The pinned SDK rejects external buffers for SRM operations when flash encryption is active. (components/esp_driver_ppa/src/ppa_core.c:622, 362a1776ec21)
• Inherited camera patch has an explicit dependency boundary: The introduced patch directly modifies this component's mapper. At the recorded revision, the pre-IDF-6 branch selects the sensor format as CSI input even after ISP processing; the target patch changes only that selection to the requested output format. (esp_video/src/device/esp_video_csi_format.c:512, 67a555a517b7)
• Cumulative scope and security checks: The verified merge-base-to-head delta contains 20 files. Camera patching checks the SDK, component, source and patch identities; allocation diagnostics emit bounded numeric records; existing encrypted-bundle rejection remains present. No introduced permission expansion, credential handling change, or authorization transfer was found. (scripts/tab5_camera_compat.py:74, 0160b153db80)
• Native CI passed for the reviewed head: The GitHub run reports success for head 0160b15. The captured body reports both encryption regressions and all five native jobs passing, while explicitly distinguishing build evidence from hardware qualification. No tests or target helpers were executed during this read-only review. (.github/workflows/ci.yml:75, 0160b153db80)

Likely related people:

• vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (4 earlier review cycles)

• reviewed 2026-09-10T13:53:36.418Z sha e166026 :: blocked before merge. :: [P1] Preserve internal rotation buffers when flash encryption is active
• reviewed 2026-09-10T14:44:43.285Z sha e166026 :: blocked before merge. :: [P1] Preserve DMA buffers when flash encryption is active
• reviewed 2026-09-10T16:54:32.639Z sha 0160b15 :: needs maintainer review before merge. :: none
• reviewed 2026-09-10T17:06:37.251Z sha 0160b15 :: blocked before merge. :: none