CIA shift register implementation may be incorrect?

[Rhialto]

Hi! I'm far from an expert in reading vhdl, but reading this:

https://github.com/MEGA65/mega65-core/blob/c533da5ee6e26d33a6dd5018aac888fd3ece1fcc/src/vhdl/cia6526.vhdl#L549-L554

it looks like the bits are shifted out LSB first. The data sheet however says that "SDR data is shifted out MSB first and serial input data should also appear in this format."

Also, when the processor stores data to the shift register, it's supposed to be double buffered. From the data sheet: "The data in the Serial Data Register will be loaded into the shift register then shift out to the SP pin when a CNT pulse occurs. Data shifted out becomes valid on the falling edge of CNT and remains valid until the next falling edge. After 8 CNT pulses, an interrupt is generated to indicate more data can be sent. If the Serial Data Register was loaded with new information prior to this interrupt, the new data will automatically be loaded into the shift register and transmission will continue. If the microprocessor stays one byte ahead of the shift register, transmission will be continuous.".

This seems to be the code for storing to the SDR but I see no such double-buffering, since the data is written directly into the register that is also shifted:

https://github.com/MEGA65/mega65-core/blob/c533da5ee6e26d33a6dd5018aac888fd3ece1fcc/src/vhdl/cia6526.vhdl#L761-L766

ETA: data sheet from here: http://www.zimmers.net/anonftp/pub/cbm/documents/chipdata/6526.zip

[gardners]

You are quite likely right, as the shift register stuff has not been really well tested. If you are willing to summarise the key points from the data sheet that should be implemented, and suggest what you think would be the right way. Don't worry about it being correct VHDL, just capturing the essence of how it should behave will be helpful.

[Rhialto]

Well the first part would be relatively simple:

 countout <= sdr_bit_alternate; 
 if sdr_bit_alternate='0' then 
   spout <= reg_shift_data(7); 
   reg_sdr_data(7 downto 1) <= reg_sdr_data(6 downto 0); 
   reg_sdr_data(0) <= '0'; 
   report "Shifting out bit, " & integer'image(sdr_bits_remaining-1) & " to go."; 

The second one involves an extra register of 8 bits to double-buffer the data, and a single bit to indicate it is filled with valid data. Then, when the shifter is idle, or finished, it moves the data from the new register to the actually shifting register, reg_shift_data. Something like

when x"0c" => 
   -- Begin shifting data in or out on shift register 
   if  sdr_bits_remaining == 0 then -- start shifting right away
       report "CIA" & to_hstring(unit) & " Loading shift register";
       reg_shift_data <= std_logic_vector(fastio_wdata); 
       sdr_bits_remaining <= 8; 
       sdr_bit_alternate <= '1'; 
   else -- have to double-buffer this byte
       report "CIA" & to_hstring(unit) & " Loading serial data register";
       reg_sdr_data <= std_logic_vector(fastio_wdata); 
       reg_sdr_filled <= '1';
   end if;

and later, for example after report "Asserting shift register ISR flag";

    if reg_sdr_filled then
       reg_shift_data <= reg_sdr_data;
       sdr_bits_remaining <= 8; 
       sdr_bit_alternate <= '1'; 
       reg_sdr_filled <= '0';
    end if;

This is ignoring the weird timing delays that exist around all this...

I'm trying to improve things in VICE, see discussion at https://sourceforge.net/p/vice-emu/patches/233/ and https://sourceforge.net/p/vice-emu/bugs/1219/ for example. There are interesting (read: really annoying) tests for this discussion at https://sourceforge.net/p/vice-emu/code/HEAD/tree/testprogs/CIA/shiftregister/cia-sdr-icr/ .

And there is the shifting IN of course, which I've sort of ignored here.

[gardners]

Hello, Thanks for this. If you are willing to checkout the source, make the changes and then a pull request with them, I'll test that it still builds (unless you want to get all excited and download Vivado etc and test compilation yourself), and then we can merge it into the development branch.

Getting the CIA timing just right is indeed tricky. I believe that Gideon made a harness with a CIA chip on it and FPGA connected to it, so that he could run all manner of tests and see how it behaved. We could look at doing the same, if someone was enthusiastic at pursuing it.

Paul.

[Rhialto]

Hi Paul,

see #538 for the pull request. I will leave checking it to you :) I suspect that Vivado is a mswindows-only program and won't run on my NetBSD machine.

I found a CIA in Verilog here: https://github.com/MiSTer-devel/C64_MiSTer/blob/master/rtl/mos6526.v It says it's not passing VICE's dd0d test; since it's the ICR I suspect that one also doesn't have the timing right, but it may have some ideas in other areas (I didn't really look at it deeply).

Cheers, -Olaf

[lgblgblgb]

@Rhialto

I suspect that Vivado is a mswindows-only program and won't run on my NetBSD machine.

Just a comment here, that Vivado also runs on Linux. No idea if NetBSD has some kind of easy & good Linux emulation layer or something like that, but maybe still a better hope than it would be Windows-only.

[lydon42]

@Rhialto I'd like to take a look at your pull, and I also did take a look at the test suite on sourceforge. Can you explain to me what I want to see with the test programs? I tried one and get a red border screen with filled with 'I' or 'A', toggeling after a press of space.

Ah, it's in the last section: red is bad. I am looking for green.

And if I am reading this correctly: this effects the IEC, so I should check if my 1541 still works (only have a pi1541, my 1571 is in a bad state, and I don't know if it works or not).

[lydon42]

I did build a bitstream with your changes, and the referenced CIA tests still show up mostly red. Some have a some green characters, but in essence I don't see much difference between development head with or without your changes. So... do you happen to have some test program that shows me that the CIA is now behaving as it should?

[Rhialto]

Great that you're looking at this!

Unfortunately I don't have the illusion that with my changes, the shift register is perfect. The CIA is just too weird for it. It has various delays in it that don't make any sense. Somebody made a very detailed model of how timers and some other things work, but unfortunately it doesn't include the shift register. Maybe you know it, it is this one: https://ist.uwaterloo.ca/~schepers/MJK/cia6526.html (also available from http://www.zimmers.net/anonftp/pub/cbm/documents/chipdata/6526cia.zip )

So the best I was hoping for here was to make it at least a bit better... and let somebody who knows what they're doing with VHDL take some next step.

Now about the tests. They aren't well documented. Each directory has a readme file and that's it.

In general, many test programs work in a similar way. They all report a global succes/failure code via some virtial I/O register, so that the testbench.sh script can run all of them headless and print results. This is also reflected in the border colour: red/green. This is for instance the current state of VICE:

.../vice-emu/testprogs/testbench$ ./testbench.sh x64sc shiftregister
running tests for x64sc (shiftregister):
Reverted '/mnt/vol1/rhialto/cvs/other/vice/vice-emu/testprogs/drive/rpm/rpm.p64'
gmake[1]: micro64disktool: No such file or directory
gmake[1]: *** [Makefile:38: rpm.p64] Error 127
../CIA/shiftregister/ cia-icr-test2-continues.prg - ok 
../CIA/shiftregister/ cia-icr-test2-oneshot.prg - ok 
../CIA/shiftregister/ cia-icr-test-continues-new.prg - ok 
../CIA/shiftregister/ cia-icr-test-continues-old.prg - ok 
../CIA/shiftregister/ cia-icr-test-oneshot-new.prg - ok 
../CIA/shiftregister/ cia-icr-test-oneshot-old.prg - ok 
../CIA/shiftregister/ cia-sdr-load.prg - ok 
../CIA/shiftregister/ cia-sdr-init.prg - ok 
../CIA/shiftregister/ cia-sdr-delay.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-0_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-0.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-1_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-19.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-39.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-3.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-4485-0_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-4485-0.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-4485-1_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-4485-19.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-4485-39.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-4485-3.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-4485-4_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-4_7f.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-generic-0.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-generic-0_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-generic-19.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-generic-1_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-generic-3.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-generic-39.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia1-sdr-icr-generic-4_7f.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-0_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-0.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-1_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-19.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-39.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-3.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-0_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-0.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-1_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-19.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-39.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-3.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-4_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4_7f.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-0.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-0_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-19.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-1_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-3.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-39.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-3.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-0_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-0.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-1_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-19.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-39.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-3.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4485-4_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-4_7f.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-0.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-0_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-19.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-1_7f.prg - error 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-3.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-39.prg - ok 
../CIA/shiftregister/cia-sdr-icr/ cia2-sdr-icr-generic-4_7f.prg - ok 
../CIA/shiftregister/ cia-sp-test-continues-new.prg - ok 
../CIA/shiftregister/ cia-sp-test-continues-old.prg - ok 
../CIA/shiftregister/ cia-sp-test-oneshot-new.prg - ok 
../CIA/shiftregister/ cia-sp-test-oneshot-old.prg - ok 
2 minutes and 5 seconds elapsed.
.../vice-emu/testprogs/testbench$

(The cia-sdr-icr tests are testing some weird thing that happens to the interrupt flags if you enable/disable the timer while the shift register isn't idle. Tests with 4485 in them, or numbers lower than 3, are not expected to work)

All tests show a final success indicator with a green or red border.

Many tests have sub-tests, numbered A ... something. You can switch to the result screen for that test by typing the letter. (Not so many in CIA/shiftregister but in CIA/ciavarious all tests are like that).

Most (sub)tests do some set-up, then take a bunch of measurements (typically reading a particular register as fast as possible). The readings are put in the top half of screen memory. If there is something in the bottom half, it shows the expected values (like in VIC20/viavarious). Here the red/green colouring is used per character.

If something is even 1 cycle off, most tests fail.

I don't think there are many tests that actually look at the contents of the shift register; there seem to be only tests that check if interrupts are generated at the right times. This one is hard enough by itself...