arduino-libraries / Arduino_AdvancedAnalog

Advanced Analog Library
GNU Lesser General Public License v2.1
7 stars 5 forks source link

ADC read speed #41

Closed CrowMagnumMan closed 6 months ago

CrowMagnumMan commented 1 year ago

First, great job with this library. I wouldn't necessarily call my post an issue, but since you say the ADC is running the "slowest clock", I have to ask.

I'm using your library with an Arduino Portenta H7 lite and reading two ADC channels simultaneously at 100,000 sample pairs/sec, 16-bit resolution, with what appears to be high accuracy. Here's the setup: adc.begin(AN_RESOLUTION_16, 400000, 4, 128)

That's as fast as I need for sure, but I'm intrigued with the idea of faster based on your "slowest clock" comment. So, how much faster could you make it?

Also, could you shed some light on how the parameter queue depth affects the speed/accuracy? (I found this which answers an earlier question I had about the sample per channel parameter: https://www.st.com/resource/en/application_note/an5354-getting-started-with-the-stm32h7-series-mcu-16bit-adc-stmicroelectronics.pdf).

iabdalkader commented 9 months ago

So, how much faster could you make it?

AN5354 has the max possible data rates for all different modes and all different device packages. For 16 bits the maximum data rate is about ~3.6 Msps, so the current ADC source clock is enough and a higher clock won't help. At lower resolutions, like 8 bit, it can do up to ~8Msps, but this requires a higher source clock from a PLL, and possibly less sampling cycles, also there's the question of whether we can keep up with the higher sample rate. You can try a few different variations, for example 64MHz clock (current clock) and 8-bit and 2.5 cycles is ~5.8Msps

Data rate = 1 / ((Sample Time + Resolution/2 + 0.5) * (1 / Sample Frequency * 10^6)) Msps

Also, could you shed some light on how the parameter queue depth affects the speed/accuracy

It doesn't affect speed or accuracy, it just gives you more time to process the buffers, the more buffers you use the more time you have before the ADC runs out of buffers and starts overwriting the buffer it holds.

jmdodd95682 commented 8 months ago

I am trying to push the sample rate pretty hard on the Arduino GIGA R1. I'm seeing max sample rate of about 1.8 MSamples/sec. I'm trying to understand if there is something else I need to be doing to get to the 3.6 MSamples/sec you mention above.

I'm running the ADC on a single channel (A0), 16b per sample, 256 samples per buffer with a queue depth of 32 buffers. I'm stopping after one sample. So, I'm sure I'm not hitting a buffering issue. Everything works great up to 1.8M Samples/sec. What I see is that 2MSamples/sec actually looks like 1 MSamples/sec. Its as if it is ignoring any bits in the sample-rate fieldf above bit 20.

iabdalkader commented 7 months ago

@jmdodd95682 The BGA package we're using on the GIGA R1 has a maximum data rate of about 2.5 Msamples/s at 16-bit resolution. Note that maximum data rate implies using the maximum ADC clock (80MHz) and the minimum sampling cycles (1.5 cycles), so this is the absolute maximum data rate you can get, however we're using a 64MHz ADC clock and 8.5 sampling cycles. Note there's also an additional delay when using a timer trigger, I think it's about 2.5 cycles (see datasheet ADC characteristics, trigger conversion latency) which is also not accounted for in the application note numbers.

image

I tested this by reducing the sampling time to 1.5 cycles ADC_SAMPLETIME_1CYCLE_5, and used the 80MHz from a PLL, and with a 5MHz sample rate, the max MSPS I see is still 2.5MSPS (sometimes with a slower clock ~3MHz I do see 3MSPS but it's not stable and probably out of spec).

image

For lower resolutions there's definitely room for improvement, we can either make the sampling time configurable then you can reduce it, but lower sampling time will have effects on accuracy, or if I can find a spare/usable PLL that I can use for the ADC clock source I can switch it to 80MHz (at 12-bit resolution with 80MHz I can get up to 5MSPS).

BTW how are you measuring the data rate I'm assuming with timestamps ? Can you post a sketch ?

jmdodd95682 commented 7 months ago

Thanks! This is very helpful data.

I am measuring sample rate by generating a known square-wave input with a fixed frequency of ~4.8KHz and then measuring the number of samples per period. As the sample rate increases, I see the number of samples per period of the input signal increase as expected until I hit 1.8M samp/sec. Beyond this, the number of samples/sec actually drops ( I see fewer captured samples per period of the input signal). If I drop the resolution I see a slight increase in sample rate. But even at 8b I cannot get beyond about 2.3M samp/sec.

I was suspecting that it might have something to do with the DMA engine until I saw that sample resolution impacted the results. So, now I suspect there is something additional related to the actual timer, ADC or my own code. I'm still poking around in my code and trying to understand. I could very well be something in my code that is limiting it, but analysis with a logic analyzer shows I spend most of the time waiting for the next buffer to be available.

iabdalkader commented 7 months ago

Try this patch you should see an improvement

https://github.com/arduino-libraries/Arduino_AdvancedAnalog/commit/976c3ea1a80a3dbb9be7d8aa188b45435ecf3d2f

jmdodd95682 commented 7 months ago

Thanks. I will definitely try it out.

When I was poking around in the HAL configuration and read the reference manual (3000+ pages!...very detailed) I reasoned out that the clock the ADC was using was the 64MHz clock. According to what I read in the reference manual, for 16b resolution, it should take 8.5 clocks, and the sampling time was 1.5 clocks for a total of 10 clocks. That would mean 64MHz/10 = 6.4M Samples/second. Clearly that it not correct, because as you mentioned the max sample rate should be 2.5M Samples/second. I guess I'm missing some factor or something is not mentioned in the reference manual. This was the table I was looking at in the reference manual:

image

Your patch will move the clock to 80MHz...so I should see some improvement I'm guessing. About a 25% gain.

jmdodd95682 commented 6 months ago

I think I figured out why the sample rate is only 1.8MS/sec. In the HALConfig.cpp there is a line:

sConfig.SamplingTime = ADC_SAMPLETIME_8CYCLES_5;

in the hal_adc_config() function.

In the Reference manual there are two times which are important in sample rate.

1) Conversion time (tconv) which is automatically set to 8.5 clocks for 16b resolution (this is NOT the 8.5 clocks described above as sConfig.SamplingTime). This is the amount of time to convert the analog voltage to a digital value.

2) Sample Time (tsamp) which is the settling time of voltage on the input cap prior to conversion. This what is called above sConfig.SamplingTime above.

If you take the 64Mhz input clock (which I already figured out was what was selected via the RCC register), divide by 2 (which is always done), and then divide by (8.5+8.5) clocks you get 1.882 MS/sec. Voila!

And it seems this sConfig.SamplingTime value is being programmed very conservatively according to the STM32H747xI datasheet which shows a value of 2.5clocks is acceptable up to a temperature of 125 degrees C.

image

When I program it to 2.5Clocks I can reach 2.9MS/sec at 16b resolution reliably! And this agrees with the datasheet.

iabdalkader commented 6 months ago

If you take the 64Mhz input clock (which I already figured out was what was selected via the RCC register), divide by 2 (which is always done), and then divide by (8.5+8.5) clocks you get 1.882 MS/sec. Voila!

This is not quite right. The conversion time is 8.5 for 16 bits because it's Resolution / 2 + 0.5, and it's already factored in the equation I posted many times here. The ADC clock is Not divided by two, a prescaler of 1 is used in HALConfig.cpp. There are many other factors in play, like device package, the timer trigger latency (in the table you posted, further down that would be trigger conversion latency), and the channel speed (slow, fast and direct channels). The actual datarate should be capped around what you see in AN5354.

Data rate = 1 / ((Sample Time + Resolution / 2 + 0.5) * (1 / Sample Frequency * 10^6)) Msps

Here's a real example with numbers: image

In [9]: 1 / ((2.5 + (16/2) + 0.5) * ((1/32000000) * 10**6))
Out[9]: 2.909090909090909

And it seems this sConfig.SamplingTime value is being programmed very conservatively according to the STM32H747xI datasheet which shows a value of 2.5clocks is acceptable up to a temperature of 125 degrees C.

It might be, but it's better to support a wide range of use cases/devices than a very specific one. We could still consider lowering the cycles or making it configurable. Note the frequency in that table is only 50MHz, and you've missed footnote 3:

These values are valid for UFBGA169 and one ADC. Refer to Getting started with the STM32H7 Series MCU 16-bit ADC (AN5354) for values of other packages and multiple ADCs operation.

jmdodd95682 commented 6 months ago

My only point was that I finally was able to understand why I was stuck at 1.8MS/s and not getting the 2.9MS/sec that the data sheet was showing. That made me happy. :-)

There may be reasons Sample Time was hard-coded to 8.5 clocks in the library. My minimal observation is that under "normal" circumstances Tsamp=2.5 is working reliably.

I agree that perhaps instead of hardcoding you can add a switch to change it to an "aggressive" setting of 2.5 clocks with the appropriate warnings about reliability and operating conditions.

iabdalkader commented 6 months ago

I agree that perhaps instead of hardcoding you can add a switch to change it to an "aggressive" setting of 2.5 clocks with the appropriate warnings about reliability and operating conditions.

Yeah I was thinking about that too, a new arg like a hint for sampling speed (SAMPLE_FAST, SAMPLE_SLOW etc..). Please open a new issue for tracking and I'll get around to implementing it.

I'm closing this issue now, I think we've posted enough information about this topic.