Menu

#1885 Use the new fast noise LFSR reset by Dag Lem to improve the tests

v3.x
closed-fixed
Dag Lem
None
test programs
2023-06-07
2023-05-19
No

As discussed privately with @gpz we could employ the new fast noise reset routine [1] to make some SID tests faster and more reliable.
Ideally we should include both method, allowing compile time choice, to compare the results.
The tests include at least:
* wb_testsuite/noise_writeback_check.asm
* wf12nsr/wf12nsr.asm
* noisewriteback

Opening this ticket as a reminder, will post patches as time permits.

[1] https://github.com/daglem/reDIP-SID/blob/master/research/noise-reset.a65

Discussion

  • Dag Lem

    Dag Lem - 2023-05-19

    Make sure to fetch the program again after reading this, it has been fixed to restore the I flag after calls to CHROUT.

     
    • Leandro Nini

      Leandro Nini - 2023-05-21

      Thanks! I've just exctracted the reset routine part from the source to fit it into the existing tests.

       
  • Leandro Nini

    Leandro Nini - 2023-05-21

    Here's a first patch. It still uses the old method by default but switching to the new one seems to work pretty well.

    There are still a few tests that can be converted:
    * SID/noisewriteback/noise_writeback_test1.asm
    * SID/noisewriteback/noise_writeback_test2.asm
    * SID/waveforms/waveforms.asm

    One thing to keep in mind is that the new method messes with the frequency registers.

     
  • Dag Lem

    Dag Lem - 2023-05-22

    Nice! I think you may be missing a final setting of the test bit in noise-reset_new.asm, i.e. something like this at the end (the first part of point 4 - reset_lfsr, in the original program):
    LDA #$08
    STA $D412
    After this, the clearing of the test bit in the test programs will finalize the reset.

     
  • Dag Lem

    Dag Lem - 2023-05-22

    One more thing - set_lfsr_18_22 is timing sensitive, so make sure that no badlines or interrupts can interfere. I'm guessing all the tests already ensure this, but I mention it anyway - better safe than sorry :-)

     
  • Leandro Nini

    Leandro Nini - 2023-05-22

    patch updated, thanks!

     
    • Dag Lem

      Dag Lem - 2023-05-23

      I believe you should set the desired frequency in noise_writeback_check.asm and wf12nsr.asm just before clearing the test bit, otherwise the oscillator will start running earlier than without the changes, and you'll be changing the tests ever so slightly.

       
    • Dag Lem

      Dag Lem - 2023-05-23

      Just a final small nitpick :-D
      In the Makefiles, you could set NEWRESET=0 just once, and then use that as -DNEWRESET=$(NEWRESET) in the recipes.
      This would make it simpler to override the setting.

       
  • Leandro Nini

    Leandro Nini - 2023-05-23

    Thanks again for reviewing! Here's the third round.

     
    • Dag Lem

      Dag Lem - 2023-05-24

      Even though the result is the same, I think I would still set the frequency immediately before releasing the test bit in wb_testsuite/noise_writeback_check.asm, so that one isn't fooled into thinking that the setting before the noise reset makes any difference.

      Other than this nitpick, this looks good to me :-)

       
  • Leandro Nini

    Leandro Nini - 2023-05-24

    Uh right, missed that!

    BTW, any plan moving to git? ;)

     
    • gpz

      gpz - 2023-05-24
       
      👍
      1
      • Leandro Nini

        Leandro Nini - 2023-05-24

        Nice!

         
    • Dag Lem

      Dag Lem - 2023-05-28

      I have tested the patch on a C64, and there is still a minor issue - badlines disrupt the timing of the LFSR clocking in noisewriteback/noise_writeback_check.asm and wb_testsuite/noise_writeback_check.asm

      Changing the start of both files to the following fixes the issue:

      ; disallow interrupts and disable screen to get stable timing
      sei
      lda #0
      sta $d011
      sta $d015
      raslo:
      bit $d011
      bpl raslo
      rashi:
      bit $d011
      bmi rashi

      I haven't tested wf12nsr.asm, but I think it should be changed like this:

      ; disallow interrupts and disable screen to get stable timing
      sei
      jsr clrscreen
      lda #5
      sta finalresult+1
      lda #0
      sta $d011
      sta $d015
      sta currenttest
      raslo:
      bit $d011
      bpl raslo
      rashi:
      bit $d011
      bmi rashi

      start

      One final thing I noticed when reading through noise_writeback_check.asm is that 9 -> 9 is marked as unstable for the 8580. So perhaps it's better to change the new reset routine to use noise + sawtooth + triangle instead of just noise + triangle?

       
  • Leandro Nini

    Leandro Nini - 2023-05-29

    Updated patch attached.

    One final thing I noticed when reading through noise_writeback_check.asm is that 9 -> 9 is marked as unstable for the 8580. So perhaps it's better to change the new reset routine to use noise + sawtooth + triangle instead of just noise + triangle?

    Right, on the chips I've tested it seems that $b or $f are more reliable. Changed it to N+S+T

     
    👍
    1
  • Dag Lem

    Dag Lem - 2023-05-31
    • status: open --> closed-fixed
    • assigned_to: Dag Lem
     
  • gpz

    gpz - 2023-06-07

    applied in r43934

     

Log in to post a comment.