keirf / flashfloppy-osd

On Screen Display and keyboard controller for FlashFloppy
The Unlicense
57 stars 15 forks source link

Fix notify display for SPI1 w/ 36 MHz clock (VGA hires mode) #46

Closed tkurbad closed 2 years ago

tkurbad commented 2 years ago

Fixes issue #45

The problem lies in the resizing of the OSD box outline. For SPI frequencies > 18 MHz, this leads to the OSD text disappearing. The reason is not entirely clear, but keeping the OSD box at a constant (maximum) size eliminates the issue.

This PR also fixes SPI initialization by properly resetting and (re-)enabling the peripheral first.

tkurbad commented 2 years ago

@keirf Please don't merge this yet. I'll add similar handling for the user definable hotkeys tomorrow to keep the OSD look consistent.

keirf commented 2 years ago

It's quite promising that you've found that not changing OSD box size "fixes" the problem. This surely does mean it's some sort of software race rather than some unfixable hardware glitch.

So it's something to do with pressing the mode switch hotkey, which changes mode and displays a notifier or hotkey at the exact same time? Is that right?

Or is it related to OSD box resize in VGA mode and not the preceding mode switch itself.

tkurbad commented 2 years ago

Well, not entirely. Even if not a single mode switch occured (or is about to occur), pressing the 'OSD Off' key sequence while using SPI1 in 36 MHz mode "empties" all subsequent OSD boxes. This even happens for bare v1.9 w/o any configured hot keys. I.e. I fire up the Amiga with the flicker fixer enabled, the OSD will enter VGA mode right away and I'll be greeted by the OSD output Flashfloppy sends via I2C. If I then press LCTRL-LALT-Del, I see a smaller OSD box (1 row in height and 7 columns wide, to be precise), but the text 'OSD Off' isn't displayed. If I press the key combo again to re-enable the OSD, another small empty box appears, where 'OSD On' should go. Subsequent I2C OSD outputs enlarge the box again, but it stays empty until the blue pill is reset completely.

Apart from that, there's also a mode switching issue that is mitigated by the SPI hard reset: If the Amiga is switched on with the flicker fixer enabled, and SPI1 is thus first driven by the 36MHz clock, switching the flicker fixer off at runtime doesn't reset the SPI peripheral to 9 MHz via do_autosync. Therefore, the OSD gets very distorted and squeezed to the left side of the screen. Hard resetting the SPI port first makes it switch frequencies according to the Amiga either driving 15 or 31kHz at run time.

keirf commented 2 years ago

Ah I see. Okay perhaps I can work out the underlying cause based on testing with vs without your patches. I will stick all your stuff on a branch for now, I expect.

tkurbad commented 2 years ago

Thanks! I'd suggest to merge the SPI reset stuff into master. It doesn't hurt and - at least to me - looks like a cleaner way to reinit the peripheral. In all the examples I found about how it's done in STM's own IDE, they do a _deinit first when reconfiguring peripherals. It's not entirely clear what their _deinit stuff does, but their forum suggests it just resets the ports.

keirf commented 2 years ago

Yes that probably makes sense!

tkurbad commented 2 years ago

While I'm sure you will work out the underlying cause of the disappearing OSD contents eventually, for the time being I added a second commit that ensures the OSD size stays the same for hotkey output strings as well while formatting them somewhat pleasantly. It's far from an ideal and even further from an elegant solution but works for me. ;-)

keirf commented 2 years ago

This is fine, and I'm happy to carry it all on a branch for now. And I will work on it when I have time. Greaseweazle is taking my time at the moment!

tkurbad commented 2 years ago

Yeah, I can imagine that. Greaseweazle is very nice, and I know how much time the little details can cost.

May I suggest that the branch be the videoswitch-branch that will eventually incorporate the videoswitch-functionality that's been - and still is - my initial intention in all of this? If you find a better solution for the disappearing OSD, this can be cherry-picked into that branch later...

keirf commented 2 years ago

Yes that's the plan: videoswitch branch incorporating that feature plus the required fixes.

tkurbad commented 2 years ago

Will close this now in favor if PR #47, where I'll add the videoswitch stuff as well.