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

Slow analogRead() - wrong ADCSRA/ADPS setting #31

Closed
jayzakk opened this issue Jul 27, 2020 · 10 comments
Closed

Slow analogRead() - wrong ADCSRA/ADPS setting #31

jayzakk opened this issue Jul 27, 2020 · 10 comments
Labels

Comments

@jayzakk
Copy link
Collaborator

jayzakk commented Jul 27, 2020

Out of curiosity, I did some speed tests with several boards I have (results see attachment).

The code for this numbers is based of parts from https://playground.arduino.cc/Main/ShowInfo/ and the PI calcs from https://forum.arduino.cc/index.php?topic=451743.15 from jurs.

From the sheet you can see a high cycle count for analogRead function(), independent of the CPU frequency, compared to the atmega328p nano (1792 cycles on atmega, 5733 cycles on lgt). Even 32MHz does not compensate this.

Is it because of a slower conversion on silicon, or is there some problem in the libraries?

ardu-speeds.xlsx

Feel free to add/link these sheets/numbers to the readme.md, as the paragraph
"32Mhz is twice as fast as a conventional arduino nano! Actually even faster as many operations take less clock cycles than in the atmega328p."
does not have any numbers.

@jayzakk
Copy link
Collaborator Author

jayzakk commented Jul 27, 2020

I think I already found it...

The ADC prescaler in ADCSRA (ADPS[2:0]) is set to 7, which is the Atmega default (sysclk/128).
(datasheet): By default, the successive approximation circuitry requires an input clock frequency between 50kHz and 200kHz to get maximum resolution
16MHz / 128 prescaler => 125kHz.

On LGT, the default reset value is 5 (sysclk/32) (and may be even 4 (sysclk/16), depending on sysclk).
(datasheet): By default, the successive approximation circuitry requires an 300KHz To 3MHz.
ADPS [2: 0] Prescale factor
0= 2
1 = 2
2 = 4
3 = 8
4 = 16
5 = 32 (default)
6 = 64
7 = 128

But somewhere, a prescaler init of of 7 (sysclk/128 = atmega default) looks like to happen.

If I run my speed test after switching the prescaler:

ADCSRA=ADCSRA&0xF8|4; // 32MHz sysclk/16 == 2MHz

I get
analogRead() : 24.000 us / 768 cyles

which is MUCH better than the 5733 cycles before.

Depending on sysclk, the prescaler should be set to:
32MHz: 4 (sysclk/16) = 2MHz
16MHz: 3 (sysclk/8) ) = 2MHz
by default, or even, if only using 10 bits (arduino default resolution before setting it to something different), can be 3 and 2.

I did not see much of a difference running the ADC at 4MHz with 10 bits resolution. It then runs at 437 cpu cycles.

@jayzakk jayzakk changed the title Slow analogRead()? Slow analogRead() - wrong ADCSRA/ADPS setting Jul 27, 2020
@jayzakk
Copy link
Collaborator Author

jayzakk commented Jul 27, 2020

As a github noob, I accidently opened the pull request as new iss #32
Sorry for that.

@dbuezas
Copy link
Owner

dbuezas commented Jul 28, 2020

Thanks for the PR and the explanations!

For anybody wanting to know more about the topic in the future, including examples of setting up free running mode with interrupts and reasons behind the changes, visit the conversations in @jayzakk merged PR #32

@thegoodhen
Copy link

Warning: changing the prescaler settings for the ADC will likely affect the apparent input impedance of the adc. This means that the readings of high impedance sources will vary, depending on the version of the core it was compiled against.

This is probably acceptable, but I just thought I'd mention it. :D

@dbuezas
Copy link
Owner

dbuezas commented Sep 9, 2020

@thegoodhen that's a good point.
I don't think it will make it "worse" tho:

According to the datasheets, sample and hold takes 1.5 ADC clocks, and I assume it is implemented the typical way (load a cap and then disconnect the input switch).

In this case, the signal will be less time exposed to a capacitor. Anyway, same as you:

This is probably acceptable, but I just thought I'd mention it. :D

I'd be curious to know if what I wrote is actually correct :)

@LaZsolt
Copy link
Collaborator

LaZsolt commented Oct 23, 2020

I found in wiring_analog.c from line 153 ADC readed two times. I don't know why.

	#if defined(__LGT8FX8P__)
	sbi(ADCSRC, SPN);
	nVal = adcRead();          // first read
	cbi(ADCSRC, SPN);
	#endif
	
	pVal = adcRead();          // second read

	#if defined(__LGT8FX8P__)
	pVal = (pVal + nVal) >> 1; // making average
	#endif


. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .

And in line 103 to 105

tmp = ADCL;
return (ADCH << 8) | tmp;

It could be a bit faster and shorter:
return ADC;

ADC is an _SFR_MEM16() macro too, like at timer registers definitions.

@LaZsolt
Copy link
Collaborator

LaZsolt commented Oct 23, 2020

From LGT8Fx8P v1.0.5 databook:

Dynamic detuning compensation algorithm procedures:

  1. As per application demand to initialize ADC conversion parameter
  2. Setting SPN=1, start ADC sampling, record ADC sampling result as VADC1
  3. Setting SPN=0, start ADC sampling, record ADC sampling result as VADC2
  4. If (VADC1 + VADC2) >> 1, then it is this ADC conversion result

In actual application, to acheive much better result by combination of above algorithm and sampling average method.

@dbuezas
Copy link
Owner

dbuezas commented Oct 23, 2020

Warning: changing the prescaler settings for the ADC will likely affect the apparent input impedance of the adc. This means that the readings of high impedance sources will vary, depending on the version of the core it was compiled against.

I tried to change the ADC prescaler and read the DAC output which is not buffered (and high impedance). The signal distortion looks exactly the same to me

@dwillmore
Copy link
Collaborator

Has this discussion reached a conclusion?

@jayzakk
Copy link
Collaborator Author

jayzakk commented Jan 26, 2023

Closing, as #32 got merged.

@jayzakk jayzakk closed this as completed Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants