Skip to content

PM4 type 2 in acb #3047

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

Merged
merged 6 commits into from
Jun 9, 2025
Merged

PM4 type 2 in acb #3047

merged 6 commits into from
Jun 9, 2025

Conversation

mailwl
Copy link
Contributor

@mailwl mailwl commented Jun 6, 2025

this PR passes CUSA00625 - Battlefield™ Hardline through main menu

@StevenMiller123
Copy link
Collaborator

This PR progresses updated versions of Call of Duty®: Advanced Warfare (CUSA00803) further, though they still crash before reaching ingame.
CUSA00803 v1.24 PR.zip
CUSA00803 v1.24 Main.txt

@Randomuser8219
Copy link
Contributor

shad_log.txt
Knack now gives another garbage PM4 without debug filter.

@mailwl mailwl marked this pull request as draft June 6, 2025 16:18
@squidbus
Copy link
Collaborator

squidbus commented Jun 6, 2025

Last I remember, seeing errors for PM4 like this are usually other issues with processing the command buffer and not actually unimplemented packets.

@StevenMiller123
Copy link
Collaborator

Type 0 case definitely feels like a hack, though I'm pretty sure type 2 is valid.
Call of Duty®: Advanced Warfare hits the type 2 error, and I think if it were a bad packet, then implementing type 2 properly wouldn't fix it.

@mailwl
Copy link
Contributor Author

mailwl commented Jun 6, 2025

Wrong command size, return as been in first commit, now error:

[Debug] <Critical> vk_presenter.cpp:646 operator(): Assertion Failed!
Device lost during waiting for a frame

in knack, and all screen blink black. Something really broken.

Battlefield still softlock on game start

@mailwl
Copy link
Contributor Author

mailwl commented Jun 6, 2025

@StevenMiller123, do you think, something wrong with offsets, and PM4t2 - is part (data) of other command?

@Randomuser8219
Copy link
Contributor

Wrong command size, return as been in first commit, now error:

[Debug] <Critical> vk_presenter.cpp:646 operator(): Assertion Failed!
Device lost during waiting for a frame

in knack, and all screen blink black. Something really broken.

Battlefield still softlock on game start

Knack has a pretty bad race condition going on still. This PR won't fix it.

@StevenMiller123
Copy link
Collaborator

There are a lot of reasons why games might be triggering the PM4 type 0 unreachable, and it will need to be looked into at some point. It's pretty clear in most games that the PM4 type 0 error is caused by some race condition. No amount of properly handling type 0 packets will fix these crashes in games like Knack or The Last of Us Remastered.
The type 2 packet handling is fine, if there was an issue there then Call of Duty wouldn't work on my end.

@georgemoralis
Copy link
Collaborator

ok maybe we can keep type 2 for this pr?

@mailwl mailwl changed the title Stub PM4 type 0 in dcb, PM4 type 2 in acb PM4 type 2 in acb Jun 8, 2025
@mailwl
Copy link
Contributor Author

mailwl commented Jun 8, 2025

i leave the possibility to skip Type0, for developers

Comment on lines 237 to 238
// comment "UNREACHABLE_MSG()" above ^^^ to skip "PM4 type 0" command
dcb = NextPacket(dcb, header->type0.NumWords() + 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we should have this kind of thing committed into the main branch source

Copy link
Contributor Author

@mailwl mailwl Jun 8, 2025

Choose a reason for hiding this comment

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

why? this code not really executed without editing code
and source code full of stubs, which cannot be reversed from fw ATM

Copy link
Collaborator

@squidbus squidbus Jun 8, 2025

Choose a reason for hiding this comment

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

Because it’s clutter, and this isn’t even a stub, it’s an invalid packet. This is just hiding another issue. Hacks don’t belong in mainline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this code is NOT executing in mainline at all, it crash early on UNREACHABLE_MSG()
it's just for devs, to see, how long a game goes on w/o this crash.
After fixing reasons, why games go to this case, the code probably never reach this point (but, as it depricated, may still be used)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, and I think even commented out we should not be committing incorrect hacks into mainline. There’s nothing stopping someone from making this change for debugging or having a branch but I won’t approve on this as-is.

@mailwl mailwl marked this pull request as ready for review June 9, 2025 06:40
@squidbus squidbus merged commit 046cf50 into shadps4-emu:main Jun 9, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants