lgt8fx icon indicating copy to clipboard operation
lgt8fx copied to clipboard

Slow analogRead() - wrong ADCSRA/ADPS setting

Open jayzakk opened this issue 3 years ago • 8 comments

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 avatar Jul 27 '20 09:07 jayzakk

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 avatar Jul 27 '20 11:07 jayzakk

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

jayzakk avatar Jul 27 '20 13:07 jayzakk

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

dbuezas avatar Jul 28 '20 19:07 dbuezas

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

thegoodhen avatar Aug 10 '20 15:08 thegoodhen

@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 :)

dbuezas avatar Sep 09 '20 06:09 dbuezas

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 avatar Oct 23 '20 14:10 LaZsolt

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.

LaZsolt avatar Oct 23 '20 16:10 LaZsolt

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

dbuezas avatar Oct 23 '20 16:10 dbuezas

Has this discussion reached a conclusion?

dwillmore avatar Jan 12 '23 20:01 dwillmore

Closing, as #32 got merged.

jayzakk avatar Jan 26 '23 17:01 jayzakk