Skip to content

Some clean ups around sceGu #285

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 2 commits into from
May 9, 2025
Merged

Conversation

fjtrujy
Copy link
Member

@fjtrujy fjtrujy commented May 8, 2025

Description

Some improvements and clean-ups in sceGu:

  • Putting better constants names for PspDisplaySetBufSync
  • Separate into 2 files sceGuFinish and sceGUFinishId
  • Modify sendCommandiStall function, it was previously adding the command and then updating the stall address, now we have a new function called _sceGuUpdateStallAddr which updates the stall address.
  • Add return value to sceGuInit
  • Add return value to sceGuCallList
  • Rename resetValues to _sceGuResetGlobalVariables, which is a more appropriate name for a "kind of private function"
  • Remove some remaining magic numbers.
  • Improve sceGuInit by being sure we cover failure scenarios and make it more robust for concurrency scenarios.
  • Improve sceGuSendList
  • Some other minor changes

Cheers!

};

/** Values for retro compatibility */
#define PSP_DISPLAY_SETBUF_IMMEDIATE PSP_DISPLAY_SETBUF_NEXTHSYNC
Copy link
Member Author

Choose a reason for hiding this comment

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

Keeping previous values for retro compatibility

@@ -116,7 +116,7 @@ extern GuLightSettings light_settings[4];

void callbackSig(int id, void *arg);
void callbackFin(int id, void *arg);
void resetValues();
void _sceGuResetGlobalVariables();
Copy link
Member Author

@fjtrujy fjtrujy May 8, 2025

Choose a reason for hiding this comment

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

Use better function name to avoid name conflicts when linking the binary.

Copy link
Member Author

Choose a reason for hiding this comment

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

The same content was before in sceGuFinish

Copy link
Member Author

Choose a reason for hiding this comment

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

Improving sceGuInit now it has better error check

@fjtrujy fjtrujy force-pushed the magic_numbers_gu branch from bfca421 to a4e09fa Compare May 8, 2025 18:46
@fjtrujy fjtrujy force-pushed the magic_numbers_gu branch from a4e09fa to 2ecbb9d Compare May 8, 2025 18:48
@fjtrujy fjtrujy marked this pull request as ready for review May 8, 2025 18:49
@fjtrujy fjtrujy force-pushed the magic_numbers_gu branch from e4a95a7 to 4743678 Compare May 9, 2025 22:11
Copy link
Member

@sharkwouter sharkwouter left a comment

Choose a reason for hiding this comment

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

I'm not able to do testing with this this weekend unfortunately, but the code looks pretty good to me. I just had some small things I don't understand, but I did not spot any issues.

GuContext *context = &gu_contexts[ctype];
unsigned int *local_list = (unsigned int *)(((unsigned int)list) | 0x40000000);

intr = sceKernelCpuSuspendIntr();
Copy link
Member

Choose a reason for hiding this comment

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

Is this expensive to do? Do we have a chance to lose input because of this?

Copy link
Member Author

Choose a reason for hiding this comment

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

it shouldn't be expensive, is the best way to assure atomic operations

Copy link
Member

Choose a reason for hiding this comment

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

Fair


if (!gu_object_stack_depth && !gu_curr_context)
sceGeListUpdateStallAddr(ge_list_executed[0], gu_list->current);
static inline int _sceGuUpdateStallAddr(void) {
Copy link
Member

Choose a reason for hiding this comment

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

So if I understand correctly the stalli function was used to make sure that if you are using direct mode, the function you called before cannot be interrupted? This looks more readable, but I do wonder how this looks in cpu cycles.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the GCC compiler is clever enough like to put the exactly same ASM code as before

@fjtrujy fjtrujy merged commit f2b1155 into pspdev:master May 9, 2025
1 check 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.

2 participants