You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/05/01 21:18:28 UTC

[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6186: Stm32f746 audio

pkarashchenko commented on code in PR #6186:
URL: https://github.com/apache/incubator-nuttx/pull/6186#discussion_r862521667


##########
arch/arm/src/stm32f7/stm32_sai.c:
##########
@@ -300,6 +283,7 @@ static const struct i2s_ops_s g_i2sops =
 static struct stm32f7_sai_s g_sai1a_priv =
 {
   .dev.ops     = &g_i2sops,
+  .sampleratecb = NULL,

Review Comment:
   this is not needed. unspecified field will be zero initialized anyway



##########
boards/arm/stm32f7/stm32f746g-disco/src/stm32_wm8994.c:
##########
@@ -183,13 +271,26 @@ int stm32_wm8994_initialize(int minor)
           goto error;
         }
 
+      /* No we can embed the WM8994/I2C/I2S conglomerate into a PCM decoder
+       * instance so that we will have a PCM front end for the the WM8994
+       * driver.
+       */
+
+      pcm = pcm_decode_initialize(wm8994);
+      if (!pcm)
+        {
+          auderr("ERROR: Failed create the PCM decoder\n");
+          ret = -ENODEV;
+          goto error;
+        }
+
       /* Create a device name */
 
       snprintf(devname, 12, "pcm%d", minor);
-#if 0
+#if 1

Review Comment:
   I think `#if 1` wrap can be removed now



##########
arch/arm/src/stm32f7/stm32_sai.h:
##########
@@ -69,6 +69,9 @@ extern "C"
 #define EXTERN extern
 #endif
 
+typedef CODE uint32_t (*stm32_sai_sampleratecb_t)(FAR struct i2s_dev_s *dev,
+                                                  uint32_t rate);

Review Comment:
   Let's remove `FAR` and `CODE`. It will be wiped out for all ARM files soon



##########
arch/arm/src/stm32f7/stm32_sai.c:
##########
@@ -1151,6 +1138,13 @@ static uint32_t sai_samplerate(struct i2s_dev_s *dev, uint32_t rate)
 
   DEBUGASSERT(priv && rate > 0);
 
+  /* Call callback to change system clock (needed for STM32F746 Disco) */
+
+  if (priv->sampleratecb != NULL)
+    priv->frequency = priv->sampleratecb (dev, rate);
+  else
+    syslog (LOG_INFO, "No Sample Rate CB set!\n");

Review Comment:
   Please add `{}`



##########
drivers/audio/wm8994.c:
##########
@@ -84,12 +84,25 @@ static
                   uint16_t regaddr);
 static void     wm8994_writereg(FAR struct wm8994_dev_s *priv,
                   uint16_t regaddr, uint16_t regval);
-static void     wm8994_takesem(sem_t *sem);
+static int      wm8994_takesem(FAR sem_t *sem);

Review Comment:
   let's remove `FAR` here



##########
drivers/audio/wm8994.c:
##########
@@ -365,6 +377,60 @@ static void wm8994_writereg(FAR struct wm8994_dev_s *priv, uint16_t regaddr,
     }
 }
 
+/****************************************************************************
+ * Name: wm8994_setsamplefreq
+ *
+ * Description:
+ *  Sets the sample frequency for AIF1
+ *
+ ****************************************************************************/
+
+static void wm8994_setsamplefreq(FAR struct wm8994_dev_s *priv)
+{
+  uint16_t regval;
+
+  /* Table 106 in WM8994 manual */
+
+  switch (priv->samprate)
+    {
+    case 8000: regval = WM8994_AIF1_SR_8K;
+      break;
+    case 11025:regval = WM8994_AIF1_SR_11K;
+      break;
+    case 12000:regval = WM8994_AIF1_SR_12K;
+      break;
+    case 16000:regval = WM8994_AIF1_SR_16K;
+      break;
+    case 22050:regval = WM8994_AIF1_SR_22K;
+      break;
+    case 24000:regval = WM8994_AIF1_SR_24K;
+      break;
+    case 32000:regval = WM8994_AIF1_SR_32K;
+      break;
+    case 44100:regval = WM8994_AIF1_SR_44K;
+      break;
+    case 48000:regval = WM8994_AIF1_SR_48K;

Review Comment:
   Please move `regval` assignment to a new line



##########
drivers/audio/wm8994.c:
##########
@@ -400,61 +459,192 @@ static inline uint16_t wm8994_scalevolume(uint16_t volume, b16_t scale)
 }
 #endif
 
-/* Name: wm8994_setbitrate
+/****************************************************************************
+ * Name: wm8994_setvolume
  *
  * Description:
- *   Program the FLL to achieve the requested bitrate (fout).  Given:
- *
- *     samprate  - Samples per second
- *     nchannels - Number of channels of data
- *     bpsamp    - Bits per sample
+ *   Set the right and left volume values in the WM8994 device based on the
+ *   current volume and balance settings.
  *
- *   Then
- *     fout = samprate * nchannels * bpsamp
- *
- *   For example:
- *     samplerate = 11,025 samples/sec
- *     nchannels  = 1
- *     bpsamp     = 16     bits
+ ****************************************************************************/
+
+#ifndef CONFIG_AUDIO_EXCLUDE_VOLUME
+static void wm8994_setvolume(FAR struct wm8994_dev_s *priv, uint16_t volume,
+                             bool mute)
+{
+  uint32_t leftlevel;
+  uint32_t rightlevel;
+  uint16_t regval;
+
+  audinfo("volume=%u mute=%u\n", volume, mute);
+
+#ifndef CONFIG_AUDIO_EXCLUDE_BALANCE
+  /* Calculate the left channel volume level {0..1000} */
+
+  if (priv->balance <= (b16HALF - 1))
+    {
+      leftlevel = volume;
+    }
+  else if (priv->balance == b16ONE)
+    {
+      leftlevel = 0;
+    }
+  else
+    {
+      /* Note: b16ONE - balance goes from 0 to 0.5.
+       * Hence need to multiply volume by 2!
+       */
+
+      leftlevel = wm8994_scalevolume(2 * volume,
+                                     b16ONE - (b16_t)priv->balance);
+    }
+
+  /* Calculate the right channel volume level {0..1000} */
+
+  if (priv->balance >= (b16HALF - 1))
+    {
+      rightlevel = volume;
+    }
+  else if (priv->balance == 0)
+    {
+      rightlevel = 0;
+    }
+  else
+    {
+      /* Note: b16ONE - balance goes from 0 to 0.5.
+       * Hence need to multiply volume by 2!
+       */
+
+      rightlevel = wm8994_scalevolume(2 * volume,
+                                      (b16_t)priv->balance);
+    }
+#else
+  leftlevel  = priv->volume;
+  rightlevel = priv->volume;
+#endif
+
+  /* Set the volume */
+
+  regval = WM8994_HPOUT1_VU_ENABLED | WM8994_HPOUT1L_VOL(leftlevel);
+  if (!mute)
+    {
+      regval |= WM8994_HPOUT1L_MUTE_N_NO;
+    }
+  wm8994_writereg(priv, WM8994_LEFT_OUTPUT_VOL, regval);
+  wm8994_writereg(priv, WM8994_SPEAKER_VOL_LEFT, regval);
+
+  regval = WM8994_HPOUT1_VU_ENABLED | WM8994_HPOUT1R_VOL(rightlevel);
+  if (!mute)
+    {
+      regval |= WM8994_HPOUT1R_MUTE_N_NO;
+    }
+  wm8994_writereg(priv, WM8994_RIGHT_OUTPUT_VOL, regval);
+  wm8994_writereg(priv, WM8994_SPEAKER_VOL_RIGHT, regval);
+
+  /* Remember the volume level and mute settings */
+
+  priv->volume = volume;
+  priv->mute   = mute;
+}
+#endif /* CONFIG_AUDIO_EXCLUDE_VOLUME */
+
+/****************************************************************************
+ * Name: wm8994_setbass
  *
- *   Then
- *     fout = 11025 samples/sec * 1 * 16 bits/sample = 176.4 bits/sec
+ * Description:
+ *   Set the bass level.
  *
- *   The clocking is configured like this:
- *     MCLK   is the FLL source clock
- *     Fref   is the scaled down version of MCLK
- *     Fvco   is the output frequency from the FLL
- *     Fout   is the final output from the FLL that drives the SYSCLK
- *     SYSCLK can be divided down to generate the BCLK
+ *   The level and range are in whole percentage levels (0-100).
  *
- *   The FLL output frequency is generated at that fout by:
+ ****************************************************************************/
+
+#ifndef CONFIG_AUDIO_EXCLUDE_TONE
+static void wm8994_setbass(FAR struct wm8994_dev_s *priv, uint8_t bass)
+{
+  audinfo("bass=%u\n", bass);
+#warning Missing logic
+}
+#endif /* CONFIG_AUDIO_EXCLUDE_TONE */
+
+/****************************************************************************
+ * Name: wm8994_settreble
  *
- *     Fout = (Fvco / FLL_OUTDIV)
+ * Description:
+ *   Set the treble level .
  *
- *   The FLL operating frequency is set according to:
+ *   The level and range are in whole percentage levels (0-100).
  *
- *     Fvco = Fref * N.K * FLL_RATIO
+ ****************************************************************************/
+
+#ifndef CONFIG_AUDIO_EXCLUDE_TONE
+static void wm8994_settreble(FAR struct wm8994_dev_s *priv, uint8_t treble)
+{
+  audinfo("treble=%u\n", treble);
+#warning Missing logic
+}
+#endif /* CONFIG_AUDIO_EXCLUDE_TONE */
+
+/****************************************************************************
+ * Name: wm8994_setdatawidth
  *
- *   Where Fref is the input frequency frequency as determined by
- *   FLL_CLK_REF_DIV. Fvco must be in the range of 90-100MHz.
+ * Description:
+ *   Set the 8- or 16-bit data modes
  *
- *   As an example:
- *     FLL_CLK_REF_DIV = 16
- *     FLL_OUTDIV = 8
- *     N.K = 187.25
- *     FLL_RATIO=16
- *     Fref =32,768
+ ****************************************************************************/
+
+static void wm8994_setdatawidth(FAR struct wm8994_dev_s *priv)
+{
+  /* TODO */
+
+  return;
+}
+
+/* Name: wm8994_setbitrate
  *
- *     Fvco = 32,768 * 187.25 / 16 = 383,488 Hz
- *     Fout = 383,488 / 8 = 47,936 Hz (approx. 48Khz)
+ * Description:
+ *   Enter callback function to let the board set
+ *   the I2S Frequency appropriately.
  *
+ * TODO: Currently the FLL is not used as in the current application
+ * the WM8994 will operate in Slave mode. Code snippet
+ * may be helpful to generalize this code to multiple
+ * outputs and other I2S frame formats.
  */
 
 static void wm8994_setbitrate(FAR struct wm8994_dev_s *priv)
 {
+  uint32_t fout;
+  unsigned int framelen;
+
   DEBUGASSERT(priv && priv->lower);
 
-  /* TODO */
+  /* First calculate the desired bitrate (fout).  This is based on
+   *
+   * 1. The I2S frame length (in bits)
+   * 2. The number of frames per second = nchannels * samplerate
+   *
+   */
+
+  framelen = (priv->bpsamp == 8) ? WM8994_FRAMELEN8 : WM8994_FRAMELEN16;
+  fout = (uint32_t)priv->samprate * (uint32_t)priv->nchannels * framelen;
+
+  audinfo("sample rate=%u nchannels=%u bpsamp=%u framelen=%d fout=%lu\n",
+          priv->samprate, priv->nchannels, priv->bpsamp, framelen,
+          (unsigned long)fout);
+
+  /* The WM8994 does have an internal FLL
+   * However, for the application here, the STM32 I2S PLL is used
+   * Only way to achieve right clock rate is by setting the
+   * SAI clock accordingly (for STM32F746G Discovery)
+   *
+   * TODO: Generalize.
+   */
+
+  wm8994_setsamplefreq (priv);
+
+  priv->i2s->ops->i2s_rxsamplerate(priv->i2s, priv->samprate);

Review Comment:
   ```suggestion
     I2S_RXSAMPLERATE(priv->i2s, priv->samprate);
   ```



##########
arch/arm/src/stm32f7/stm32_sai.c:
##########
@@ -146,41 +147,19 @@
 #  define SAI_RXDMA16_CONFIG   (DMA_SCR_PFCTRL | DMA_SCR_DIR_P2M|DMA_SCR_MINC | \
                                   DMA_SCR_PSIZE_16BITS | DMA_SCR_MSIZE_16BITS | \
                                   DMA_SCR_PBURST_INCR4 | DMA_SCR_MBURST_INCR4)
-
 #  define SAI_RXDMA32_CONFIG   (DMA_SCR_PFCTRL | DMA_SCR_DIR_P2M|DMA_SCR_MINC | \
                                   DMA_SCR_PSIZE_32BITS | DMA_SCR_MSIZE_32BITS | \
                                   DMA_SCR_PBURST_INCR4 | DMA_SCR_MBURST_INCR4)
-#  define SAI_TXDMA8_CONFIG   (DMA_SCR_PFCTRL | DMA_SCR_DIR_M2P | DMA_SCR_MINC | \
+
+#  define SAI_TXDMA8_CONFIG   (DMA_SCR_DIR_M2P | DMA_SCR_MINC | \
                                   DMA_SCR_PSIZE_32BITS | DMA_SCR_MSIZE_32BITS | \
                                   DMA_SCR_PBURST_INCR4 | DMA_SCR_MBURST_INCR4)
-#  define SAI_TXDMA16_CONFIG   (DMA_SCR_PFCTRL | DMA_SCR_DIR_M2P | DMA_SCR_MINC | \
-                                  DMA_SCR_PSIZE_32BITS | DMA_SCR_MSIZE_32BITS | \
+#  define SAI_TXDMA16_CONFIG   (DMA_SCR_DIR_M2P | DMA_SCR_MINC |	\

Review Comment:
   ```suggestion
   #  define SAI_TXDMA16_CONFIG   (DMA_SCR_DIR_M2P | DMA_SCR_MINC | \
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org