"bne" assembly command isn't doing what it should

Question

I was trying to see if I could make a square wave tone generator on my 6502 cpu. After that I tried to make it change frequency over time but it always got stuck in a loop that counts the cycles of the output and changes the frequency after a speciffic amount of cycles. Here's the code:

PORTA = $6001
PORTB = $6000
DDRA = $6003
DDRB = $6002

TONE = $0200    ;1 byte
TEMPX = $0201   ;1 byte
TEMPY = $0202   ;1 byte
INDEX = $0203   ;3 bytes

CLEAR = %00000001

E = %10000000
RW = %01000000
RS = %00100000

 .org $c000
MUSIC:
T1:
 ldy #12
 ldx #75
 jmp WAIT_LOOP
T2:
 ldy #8
 ldx #5
 jmp WAIT_LOOP
T3:
 ldy #6
 ldx #37
 jmp WAIT_LOOP
T4:
 ldy #5
 ldx #1
 jmp WAIT_LOOP
T5:
 ldy #4
 ldx #25
 jmp WAIT_LOOP
T6:
 ldy #3
 ldx #64
 jmp WAIT_LOOP
T7:
 ldy #3
 ldx #18
 jmp WAIT_LOOP
T8:
 ldy #2
 ldx #83
 jmp WAIT_LOOP
T9:
 ldy #2
 ldx #55
 jmp WAIT_LOOP
JH:
 jmp HLT

 .org $8000
RESET:               The programm starts here
 ldx #$ff
 txs

 cli

 lda #%11111111   ;setting port B as all output
 sta DDRB
 lda #%11100001   ;setting first 3 pins and last pin of port A as output
 sta DDRA

 lda #%00111000   ;initialize LCD in 8-bit mode, 5x8 font and 2 lines
 jsr LCD_INSTRUCTION
 lda #%00001110   ;initialize LCD to be on and have cursor on
 jsr LCD_INSTRUCTION
 lda #%00000110   ;set cursor move direction and set LCD to not scroll
 jsr LCD_INSTRUCTION

 jsr LCD_CLEAR

 lda #$4c
 sta INDEX
 lda #$00
 sta INDEX +1
 lda #$c0
 sta INDEX +2

 lda #%01010101
 sta TONE

 ldx #0
PRINT_MESSAGE:
 lda MESSAGE,x
 beq PLAY
 jsr PRINT_CHAR
 inx
 jmp PRINT_MESSAGE

PLAY:
 ldy #$ff
 ldx #$ff
PLAY_TONE:
 sty TEMPY
 stx TEMPX

 lda TONE
 and #%0000001
 sta PORTA

 lda TONE
 eor #$ff
 sta TONE

 jsr WAIT

 ldy TEMPY
 ldx TEMPX
 dex
 bne PLAY_TONE
 dey
 bne PLAY_TONE            it always gets stuck jumping to "PLAY_TONE"

INC_INDEX:
 clc
 lda INDEX +1
 adc #7
 sta INDEX +1
 lda INDEX +2
 adc #0
 sta INDEX +2

 jmp PLAY

HLT:
 jmp HLT

MESSAGE: .asciiz "Frequency: var                         Amplitude: 5V   "

WAIT:
 jmp INDEX
WAIT_LOOP:
 dex
 bne WAIT_LOOP
 dey
 bne WAIT_LOOP
 rts

LCD_WAIT:
 pha
 lda #%00000000   ;set port B as all input
 sta DDRB
LCD_BUSY:
 lda #RW
 sta PORTA
 lda #(RW|E)
 sta PORTA
 lda PORTB
 and #%10000000   ;only check busy flag
 bne LCD_BUSY

 lda #RW
 sta PORTA
 lda #%11111111   ;set port B back to all output
 sta DDRB
 pla
 rts

LCD_CLEAR:
 lda #CLEAR       ;clear LCD
 jsr LCD_INSTRUCTION

LCD_INSTRUCTION:
 jsr LCD_WAIT
 sta PORTB
 lda #%0          ;clear RS/RW/E bits
 sta PORTA
 lda #E           ;enable the LCD to send instruction
 sta PORTA
 lda #%0          ;clear RS/RW/E bits
 sta PORTA
 rts

PRINT_CHAR:
 jsr LCD_WAIT
 sta PORTB
 lda #RS         ;clear RW/E bits and switch RS on
 sta PORTA
 lda #(RS|E)     ;enable the LCD to send instruction
 sta PORTA
 lda #RS         ;clear RW/E bits and switch RS on
 sta PORTA
 rts

NMI:
 rti

BRK_IRQ:
 rti

EXIT_INT:
 rti

VECTORS:
 .org $fffa
 .word NMI
 .org $fffc
 .word RESET
 .org $fffe
 .word BRK_IRQ

I know that the code is probably horrably inefficient but i just want it to work (I am using the setup from ben eater) There is also an emulator of the setup I am using at https://www.tejotron.com/

After a certain amount of cycles the code should change frequency a few times and then stop by getting stuck in the loop "HLT". I have tried a few things but nothing worked.

Answer 1

The first time you enter WAITLOOP X contains 75 and Y contains 12. It will count X 75 down to 0, then it will decrement Y to 11. Then it goes back to the top of the loop and decrements X (which contains zero at this point) from 0 to 0 (going the long way) and decrements Y again and do on until Y is zero.

So the first time through, you have 75 + 12 x 256 iterations of WAITLOOP and that is 3147 iterations. The code that does this takes 5 cycles for each X iteration plus a few more for each Y iteration. It's roughly 16,000 clock cycles.

In the loop that calls WAITLOOP you iterate 256 x 256 iterations. This is calling WAITLOOP 65,536 times. So in your first set of PLAY_TONE iterations, you spend around 1030,000,000 cycles just executing WAITLOOP. That's 1030 seconds or 17 minutes (assuming a 1MHz clock) not including all the other instructions in your loop.

I don't think it's stuck, I just think your loops are far too big.

---

Side note

In my opinion, your JMP INDEX is a little hincky. It kind of obfuscates what happens. If INDEX and INDEX+1 just contain the address you want to jump to, you can just use the indirect form JMP (INDEX). It will save you one clock cycle and one byte and also be more readable.