Suspicious logic

[jotego]

This change by @greyrogue does not look right:

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

This was reported in this issue. In my original code en_ch[0] was used, whereas en_ch[2] is used twice here... That looks like a typo.

[jotego]

Shouldn't it be?

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

[greyrogue]

It's been a while since I made that change, but I believe it is correct. If I remember correctly, it's trying to happen two cycles early (see the commented line above which has target numbers [0,0],[1,1]...etc. As I recall, en_ch actually counts backwards, so two cycles early for each one looks like this: [en,cur] + two cycles = target, but if target cur>=6 then --en, cur-=6 [1,4] + 2 = [1,4+2] = [1,6]=[1-1,6-6]=[0,0] [2,5] + 2 = [2,5+2] = [2,7]=[2-1,7-6]=[1,1] [2,0] + 2 = [2,0+2] = [2,2] [3,1] + 2 = [3,1+2] = [3,3] [4,2] + 2 = [4,2+2] = [4,4] [5,3] + 2 = [5,3+2] = [5,5]

[jotego]

Thank you. But why is en_ch[2] used twice and en_ch[0] ommitted. That does not look right on its own.

[greyrogue]

It's because of the mismatch between 36 cycles per total vs a gap of 5 between each channel. Probably would help to see it as a table: ([en,cur] with en counting backwards) [5,0],[5,1],[5,2],[5,3],[5,4],[5,5] [4,0],[4,1],[4,2],[4,3],[4,4],[4,5] [3,0],[3,1],[3,2],[3,3],[3,4],[3,5] [2,0],[2,1],[2,2],[2,3],[2,4],[2,5] [1,0],[1,1],[1,2],[1,3],[1,4],[1,5] [0,0],[0,1],[0,2],[0,3],[0,4],[0,5] You can see the targets are spaced 5 apart (which means the active5 needs to be as well). Note that when [2,0] is active, [2,5] is the 5 later. So using this table, you can see that the first one happens at [5,5] (which is the [5] out of the [36]), 25 later will be [0,0] (which is [30] out of [36]). So the last 6 are skipped, as well as the first 5 (11 total), which brings it back to [5,5]. 36 -25 = 11. If it was happening with a gap of 6, this weirdness wouldn't happen, but that would only happen if always using cur_ch=0. My memory is fuzzy, but I think other places in the code assume that all calculations are done when en=0, which is where they are summed.

[sorgelig]

I believe this is already finished discussion.