Skip to content

Commit ebd08d4

Browse files
committed
Preserve YM2610 context for port A and B
Address context must be preserved for both ports A and B, because a write to data port X is only effective if the port previously written to was the same port X. This fixes a race condition that would cause invalid setup of ADPCM-A sample, and trigger a wrong communication pattern to the YM2610.
1 parent f46ae7a commit ebd08d4

2 files changed

Lines changed: 51 additions & 22 deletions

File tree

nullsound/timer.s

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
;;;
22
;;; nullsound - modular sound driver
3-
;;; Copyright (c) 2023-2024 Damien Ciabrini
3+
;;; Copyright (c) 2023-2026 Damien Ciabrini
44
;;; This file is part of ngdevkit
55
;;;
66
;;; ngdevkit is free software: you can redistribute it and/or modify
@@ -87,8 +87,8 @@ update_timer_state_tracker::
8787
;; ym2610 register context from a ongoing ym2610_write_port_a.
8888
;; so we have to update the YM2610 with care
8989

90-
;; step1: wait before reading/writing anything, so that
91-
;; if we interrupted a ym2610_write_port_a, it gets a chance to
90+
;; step 1: wait before reading/writing anything, so that if we
91+
;; interrupted a write to a YM2610 register, it gets a chance to
9292
;; update the YM2610 properly
9393
call ym2610_wait_available
9494

@@ -101,10 +101,10 @@ update_timer_state_tracker::
101101
ld c, a
102102
call ym2610_write_port_a_no_ctx
103103

104-
;; step2: at this stage, if we interrupted a ym2610_write_port_a,
105-
;; restore the YM2610 register context before returning from the
106-
;; interrupt handler
107-
call ym2610_restore_context_port_a
104+
;; step 2: at this stage, if we interrupted a ym2610_write_port_a
105+
;; or ym2610_write_port_b, restore the YM2610 register context
106+
;; before returning from the interrupt handler
107+
call ym2610_restore_port_context
108108

109109
pop bc
110110
pop af

nullsound/ym2610.s

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
;;;
22
;;; nullsound - modular sound driver
3-
;;; Copyright (c) 2020-2023 Damien Ciabrini
3+
;;; Copyright (c) 2020-2026 Damien Ciabrini
44
;;; This file is part of ngdevkit
55
;;;
66
;;; ngdevkit is free software: you can redistribute it and/or modify
@@ -22,16 +22,28 @@
2222
.include "ym2610.inc"
2323

2424

25-
.lclequ CTX_NO_REGISTER, 0xff
25+
.lclequ CTX_NO_WRITE, 0
26+
.lclequ CTX_WRITE_PORT_A, 1
27+
.lclequ CTX_WRITE_PORT_B, 2
2628

2729

2830
.area DATA
2931

3032

31-
;;; current YM2610 context when a write to port A is ongoing, 0xff otherwise
32-
;;; (the interrupt handler currently writes to the YM2610 on port A,
33-
;;; so ym2610_write_port_a must be reentrant, hence this context)
34-
state_ym2610_context_port_a::
33+
;;; The interrupt handler currently writes to the YM2610 to reset and
34+
;;; rearm interrupts, so ongoing port writes from user space must
35+
;;; be reentrant. Possible values:
36+
;;; - 0: no write in progress
37+
;;; - 1: write to port A in progress
38+
;;; - 2: write to port B in progress
39+
;;; Address context must be preserved for both ports A and B, because
40+
;;; a write to data port X is only effective if the port previously
41+
;;; written to was the same port X (see ngdevkit#123).
42+
state_ym2610_context_write::
43+
.blkb 1
44+
45+
;;; target address when a write to port A or port B was initiated
46+
state_ym2610_context_address::
3547
.blkb 1
3648

3749

@@ -48,7 +60,9 @@ ym2610_write_port_a::
4860
push af
4961
;; reentrant: save register context
5062
ld a, b
51-
ld (state_ym2610_context_port_a), a
63+
ld (state_ym2610_context_address), a
64+
ld a, #CTX_WRITE_PORT_A
65+
ld (state_ym2610_context_write), a
5266
;; select register address
5367
ld a, b
5468
out (PORT_YM2610_A_ADDR), a
@@ -58,8 +72,8 @@ ym2610_write_port_a::
5872
out (PORT_YM2610_A_VALUE), a
5973
call _ym2610_wait_data_write
6074
;; reentrant: clear register context
61-
ld a, #CTX_NO_REGISTER
62-
ld (state_ym2610_context_port_a), a
75+
xor a
76+
ld (state_ym2610_context_write), a
6377
pop af
6478
ret
6579

@@ -90,6 +104,11 @@ ym2610_write_port_a_no_ctx::
90104
;;; (all registers are preserved)
91105
ym2610_write_port_b::
92106
push af
107+
;; reentrant: save register context
108+
ld a, b
109+
ld (state_ym2610_context_address), a
110+
ld a, #CTX_WRITE_PORT_B
111+
ld (state_ym2610_context_write), a
93112
;; select register address
94113
ld a, b
95114
out (PORT_YM2610_B_ADDR), a
@@ -98,6 +117,9 @@ ym2610_write_port_b::
98117
ld a, c
99118
out (PORT_YM2610_B_VALUE), a
100119
call _ym2610_wait_data_write
120+
;; reentrant: clear register context
121+
xor a
122+
ld (state_ym2610_context_write), a
101123
pop af
102124
ret
103125

@@ -135,17 +157,24 @@ ym2610_wait_available::
135157
ret
136158

137159

138-
;;; ym2610_restore_context_port_a
139-
;;; -----------------------------
160+
;;; ym2610_restore_port_context
161+
;;; ---------------------------
140162
;;; If an interrupt was triggered while the YM2610 was being
141163
;;; written to by the sound driver, restore the register context
142164
;;; before exiting from the interrupt handler.
143-
;;; (all registers are preserved)
144-
ym2610_restore_context_port_a::
145-
ld a, (state_ym2610_context_port_a)
146-
cp #CTX_NO_REGISTER
165+
;;; (registers preserved, except a)
166+
ym2610_restore_port_context::
167+
ld a, (state_ym2610_context_write)
168+
cp #CTX_NO_WRITE
147169
jr z, _end_restore_context
170+
cp #CTX_WRITE_PORT_A
171+
ld a, (state_ym2610_context_address)
172+
jr z, _restore_context_port_a
173+
out (PORT_YM2610_B_ADDR), a
174+
jr _restore_wait_write
175+
_restore_context_port_a:
148176
out (PORT_YM2610_A_ADDR), a
177+
_restore_wait_write:
149178
call _ym2610_wait_address_write
150179
_end_restore_context:
151180
ret

0 commit comments

Comments
 (0)