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

Refactor RTC subsystem [WIP] #563

Draft
wants to merge 5 commits into
base: preview
Choose a base branch
from
Draft

Conversation

meeq
Copy link
Contributor

@meeq meeq commented Jun 6, 2024

  1. Optimizes away all RTC read operations except the initial sync and recalculates the date/time based on sync point using ticks.
  2. Support for "Software RTC with no persistence": if your flashcart doesn't have an RTC, just set the time and we'll count forward from there.
  3. Support for switching between clock sources: Joybus and 64DD (highly experimental)
  4. Deprecates rtc_time_t struct in favor of standard C time types.
  5. Adds support for settimeofday so standard C APIs can be used to get and set RTC

Still left to do:

  • Update documentation
  • Strip out the 64DD RTC stuff (it's not fully-baked yet)
  • Document SummerCart quirks
  • Handling the 2038 problem?

Copy link

@gyrovorbis gyrovorbis 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 the RTC guy for the KOS Sega Dreamcast SDK, and we also are using standard C time_t in our RTC API like you are now... I know my opinion doesn't matter even remotely here, but I think this a fantastic API and just wanted to make like 2 teensy comments about marking the API deprecated for the Doxygen so it gets added to the top-level "deprecated" page. lol.

Keep up the good work.

include/rtc.h Outdated Show resolved Hide resolved
include/rtc.h Outdated Show resolved Hide resolved
@gyrovorbis
Copy link

By the way, if you guys ever do anything about "handling the 2038 problem," I'd love to hear it... We have our own 2086 problem over on the Dreamcast. lmao.

@meeq
Copy link
Contributor Author

meeq commented Jun 7, 2024

Rasky informed me that LibDragon is already good on the 2038 problem: time_t is 64-bit in LibDragon.

I haven't actually tested to see what our practical limit is on the newlib side. On the RTC side, we're realistically constrained between 1900-2099 (Joybus RTC limit) or 1996-2095 (64DD RTC limit)

#include <libdragon.h>

#define BLACK 0x000000FF
#define WHITE 0xFFFFFFFF

#define JOYSTICK_DEAD_ZONE 32

#define MAX(a,b) ({ typeof(a) _a = a; typeof(b) _b = b; _a > _b ? _a : _b; })
#define MIN(a,b) ({ typeof(a) _a = a; typeof(b) _b = b; _a < _b ? _a : _b; })
#define CLAMP(x, min, max) (MIN(MAX((x), (min)), (max)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are these part of the C standard library yet?

Choose a reason for hiding this comment

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

NOPE. I think they just got added to C23 for floats, but nah, nothing that I know of as a generic macro... I even have these in my own codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LibDragon has them in the "private header" #include "utils.h":

libdragon/src/utils.h

Lines 12 to 14 in 8fcaba6

#define MAX(a,b) ({ typeof(a) _a = a; typeof(b) _b = b; _a > _b ? _a : _b; })
#define MIN(a,b) ({ typeof(a) _a = a; typeof(b) _b = b; _a < _b ? _a : _b; })
#define CLAMP(x, min, max) (MIN(MAX((x), (min)), (max)))

They seem useful enough that we should expose them to LibDragon devs, but they're generic enough that exposing them in <libdragon.h> could cause naming conflicts. (Basically every C code base has these peppered around in some form or another).

I know LibDragon strongly encourages including the entire library as a single header, but I definitely see the benefits for utils.h to be exposed as an "optional public header": #include <libdragon/utils.h> or something.

This is another one of those "it's a little late to change now" things, but sometimes I wish LibDragon used a common prefix so that we wouldn't have to worry (as much) about namespace collisions with other libraries and user code.

Copy link
Contributor

Choose a reason for hiding this comment

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

A solution could be hiding them behind an opt-in preprocessor define. Like Microsoft does for defining M_PI and friends in math.h. I think you always get them with GCC but with Microsoft's implementation, you need #define _USE_MATH_DEFINES to have them defined.

@meeq
Copy link
Contributor Author

meeq commented Jun 10, 2024

Should the RTC subsystem be responsible for enforcing min/max dates based on RTC hardware limitations?

  • The 1996-2095 range is dictated by the 64DD RTC, which only stores a two-digit year value (and canonically infers XX96-XX99 as 1996-1999).
  • Joybus RTC is theoretically limited to the range 1900-2099, but practically limited to 2001-2099 due to emulation quirks in the various implementations.
Behavior Pro Con My Opinion
settimeofday fails when setting a timeval that the RTC source clock cannot consistently read-back. RTC source will roll-back if read-back is not consistent with written timeval. Conceptually simple: "If successful, the value I provided was written to the RTC source as-is. Otherwise, the RTC source will be unchanged." Requires caller to check result for error. Requires caller to know intimate knowledge of RTC hardware quirks to avoid errors and provide UI affordances. Non-viable: LibDragon should be shielding developers from the rough-edges of N64 hardware quirks in high-level APIs.
settimeofday accepts any timeval, attempts to write it to the RTC source, and keeps counting up from the provided timeval regardless of what was actually written to the RTC. Easiest to implement for me Violates the "principle of least-astonishment" Non-viable: Irresponsibly lazy approach
settimeofday accepts any timeval, attempts to write it to the RTC source, and reads the time back from the RTC source, counting forward from the read-back value. The RTC subsystem time will be consistent with the actual RTC time. This will result in different behavior across emulators and flashcarts Potentially viable
settimeofday silently clamps timeval (to {1996,2000,2001}-2095), attempts to write the clamped value to the RTC source, and keeps counting up from the clamped timeval. This will generally result in consistent behavior across reasonably-compliant emulators and flashcarts, and keeps RTC subsystem time consistent with actual RTC time Some users may be surprised that their input timeval is not respected Potentially viable

Unfortunately, both "potentially viable" options have a huge gaping caveat: EverDrive 3.0/X7 does not support Joybus RTC write commands. If we read-back, the write will always have failed, so the player can't even just set the time for the current play session. If we don't read-back, the write will always silently fail to persist, but at least the time can be set ephemerally.

Animal Forest reads the Joybus RTC continously to update the time, so it would seem to favor the "read-back" approach, and I think this is actually the least-suprising, most-useful of the options.

(Discussion in N64brew Discord thread 💬 )

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.

3 participants