[alsa-devel] [PATCH] ASoC: rsnd: adg :: AUDIO-CLKOUTn can synchronizes with L/R clock.
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> AUDIO-CLKOUTn can synchronizes with L/R clock, and Salvator board needs it. Otherwise, specific frequency sound will be noisy. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- .../devicetree/bindings/sound/renesas,rsnd.txt | 2 ++ sound/soc/sh/rcar/adg.c | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.txt b/Documentation/devicetree/bindings/sound/renesas,rsnd.txt index c7b29df..065d9c8 100644 --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.txt +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.txt @@ -373,6 +373,8 @@ Optional properties: - #clock-cells : it must be 0 if your system has audio_clkout it must be 1 if your system has audio_clkout0/1/2/3 - clock-frequency : for all audio_clkout0/1/2/3 +- clkout-lr-synchronous : boolean property. it indicates that audio_clkoutn + synchronizes with lr-clock. SSI subnode properties: - interrupts : Should contain SSI interrupt for PIO transfer diff --git a/sound/soc/sh/rcar/adg.c b/sound/soc/sh/rcar/adg.c index c4c51a4..1586271 100644 --- a/sound/soc/sh/rcar/adg.c +++ b/sound/soc/sh/rcar/adg.c @@ -33,11 +33,15 @@ struct rsnd_adg { struct clk *clkout[CLKOUTMAX]; struct clk_onecell_data onecell; struct rsnd_mod mod; + u32 flags; int rbga_rate_for_441khz; /* RBGA */ int rbgb_rate_for_48khz; /* RBGB */ }; +#define LRCLK_SYNC (1 << 0) +#define adg_mode_flags(adg) (adg->flags) + #define for_each_rsnd_clk(pos, adg, i) \ for (i = 0; \ (i < CLKMAX) && \ @@ -355,6 +359,16 @@ found_clock: rsnd_adg_set_ssi_clk(ssi_mod, data); + if (adg_mode_flags(adg) & LRCLK_SYNC) { + struct rsnd_mod *adg_mod = rsnd_mod_get(adg); + u32 ckr = 0; + + if (0 == (rate % 8000)) + ckr = 0x80000000; + + rsnd_mod_bset(adg_mod, SSICKR, 0x80000000, ckr); + } + dev_dbg(dev, "ADG: %s[%d] selects 0x%x for %d\n", rsnd_mod_name(ssi_mod), rsnd_mod_id(ssi_mod), data, rate); @@ -532,6 +546,7 @@ int rsnd_adg_probe(struct rsnd_priv *priv) { struct rsnd_adg *adg; struct device *dev = rsnd_priv_to_dev(priv); + struct device_node *np = dev->of_node; adg = devm_kzalloc(dev, sizeof(*adg), GFP_KERNEL); if (!adg) { @@ -545,6 +560,9 @@ int rsnd_adg_probe(struct rsnd_priv *priv) rsnd_adg_get_clkin(priv, adg); rsnd_adg_get_clkout(priv, adg); + if (of_get_property(np, "clkout-lr-synchronous", NULL)) + adg->flags = LRCLK_SYNC; + priv->adg = adg; return 0; -- 1.9.1
On Tue, Jun 07, 2016 at 06:21:33AM +0000, Kuninori Morimoto wrote:
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
AUDIO-CLKOUTn can synchronizes with L/R clock, and Salvator board needs it. Otherwise, specific frequency sound will be noisy.
Why would a user not want these clocks to be synchronous? A lot of CODECs will at least have better performance if their master clock is synchronous to the audio clocks so it'd be a better default, is there an advantage to not doing it?
Hi Mark
AUDIO-CLKOUTn can synchronizes with L/R clock, and Salvator board needs it. Otherwise, specific frequency sound will be noisy.
Why would a user not want these clocks to be synchronous? A lot of CODECs will at least have better performance if their master clock is synchronous to the audio clocks so it'd be a better default, is there an advantage to not doing it?
I'm now confusing. We can set system clock on audio card, for example simple-card case, it is called as "system-clock-frequency". In my understanding, this "system clock" and above "master clock" are same clock. but "system clock" is fixed rate (= not related to audio clock in many cases). Because of this, some codec doesn't request synchronous between master clock <-> audio clock, but, some codec requests synchronous it. Am I wrong ??
On Wed, Jun 08, 2016 at 12:27:48AM +0000, Kuninori Morimoto wrote:
Why would a user not want these clocks to be synchronous? A lot of CODECs will at least have better performance if their master clock is synchronous to the audio clocks so it'd be a better default, is there an advantage to not doing it?
I'm now confusing. We can set system clock on audio card, for example simple-card case, it is called as "system-clock-frequency". In my understanding, this "system clock" and above "master clock" are same clock. but "system clock" is fixed rate (= not related to audio clock in many cases). Because of this, some codec doesn't request synchronous between master clock <-> audio clock, but, some codec requests synchronous it. Am I wrong ??
A lot of CODECs can to varying degrees tolerate this but it will tend to show up in performance numbers, it's often not immediately obvious just from a listening test. Usually those that don't mind have a FLL or PLL they're using to generate the actual audio master clock at a useful rate for dividing down, or sometimes fancy digital logic to match things up. It's not a problem to have this configurable, I'm just thinking it might be better to have the default be the other way around so that we default to synchronous but can turn that off if it's required.
Hi Mark Thank you for your explanation
I'm now confusing. We can set system clock on audio card, for example simple-card case, it is called as "system-clock-frequency". In my understanding, this "system clock" and above "master clock" are same clock. but "system clock" is fixed rate (= not related to audio clock in many cases). Because of this, some codec doesn't request synchronous between master clock <-> audio clock, but, some codec requests synchronous it. Am I wrong ??
A lot of CODECs can to varying degrees tolerate this but it will tend to show up in performance numbers, it's often not immediately obvious just from a listening test. Usually those that don't mind have a FLL or PLL they're using to generate the actual audio master clock at a useful rate for dividing down, or sometimes fancy digital logic to match things up.
It's not a problem to have this configurable, I'm just thinking it might be better to have the default be the other way around so that we default to synchronous but can turn that off if it's required.
OK, I see. I checked existing DT, and default synchronous <-> asynchronous exchange seems no problem. So, synchronous will be default in v2
participants (2)
-
Kuninori Morimoto -
Mark Brown