PmodAD2 log performance (ADC sampling performance)

[cathalmccabe]

Adding this as a note. Unless someone else can take this on, I will try to modify this. I think this is a good example of where we can be more efficient with the MicroBlaze code.

The current maximum sampling rate of the ADC is limited by the MicroBlaze code.

adc_config should be outside while loop.

Reading all 4 channels limits the sampling rate when only a single channel is required.

Logging the floating point value of the voltage reduces performance further. return (float)(adc_read_raw() * vref / 4096.0);

Perhaps we could be smarter, and log raw values, and when the reset is signaled, we could do the floating point conversion on whatever we sampled.

For now, I suggest we retain the existing functionality (as it works) but add another case statement where we try to implement a more optimized read and log.

This is the code:

case READ_AND_LOG_VOLTAGE: // set the delay in ms between samples delay = MAILBOX_DATA(0); // set the channel index, from 0 to 2 channel = MAILBOX_DATA(1); // initialize logging variables, reset cmd cb_init(&pmod_log, LOG_BASE_ADDRESS, LOG_CAPACITY, LOG_ITEM_SIZE); while(MAILBOX_CMD_ADDR != RESET_ADC){ adc_config(useChan3,useChan2,useChan1,useChan0, useVref,useFILT,useBIT,useSample); for(j=0; j<num_channels; j++) { adc_voltage = adc_read_voltage(VREF); if (j == channel){ // push samples only for specified channel cb_push_back_float(&pmod_log, &adc_voltage); } } delay_ms(delay); }

[schelleg]

@yunqu - can you remove redundant adc_configs if not needed and close this issue.

For float/raw optimization - this is low priority IMHO, we are sampling IIC/SPI devices and I don't want to spend extra time optimizing low speed device sampling unless compelling example.

[yunqu]

Hi Cathal,

I added adc_config inside the while loop intentionally. Otherwise the ADC each time read a different channel.

For example, If you do: adc.read() adc.read()

If the first read reads the first channel, the second adc.read() will read the data from the second channel.

If you look at the original code (the one before I changed), you will also find adc_config() inside the while loop.

I enabled 3 channels, otherwise in each case statement, we have to first enable the channels we want to use; even if you do that, we will also need adc_config() inside the while loop. But you are right, maybe for single reads, we want to do this (enabling channels first and reading them).

For logging, I still think logging floating point values is better. Well, that depends on who (microblaze or the PS) can do the work faster...

[cathalmccabe]

Graham, looks to me like we can only sample 50 Hz max, which is fairly limiting, especially as the spec of the adc is much higher. I think this would justify giving it higher priority? Depends what else we need to do.

Rock, my idea is that we log raw values, and once the adc reset command is received from Python, the microblaze code (not Python) would go back and convert the raw to float in the buffer before acknowledging to Python. There would be an overhead after sampling, before the log is ready, but should mean the sampling rate could be increased.

[yunqu]

Cathal, 50Hz is just an example to show a better waveform. Based on Nyquist theorem, it is required to have a sampling rate double the signal frequency in order to recover the signal.

So suppose we have 0.1ms sampling delay (10KHz), you can recover signal frequency of up to 5KHz.

My calculation in notebook is pessimistic because I want to show a better waveform...

[cathalmccabe]

Ok, I tried 100 Hz and it didn't work so I assumed I had hit a limit. I must have been doing something wrong. If we can get to 5KHz it is much more reasonable. I will close this now, and reopen if there is still an issue.

[cathalmccabe]

Checked again today. Sampling rate is in ms and must be an int, so 1 ms is minimum and ~50Hz is the limit as I thought yesterday.

I know we want consistence across modules, but I don't think it makes sense to capture a log for the adc in the same way as the ALS/temp sensor (typically changing at a much slower rate).

Rather than a specifying the delay between samples for the ADC, I think we should be specifying this as the sampling rate (and hopefully approaching >KHz rates). This would also allow us to specify a much slower rate if necessary.

Also there seems to be a bug with the log capture.

Doing this: adc.start_log(0, sampling_rate_ms) sleep(3) log = adc.get_log()

Fills the log buffer. Sleeping for longer than 3 seconds gives a log with less than 999 items.

[schelleg]

@cathalmccabe - can you look into increasing the sampling performance? I think it is a good idea and I'm not so worried about consistency here.

[yunqu]

Hi Graham and Cathal,

It is not necessary to change all the codes.

It is okay to change delay_ms to delay_us and you will be able to sample higher frequency....

[schelleg]

Whatever happens, sample rate may be a key differentiation AND exemplar for why you want independent microcontrollers.

On Thu, May 12, 2016 at 10:12 AM, Yun Rock Qu notifications@github.com wrote:

Hi Graham and Cathal,

It is not necessary to change all the codes.

It is okay to change delay_ms to delay_us and you will be able to sample higher frequency....

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/Xilinx/Pynq/issues/74#issuecomment-218807074

Graham

[yunqu]

Give me some time and I am cleaning the PMOD ADC code...

[yunqu]

Just changed the delay_ms to delay_us. I was able to sample a waveform of 200Hz. However, I could not get any better.

It seems the delay_us only contributes to a small portion of the total delay between samples. Right now with delay_us(1), the smallest configurable delay between samples, we still get a sample every ~1.5ms. This means we probably need a major change in our codes to improve the performance...

[schelleg]

We presumably are limited by the IIC channel as well correct?

On Thu, May 12, 2016 at 11:54 AM, Yun Rock Qu notifications@github.com wrote:

Just changed the delay_ms to delay_us. I was able to sample a waveform of 200Hz. However, I could not get any better.

It seems the delay_us only contributes to a small portion of the total delay between samples. Right now with delay_us(1), the smallest configurable delay between samples, we still get a sample every ~1.5ms. This means we probably need a major change in our codes to improve the performance...

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/Xilinx/Pynq/issues/74#issuecomment-218835014

Graham

[yunqu]

Yes, I checked their data sheet. It does not say anything about the maximum frequency that can be sampled.

http://store.digilentinc.com/pmodad2-4-channel-12-bit-a-d-converter/

Even if we log raw values, it would not be much better. The actual delay between samples is ~1ms instead of ~1.5ms. @cathalmccabe

[plysaght]

Pls make available the highest sustainable speed of operation supported by Pmod AD2, not a limitation imposed by our s/w

Best .. Patrick

From: Yun Rock Qu [mailto:notifications@github.com] Sent: Thursday, May 12, 2016 11:06 AM To: Xilinx/Pynq Pynq@noreply.github.com Subject: Re: [Xilinx/Pynq] PmodAD2 log performance (ADC sampling performance) (#74)

Yes, I checked their data sheet. It does not say anything about the maximum frequency that can be sampled.

Even if we log raw values, it would not be much better. The actual delay between samples is ~1ms instead of ~1.5ms. @cathalmccabehttps://github.com/cathalmccabe

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHubhttps://github.com/Xilinx/Pynq/issues/74#issuecomment-218838259

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[cathalmccabe]

@yunqu are you changing the c code? The delay functions are limiting the minimum possible delay (100x for the us delay) Let me know where you get to and I can try look into it tomorrow.

void delay_us(int usdelay){ // us delay XTmrCtr_SetResetValue(&TimerInst_0, 1, usdelay*100); // Start the timer0 for usdelay us delay XTmrCtr_Start(&TimerInst_0, 1); // Wait for usdelay us to lapse while(!XTmrCtr_IsExpired(&TimerInst_0,1)); // Stop the timer0 XTmrCtr_Stop(&TimerInst_0, 1); }

void delay_ms(u32 msdelay){ // ms delay XTmrCtr_SetResetValue(&TimerInst_0, 1, msdelay_100_1000); // Start the timer0 for usdelay us delay XTmrCtr_Start(&TimerInst_0, 1); // Wait for usdelay us to lapse while(!XTmrCtr_IsExpired(&TimerInst_0,1)); // Stop the timer0 XTmrCtr_Stop(&TimerInst_0, 1); }

[yunqu]

Hi Cathal,

I don't think delay_us() is the major issue here. I tried delay_us(10) and delay_us(1) in my codes. They don't show significant difference. The time between two samples is still ~1ms.

I think our code is limited by other factors. For example, it might be the circular buffer is slow, the memory access in microblaze is slow, etc. But I am not sure how we can improve that...

[cathalmccabe]

@yunqu Hi Rock, sorry, I didn't get to look at this today. Did you remove the call to delay_us completely? I'm not sure what the overhead is to call the timer, even with Zero delay. i.e. what is the minimum delay calling the timer will cause. Memory should be BRAM, so fast. We can check buffer code.

[schelleg]

@parimalp has agreed to own PMOD ADC performance issue here. Let's give him time to work the problem, while the rest of us address other issues.

[parimalp]

I have instrumented the code. The MB code takes 70 us between a call is made to read an analog channel and sample is received via I2C. It means rest of the delays are between MB and Python. In order to reduce the 70 us sampling rate, we can increase I2C speed which is set to 100 KHz for now. The valid range is from 100 KHz to 1 MHz. Potential problem with higher speed could be that some I2C devices may break communication. I think doing at 400 KHz is safe.

I recommend using 400 KHz for the PYNQ board based development.

Regards,

Parimal

From: schelleg [mailto:notifications@github.com] Sent: Friday, May 13, 2016 1:42 PM To: Xilinx/Pynq Pynq@noreply.github.com Cc: Parimal Patel parimalp@xilinx.com; Mention mention@noreply.github.com Subject: Re: [Xilinx/Pynq] PmodAD2 log performance (ADC sampling performance) (#74)

@parimalphttps://github.com/parimalp has agreed to own PMOD ADC performance issue here. Let's give him work the problem, while the rest of us address other issues.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHubhttps://github.com/Xilinx/Pynq/issues/74#issuecomment-219153983

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[schelleg]

For highest sample rates, the Microblaze should be reading and logging samples independent of the A9s/Python.

Questions: What sample rate can be achieve with higher clockrate?
What is work involved to make that clockrate configurable upto 1 MHz?

[cathalmccabe]

@parimalp to clarify, this is 70us when doing something like this in the MicroBlaze?

while(1){
   read_adc()
}

I estimated ~280us was fastest I could get which included microblaze logging (i.e. no python but writing to buffer)

[yunqu]

https://en.wikipedia.org/wiki/Audio_frequency

100us corresponds to 10k samples per second, fast enough to recover a waveform of 5kHz (~ highest note on a piano). For human voice, I think ~2kHz is high enough (~ 200us delay between samples).

[plysaght]

Human voice in telecomms is usually band limited at 4kHz and sampled at 8kHz. This is not considered high quality but it is acceptable.

For music, ~20 kHz is considered high quality so you would sample at ~40 ksps. The most common standard sampling rate for digital audio (the one used for CDs) is 44.1 kHz, giving us a Nyquist frequency (defined as half the sampling rate) of 22.05 kHz. 48kHz is used for DV, DVD-Video.

The harmonics of the piano key notes are important to capture the timbre of the sound. The different combinations of harmonics is what allows us to distinguish a 5 kHz signal played on different instruments.

[yunqu]

Just tested the new PMOD ADC. The sampling rate is around 300us, indicating a maximum input frequency of 1.5kHz. This is consistent with @cathalmccabe 's observation.

Not sure if we still want to put more time on this. But if we do, we need to modify the circular buffer to be faster.

[schelleg]

We are still running I2C at 100 KHz correct?

If so, then to send 7b address + receive 2B over IIC takes at a minimum 28 IIC cycles ( 23 addr/data + ~5 IIC protocol overhead) - equating to 280 us. I imagine this is vast majority of overhead correct?

[yunqu]

Yes, that is correct.

[schelleg]

Any more comments on this? As of now, we have a very low speed ADC. I'll close by EOD as 'won't fix' label unless others chime in.

[yunqu]

@schelleg , I went through all the comments; @parimalp wanted to use 400kHz for pynq-z1. Should we do this now?

If we use 400kHz, 28 IIC cycles will be 70us (the frequency of the input can be 3kHz, not very high, but much better).

The challenge is that we may have to check all the other devices to regress.

[schelleg]

yes - we want MSPS I suppose. I think we'll need different device to get MSPS.