Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPU: Implement "double" SNR storage #12258

Merged
merged 2 commits into from
Jun 20, 2022
Merged

SPU: Implement "double" SNR storage #12258

merged 2 commits into from
Jun 20, 2022

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Jun 19, 2022

What Sonic The Hedgehog does is to send a pair of succeeding values to the SPU SNR. If the SPU does not wait on SNR reading, the first value should be overwritten by the second and this is what RPCS3 does.
On real hw however concluded by reversing this game, If the SPU currently reads from the empty SNR channel, the insertion of the first value keeps the channel empty and the SPU quits reading SNR, the SNR is available the obtain the second value freely.
This implements this mechanism and allows Sonic The Hedgehog to display some images and fixes the random crashes with its loading, but it is still not the same as the hacked build of mine which also waits for SNR to empty before each write and thus ensuring values are not overwritten. Because on real hw the SPU finished its execution path faster than PPU and is already executing SPU SNR read instruction when the PPU writes the pair of values allowing it to work on real hw.

@elad335 elad335 marked this pull request as ready for review June 19, 2022 19:02
@elad335 elad335 force-pushed the sonic-2 branch 8 times, most recently from 68909dc to cd822a7 Compare June 20, 2022 03:05
const u64 old = data.fetch_op([&](u64& data)
{
if (pushed_to_jostling)
{
Copy link
Member

Choose a reason for hiding this comment

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

It looks like setting data to 0 may be abandoned here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the cmpxchg in that situation is allowed to fail in case other thread pushed another value and set a count of 1 and a value. In both cases the wait bit is cleared so what needs to be done is done.

Copy link
Member

Choose a reason for hiding this comment

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

Still looks like a reckless "optimization". It should be assumed that the fail can be sponatneous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@Nekotekina Nekotekina merged commit d0e9108 into RPCS3:master Jun 20, 2022
@elad335 elad335 deleted the sonic-2 branch June 20, 2022 19:14
@RPCS3 RPCS3 deleted a comment from MasterChief747 Aug 5, 2024
@RPCS3 RPCS3 locked as resolved and limited conversation to collaborators Aug 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants