Skip to content

Fixed SPU code on real hardware#1990

Merged
nicolasnoble merged 2 commits intogrumpycoders:mainfrom
CloudMracek:main
Mar 24, 2026
Merged

Fixed SPU code on real hardware#1990
nicolasnoble merged 2 commits intogrumpycoders:mainfrom
CloudMracek:main

Conversation

@CloudMracek
Copy link
Contributor

SPU fixes

  • SPU not being on for dummy block DMA transfer
  • Volume set to channels was over the maximum volume value for fixed volume mode
  • SPU_KEY_LOW and HIGH are readonly. Using a bitmask on them caused a read which caused undefined volume behavior on real hardware
  • Added a more robust detection of free channels using ENDX registers, which have also been added to common/hardware/spu.h

…e robust free channel detection, other various fixes
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1790048e-e326-4528-88c8-20fffd710a66

📥 Commits

Reviewing files that changed from the base of the PR and between 63d85bd and 0b31743.

📒 Files selected for processing (1)
  • src/mips/psyqo/src/spu.cpp

📝 Walkthrough

Walkthrough

Single-file update to SPU audio driver adjusting voice sample rate values, output volume registers, hardware register operation sequencing during initialization, and key on/off register write behavior from bitwise OR operations to direct assignment.

Changes

Cohort / File(s) Summary
SPU Audio Driver
src/mips/psyqo/src/spu.cpp
Modified channel silencing to set sample rate to 0x1000 instead of 0; reduced main output volume from 0x7fff to 0x3fff; reordered dmaWrite to execute after SPU_CTRL setup; changed SPU_KEY_OFF_* and SPU_KEY_ON_* register writes from OR-ing bits to overwriting with direct assignment (1 << bit).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Whiskers twitch with audio cheer,
Sample rates and volumes clear,
Register writes now stand alone,
No more OR-ing—assignment's shown,
SPU sings with voice so true! 🎵

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fixed SPU code on real hardware' accurately summarizes the main purpose of the changeset—fixing SPU-related issues for real hardware compatibility.
Description check ✅ Passed The description provides specific details about SPU fixes including DMA transfers, volume settings, and SPU_KEY register handling, all of which relate directly to the changes in the code.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Member

@nicolasnoble nicolasnoble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look correct. The init sequence now properly disables the SPU first, configures registers, then enables before DMA - fixing the DMA-with-SPU-off issue while preserving the clean-slate guarantee for unknown power-on state. KEY_ON/KEY_OFF plain writes are correct per hardware behavior (registers are consumed by the SPU and don't retain readable state, so OR-ing with the read was meaningless). Sample rate 0x1000 for silenced voices makes sense to let them process the dummy sample.

@nicolasnoble nicolasnoble enabled auto-merge March 23, 2026 21:37
@nicolasnoble nicolasnoble merged commit a7d01bb into grumpycoders:main Mar 24, 2026
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants