search

ramapcsx2 / gbs-control

GNU General Public License v3.0
773 stars 110 forks source link

Update Si5351mcu to unmodified upstream 0.7.1 #435

Closed nyanpasu64 closed 1 year ago

nyanpasu64 commented 1 year ago

This updates src/si5351mcu.{cpp,h} to the latest versions from https://github.com/pavelmc/Si5351mcu.

Previously we had a copy of Si5351mcu at src/ modified to fix building on ESP8266, and an unmodified copy at 3rdparty/Si5351mcu/. Upstream has already incorporated the build fix, so I also removed the copy at 3rdparty/Si5351mcu/ since the updated files are completely unchanged from upstream.

I have verified that external clock gen still works, and outputs at the correct refresh rate when given NTSC, PAL, and 480p signals (from 240p Test Suite Wii) as input. Also clockgen/frequency-based frametime lock works in both NTSC and PAL as well.

ramapcsx2 commented 1 year ago

It sounds good to me, but the doubled frequency problem is a gamble. There is a good chance it is fixed by now, from either direct fixes in the library, or from work on this project. It can be tested to remove it, but watch for any issue reports..

smgoldade commented 1 year ago

I have tested this locally and everything is working as well. I have never had the double frequency issue so no comment there.

nyanpasu64 commented 1 year ago

This PR does not remove the hack in externalClockGenResetClock() to prevent doubled output frequency, so that code should be unchanged. If we need to do so (eg. if we calculate output dot clocks based on custom htotal and vtotal values, decoupled from the chip's preset frequencies, and don't know whether to switch to a different frequency first), I'd have to open a new PR and test that first. Alternatively, to minimize change in behavior, I could only skip the frequency hack when using custom output frequencies/dot clocks.

ramapcsx2 commented 1 year ago

It would be best to get rid of it everywhere. Test the cold waters ;p