[RFC] periph/pdm: propose API and add nrf52 implementation #16125

aabadie · 2021-03-02T13:22:27Z

Contribution description

This PR proposes an API for the PDM peripheral available at least in nRF52 CPUs. An implementation is proposed for nRF and there's a configuration for the adafruit-clue board.

I could never really convert the "sound" to a hearable signal so I'm wondering if it works correctly. The values printed seems correct though (higher when make noise, lower otherwise) but I have no ground truth.

I'm also wondering if this could fit in the I2S API proposed by @bergzand in #15131, and thus there's no need for PDM API of this PR.

Testing procedure

Run tests/periph_pdm on adafruit-clue.

Issues/PRs references

None

benpicco

What is PDM? Is it similar to PWM?
If it's intended for sound, it could also implement the DAC DDS API.

benpicco · 2021-03-02T13:41:24Z

cpu/nrf52/periph/pdm.c

+    /* Configure sampling rate */
+    switch (rate) {
+        case PDM_SAMPLE_RATE_16KHZ:
+#ifdef CPU_MODEL_NRF52840XXAA


Suggested change

#ifdef CPU_MODEL_NRF52840XXAA

#ifdef PDM_PDMCLKCTRL_FREQ_1280K

chrysn · 2021-03-02T14:02:49Z

PDM is not so much comparable to PWM but more to PCM. Think of it as PCM with a very high sampling rate but a sampling resolution of 1 bit.

As a result, it can represent much higher frequencies (up to half the bit rate) at a low dynamic range (down to 1 bit), and can represent lower frequencies with a much higher dynamic range.

There's two things where it is used:

Sony used it in its SACDs. They claim superior quality, but I haven't found anyone who could hear the difference (and their SACDs contain more data than a regular CD).
It's super cheap to get from a digital microphone, as is on the thingy:52.

The DSS API doesn't realy work with it, as it assumes a sampling resolution which is not there. (It's similar in general, though, but has different parameters. There's a sampling rate, but no sampling depth).

aabadie · 2021-03-02T15:08:54Z

It's super cheap to get from a digital microphone, as is on the thingy:52.

Good to know that there's a microphone on the thingy:52, at least someone can test this PR (and eventually find bugs)

chrysn · 2021-03-23T23:22:25Z

Like on the thingy52 board, the microbit-v2 also has such a microphone built in.

As for PDM and PCM, this API looks a bit mixed to me: The pin is addressed as a PDM pin, but the hardware peripheral eventually provides PCM data, if I understand correctly. Is this a kind of peripheral that's present on anything else except the nrf5x?

stale · 2022-03-02T13:18:47Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

aabadie · 2022-03-23T11:39:48Z

Needs rebase. @chrysn are still interested by this one ? (and do you have the hardware to test it ?)

stale · 2022-11-02T05:36:52Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

Kcebnelloh

Also adding some information about stereo mode would be good. For example letting the user know that the first sample in each buffer is always the Left Channel.

I'm currently working on fully supporting the feather-nrf52-sense board and for this I would like this api to be merged, so i could help with the completion of the code, maybe writing a stereo example and testing it etc.

Please let me know what you think about this.

Kcebnelloh · 2023-12-11T13:02:43Z

cpu/nrf52/periph/pdm.c

+                        (PDM_INTEN_STOPPED_Enabled << PDM_INTEN_STOPPED_Pos));
+
+    /* Configure internal RAM buffer size, divide by 2 for stereo mode */
+    NRF_PDM->SAMPLE.MAXCNT = (PDM_BUF_SIZE >> mode);


Suggested change

NRF_PDM->SAMPLE.MAXCNT = (PDM_BUF_SIZE >> mode);

NRF_PDM->SAMPLE.MAXCNT = (PDM_BUF_SIZE);

According to the datasheet and also logically the SAMPLE.MAXCNT shouldn't change if we run the pdm in stereo mode, because we still want to fill the double buffer _pdm_buff and not only half of it.

The only thing that changes is that in stereo mode "the buffer in RAM can contain half the stereo sampling time as compared to the mono sampling time".

Kcebnelloh · 2023-12-11T13:05:30Z

cpu/nrf52/periph/pdm.c

+    if (NRF_PDM->EVENTS_STARTED == 1) {
+        NRF_PDM->EVENTS_STARTED = 0;
+        uint8_t next_buf_pos = (_pdm_current_buf + 1) & 0x1;
+        NRF_PDM->SAMPLE.PTR = (uint32_t)&_pdm_buf[next_buf_pos * (PDM_BUF_SIZE >> 1)];


Suggested change

NRF_PDM->SAMPLE.PTR = (uint32_t)&_pdm_buf[next_buf_pos * (PDM_BUF_SIZE >> 1)];

NRF_PDM->SAMPLE.PTR = (uint32_t)&_pdm_buf[next_buf_pos * (PDM_BUF_SIZE)];

Here the buffer shouldn't be divided by 2 because the _pdm_buf is PDM_BUF_SIZE * 2. So dividing PDM_BUF_SIZE here would result in us only looping one quarter of the whole buffer.

Kcebnelloh · 2023-12-11T13:06:44Z

cpu/nrf52/periph/pdm.c

+        NRF_PDM->EVENTS_END = 0;
+
+        /* Process received samples frame */
+        isr_ctx.cb(isr_ctx.arg, &_pdm_buf[_pdm_current_buf * (PDM_BUF_SIZE >> 1)]);


Suggested change

isr_ctx.cb(isr_ctx.arg, &_pdm_buf[_pdm_current_buf * (PDM_BUF_SIZE >> 1)]);

isr_ctx.cb(isr_ctx.arg, &_pdm_buf[_pdm_current_buf * (PDM_BUF_SIZE)]);

Similarly to the change on line 148 dividing the PDM_BUF_SIZE here would only return one quarter of the double buffer _pdm_buff

aabadie added 4 commits February 18, 2021 18:45

drivers/periph: introduce PDM peripheral interface

05b5169

cpu/nrf52: add PDM peripheral driver

a6b4d35

tests/periph_pdm: add test application

f6fcadc

boards/adafruit-clue: configure PDM peripheral

4fd8bd1

aabadie added Type: new feature The issue requests / The PR implemements a new feature for RIOT Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports labels Mar 2, 2021

benpicco requested a review from chrysn March 2, 2021 13:40

benpicco reviewed Mar 2, 2021

View reviewed changes

MrKevinWeiss added this to the Release 2021.07 milestone Jun 22, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Mar 2, 2022

stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Mar 23, 2022

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Nov 2, 2022

Kcebnelloh suggested changes Dec 11, 2023

View reviewed changes

stale bot removed State: stale State: The issue / PR has no activity for >185 days labels Dec 11, 2023

baharehf mentioned this pull request Mar 26, 2024

drivers/periph/pdm: initial pdm implementation #20509

Draft

MrKevinWeiss assigned MrKevinWeiss and unassigned MrKevinWeiss Mar 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] periph/pdm: propose API and add nrf52 implementation #16125

[RFC] periph/pdm: propose API and add nrf52 implementation #16125

aabadie commented Mar 2, 2021

benpicco left a comment

benpicco Mar 2, 2021

chrysn commented Mar 2, 2021 •

edited

Loading

aabadie commented Mar 2, 2021

chrysn commented Mar 23, 2021

stale bot commented Mar 2, 2022

aabadie commented Mar 23, 2022

stale bot commented Nov 2, 2022

Kcebnelloh left a comment

Kcebnelloh Dec 11, 2023

Kcebnelloh Dec 11, 2023

Kcebnelloh Dec 11, 2023

	#ifdef CPU_MODEL_NRF52840XXAA
	#ifdef PDM_PDMCLKCTRL_FREQ_1280K

	NRF_PDM->SAMPLE.MAXCNT = (PDM_BUF_SIZE >> mode);
	NRF_PDM->SAMPLE.MAXCNT = (PDM_BUF_SIZE);

	NRF_PDM->SAMPLE.PTR = (uint32_t)&_pdm_buf[next_buf_pos * (PDM_BUF_SIZE >> 1)];
	NRF_PDM->SAMPLE.PTR = (uint32_t)&_pdm_buf[next_buf_pos * (PDM_BUF_SIZE)];

	isr_ctx.cb(isr_ctx.arg, &_pdm_buf[_pdm_current_buf * (PDM_BUF_SIZE >> 1)]);
	isr_ctx.cb(isr_ctx.arg, &_pdm_buf[_pdm_current_buf * (PDM_BUF_SIZE)]);

[RFC] periph/pdm: propose API and add nrf52 implementation #16125

Are you sure you want to change the base?

[RFC] periph/pdm: propose API and add nrf52 implementation #16125

Conversation

aabadie commented Mar 2, 2021

Contribution description

Testing procedure

Issues/PRs references

benpicco left a comment

Choose a reason for hiding this comment

benpicco Mar 2, 2021

Choose a reason for hiding this comment

chrysn commented Mar 2, 2021 • edited Loading

aabadie commented Mar 2, 2021

chrysn commented Mar 23, 2021

stale bot commented Mar 2, 2022

aabadie commented Mar 23, 2022

stale bot commented Nov 2, 2022

Kcebnelloh left a comment

Choose a reason for hiding this comment

Kcebnelloh Dec 11, 2023

Choose a reason for hiding this comment

Kcebnelloh Dec 11, 2023

Choose a reason for hiding this comment

Kcebnelloh Dec 11, 2023

Choose a reason for hiding this comment

chrysn commented Mar 2, 2021 •

edited

Loading