adafruit / ArduinoCore-samd

115 stars 118 forks source link

adafruit metro m4 / feather m4 (SAMD51) standard I2S library doesn't work #86

Open nekuneko opened 5 years ago

nekuneko commented 5 years ago

Hello, I'm trying to use an Adafruit I2S Mic SPH0645 with an Adafruit Metro M4 board, when I try to compile AmplitudeSerialPlotter.ino sketch from ArduinoSound Library I get the following errors:

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp: In member function 'void I2SClass::enableClock(int)':

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:421:13: error: 'GCLK_GENCTRL_SRC_DFLL48M_Val' was not declared in this scope

   int src = GCLK_GENCTRL_SRC_DFLL48M_Val;
             ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:426:11: error: 'GCLK_GENCTRL_SRC_OSC8M_Val' was not declared in this scope

     src = GCLK_GENCTRL_SRC_OSC8M_Val;
           ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:430:16: error: 'struct Gclk' has no member named 'STATUS'

   while (GCLK->STATUS.bit.SYNCBUSY);
                ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:431:9: error: 'struct Gclk' has no member named 'GENDIV'

   GCLK->GENDIV.bit.ID = _clockGenerator;
         ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:432:9: error: 'struct Gclk' has no member named 'GENDIV'

   GCLK->GENDIV.bit.DIV = div;
         ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:435:16: error: 'struct Gclk' has no member named 'STATUS'

   while (GCLK->STATUS.bit.SYNCBUSY);
                ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:436:17: error: request for member 'bit' in '1073748992u->Gclk::GENCTRL', which is of non-class type 'volatile GCLK_GENCTRL_Type [12]'

   GCLK->GENCTRL.bit.ID = _clockGenerator;
                 ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:437:17: error: request for member 'bit' in '1073748992u->Gclk::GENCTRL', which is of non-class type 'volatile GCLK_GENCTRL_Type [12]'

   GCLK->GENCTRL.bit.SRC = src;
                 ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:438:17: error: request for member 'bit' in '1073748992u->Gclk::GENCTRL', which is of non-class type 'volatile GCLK_GENCTRL_Type [12]'

   GCLK->GENCTRL.bit.IDC = 1;
                 ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:439:17: error: request for member 'bit' in '1073748992u->Gclk::GENCTRL', which is of non-class type 'volatile GCLK_GENCTRL_Type [12]'

   GCLK->GENCTRL.bit.GENEN = 1;
                 ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:442:16: error: 'struct Gclk' has no member named 'STATUS'

   while (GCLK->STATUS.bit.SYNCBUSY);
                ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:443:9: error: 'struct Gclk' has no member named 'CLKCTRL'

   GCLK->CLKCTRL.bit.ID = i2sd.glckId(_deviceIndex);
         ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:444:9: error: 'struct Gclk' has no member named 'CLKCTRL'

   GCLK->CLKCTRL.bit.GEN = _clockGenerator;
         ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:445:9: error: 'struct Gclk' has no member named 'CLKCTRL'

   GCLK->CLKCTRL.bit.CLKEN = 1;
         ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:447:16: error: 'struct Gclk' has no member named 'STATUS'

   while (GCLK->STATUS.bit.SYNCBUSY);
                ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp: In member function 'void I2SClass::disableClock()':

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:452:16: error: 'struct Gclk' has no member named 'STATUS'

   while (GCLK->STATUS.bit.SYNCBUSY);
                ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:453:17: error: request for member 'bit' in '1073748992u->Gclk::GENCTRL', which is of non-class type 'volatile GCLK_GENCTRL_Type [12]'

   GCLK->GENCTRL.bit.ID = _clockGenerator;
                 ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:454:17: error: request for member 'bit' in '1073748992u->Gclk::GENCTRL', which is of non-class type 'volatile GCLK_GENCTRL_Type [12]'

   GCLK->GENCTRL.bit.SRC = GCLK_GENCTRL_SRC_DFLL48M_Val;
                 ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:454:27: error: 'GCLK_GENCTRL_SRC_DFLL48M_Val' was not declared in this scope

   GCLK->GENCTRL.bit.SRC = GCLK_GENCTRL_SRC_DFLL48M_Val;
                           ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:455:17: error: request for member 'bit' in '1073748992u->Gclk::GENCTRL', which is of non-class type 'volatile GCLK_GENCTRL_Type [12]'

   GCLK->GENCTRL.bit.IDC = 1;
                 ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:456:17: error: request for member 'bit' in '1073748992u->Gclk::GENCTRL', which is of non-class type 'volatile GCLK_GENCTRL_Type [12]'

   GCLK->GENCTRL.bit.GENEN = 0;
                 ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:458:16: error: 'struct Gclk' has no member named 'STATUS'

   while (GCLK->STATUS.bit.SYNCBUSY);
                ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:459:9: error: 'struct Gclk' has no member named 'CLKCTRL'

   GCLK->CLKCTRL.bit.ID = i2sd.glckId(_deviceIndex);
         ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:460:9: error: 'struct Gclk' has no member named 'CLKCTRL'

   GCLK->CLKCTRL.bit.GEN = _clockGenerator;
         ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:461:9: error: 'struct Gclk' has no member named 'CLKCTRL'

   GCLK->CLKCTRL.bit.CLKEN = 0;
         ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:463:16: error: 'struct Gclk' has no member named 'STATUS'

   while (GCLK->STATUS.bit.SYNCBUSY);
                ^

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp: At global scope:

C:\Users\Neku\AppData\Local\Arduino15\packages\adafruit\hardware\samd\1.2.9\libraries\I2S\src\I2S.cpp:546:47: error: 'PIN_I2S_SD' was not declared in this scope

 I2SClass I2S(I2S_DEVICE, I2S_CLOCK_GENERATOR, PIN_I2S_SD, PIN_I2S_SCK, PIN_I2S_FS);

So, the errors are located in the following functions within the file I2S.cpp on packages/adafruit/hardware/samd/version/libraries/I2S/src:

void I2SClass::enableClock(int divider)
void I2SClass::disableClock()

The content of these functions is listed below:

void I2SClass::enableClock(int divider)
{
  int div = SystemCoreClock / divider;
  int src = GCLK_GENCTRL_SRC_DFLL48M_Val;                 // <--- ERROR SAMD51

  if (div > 255) {
    // divider is too big, use 8 MHz oscillator instead
    div = 8000000 / divider;
    src = GCLK_GENCTRL_SRC_OSC8M_Val;                     // <--- ERROR SAMD51
  }

  // configure the clock divider
  while (GCLK->STATUS.bit.SYNCBUSY);                      // <--- ERROR SAMD51
  GCLK->GENDIV.bit.ID = _clockGenerator;                  // <--- ERROR SAMD51
  GCLK->GENDIV.bit.DIV = div;                             // <--- ERROR SAMD51

  // use the DFLL as the source
  while (GCLK->STATUS.bit.SYNCBUSY);                      // <--- ERROR SAMD51
  GCLK->GENCTRL.bit.ID = _clockGenerator;                 // <--- ERROR SAMD51   
  GCLK->GENCTRL.bit.SRC = src;                            // <--- ERROR SAMD51
  GCLK->GENCTRL.bit.IDC = 1;                              // <--- ERROR SAMD51
  GCLK->GENCTRL.bit.GENEN = 1;                            // <--- ERROR SAMD51

  // enable
  while (GCLK->STATUS.bit.SYNCBUSY);                      // <--- ERROR SAMD51
  GCLK->CLKCTRL.bit.ID = i2sd.glckId(_deviceIndex);       // <--- ERROR SAMD51
  GCLK->CLKCTRL.bit.GEN = _clockGenerator;                // <--- ERROR SAMD51
  GCLK->CLKCTRL.bit.CLKEN = 1;                            // <--- ERROR SAMD51

  while (GCLK->STATUS.bit.SYNCBUSY);                      // <--- ERROR SAMD51
}

void I2SClass::disableClock()
{
  while (GCLK->STATUS.bit.SYNCBUSY);                      // <--- ERROR SAMD51
  GCLK->GENCTRL.bit.ID = _clockGenerator;                 // <--- ERROR SAMD51
  GCLK->GENCTRL.bit.SRC = GCLK_GENCTRL_SRC_DFLL48M_Val;   // <--- ERROR SAMD51
  GCLK->GENCTRL.bit.IDC = 1;                              // <--- ERROR SAMD51
  GCLK->GENCTRL.bit.GENEN = 0;                            // <--- ERROR SAMD51

  while (GCLK->STATUS.bit.SYNCBUSY);                      // <--- ERROR SAMD51
  GCLK->CLKCTRL.bit.ID = i2sd.glckId(_deviceIndex);       // <--- ERROR SAMD51
  GCLK->CLKCTRL.bit.GEN = _clockGenerator;                // <--- ERROR SAMD51
  GCLK->CLKCTRL.bit.CLKEN = 0;                            // <--- ERROR SAMD51

  while (GCLK->STATUS.bit.SYNCBUSY);                      // <--- ERROR SAMD51
}

The last one error can be corrected by replacing the last part of the file for this code:

#if defined(__SAMD51__)
  #if I2S_INTERFACES_COUNT > 0
  I2SClass I2S(I2S_DEVICE, I2S_CLOCK_GENERATOR, PIN_I2S_SDI, PIN_I2S_SCK, PIN_I2S_FS);
  #endif
#else 
  #if I2S_INTERFACES_COUNT > 0
  I2SClass I2S(I2S_DEVICE, I2S_CLOCK_GENERATOR, PIN_I2S_SD, PIN_I2S_SCK, PIN_I2S_FS);
  #endif
#endif

I think all this issues are generated because of the way I2S works on SAMD-E5x microcontrollers, in where I2S_SD function is splitted on I2S_SDI & I2S_SDO internal pin functions. I will try to correct this behaviour by my own.

ladyada commented 5 years ago

ok thanks! yes we haven't tried microphone i2s in on SAMD51 - just I2s output!

tgvarik commented 5 years ago

Having the same problem here, with Feather M4 Express board.

ladyada commented 5 years ago

you can try our fork of the Audio library - https://github.com/adafruit/audio but we haven't done a lot of I2S testing on the SAMD51 so there may be bugs. I2S is poorly supported in arduino in general

nekuneko commented 5 years ago

Standard I2S library will never work as is right now, both to transmit nor receive data with SAMD51 microcontroller, and finally I understand why it will doesn't work. Adafruit's ArduinoCore I2S for SAMD51 is uncomplete, and after a couple of months studying the code I know the reasons. There are so many points, so I will try to explain it as simplier as I can.

To do this study I compare the implementations of the I2S standard API and the Adafuit_ZeroI2S library. Adafruit_ZeroI2S manage directly SAMD51 I2S registers through CMSIS driver API. This driver is divided in some files (named i2s.h) located in Windows on these paths:

Constants: C:\Users\User\AppData\Local\Arduino15\packages\arduino\tools\CMSIS-Atmel\1.2.0\CMSIS\Device\ATMEL\samd51\include\instance

Structures: C:\Users\User\AppData\Local\Arduino15\packages\arduino\tools\CMSIS-Atmel\1.2.0\CMSIS\Device\ATMEL\samd51\include\component

Standard I2S library API (found on arduino core library folder) abstracts I2S CMSIS driver module into a c++ class named _I2SDeviceSAMD21 located in the file _SAMD21I2SDevice.h. This class provide some functions in order to access the microcontroller registers in a high level way. SAMD51_I2SDevice.h file tries to manage the device using the standard arduino I2S API but there are needed another functions and parameters (which accessing to other registers who has SAMD51 and hasn't SAMD21) in order to make work the clock hierarchy and serializers, this is because TX and RX serializers output are splitted into two different physical pins.

There are needed another changes into I2S.cpp code, for example:

Functions enableClock() & disableClock() I mentioned before, fails during compilation because they are designed for the SAMD21 clock structure. SAMD51 uses another different clock sources, as you can check on begin() function on Adafruit_ZeroI2S library.

Also on standard I2S begin() function need this change: I2S gpios function on SAMD21 are on PIO_COM column, meanwhile I2S gpios function on SAMD51 are on PIO_I2S column.

#if defined(__SAMD51__)
  pinPeripheral(_sckPin, PIO_I2S);
  pinPeripheral(_fsPin,  PIO_I2S);
  pinPeripheral(_sdPin, PIO_I2S);
#else // SAMD21
  pinPeripheral(_sckPin, PIO_COM);
  pinPeripheral(_fsPin,  PIO_COM);
  pinPeripheral(_sdPin, PIO_COM);
#endif

Adafruit_ZeroI2S library works with SAMD51, but it can only read (with blocking) two samples at time. This is very unefficient in order to recording audio data. I2S standard library can read a buffer of 512 bytes because it uses DMA in background with a double buffer in order to read data. This double buffer implementation is very simply, there are two buffers with a size of 512 bytes. Meanwhile DMA fills one buffer, the user reads the other buffer, which has been filled by DMA previously and so on.

Until now, I have manage to get two samples in blocking read with I2S standard library by merging Adafruit_ZeroI2S library with SAMD51_I2SDevice.h and I2S.cpp files. I think is also needed to modify the DMA utility file used by I2S standard library (which actually works for SAMD21 microcontroller) in order to make it compatible with SAMD51 microcontroller. With the modifications I performed, if I try to read a buffer of 512 bytes, I get weird data like a pulse train without sense nor correlation.

After this DMA problem is solved, we could read audio samples into a buffer with I2S standard library. BUT if you want to write them back into a WAV file it would be another problem: Adafruit SPH0645 microphone has 24-bit resolution. Both DMA and AdafruitZeroI2S library only works with 8, 16, 32 bit depth resolution. So you must to begin I2S in orther to read at 32 bits resolution and you will get your samples in format: [MSB][BYTE][LSB][0xFF] [MSB][BYTE][LSB][0xFF] (left sample & right sample, is needed to read RXDATA register twice).

For some reason CMSIS I2S driver RXCTRL register I2S_RXCTRL_EXTEND_ZERO option actually extends the read data with ONEs (0xFF), and I2S_RXCTRL_EXTEND_ONE option actually extends the read data with ZEROs (0x00). You can check this behaviuour by replacing this option on AdafruitZeroI2S begin() function.

Furthermore, data you get on SAMD51 RXDATA register is in BIG ENDIAN (because I2S standard sends first MSB), and data on WAV files are stored on LITTLE ENDIAN. So is needed to swap MSB with LSB, this take time and can ruin a recording.

ArduinoSound WavPlayer and Teensy WavPlayer example sketches can just play WAV files at 16 bits resolution and 44100 sampleRate. I have testing this by modifying samplerates and bits resolutions with an online audio converter and it didn't play anything.

None of the examples I found on https://github.com/adafruit/audio compile for Adafruit M0 Express, Adafruit Metro M4 (I mean for example: Recorder, WavFilePlayer, WavFilePlayerCircuitpythonFS) . I get several compilations errors due to missing libraries like kinetis.h library. This library is designed for the teensy 3.6 and it works 100% on it I have tested to record and play wav files with this board/library.

As @ladyada says and I could check by myself, I2S support is very poorly on arduino, very disappointing.

I share my changes and example sketches on pull request #85 https://github.com/adafruit/ArduinoCore-samd/pull/85