Skip to content

samd51: DAC: Make clock usage more like Arduino core #2104

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

Closed
wants to merge 1 commit into from

Conversation

jepler
Copy link

@jepler jepler commented Sep 2, 2019

This improves the behavior around #1992. The samples stay in sync now, though full scale changes still behave erratically.

Testing performed: On a Metro M4, with both analog channels going to a scope, I looked for synchronization and waveform shape.

Original reproducer: Stays synchronized.

#1992 (comment) reproducer: Stays synchronized and plays at the nominal 100kHz sample rate. However, waveform peaks are "regularly irregular", with every other high peak being truncated, usually.

A difference I'm aware of between this and Arduino's M4 core is that Arduino uses CCTRL_CC100K; however, doing this makes CP worse, so I went with the more "correct" CC12M setting.

Closes: #1992

This improves the behavior around adafruit#1992.  The samples stay in
sync now, though full scale changes still behave erratically.

Testing performed: On a Metro M4, with both analog channels going to
a scope, I looked for synchronization and waveform shape.

Original reproducer: Stays synchronized.

adafruit#1992 (comment) reproducer:
Stays synchronized and plays at the nominal 100kHz sample rate.
However, waveform peaks are "regularly irregular", with every other
high peak being truncated, usually.

A difference I'm aware of between this and Arduino's M4 core is that
Arduino uses CCTRL_CC100K; however, doing this makes CP worse, so
I went with the more "correct" CC12M setting.

Closes: adafruit#1992
@jepler
Copy link
Author

jepler commented Sep 2, 2019

Obviously I feel quite uneasy about this and wish I had more answers.

Here's how the DAC looks with the sequence [0,65535] repeated forever:

MSO1104Z_2019-09-02_11 06 21

Something's obviously still quite wrong with how the DAC or its clocking is set up.

@ladyada
Copy link
Member

ladyada commented Sep 2, 2019

lol - out of curiosity, do you have a 'mimic' set up with arduino for this kind of output? what if you do [0, 1000] is it the 65535 thats messing it up?

@jepler
Copy link
Author

jepler commented Sep 2, 2019

@ladyada The screenshot above is from a CircuitPython program with my patch:

import board
import audioio
import array
import time

X = 4000
Y = 65535
data = array.array('H', [0,0,Y,X])
sample = audioio.RawSample(data, sample_rate=100 * 1000, channel_count=2)
a = audioio.AudioOut(board.A0, right_channel=board.A1)
a.play(sample, loop=True)

while 1: time.sleep(1)

My arduino program:

void setup() {
  pinMode(A0, OUTPUT);
}

void loop() {
  analogWrite(A0, 0);
  analogWrite(A0, 4059);
}

MSO1104Z_2019-09-02_11 23 24

@ladyada
Copy link
Member

ladyada commented Sep 2, 2019

🤷‍♀ i just play 20khz max audio thru this thing :D

@jepler
Copy link
Author

jepler commented Sep 2, 2019

The rise/fall rate seems to have to do with the CCTRL register:

Bits 3:2 – CCTRL[1:0] Current Control
This field defines the current in output buffer according to conversion rate.

One thing I notice is that when doing analogWrite, Arduino does this:

                                while ( !DAC->STATUS.bit.READY0 );

                                while (DAC->SYNCBUSY.bit.DATA0);
                                DAC->DATA[0].reg = value;  // DAC on 10 bits.

in CP AnalogOut, we're not looking at READY0 or DATA0. Ardino also doesn't use LEFTADJ but that doesn't seem to be the source of the problem.

@jepler
Copy link
Author

jepler commented Sep 2, 2019

@ladyada I think 20kHz sample rate audio works pretty good, especially if it's also mono. Stereo, very high sample rates, and unusual waveforms are where the problems @kevinjwalters reports seem to enter the picture.

@tannewt tannewt self-requested a review September 3, 2019 20:41
tannewt
tannewt previously approved these changes Sep 3, 2019
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Have you done a diff of the DAC's registers between CircuitPython and Arduino? I'd be curious if it shows anything.

@jepler
Copy link
Author

jepler commented Sep 3, 2019

@tannewt don't merge yet, I think this change may have adversely affected sqpi flash I/O and need to do further checking to exclude that theory. Yes, I do have a diff of DAC registers but it's not handy. Ping me again if I don't attach it.

@tannewt
Copy link
Member

tannewt commented Sep 3, 2019

Kk, looks like it didn't run the checks either...

@tannewt tannewt dismissed their stale review September 3, 2019 20:47

not ready

@jepler
Copy link
Author

jepler commented Sep 3, 2019

Huh I wonder why it didn't run checks.

@dhalbert
Copy link
Collaborator

dhalbert commented Sep 5, 2019

If you merge from upstream and push, it will run the checks. That has worked for me.

@tannewt
Copy link
Member

tannewt commented Sep 11, 2019

Where are we at on this?

@jepler
Copy link
Author

jepler commented Sep 11, 2019

Needs more testing on my part, I had inconclusive results

@jepler
Copy link
Author

jepler commented Sep 15, 2019

I am closing this in favor of a new PR to be issued shortly.

@jepler jepler closed this Sep 15, 2019
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

Successfully merging this pull request may close these issues.

audioio does not maintain synchonisation of A0/A1 looping some audioio.RawSample data
4 participants