About the ADPCM's active5 signal.

[kunichiko]

I have a question about the HDL code on this line.

jt12/hdl/adpcm/jt10_adpcm_cnt.v

Line 74 in 42b1918

wire active5 = (en_ch[1] && cur_ch[4]) || (en_ch[2] && cur_ch[5]) || (en_ch[2] && cur_ch[0]) || (en_ch[3] && cur_ch[1]) || (en_ch[4] && cur_ch[2]) || (en_ch[5] && cur_ch[3]);//{ cur_ch[3:0], cur_ch[5:4] } == en_ch;

This combinational logic looks asymmetric. Because..

(en_ch[1] && cur_ch[4])   ← diff = 3
(en_ch[2] && cur_ch[5])  ← diff = 3
(en_ch[2] && cur_ch[0])  ← diff = 4
(en_ch[3] && cur_ch[1])  ← diff = 4
(en_ch[4] && cur_ch[2])  ← diff = 4
(en_ch[5] && cur_ch[3])  ← diff = 4

Of course, I haven't fully understood the pipeline logic, but I believe this logic is incorrect.

Could you please inform me if there are any reasons why it is asymmetric?

[jotego]

I didn't make that change. That line looks wrong indeed, it isn't using bit 0 in en_ch either...

After I released JT10, MiSTer developers tinkered with it independently and I lost track of some of the edits. This is marked in MiSTer's repo as an experimental change:

https://github.com/MiSTer-devel/NeoGeo_MiSTer/blame/master/rtl/jt12/hdl/adpcm/jt10_adpcm_cnt.v#L74

I have raised the question in MiSTer's repo, let's wait and see. I think en_ch[0] should be used instead of using twice en_ch[2]. I would try this instead:

 wire active5 =  |({ cur_ch[3:0], cur_ch[5:4] }&en_ch);