| Message ID | 20200409195841.18901-6-pierre-louis.bossart@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC/SOF/clk/gpio/dt: add Hifiberry DAC+ PRO support | expand |
On Thu, Apr 09, 2020 at 02:58:30PM -0500, Pierre-Louis Bossart wrote: > The SCLK is resumed by the codec driver. In case the rate specified in > hw_params does not match the current configuration, disable, set the > new rate and restart the clock. > > There is no operation on hw_free, the codec suspend routine will > disable/deprepare the clock. > > Note that we don't change the DAI configuration when the DAC+ PRO is > detected. All changes for the codec master mode are handled in the > topology file (DAI configuration change and scheduling change) ... > + err = snd_interval_ratnum(hw_param_interval(params, > + SNDRV_PCM_HW_PARAM_RATE), > + 1, &rats_no_pll, &num, &den); > + if (err >= 0 && den) { Perhaps usual pattern, i.e. if (err < 0 || !den) return 0; (so, above seems optional configuration) params...; return 0; > + params->rate_num = num; > + params->rate_den = den; > + } > + > + return 0; ... > +static int aif1_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct sof_card_private *ctx = snd_soc_card_get_drvdata(rtd->card); > + struct device *dev = rtd->card->dev; > + int current_rate; > + int sclk_rate; > + int channels; > + int width; > + int rate; > + int ret = 0; > + > + if (ctx->is_dac_pro) { if (!...) return 0; ...and drop the redundant ret assignment above. > + rate = params_rate(params); > + channels = params_channels(params); > + width = snd_pcm_format_physical_width(params_format(params)); > + > + if (rate % 24000) > + sclk_rate = 22579200; > + else > + sclk_rate = 24576000; > + > + current_rate = clk_get_rate(ctx->sclk); > + if (current_rate != sclk_rate) { > + /* > + * The sclk clock is started and stopped by the codec > + * resume/suspend functions. If the rate isn't correct, > + * stop, set the new rate and restart the clock > + */ > + > + dev_dbg(dev, "reconfiguring SCLK to rate %d\n", > + sclk_rate); > + > + clk_disable_unprepare(ctx->sclk); > + > + ret = clk_set_rate(ctx->sclk, sclk_rate); > + if (ret) { > + dev_err(dev, "Could not set SCLK rate %d\n", > + sclk_rate); > + return ret; > + } > + > + ret = clk_prepare_enable(ctx->sclk); > + if (ret) { > + dev_err(dev, "Failed to enable SCLK: %d\n", > + ret); > + return ret; > + } > + } > + > + ret = aif1_update_rate_den(substream, params); > + if (ret) { > + dev_err(dev, "Failed to update rate denominator: %d\n", ret); > + return ret; > + } Do you still need below steps when current_rate == sclk_rate? > + ret = snd_soc_dai_set_bclk_ratio(rtd->codec_dai, > + channels * width); > + if (ret) { > + dev_err(dev, "Failed to set bclk ratio : %d\n", ret); > + return ret; > + } > + } > + > + return ret; > +}
>> + err = snd_interval_ratnum(hw_param_interval(params, >> + SNDRV_PCM_HW_PARAM_RATE), >> + 1, &rats_no_pll, &num, &den); > >> + if (err >= 0 && den) { > > Perhaps usual pattern, i.e. > > if (err < 0 || !den) > return 0; > (so, above seems optional configuration) > > params...; > return 0; ok > >> + if (ctx->is_dac_pro) { > > if (!...) > return 0; > > ...and drop the redundant ret assignment above. yes, this was suggested by Guennadi today as well. >> + ret = aif1_update_rate_den(substream, params); >> + if (ret) { >> + dev_err(dev, "Failed to update rate denominator: %d\n", ret); >> + return ret; >> + } > > Do you still need below steps when current_rate == sclk_rate? Good question. I assume the values are properly stored by the regmap cache, but if these channel and width do change (something we don't support for now) then yes we should move this out of the if case. I'll give it a try, thanks for flagging this. >> + ret = snd_soc_dai_set_bclk_ratio(rtd->codec_dai, >> + channels * width); >> + if (ret) { >> + dev_err(dev, "Failed to set bclk ratio : %d\n", ret); >> + return ret; >> + } >> + } >> + >> + return ret; >> +} >
diff --git a/sound/soc/intel/boards/sof_pcm512x.c b/sound/soc/intel/boards/sof_pcm512x.c index c1d2a53c1ec8..b5153ce954c7 100644 --- a/sound/soc/intel/boards/sof_pcm512x.c +++ b/sound/soc/intel/boards/sof_pcm512x.c @@ -145,6 +145,31 @@ static int sof_pcm512x_codec_init(struct snd_soc_pcm_runtime *rtd) return 0; } +static int aif1_update_rate_den(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct sof_card_private *ctx = snd_soc_card_get_drvdata(rtd->card); + struct snd_ratnum rats_no_pll; + unsigned int num = 0, den = 0; + int err; + + rats_no_pll.num = clk_get_rate(ctx->sclk) / 64; + rats_no_pll.den_min = 1; + rats_no_pll.den_max = 128; + rats_no_pll.den_step = 1; + + err = snd_interval_ratnum(hw_param_interval(params, + SNDRV_PCM_HW_PARAM_RATE), + 1, &rats_no_pll, &num, &den); + if (err >= 0 && den) { + params->rate_num = num; + params->rate_den = den; + } + + return 0; +} + static int aif1_startup(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; @@ -156,6 +181,74 @@ static int aif1_startup(struct snd_pcm_substream *substream) return 0; } +static int aif1_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct sof_card_private *ctx = snd_soc_card_get_drvdata(rtd->card); + struct device *dev = rtd->card->dev; + int current_rate; + int sclk_rate; + int channels; + int width; + int rate; + int ret = 0; + + if (ctx->is_dac_pro) { + rate = params_rate(params); + channels = params_channels(params); + width = snd_pcm_format_physical_width(params_format(params)); + + if (rate % 24000) + sclk_rate = 22579200; + else + sclk_rate = 24576000; + + current_rate = clk_get_rate(ctx->sclk); + if (current_rate != sclk_rate) { + /* + * The sclk clock is started and stopped by the codec + * resume/suspend functions. If the rate isn't correct, + * stop, set the new rate and restart the clock + */ + + dev_dbg(dev, "reconfiguring SCLK to rate %d\n", + sclk_rate); + + clk_disable_unprepare(ctx->sclk); + + ret = clk_set_rate(ctx->sclk, sclk_rate); + if (ret) { + dev_err(dev, "Could not set SCLK rate %d\n", + sclk_rate); + return ret; + } + + ret = clk_prepare_enable(ctx->sclk); + if (ret) { + dev_err(dev, "Failed to enable SCLK: %d\n", + ret); + return ret; + } + } + + ret = aif1_update_rate_den(substream, params); + if (ret) { + dev_err(dev, "Failed to update rate denominator: %d\n", ret); + return ret; + } + + ret = snd_soc_dai_set_bclk_ratio(rtd->codec_dai, + channels * width); + if (ret) { + dev_err(dev, "Failed to set bclk ratio : %d\n", ret); + return ret; + } + } + + return ret; +} + static void aif1_shutdown(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; @@ -167,6 +260,7 @@ static void aif1_shutdown(struct snd_pcm_substream *substream) static const struct snd_soc_ops sof_pcm512x_ops = { .startup = aif1_startup, + .hw_params = aif1_hw_params, .shutdown = aif1_shutdown, };
The SCLK is resumed by the codec driver. In case the rate specified in hw_params does not match the current configuration, disable, set the new rate and restart the clock. There is no operation on hw_free, the codec suspend routine will disable/deprepare the clock. Note that we don't change the DAI configuration when the DAC+ PRO is detected. All changes for the codec master mode are handled in the topology file (DAI configuration change and scheduling change) Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- sound/soc/intel/boards/sof_pcm512x.c | 94 ++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+)