STM core: some ADC tests and feature request for improvements

Post here all questions related to STM32 core if you can't find a relevant section!
Post Reply
ag123
Posts: 1655
Joined: Thu Dec 19, 2019 5:30 am
Answers: 24

STM core: some ADC tests and feature request for improvements

Post by ag123 »

hi,
I did some tests with some codes calling directly into LL_ADC codes in HAL vs analogRead(). The sketch is rather lengthy as such:

Code: Select all

#include <Arduino.h>
#include <variant_BLACKPILL_F401CC.h>

void checkcmd(void);
void printtempvbat(void);
//void enablefpu(void);
void ARTtoggle();
uint16_t readADC(uint32_t pin);
void testADC();
void adcregs();
uint32_t get_adc_channel(PinName pin);
void sleep(uint16_t ms);

int ledPin = LED_BUILTIN;
int print = false;

//used by cosfade()
#define PER 15
#define REP 20
int8_t led = 0;
int n = PER, cnt = 0;
uint8_t p = 0;

//#define DEBUG

// the setup() method runs once when the sketch starts
void setup() {

// not needed fpu is enabled in the core
//	enablefpu();

	ARTtoggle();

	//initialize the digital pin as an output:
	pinMode(ledPin, OUTPUT);
	digitalWrite(ledPin, LOW);

	//these are codes for steve's libmaple core, left here as a ref
	// ADC_CCR_VBATE;  // f401 allows only vbat or temp sensor read, setting VBATE reads vbat
	//ADC_COMMON->CCR |= ADC_CCR_TSVREFE; //| ADC_CCR_VBATE;    // enable VREFINT, VBAT or temp sensor
	//adc_calibrate(ADC1);
	//adc_set_sampling_time(ADC1, ADC_SMPR_480); // sample time for temp sensor

	pinMode(ATEMP, INPUT_ANALOG);
	pinMode(AVREF, INPUT_ANALOG);
	pinMode(PA0, INPUT_ANALOG);

	analogReadResolution(12);

	while(!Serial.available()) {
		Serial.println("press any key to start");
		sleep(1000);
	}
	Serial.read();

}


// Enable the FPU (Cortex-M4 - STM32F4xx and higher)
// http://infocenter.arm.com/help/topic/com.arm.doc.dui0553a/BEHBJHIG.html
//void enablefpu() {
//	  __asm volatile
//	  (
//	    "  ldr.w r0, =0xE000ED88    \n"  /* The FPU enable bits are in the CPACR. */
//	    "  ldr r1, [r0]             \n"  /* read CAPCR */
//	    "  orr r1, r1, #( 0xf << 20 )\n" /* Set bits 20-23 to enable CP10 and CP11 coprocessors */
//	    "  str r1, [r0]              \n" /* Write back the modified value to the CPACR */
//	    "  dsb                       \n" /* wait for store to complete */
//	    "  isb"                          /* reset pipeline now the FPU is enabled */
//	  );
//}


//the loop() method runs over and over again,
//as long as maple has power
void loop() {

	digitalToggle(LED_BUILTIN);
	checkcmd();
	printtempvbat();
	testADC();
	sleep(2000);
}

void testADC() {
	const int N = 10000;

	//adc test
	Serial.print(N);
	Serial.println(" analog reads");
	float ave = 0.0F;
	uint32_t start = millis();
	for(uint16_t i=0; i<N; i++) {
		ave += analogRead(PA0) / 4096.0F * 3.3F;
	}
	uint32_t dur = millis() - start;
	ave = ave / (N * 1.0F);
	Serial.print("dur (ms):");
	Serial.println(dur);
	Serial.print("ave:");
	Serial.println(ave);
	// fast
	start = millis();
	ave = 0.0F;

	// enable adc clocks
	__HAL_RCC_ADC1_CLK_ENABLE();

	LL_ADC_REG_SetSequencerDiscont(ADC1, LL_ADC_REG_SEQ_DISCONT_1RANK);
	LL_ADC_Enable(ADC1);

	adcregs();
	while(!LL_ADC_IsEnabled(ADC1));
	//while(!(ADC1->CR2 & ADC_CR2_ADON_Msk));
	for(uint16_t i=0; i<N; i++) {
		ave += readADC(PA0) / 4096.0F * 3.3F;
	}
	dur = millis() - start;
	ave = ave / (N * 1.0F);
	Serial.print("dur (ms):");
	Serial.println(dur);
	Serial.print("ave:");
	Serial.println(ave);
}

uint16_t readADC(uint32_t pin) {
	// "analogRead"
	PinName pn = digitalPinToPinName(analogInputToDigitalPin(pin));
	uint32_t channel = get_adc_channel(pn);
	LL_ADC_REG_SetSequencerLength(ADC1, LL_ADC_REG_SEQ_SCAN_DISABLE);
	LL_ADC_REG_SetSequencerRanks(ADC1, LL_ADC_REG_RANK_1, channel);
	LL_ADC_SetChannelSamplingTime(ADC1, channel, LL_ADC_SAMPLINGTIME_15CYCLES);

	// start conversion
	LL_ADC_ClearFlag_EOCS(ADC1);
	LL_ADC_REG_StartConversionSWStart(ADC1);
	// another spinlock to check end of conversion;
	while (!LL_ADC_IsActiveFlag_EOCS(ADC1));
	// ok we've got an end-of-conversion
	LL_ADC_ClearFlag_EOCS(ADC1);
	// now read the register
	uint16_t value = LL_ADC_REG_ReadConversionData12(ADC1);
	return value;
}

uint32_t get_adc_channel(PinName pin)
{
  uint32_t function = pinmap_function(pin, PinMap_ADC);
  uint32_t channel = 0;
  switch (STM_PIN_CHANNEL(function)) {
    case 0:
      channel = LL_ADC_CHANNEL_0;
      break;
    case 1:
      channel = LL_ADC_CHANNEL_1;
      break;
    case 2:
      channel = LL_ADC_CHANNEL_2;
      break;
    case 3:
      channel = LL_ADC_CHANNEL_3;
      break;
    case 4:
      channel = LL_ADC_CHANNEL_4;
      break;
    case 5:
      channel = LL_ADC_CHANNEL_5;
      break;
    case 6:
      channel = LL_ADC_CHANNEL_6;
      break;
    case 7:
      channel = LL_ADC_CHANNEL_7;
      break;
    case 8:
      channel = LL_ADC_CHANNEL_8;
      break;
    case 9:
      channel = LL_ADC_CHANNEL_9;
      break;
    case 10:
      channel = LL_ADC_CHANNEL_10;
      break;
    case 11:
      channel = LL_ADC_CHANNEL_11;
      break;
    case 12:
      channel = LL_ADC_CHANNEL_12;
      break;
    case 13:
      channel = LL_ADC_CHANNEL_13;
      break;
    case 14:
      channel = LL_ADC_CHANNEL_14;
      break;
    case 15:
      channel = LL_ADC_CHANNEL_15;
      break;
    case 16:
      channel = LL_ADC_CHANNEL_16;
      break;
    case 17:
      channel = LL_ADC_CHANNEL_17;
      break;
    case 18:
      channel = LL_ADC_CHANNEL_18;
      break;
    default:
      channel = 0;
      break;
  }
  return channel;
}

void adcregs() {

#ifdef DEBUG
	Serial.print("addr:");
	Serial.println((uint32_t) ADC1, HEX);
	//uint32_t control asm("control");
	uint32_t control = __get_CONTROL();
	Serial.print("control:");
	Serial.println((uint32_t) control, HEX);

	uint32_t psr = __get_APSR();
	Serial.print("psr:");
	Serial.println((uint32_t) psr, HEX);

	Serial.print("SR:");
	Serial.println(ADC1->SR, HEX);
	Serial.print("CR1:");
	Serial.println(ADC1->CR1, HEX);
	Serial.print("CR2:");
	Serial.println(ADC1->CR2, HEX);
	Serial.print("SQR1:");
	Serial.println(ADC1->SQR1, HEX);
	Serial.print("SQR3:");
	Serial.println(ADC1->SQR3, HEX);
	Serial.print("SMPR2:");
	Serial.println(ADC1->SMPR2, HEX);
	Serial.print("CCR:");
	Serial.println(ADC1_COMMON->CCR, HEX);

#endif //DEBUG

}


void printtempvbat() {
	//ADC1 channel 18 is vrefint 1.23v
	//uint16_t vrefint = adc_read(ADC1, 17);
	uint16_t vrefint = analogRead(AVREF);
	Serial.print("Vref int (1.21v):");
	Serial.print(vrefint);
	Serial.println();

	//ADC1 channel 16 is temperature sensor
	//uint16_t vtemp = adc_read(ADC1, 18);
	uint16_t vtemp = analogRead(ATEMP);
	Serial.print("temp sensor:");
	Serial.print(vtemp);
	Serial.println();

	uint16_t mv = (1210 * vtemp) / vrefint;
	Serial.print("mvolt:");
	Serial.print(mv);
	Serial.println();

	// specs 5.3.22 temp sensor characteristics
	// V 25 deg ~ 0.76v
	// slope 2.5 mv/C
	uint16_t v25 = 760;
	float temp = (mv - v25) * 1.0 / 2.5 + 25.0;
	Serial.print("temp:");
	Serial.print(temp);
	Serial.println();

	/* for stm32f401 it is either temp or vbat
	//ADC1 channel 17 is VBAT / 2
	uint16_t vbatsens = adc_read(ADC1, 18);
	Serial.print("VBAT/2 sensor:");
	Serial.print(vbatsens);
	Serial.println();

	uint16_t vbat = vbatsens * 2.0 * 1230 / vrefint;
	Serial.print("VBAT (mv):");
	Serial.print(vbat);
	Serial.println();
	*/

}


void checkcmd() {
	if(Serial.available()) {
		char r = Serial.read();
		if(r==' ') {
			Serial.println("paused");
			while(!Serial.available())
				sleep(100);
			Serial.read();
		} else {
		}
	}
}

void ARTtoggle() {

	if((FLASH->ACR & FLASH_ACR_ICEN)!=FLASH_ACR_ICEN) { // art enabled
		/* enable the ART accelerator */
		/* enable prefetch buffer */
		FLASH->ACR |= FLASH_ACR_PRFTEN;
		/* Enable flash instruction cache */
		FLASH->ACR |= FLASH_ACR_ICEN;
		/* Enable flash data cache */
		FLASH->ACR |= FLASH_ACR_DCEN;
		asm("wfi"); //wait for a systick interrupt i.e. delay(1)
		Serial.println("ART enabled");
	} else {
		/* disable the ART accelerator */
		/* disable flash instruction cache */
		//FLASH->ACR &= ~FLASH_ACR_ICEN;
		/* disable flash data cache */
		//FLASH->ACR &= ~FLASH_ACR_DCEN;
		/* enable prefetch buffer */
		//FLASH->ACR |= FLASH_ACR_PRFTEN;
		//asm("wfi"); //wait for a systick interrupt, i.e. delay(1)
		//Serial.println("ART disabled");
	}
}

void sleep(uint16_t ms) {
	for(uint16_t i=0; i<ms; i++)
		asm("wfi");
}

initially I ran into a lot of issues that the ADC registers apparently don't get updated, e.g. I can't enable/disable the ADC.
I debugged it a little and found out that apparently I'd need to call __HAL_RCC_ADC1_CLK_ENABLE(); to enable the ADC clocks.
Otherwise, all the updates are void and nothing happens.

The codes compare reading 10,000 adc values on pin PA0 comparing between analogRead() vs the low layer HAL functions.
Actually, this is a poor comparison as it'd probably be better to compare between the more abstracted HAL, but i used LL codes in part to avoid possible 'overlaps' in accessing ADC handles etc.

the results looks like such. The mcu is stm32f401ccu the first set of readings after 10000 analog reads() is done using analogRead(), the second set using LL HAL functions.

Code: Select all

Vref int (1.21v):1502
temp sensor:956
mvolt:770
temp:29.00
10000 analog reads
dur (ms):509
ave:1.23
dur (ms):28
ave:1.25
Vref int (1.21v):1502
temp sensor:952
mvolt:766
temp:27.40
10000 analog reads
dur (ms):509
ave:1.23
dur (ms):28
ave:1.25
paused
I've checked in the registers that both sets are done with 15 adc clocks for sample time and the same pre-scalar settings.

The differences are rather large, for 10000 ADC reads(), the duration is 509 ms using analogRead() while the same done with LL codes is 28 ms.
It turns out that much of that difference is due to that analogRead() initializes the ADC each time analogRead() is called, it clocks the ADC,
setup all the registers, enable the adc.
Then the actual sampling is done to start conversion, poll for end of conversion and read the data register.
Next it proceed to deInit() the adc before returning the results.

The ADC turn on clocks and init() is very costly in terms of processing time as the codes often need to spin lock and wait for the adc to be ready in a particular state. e.g. check that the clocks are on, then set all the relevant registers and enable the adc, then spin lock to see that the adc is enabled before proceeding to perform conversions.

while in my LL based codes, the adc initialization is done once, clock the adc and enable it, then spinlock for adc enabled.
while for each adc read(), the LL based codes only does is select the ADC channel/pin, SWSTART to start conversion and poll for EOC (end of conversion) and read results.
This accounts for the significant differences in run times.

I'd think these changes if they are to be implemented are rather large in the core codes. i.e. the adc pretty much needs to be initialized at startup / reset.
and analogRead() pretty much does selecting ADC channel/pin, SWSTART to start conversion and poll for EOC and read results. This would significantly reduce the time lost caused by the init and deinit phase of analogRead().

I think this attempt is still worth doing despite the effort as the use cases of analogRead() are pretty large, I'd think there are quite many codes that probably calls analogRead() from interrupts e.g. timer to poll the adc periodically. one could think of 3d printers (Marlin) using analogRead() to poll thermistors say every 1s and maybe audio processing apps that does pretty much the same from interrupts. And as analogRead() is pretty much the only interface spec in the Arduino / Wiring api that does analog reads, a lot of codes reference it for various sensors etc.

A trouble is due to the rather large different capabilities of different stm32 processors, this change may be 'hard to implement'. I'd guess a focus would mainly be just providing ADC1 for analogRead(). I'm not sure if after all there may be stm32 mcus that has no adc, that'd be a surprise, but it'd take checks in such a large portfolio of skus.
Last edited by ag123 on Fri Oct 29, 2021 5:22 am, edited 2 times in total.
mrburnette
Posts: 633
Joined: Thu Dec 19, 2019 1:23 am
Answers: 7

Re: STM core: some ADC tests and feature request for improvements

Post by mrburnette »

Interesting experiment.

How does your "feature request" depart from Arduino, if at all?

STM32duino has been crafted to mimic Arduino, a decision which unfortunately introduces inefficiencies. I'm all for efficiency, but not at the expense of compatibility. Your code may be drop-in compatible, but the value in your testing is that you have documented an alternate, faster approach. There is IMO nothing wrong with departing the path as long as the decision is being made with eyes wide open and armed with knowledge of pitfalls.

Ray
ag123
Posts: 1655
Joined: Thu Dec 19, 2019 5:30 am
Answers: 24

Re: STM core: some ADC tests and feature request for improvements

Post by ag123 »

I'd think this change is after all feasible, as the main parts are to move the turning on clocks for ADC and various generic initializations to the reset/startup initializations part, before setup() runs. In addition, some additional APIs are needed to set various parameters, e.g. number of bits, sample time etc.

Arduino/Wiring evolves from the Uno and it is still valid today, hence it is a good design. But that Atmega328 aren't as elaborate as these processors such as stm32. Hence, the additional apis to set number of bits, sample time are after all relevant. This change keeps the analogRead() api unchanged, but that it would significantly speed up analogRead(), simply as the clocking and various initializations are moved to pre-start / reset codes.

This would also fix many 'woes' for those using analogRead() for rather low sample rates ADC samplings, and the use cases are *huge*.
3d printers (Marlin) uses analogRead() to sample thermistor temperatures, some uses analogRead() driven by a hardware timer to sample audio at 44 kHz. Not 'terribly fast', but yet moderately high sample rates. Then there are so many sketches build around sensors that use analogRead(), e.g. field effect sensors with stm32 driving a 3 phase BLDC motor? 2 of those analog sensors placed orthogonally makes it possible to tell the position of the motor rotor. Then maybe add IR proximity sensing for obstacle avoidance etc (e.g. those micro mouse in those competitions, line following robots etc).

It is a reason I'd think this change is 'worth doing', the benefit is large to a good portions of users in these and similar use cases.
mrburnette
Posts: 633
Joined: Thu Dec 19, 2019 1:23 am
Answers: 7

Re: STM core: some ADC tests and feature request for improvements

Post by mrburnette »

ag123 wrote: Fri Oct 29, 2021 4:42 am I'd think this change is after all feasible, as the main parts are to move the turning on clocks for ADC and various generic initializations to the reset/startup initializations part, before setup() runs. In addition, some additional APIs are needed to set various parameters, e.g. number of bits, sample time etc.
...
It is a reason I'd think this change is 'worth doing', the benefit is large to a good portions of users in these and similar use cases.
To reiterate my point; It does not matter if it is a good idea (I like it) but what matters is if the "Arduino Way" is not violated. Which is to say that with an Arduino Due (representative 32-bit device) what is the A/D methodology? I'm too lazy to build a state diagram for the A/D code for the Due, but when done, that is the template for the STM32 A/D implementation. Heck, everyone knows that Arduino can be inefficient, there is no point in my way of thinking to optimize if we go out of the Arduino paradigm. But, I am not the one you need to convince, if Frederic & team agrees, then I am sure it meets his "smell test."

But, just my opinion on things: One can simply inherit the Analog Class and roll-your-own without overthinking stuff.


Ray

Another thought, Arduino should never be considered the optimum solution, best of breed, or preferred language approach. It is akin to BASIC: you can do anything, but not necessarily the best of anything.

STM provides professional tools for serious programmers to write efficient, supportable, and all-around good code. This is not to suggest Arduino port is not a quality product, but it is in another class and intended for other useful needs.
User avatar
fpiSTM
Posts: 1738
Joined: Wed Dec 11, 2019 7:11 pm
Answers: 91
Location: Le Mans
Contact:

Re: STM core: some ADC tests and feature request for improvements

Post by fpiSTM »

Thanks @ag123
Improving analog features or extend the offer is in the bannette... while the basic arduino API is compatible :)
But as always time is the missing ingredient.
mrburnette
Posts: 633
Joined: Thu Dec 19, 2019 1:23 am
Answers: 7

Re: STM core: some ADC tests and feature request for improvements

Post by mrburnette »

fpiSTM wrote: Fri Oct 29, 2021 1:29 pm Thanks @ag123
Improving analog features or extend the offer is in the bannette... while the basic arduino API is compatible :)
But as always time is the missing ingredient.
Yea ... AG123 will be most gratified :lol:

For others, Mike has done a great job at documenting the Arduino classes.
https://mikaelpatel.github.io/Arduino-O ... duino.html
ag123
Posts: 1655
Joined: Thu Dec 19, 2019 5:30 am
Answers: 24

Re: STM core: some ADC tests and feature request for improvements

Post by ag123 »

fpiSTM wrote: Fri Oct 29, 2021 1:29 pm Thanks @ag123
Improving analog features or extend the offer is in the bannette... while the basic arduino API is compatible :)
But as always time is the missing ingredient.
Thanks fpiSTM, I'd think no hurry as we are after all busy with various things to follow up on, I'd refrain from sending a PR for this as this is codes in the stem. Doing so could 'complicate things' as our ideas for implementing this is likely somewhat different sort of.
Just that I'd think this is one area it (analogRead()) is good to look into given the rather wide use cases.
Post Reply

Return to “General discussion”