mamedev / mame

MAME
https://www.mamedev.org/
Other
7.92k stars 1.98k forks source link

Commodore PET sound via shift register plays at wrong frequency #7559

Open utz82 opened 3 years ago

utz82 commented 3 years ago

Sound generated through the Shift register on the Commodore PET family (cbm8032 et al) plays at the wrong frequency in MAME. The following code produces a 245 Hz tone on real hardware, but will produce a 490Hz tone in MAME.

    lda #$10        ;shift out free running at T2 rate
    sta VIA_CR
    lda #255        ;lowest possible frequency, 1000000/16/255=245hz
    sta VIA_T2CL
    lda #%00001111
    sta VIA_SR
cuavas commented 3 years ago

Which version of MAME are you using? I’m asking because there was a bug a while ago that caused the VIA to shift at twice the rate it should.

utz82 commented 3 years ago

I'm running 0.226, so I don't think it's resolved yet.

Rhialto commented 2 years ago

I checked on 0.240 and the problem still seems to be there. However, I think that this patch fixes it:

--- ./src/devices/machine/6522via.cpp   2022-01-29 19:53:04.000000000 +0100
+++ ./src/devices/machine/6522via.cpp.new       2022-02-20 19:05:50.791266510 +0100
@@ -512,7 +512,7 @@
                                }
                                else if (SO_T2_RATE(m_acr) || SO_T2_CONTROL(m_acr) || SI_T2_CONTROL(m_acr))
                                {
-                                       m_shift_timer->adjust(clocks_to_attotime(m_t2ll + 2) / 2);
+                                       m_shift_timer->adjust(clocks_to_attotime(m_t2ll + 2));
                                }
                                else // otherwise we stop it
                                {
@@ -781,7 +781,7 @@
                        }
                        else if (SI_T2_CONTROL(m_acr) || SO_T2_CONTROL(m_acr))
                        {
-                               m_shift_timer->adjust(clocks_to_attotime(m_t2ll + 2) / 2);
+                               m_shift_timer->adjust(clocks_to_attotime(m_t2ll + 2));
                                LOGSHIFT(" - read SR starts T2 timer ");
                        }
                        else if (!SO_T2_RATE(m_acr))
@@ -977,7 +977,7 @@
                }
                else if (SO_T2_RATE(m_acr) || SO_T2_CONTROL(m_acr) || SI_T2_CONTROL(m_acr))
                {
-                       m_shift_timer->adjust(clocks_to_attotime(m_t2ll + 2) / 2);
+                       m_shift_timer->adjust(clocks_to_attotime(m_t2ll + 2));
                        LOGSHIFT(" - write starts T2 timer");
                }
                else

This is because timer 2 needs to underflow twice before a bit is shifted. Each time it underflows it toggles the handshake line CB1.

The first hunk of the patch is the main change: that's the time between bit-shifts. The other 2 hunks are about the delay before the first bit is shifted.

In the 6522 datasheet there is a drawing with the timing (but without the actual timing). However it has 2 skip-some-time wavey lines (surely there is an official name for those) each of which would likely stand for the time for the timer to time out.

It wouldn't make much sense if the handshake line would be toggled halfway a time-out period...

The other 2 delays, which I'm doubling here, are simply an extension of the same idea. Thet don't make a PET sound noticably different I would say, so I can't check them in the same way.