fix(zaparoo): restore HDMI on frontend launches and fix DualSense maps - #20
Open
wizzomafizzo wants to merge 7 commits into
Open
fix(zaparoo): restore HDMI on frontend launches and fix DualSense maps#20wizzomafizzo wants to merge 7 commits into
wizzomafizzo wants to merge 7 commits into
Conversation
The alt_launcher hook sat at the top of video_fb_config(), so while the
launcher owned (or was queued to own) the HPS framebuffer the function
returned before assigning anything: fb_width, fb_height and brd_* stayed 0
for the whole menu process. video_menu_bg() then built 0x0 wallpaper images
("Warning: bg1 is 0"), and the launch-time video_fb_enable(0) took
video_fb_set()'s menu-background fallback and programmed a 0x0 framebuffer
into the FPGA on the way into fpga_load_rbf, so HDMI output died on every
game launch from the frontend.
- Move the hook below the geometry block. It still owns the enable and the
kernel-module write, which is all it ever needed to replace, and the
launcher re-asserts its own geometry itself.
- Disable the framebuffer in alt_launcher_shutdown()'s no-child branch too,
mirroring the live-child branch below it. The framebuffer can be on with no
child alive (queued init, the respawn window, a crashed frontend), and FB_EN
carried into do_bridge(0) is the AXI deadlock fpga_load_rbf warns about.
Launcher core only: a game core never enabled it.
gcdb resolves a mapping's bN entries by enumerating the probed evdev node's EV_KEY bits (get_ctrl_index_maps), while hN.x hat entries are arithmetic. A DualSense contributes a gamepad node and a Touchpad node with the same Phys/Uniq, mergedevs() merges them, and input_cb loads the shared map from whichever node reaches it first, caching the result by vid/pid. Probed through the touchpad, every face button resolves to a touchpad code and only the d-pad still works: the beta report of "d-pad moves, Open/Options/View do nothing", seen only with keyboard dongles present, which reorder /dev/input. - zaparoo_gcdb_probe_dev() swaps a QUIRK_DS4TOUCH probe for the group's QUIRK_DS4 sibling. The upstream hook in input_cb is two lines. - The pre-warm now feeds merge-base, non-touchpad nodes only, so it cannot seed the cache from the wrong node either. - get_ctrl_index_maps bounds both key loops to btn_map's size. The array holds KEY_MAX - BTN_JOYSTICK entries at both call sites, but the loops append every key bit the node advertises, and a composite dongle can exceed that and run into the abs_map beside it. - gcdb_controller_idx skips zeroed cache slots. db_maps is zero-initialised, so a device reporting no vid/pid matched an unused slot and was handed an all-zero map as a hit.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults 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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change preserves framebuffer geometry before alternate-launcher handling, redirects DualSense GCDB probes to gamepad nodes, excludes touchpad nodes during prewarming, and hardens controller-map bounds and cache matching. ChangesRuntime input and video handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR makes localized video-output and controller-mapping fixes; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant input_cb
participant zaparoo_gcdb_probe_dev
participant DualSenseGamepadNode
participant gcdb_map_for_controller
input_cb->>zaparoo_gcdb_probe_dev: Probe merged device
zaparoo_gcdb_probe_dev->>DualSenseGamepadNode: Select associated gamepad node
zaparoo_gcdb_probe_dev-->>input_cb: Return gamepad probe index
input_cb->>gcdb_map_for_controller: Request controller mapping
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
The video_fb_enable(0) added to alt_launcher_shutdown()'s no-child branch in 10c4ceb broke every game launch: black screen, and Main gone with /tmp/CORENAME still MENU because the core never came up. app_restart() calls alt_launcher_shutdown() a second time, after fpga_load_rbf has already run socfpga_load and do_bridge(1), so the game core is in the FPGA by then. The child is gone at that point, so the new branch ran, and is_menu() is a per-process cache of orig_name (user_io.cpp:222) which is still true in the dying menu process. video_fb_set()'s menu-background fallback then turns the disable into spi_w(FB_EN | ...) at menu geometry, writing the HPS framebuffer into the new core along with input_switch(0) and a module-param write. That is the same fallback the commit message named as the original cause. The guard comment claimed "launcher core only", but is_menu() describes the process, not what the FPGA holds. Teardown video belongs on the live-child branch, which runs before the reconfiguration. The video_fb_config() hook move, which is the actual HDMI fix, is unchanged.
Every launch of a core with a per-core main= (the ini has [RA_*] main=MiSTer_RA) ended with no Main running and a black screen. What was running instead was /media/fat/zaparoo/frontend with the rbf and mgl as its arguments: app_restart had exec'd the frontend binary as Main. user_io_init passes getFullPath(cfg.main) as exe, which is a pointer into file_io's single static full_path buffer. app_restart calls alt_launcher_shutdown() before it reads exe, and on that path kill_stale_frontends() (getFullPath(s_launcher_path)) and a cold alt_launcher_installed() (FileExists) both rewrite the buffer to the frontend path. The execl then used it. Copy exe into a private buffer before the teardown. Three lines beside the existing fork hook in app_restart, so no new upstream surface.
Main's stdout goes to the console after a reboot, so the first-exit-after- boot slowness (12 s from exec to frontend spawn, versus 0.4 s on a warm cycle, and the frontend falling back to a full-res render) could not be captured with the /tmp log recipe. If /media/fat/zaparoo/main.log exists, a constructor dup2's it over stdout and stderr before main(), writes a start marker per exec, rotates to .old above 8 MB, and starts a detached heartbeat thread writing [zt <uptime ms>] every 250 ms so upstream's untimed output can be placed against zlog's t=. Marker and heartbeat use dprintf: the setvbuf in service_boot.cpp may not have run yet and is only valid before the stream's first use. Absent file, nothing runs. New file, no upstream hooks.
A cold boot where the TV had not asserted HPD yet left the frontend rendering full-res: read_edid() returns 0 without a word when the ADV7513 senses no HPD + monitor sense, video_init() falls back to 1280x720, and the frontend's startup probe sizes itself against that. Nothing corrects it afterwards. The MENU core has no HDMI interrupt pin, so upstream's video_poll() hot-plug re-init never runs; in the captured log the mode stayed wrong for 34 minutes, until the next core load. Poll the transmitter's sense bits instead. The pre-spawn retry now polls out a 3 s hold window rather than giving up after one try at 500 ms, and a spawn that still lands on the fallback mode arms a 30 s watch behind the running child: when the display answers, re-init and restart the child so it probes the real mode. video_reinit() reprograms the PLL, so it is spent only on a link down->up edge and at most 3 times per init - a display that has no EDID to give must not be flickered for it. Sense bits come from a private i2c_open(0x39) handle, the way hdmi_cec.cpp already opens the same chip. No upstream hooks.
nohup over ssh left no Main running: it blocks SIGHUP but the process still inherits the ssh session, and Main does not survive it going away. setsid from /media/fat matches how the device starts it, and the script now checks that it actually came up instead of reporting Done.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
video_fb_config(). With the hook at the top of the function, the menu process never assignedfb_width/fb_height/brd_*, sovideo_menu_bg()built 0x0 wallpaper images and the launch-timevideo_fb_enable(0)programmed a 0x0 framebuffer into the FPGA on the way intofpga_load_rbf, killing HDMI output on every game launch from the frontend. The hook still owns the enable and the kernel-module write.alt_launcher_shutdown()'s no-child branch (launcher core only), mirroring the live-child branch, soFB_ENis never carried intodo_bridge(0)from the queued-init or respawn window.input_cbloads the shared map from whichever node reaches it first and caches it by vid/pid; probed through the Touchpad node every face button resolves to a touchpad code while the d-pad keeps working, which is the beta report of "d-pad moves, Open/Options/View do nothing" seen only with keyboard dongles present.get_ctrl_index_mapstobtn_map's size, and skip zeroed cache slots ingcdb_controller_idxwhere an all-zero device id matched slot 0 and was handed an all-zero map.Based on
feat/zaparoo-kiosk-mode(#19) since it touches the same files; retarget tomasteronce that merges.Summary by CodeRabbit
Bug Fixes
Documentation