1. 12 Mar, 2024 1 commit
    • Luca Ceresoli's avatar
      ASoC: rockchip: i2s-tdm: Fix inaccurate sampling rates · 9e2ab4b1
      Luca Ceresoli authored
      The sample rates set by the rockchip_i2s_tdm driver in master mode are
      inaccurate up to 5% in several cases, due to the driver logic to configure
      clocks and a nasty interaction with the Common Clock Framework.
      
      To understand what happens, here is the relevant section of the clock tree
      (slightly simplified), along with the names used in the driver:
      
             vpll0 _OR_ vpll1               "mclk_root"
                clk_i2s2_8ch_tx_src         "mclk_parent"
                   clk_i2s2_8ch_tx_mux
                      clk_i2s2_8ch_tx       "mclk" or "mclk_tx"
      
      This is what happens when playing back e.g. at 192 kHz using
      audio-graph-card (when recording the same applies, only s/tx/rx/):
      
       0. at probe, rockchip_i2s_tdm_set_sysclk() stores the passed frequency in
          i2s_tdm->mclk_tx_freq (*) which is 50176000, and that is never modified
          afterwards
      
       1. when playback is started, rockchip_i2s_tdm_hw_params() is called and
          does the following two calls
      
       2. rockchip_i2s_tdm_calibrate_mclk():
      
          2a. selects mclk_root0 (vpll0) as a parent for mclk_parent
              (mclk_tx_src), which is OK because the vpll0 rate is a good for
              192000 (and sumbultiple) rates
      
          2b. sets the mclk_root frequency based on ppm calibration computations
      
          2c. sets mclk_tx_src to 49152000 (= 256 * 192000), which is also OK as
              it is a multiple of the required bit clock
      
       3. rockchip_i2s_tdm_set_mclk()
      
          3a. calls clk_set_rate() to set the rate of mclk_tx (clk_i2s2_8ch_tx)
              to the value of i2s_tdm->mclk_tx_freq (*), i.e. 50176000 which is
              not a multiple of the sampling frequency -- this is not OK
      
              3a1. clk_set_rate() reacts by reparenting clk_i2s2_8ch_tx_src to
                   vpll1 -- this is not OK because the default vpll1 rate can be
      	     divided to get 44.1 kHz and related rates, not 192 kHz
      
      The result is that the driver does a lot of ad-hoc decisions about clocks
      and ends up in using the wrong parent at an unoptimal rate.
      
      Step 0 is one part of the problem: unless the card driver calls set_sysclk
      at each stream start, whatever rate is set in mclk_tx_freq during boot will
      be taken and used until reboot. Moreover the driver does not care if its
      value is not a multiple of any audio frequency.
      
      Another part of the problem is that the whole reparenting and clock rate
      setting logic is conflicting with the CCF algorithms to achieve largely the
      same goal: selecting the best parent and setting the closest clock
      rate. And it turns out that only calling once clk_set_rate() on
      clk_i2s2_8ch_tx picks the correct vpll and sets the correct rate.
      
      The fix is based on removing the custom logic in the driver to select the
      parent and set the various clocks, and just let the Clock Framework do it
      all. As a side effect, the set_sysclk() op becomes useless because we now
      let the CCF compute the appropriate value for the sampling rate.  It also
      implies that the whole calibration logic is now dead code and so it is
      removed along with the "PCM Clock Compensation in PPM" kcontrol, which has
      always been broken anyway. The handling of the 4 optional clocks also
      becomes dead code and is removed.
      
      The actual rates have been tested playing 30 seconds of audio at various
      sampling rates before and after this change using sox:
      
          time play -r <sample_rate> -n synth 30 sine 950 gain -3
      
      The time reported in the table below is the 'real' value reported by the
      'time' command in the above command line.
      
           rate        before     after
         ---------     ------     ------
           8000 Hz     30.60s     30.63s
          11025 Hz     30.45s     30.51s
          16000 Hz     30.47s     30.50s
          22050 Hz     30.78s     30.41s
          32000 Hz     31.02s     30.43s
          44100 Hz     30.78s     30.41s
          48000 Hz     29.81s     30.45s
          88200 Hz     30.78s     30.41s
          96000 Hz     29.79s     30.42s
         176400 Hz     27.40s     30.41s
         192000 Hz     29.79s     30.42s
      
      While the tests are running the clock tree confirms that:
      
       * without the patch, vpll1 is always used and clk_i2s2_8ch_tx always
         produces 50176000 Hz, which cannot be divided for most audio rates
         except the slowest ones, generating inaccurate rates
       * with the patch:
         - for 192000 Hz vpll0 is used
         - for 176400 Hz vpll1 is used
         - clk_i2s2_8ch_tx always produces (256 * <rate>) Hz
      
      Tested on the RK3308 using the internal audio codec.
      
      Fixes: 081068fd ("ASoC: rockchip: add support for i2s-tdm controller")
      Signed-off-by: default avatarLuca Ceresoli <luca.ceresoli@bootlin.com>
      Link: https://msgid.link/r/20240305-rk3308-audio-codec-v4-1-312acdbe628f@bootlin.comSigned-off-by: default avatarMark Brown <broonie@kernel.org>
      9e2ab4b1
  2. 11 Mar, 2024 2 commits
  3. 07 Mar, 2024 6 commits
  4. 06 Mar, 2024 2 commits
    • Luca Ceresoli's avatar
      ASoC: trace: add event to snd_soc_dapm trace events · 7df3eb4c
      Luca Ceresoli authored
      Add the event value to the snd_soc_dapm_start and snd_soc_dapm_done trace
      events to make them more informative.
      
      Trace before:
      
                 aplay-229   [000]   250.140309: snd_soc_dapm_start:   card=vscn-2046
                 aplay-229   [000]   250.167531: snd_soc_dapm_done:    card=vscn-2046
                 aplay-229   [000]   251.169588: snd_soc_dapm_start:   card=vscn-2046
                 aplay-229   [000]   251.195245: snd_soc_dapm_done:    card=vscn-2046
      
      Trace after:
      
                 aplay-214   [000]   693.290612: snd_soc_dapm_start:   card=vscn-2046 event=1
                 aplay-214   [000]   693.315508: snd_soc_dapm_done:    card=vscn-2046 event=1
                 aplay-214   [000]   694.537349: snd_soc_dapm_start:   card=vscn-2046 event=2
                 aplay-214   [000]   694.563241: snd_soc_dapm_done:    card=vscn-2046 event=2
      Signed-off-by: default avatarLuca Ceresoli <luca.ceresoli@bootlin.com>
      Link: https://msgid.link/r/20240306-improve-asoc-trace-events-v1-2-edb252bbeb10@bootlin.comSigned-off-by: default avatarMark Brown <broonie@kernel.org>
      7df3eb4c
    • Luca Ceresoli's avatar
      ASoC: trace: add component to set_bias_level trace events · 6ef46a69
      Luca Ceresoli authored
      The snd_soc_bias_level_start and snd_soc_bias_level_done trace events
      currently look like:
      
                 aplay-229   [000]  1250.140778: snd_soc_bias_level_start: card=vscn-2046 val=1
                 aplay-229   [000]  1250.140784: snd_soc_bias_level_done: card=vscn-2046 val=1
                 aplay-229   [000]  1250.140786: snd_soc_bias_level_start: card=vscn-2046 val=2
                 aplay-229   [000]  1250.140788: snd_soc_bias_level_done: card=vscn-2046 val=2
          kworker/u8:1-21    [000]  1250.140871: snd_soc_bias_level_start: card=vscn-2046 val=1
          kworker/u8:0-11    [000]  1250.140951: snd_soc_bias_level_start: card=vscn-2046 val=1
          kworker/u8:0-11    [000]  1250.140956: snd_soc_bias_level_done: card=vscn-2046 val=1
          kworker/u8:0-11    [000]  1250.140959: snd_soc_bias_level_start: card=vscn-2046 val=2
          kworker/u8:0-11    [000]  1250.140961: snd_soc_bias_level_done: card=vscn-2046 val=2
          kworker/u8:1-21    [000]  1250.167219: snd_soc_bias_level_done: card=vscn-2046 val=1
          kworker/u8:1-21    [000]  1250.167222: snd_soc_bias_level_start: card=vscn-2046 val=2
          kworker/u8:1-21    [000]  1250.167232: snd_soc_bias_level_done: card=vscn-2046 val=2
          kworker/u8:0-11    [000]  1250.167440: snd_soc_bias_level_start: card=vscn-2046 val=3
          kworker/u8:0-11    [000]  1250.167444: snd_soc_bias_level_done: card=vscn-2046 val=3
          kworker/u8:1-21    [000]  1250.167497: snd_soc_bias_level_start: card=vscn-2046 val=3
          kworker/u8:1-21    [000]  1250.167506: snd_soc_bias_level_done: card=vscn-2046 val=3
      
      There are clearly multiple calls, one per component, but they cannot be
      discriminated from each other.
      
      Change the ftrace events to also print the component name, to make it clear
      which part of the code is involved. This requires changing the passed value
      from a struct snd_soc_card, where the DAPM context is not kwown, to a
      struct snd_soc_dapm_context where it is obviously known but the a card
      pointer is also available.
      
      With this change, the resulting trace becomes:
      
                 aplay-247   [000]  1436.357332: snd_soc_bias_level_start: card=vscn-2046 component=(none) val=1
                 aplay-247   [000]  1436.357338: snd_soc_bias_level_done: card=vscn-2046 component=(none) val=1
                 aplay-247   [000]  1436.357340: snd_soc_bias_level_start: card=vscn-2046 component=(none) val=2
                 aplay-247   [000]  1436.357343: snd_soc_bias_level_done: card=vscn-2046 component=(none) val=2
          kworker/u8:4-215   [000]  1436.357437: snd_soc_bias_level_start: card=vscn-2046 component=ff560000.codec val=1
          kworker/u8:5-231   [000]  1436.357518: snd_soc_bias_level_start: card=vscn-2046 component=ff320000.i2s val=1
          kworker/u8:5-231   [000]  1436.357523: snd_soc_bias_level_done: card=vscn-2046 component=ff320000.i2s val=1
          kworker/u8:5-231   [000]  1436.357526: snd_soc_bias_level_start: card=vscn-2046 component=ff320000.i2s val=2
          kworker/u8:5-231   [000]  1436.357528: snd_soc_bias_level_done: card=vscn-2046 component=ff320000.i2s val=2
          kworker/u8:4-215   [000]  1436.383217: snd_soc_bias_level_done: card=vscn-2046 component=ff560000.codec val=1
          kworker/u8:4-215   [000]  1436.383221: snd_soc_bias_level_start: card=vscn-2046 component=ff560000.codec val=2
          kworker/u8:4-215   [000]  1436.383231: snd_soc_bias_level_done: card=vscn-2046 component=ff560000.codec val=2
          kworker/u8:5-231   [000]  1436.383468: snd_soc_bias_level_start: card=vscn-2046 component=ff320000.i2s val=3
          kworker/u8:5-231   [000]  1436.383472: snd_soc_bias_level_done: card=vscn-2046 component=ff320000.i2s val=3
          kworker/u8:4-215   [000]  1436.383503: snd_soc_bias_level_start: card=vscn-2046 component=ff560000.codec val=3
          kworker/u8:4-215   [000]  1436.383513: snd_soc_bias_level_done: card=vscn-2046 component=ff560000.codec val=3
      Signed-off-by: default avatarLuca Ceresoli <luca.ceresoli@bootlin.com>
      Link: https://msgid.link/r/20240306-improve-asoc-trace-events-v1-1-edb252bbeb10@bootlin.comSigned-off-by: default avatarMark Brown <broonie@kernel.org>
      6ef46a69
  5. 05 Mar, 2024 2 commits
  6. 04 Mar, 2024 2 commits
    • Chancel Liu's avatar
      ASoC: soc-core.c: Prefer to return dai->driver->name in snd_soc_dai_name_get() · 755bb9a4
      Chancel Liu authored
      ASoC machine driver can use snd_soc_{of_}get_dlc() (A) to get DAI name
      for dlc (snd_soc_dai_link_component). In this function call
      dlc->dai_name is parsed via snd_soc_dai_name_get() (B).
      
      (A)	int snd_soc_get_dlc(...)
      	{
      		...
      (B)		dlc->dai_name = snd_soc_dai_name_get(dai);
      		...
      	}
      
      (B) has a priority to return dai->name as dlc->dai_name. In most cases
      card can probe successfully. However it has an issue that ASoC tries to
      rebind card. Here is a simplified flow for example:
      
       |	a) Card probes successfully at first
       |	b) One of the component bound to this card is removed for some
       |	   reason the component->dev is released
       |	c) That component is re-registered
       v	d) ASoC calls snd_soc_try_rebind_card()
      
      a) points dlc->dai_name to dai->name. b) releases all resource of the
      old DAI. c) creates new DAI structure. In result d) can not use
      dlc->dai_name to add new created DAI.
      
      So it's reasonable that prefer to return dai->driver->name in
      snd_soc_dai_name_get() because dai->driver is a pre-defined global
      variable. Also update snd_soc_is_matching_dai() for alignment.
      Signed-off-by: default avatarChancel Liu <chancel.liu@nxp.com>
      Link: https://msgid.link/r/20240304072128.2845432-1-chancel.liu@nxp.comSigned-off-by: default avatarMark Brown <broonie@kernel.org>
      755bb9a4
    • Richard Fitzgerald's avatar
      ASoC: cs-amp-lib: Add KUnit test for calibration helpers · 17786231
      Richard Fitzgerald authored
      Add a KUnit test for the cs-amp-lib library. This has test cases
      for cs_amp_get_efi_calibration_data() and cs_amp_write_cal_coeffs().
      
      A KUNIT_STATIC_STUB_REDIRECT() has been added to
      cs_amp_get_efi_variable() and cs_amp_write_cal_coeff() so that the
      KUnit test can redirect these to test harness functions.
      
      Much of the testing involves invoking the same function with different
      parameters, i.e. the number of amps and the amp index within the array.
      This uses parameterization rather than looping. The idea is to avoid
      looping over configurations within one test case as that has a higher
      chance of having a bug that doesn't actually test all the expected cases.
      Having the test run exactly one configuration, and then tear-down, is less
      prone to accidentally skipped configurations.
      Signed-off-by: default avatarRichard Fitzgerald <rf@opensource.cirrus.com>
      Link: https://msgid.link/r/20240304143705.26362-1-rf@opensource.cirrus.comSigned-off-by: default avatarMark Brown <broonie@kernel.org>
      17786231
  7. 27 Feb, 2024 2 commits
  8. 26 Feb, 2024 9 commits
  9. 24 Feb, 2024 1 commit
    • Mark Brown's avatar
      ALSA: cs35l56: Apply calibration from EFI · 0c4ebb28
      Mark Brown authored
      Merge series from Richard Fitzgerald <rf@opensource.cirrus.com>:
      
      Factory calibration of the speakers stores the calibration information
      into an EFI variable.
      
      This set of patches adds support for applying speaker calibration
      data from that EFI variable.
      
      The HDA patch (#5) depends on the ASoC patches #2 and #3
      0c4ebb28
  10. 23 Feb, 2024 7 commits
  11. 22 Feb, 2024 3 commits
  12. 21 Feb, 2024 3 commits