Re: [PATCH 20/25] ASoC: sun8i-codec: Protect the clock rate while streams are open

[Samuel Holland]

On 10/5/20 8:15 AM, Chen-Yu Tsai wrote:
> On Mon, Oct 5, 2020 at 8:01 PM Maxime Ripard <maxime@xxxxxxxxxx> wrote:
>>
>> On Wed, Sep 30, 2020 at 09:11:43PM -0500, Samuel Holland wrote:
>>> The codec's clock input is shared among all AIFs, and shared with other
>>> audio-related hardware in the SoC, including I2S and SPDIF controllers.
>>> To ensure sample rates selected by userspace or by codec2codec DAI links
>>> are maintained, the clock rate must be protected while it is in use.
>>>
>>> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx>
>>> ---
>>>  sound/soc/sunxi/sun8i-codec.c | 25 ++++++++++++++++++++++---
>>>  1 file changed, 22 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
>>> index 501af64d43a0..86065bee7cd3 100644
>>> --- a/sound/soc/sunxi/sun8i-codec.c
>>> +++ b/sound/soc/sunxi/sun8i-codec.c
...
>>> @@ -466,17 +471,30 @@ static int sun8i_codec_hw_params(struct snd_pcm_substream *substream,
>>>                          (lrck_div_order - 4) << SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV);
>>>
>>>       /* BCLK divider (SYSCLK/BCLK ratio) */
>>>       bclk_div = sun8i_codec_get_bclk_div(sysclk_rate, lrck_div_order, sample_rate);
>>>       regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL,
>>>                          SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK,
>>>                          bclk_div << SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV);
>>>
>>> -     if (!aif->open_streams) {
>>> +     /* SYSCLK rate */
>>> +     if (aif->open_streams) {
>>> +             ret = clk_set_rate(scodec->clk_module, sysclk_rate);
>>> +             if (ret < 0)
>>> +                     return ret;
>>> +     } else {
>>> +             ret = clk_set_rate_exclusive(scodec->clk_module, sysclk_rate);
>>
>> It's not really clear to me why we wouldn't want to always protect the
>> clock rate here?
> 
> I believe the intention is to allow a window, i.e. when no audio
> blocks are running,
> when it is possible to switch between sample rate families?

Yes, this is an advantage now. It would not be the case if sun4i-i2s did
something similar. It has the same problem that multiple separate sound
cards compete for one PLL.

> ChenYu

Cheers,
Samuel