Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Y8950 ADPCM implementation #1160

Open
Eugeny1 opened this issue May 20, 2019 · 10 comments
Open

Y8950 ADPCM implementation #1160

Eugeny1 opened this issue May 20, 2019 · 10 comments

Comments

@Eugeny1
Copy link

Eugeny1 commented May 20, 2019

We are performing testing on the real module, and will be adding information here on the discrepancies between openMSX and real module.

  1. openMSX gives EOS one byte later. For example, if I set start to 0 and stop to 3ff, I have to write 1025 bytes to get EOS in openMSX. Real module required exactly 1024 bytes.
  2. The following sample RAM writing code at the end of sequence
	ld	a,0fh
	out	(0c0h),a
	nop
	xor	a
	out	(0c1h),a
	ini
	ini
	ini
	ini
	ini
	ini
	ini
	ini
	ini
	ini

returns

8E 8E 8E 8E 8E 9E 9E 9E 9E 9E

thus BUFRDY flag appears set before EOS flag is being set. Thus if application reads status register once within 5 * 18 * 279= ~25 us and checks for EOS, it will miss this flag. Code similar to Unknown Reality

ld	a,0fh
out	(0c0h),a
nop
xor	a
out	(0c1h),a

ld	a,04h
out	(0c0h),a
ld	a,080h
out	(0c1h),a

in	a,(0c0h)

does also miss the EOS flag as read is performed within ~11 us. UR code does not fail because it does not track end of sequence using the flag but using number of bytes written.

When reading data from sample RAM there' no delay in EOS setting, and flag gets set immediately after reading from the data port.

  1. When writing RAM (R$07=$60) when end of selected piece is reached, EOS is set, and if CPU continues writing beyond the stop, writes continue from the start of the piece overwriting previous written data. It is actually logical - when EOS condition occurs. chip internally performs pointer reset to the start. At present openMSX's writes beyond end address do not go anywhere.
@MBilderbeek
Copy link
Member

This patch might fix issues 1 and 3...
points1_and_3.patch.txt
Not sure what to do with 2. @m9710797 any idea?

@Eugeny1
Copy link
Author

Eugeny1 commented May 21, 2019

Manuel, thank you. I am doing testing, will be editing original post (if it is possible) adding more relevant information.

@MBilderbeek
Copy link
Member

You should be able to. If not, just add a new comment. No problem! :)

Please let me know if the patch does what it should do... assuming you can compile and run the latest openMSX code and apply the patch before that.

@m9710797
Copy link
Contributor

m9710797 commented May 23, 2019

@MBilderbeek: I don't think your patch for point 1 is not correct (point 3 seems fine):
You basically changed a '<=' comparison into a '<' comparison, but while uploading samples, 'memPntr' is always even and 'stopAddr' is always odd, so these two are never equal and therefor your change has no practical effect.
I think the bug was that we only check for EOS when the next byte is send. instead we should check immediately after the last one has been send.
I've attached an updated patch, but didn't have time yet to do any testing.

@Eugeny1 : point 2 is (only) about the timing of the flags in the status register, right? Currently openMSX does not emulate this timing (see comment in src/sound/Y8950Adpcm.cc:271)

points1_and_3_revision2.patch.txt

@Eugeny1
Copy link
Author

Eugeny1 commented May 23, 2019

Wouter, let me explain and then you will decide if it will require implementation:

	; reset flags
	ld	a,04h
	out	(0c0h),a
	ld	a,080h
	out	(0c1h),a
	ld	a,0fh
	out	(0c0h),a
	ld	a,(hl)
	inc	hl
	out	(0c1h),a

notrd5:
	in	a,(0c0h)
	bit	3,a
	jr	z,notrd5

	inc	de
	ld	a,d
	or	e
	jr	nz,wr5nze

	inc	c
wr5nze:

	in	a,(0c0h)
	bit	4,a
	jr	z,wri4k

This code hangs on real module because when status is read second time EOS bit is not set yet. But this code works in openMSX because it sets EOS immediately without delay.

	ld	a,0fh
	out	(0c0h),a
	nop
	xor	a
	out	(0c1h),a

	; reset flags
	ld	a,04h
	out	(0c0h),a
	ld	a,080h
	out	(0c1h),a

	ex	(sp),hl
	ex	(sp),hl
	ex	(sp),hl
	ex	(sp),hl

	in	a,(0c0h)
	bit	4,a
	jr	z,clrram

This code works on real module, because delay is enough for EOS to raise, but it hangs on openMSX because EOS gets set immediately and flags reset following port write resets EOS bit.

@m9710797
Copy link
Contributor

@Eugeny1: now that you've created a detection routine it's really required to implement this in openMSX ;-)
I'll try to make time for this in the following weeks.

@MBilderbeek
Copy link
Member

@Eugeny1 Were you already able to test the other fixes?

@MBilderbeek
Copy link
Member

@m9710797 If you think it's safe, please commit the changes. Then @Eugeny1 can use the autobuilt binary to do some testing.

m9710797 added a commit that referenced this issue Jun 11, 2019
This fixes points 1 and 3 of
  #1160
See the discussion on that issue for more details. Very short summary:
 1) openMSX sets EOS one byte too late
 3) continue to write after EOS wraps around
Still TODO:
 2) Timing of the BUFRDY and EOS status bits
@MBilderbeek
Copy link
Member

I just found out that e5dc580 causes a regression:

  1. start openMSX with the Philips Music Module (or the openMSX audio extension) and the Philips Music Creator disk
  2. In the disk menu, move the cursor to the CREATOR item and press space to select and start it.
  3. Software hangs up with this patch included, but starts fine without this patch...

@m9710797 What is the best way to proceed?

m9710797 added a commit that referenced this issue Feb 14, 2020
In case of EOS, BUF_RDY must be set as well.
See comments in the code for details.
@m9710797
Copy link
Contributor

Regression caused by e5dc580 should be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants