[PATCH 0/2] ASoC: fsl_asrc: improve clock selection and quality
This series fixes the automatic clock selection and enables internal ratio in order to improve audio quality.
The clock selection patches have been set aside for now, as the discussion is still ongoing regarding that matter.
Arnaud Ferraris(2): ASoC: fsl_asrc: make sure the input and output clocks are different ASoC: fsl_asrc: always use internal ratio
sound/soc/fsl/fsl_asrc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
The current clock selection algorithm might select the same clock for both input and output. This can happen when, for instance, the output sample rate is a multiple of the input rate.
This patch makes sure it always selects distinct input and output clocks.
Signed-off-by: Arnaud Ferraris arnaud.ferraris@collabora.com --- sound/soc/fsl/fsl_asrc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index 02c81d2e34ad..bfd35b9c0781 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -622,7 +622,8 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index]); /* Only match a perfect clock source with no remainder */ if (clk_rate != 0 && (clk_rate / rate[j]) <= 1024 && - (clk_rate % rate[j]) == 0) + (clk_rate % rate[j]) == 0 && + (j == 0 || i != select_clk[j-1])) break; }
Le 16/07/2020 à 16:52, Arnaud Ferraris a écrit :
The current clock selection algorithm might select the same clock for both input and output. This can happen when, for instance, the output sample rate is a multiple of the input rate.
This patch makes sure it always selects distinct input and output clocks.
Signed-off-by: Arnaud Ferraris arnaud.ferraris@collabora.com
sound/soc/fsl/fsl_asrc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index 02c81d2e34ad..bfd35b9c0781 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -622,7 +622,8 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index]); /* Only match a perfect clock source with no remainder */ if (clk_rate != 0 && (clk_rate / rate[j]) <= 1024 &&
(clk_rate % rate[j]) == 0)
(clk_rate % rate[j]) == 0 &&(j == 0 || i != select_clk[j-1])) break;
Well, it looks like I sent the wrong patch for this one, will send a v2 fixing this right now. Sorry about the noise.
Regards, Arnaud
Even though the current driver calculates the dividers to be used depending on the clocks and sample rates, enabling the internal ratio can lead to noticeable improvements in the audio quality, based on my testing.
As stated in the documentation, "When USRx=1 and IDRx=0, ASRC internal measured ratio will be used", so setting this bit even when not in "Ideal Ratio" mode still makes sense.
Signed-off-by: Arnaud Ferraris arnaud.ferraris@collabora.com --- sound/soc/fsl/fsl_asrc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index bfd35b9c0781..cc0f70c9140f 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -465,7 +465,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate) regmap_update_bits(asrc->regmap, REG_ASRCTR, ASRCTR_ATSi_MASK(index), ASRCTR_ATS(index)); regmap_update_bits(asrc->regmap, REG_ASRCTR, - ASRCTR_USRi_MASK(index), 0); + ASRCTR_USRi_MASK(index), ASRCTR_USR(index));
/* Set the input and output clock sources */ regmap_update_bits(asrc->regmap, REG_ASRCSR, @@ -507,8 +507,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate)
/* Enable Ideal Ratio mode */ regmap_update_bits(asrc->regmap, REG_ASRCTR, - ASRCTR_IDRi_MASK(index) | ASRCTR_USRi_MASK(index), - ASRCTR_IDR(index) | ASRCTR_USR(index)); + ASRCTR_IDRi_MASK(index), ASRCTR_IDR(index);
fsl_asrc_sel_proc(inrate, outrate, &pre_proc, &post_proc);
This series fixes the automatic clock selection and enables internal ratio in order to improve audio quality.
The clock selection patches have been set aside for now, as the discussion is still ongoing regarding that matter.
v1 -> v2: - compare clock indexes (and not the location in the clock table) to make sure input and output clocks are different
Arnaud Ferraris(2): ASoC: fsl_asrc: make sure the input and output clocks are different ASoC: fsl_asrc: always use internal ratio
sound/soc/fsl/fsl_asrc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
The current clock selection algorithm might select the same clock for both input and output. This can happen when, for instance, the output sample rate is a multiple of the input rate.
This patch makes sure it always selects distinct input and output clocks.
Signed-off-by: Arnaud Ferraris arnaud.ferraris@collabora.com --- sound/soc/fsl/fsl_asrc.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index 02c81d2e34ad..6d43cab6c885 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -608,8 +608,8 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, { struct fsl_asrc_pair_priv *pair_priv = pair->private; struct asrc_config *config = pair_priv->config; - int rate[2], select_clk[2]; /* Array size 2 means IN and OUT */ - int clk_rate, clk_index; + int rate[2], select_clk[2], clk_index[2]; /* Array size 2 means IN and OUT */ + int clk_rate; int i = 0, j = 0;
rate[IN] = in_rate; @@ -618,11 +618,12 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, /* Select proper clock source for internal ratio mode */ for (j = 0; j < 2; j++) { for (i = 0; i < ASRC_CLK_MAP_LEN; i++) { - clk_index = asrc_priv->clk_map[j][i]; - clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index]); + clk_index[j] = asrc_priv->clk_map[j][i]; + clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index[j]]); /* Only match a perfect clock source with no remainder */ if (clk_rate != 0 && (clk_rate / rate[j]) <= 1024 && - (clk_rate % rate[j]) == 0) + (clk_rate % rate[j]) == 0 && + (j == 0 || clk_index[j] != clk_index[j-1])) break; }
On Thu, Jul 16, 2020 at 05:13:52PM +0200, Arnaud Ferraris wrote:
The current clock selection algorithm might select the same clock for both input and output. This can happen when, for instance, the output sample rate is a multiple of the input rate.
What's the issue when selecting the same clock source for both input and output? Please explain it in the commit logs.
This patch makes sure it always selects distinct input and output clocks.
Signed-off-by: Arnaud Ferraris arnaud.ferraris@collabora.com
sound/soc/fsl/fsl_asrc.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index 02c81d2e34ad..6d43cab6c885 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -608,8 +608,8 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, { struct fsl_asrc_pair_priv *pair_priv = pair->private; struct asrc_config *config = pair_priv->config;
- int rate[2], select_clk[2]; /* Array size 2 means IN and OUT */
- int clk_rate, clk_index;
int rate[2], select_clk[2], clk_index[2]; /* Array size 2 means IN and OUT */
int clk_rate; int i = 0, j = 0;
rate[IN] = in_rate;
@@ -618,11 +618,12 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, /* Select proper clock source for internal ratio mode */ for (j = 0; j < 2; j++) { for (i = 0; i < ASRC_CLK_MAP_LEN; i++) {
clk_index = asrc_priv->clk_map[j][i];clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index]);
clk_index[j] = asrc_priv->clk_map[j][i];clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index[j]]); /* Only match a perfect clock source with no remainder */
Better to update the comments here as there's a new condition.
if (clk_rate != 0 && (clk_rate / rate[j]) <= 1024 &&
(clk_rate % rate[j]) == 0)
(clk_rate % rate[j]) == 0 &&(j == 0 || clk_index[j] != clk_index[j-1]))
clk_index[j - 1]
This patch fixes the automatic clock selection so it always selects distinct input and output clocks.
v2 -> v3: - Update code comment, fix formatting and add more detailed explanations in commit message
v1 -> v2: - compare clock indexes (and not the location in the clock table) to make sure input and output clocks are different
Arnaud Ferraris(1): ASoC: fsl_asrc: make sure the input and output clocks are different
sound/soc/fsl/fsl_asrc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
The current clock selection algorithm might select the same clock for both input and output when, for instance, the output sample rate is a multiple of the input rate.
In that case, both selectable clocks will be multiples of both the input and output sample rates, and therefore the first of these clocks will be selected for both input and output, leading to miscalculation of the dividers for either the input or output side.
Example: Input uses clock A (512kHz) and has a sample rate of 8kHz Output uses clock B (1024kHz) and has a sample rate of 16kHz
In this case, the algorithm will select clock A for both input and output: the input divider will therefore be calculated properly (512 / 8 => 64), but the output divider's value will be only half the expected value (512 / 16 => 32 instead of 1024 / 16 => 64).
This patch makes sure it always selects distinct input and output clocks.
Signed-off-by: Arnaud Ferraris arnaud.ferraris@collabora.com --- sound/soc/fsl/fsl_asrc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index 02c81d2e34ad..de10c208d3c8 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -608,8 +608,8 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, { struct fsl_asrc_pair_priv *pair_priv = pair->private; struct asrc_config *config = pair_priv->config; - int rate[2], select_clk[2]; /* Array size 2 means IN and OUT */ - int clk_rate, clk_index; + int rate[2], select_clk[2], clk_index[2]; /* Array size 2 means IN and OUT */ + int clk_rate; int i = 0, j = 0;
rate[IN] = in_rate; @@ -618,11 +618,15 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, /* Select proper clock source for internal ratio mode */ for (j = 0; j < 2; j++) { for (i = 0; i < ASRC_CLK_MAP_LEN; i++) { - clk_index = asrc_priv->clk_map[j][i]; - clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index]); - /* Only match a perfect clock source with no remainder */ + clk_index[j] = asrc_priv->clk_map[j][i]; + clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index[j]]); + /* + * Only match a perfect clock source with no remainder + * and make sure the input & output clocks are different + */ if (clk_rate != 0 && (clk_rate / rate[j]) <= 1024 && - (clk_rate % rate[j]) == 0) + (clk_rate % rate[j]) == 0 && + (j == 0 || clk_index[j] != clk_index[j - 1])) break; }
On Fri, Jul 17, 2020 at 12:38:58PM +0200, Arnaud Ferraris wrote:
The current clock selection algorithm might select the same clock for both input and output when, for instance, the output sample rate is a multiple of the input rate.
In that case, both selectable clocks will be multiples of both the input and output sample rates, and therefore the first of these clocks will be selected for both input and output, leading to miscalculation of the dividers for either the input or output side.
Example: Input uses clock A (512kHz) and has a sample rate of 8kHz Output uses clock B (1024kHz) and has a sample rate of 16kHz
In this case, the algorithm will select clock A for both input and output: the input divider will therefore be calculated properly (512 / 8 => 64), but the output divider's value will be only half the expected value (512 / 16 => 32 instead of 1024 / 16 => 64).
This patch makes sure it always selects distinct input and output clocks.
Please allow me to take some time to review the use case and the changes. And I'd love to wait for a review from Shengjiu also, as he introduced this auto-selection function and he's more familiar with internal ratio mode.
Thanks
Signed-off-by: Arnaud Ferraris arnaud.ferraris@collabora.com
sound/soc/fsl/fsl_asrc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index 02c81d2e34ad..de10c208d3c8 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -608,8 +608,8 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, { struct fsl_asrc_pair_priv *pair_priv = pair->private; struct asrc_config *config = pair_priv->config;
- int rate[2], select_clk[2]; /* Array size 2 means IN and OUT */
- int clk_rate, clk_index;
int rate[2], select_clk[2], clk_index[2]; /* Array size 2 means IN and OUT */
int clk_rate; int i = 0, j = 0;
rate[IN] = in_rate;
@@ -618,11 +618,15 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, /* Select proper clock source for internal ratio mode */ for (j = 0; j < 2; j++) { for (i = 0; i < ASRC_CLK_MAP_LEN; i++) {
clk_index = asrc_priv->clk_map[j][i];clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index]);/* Only match a perfect clock source with no remainder */
clk_index[j] = asrc_priv->clk_map[j][i];clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index[j]]);/** Only match a perfect clock source with no remainder* and make sure the input & output clocks are different*/ if (clk_rate != 0 && (clk_rate / rate[j]) <= 1024 &&
(clk_rate % rate[j]) == 0)
(clk_rate % rate[j]) == 0 &&(j == 0 || clk_index[j] != clk_index[j - 1])) break;-- 2.27.0
On Fri, Jul 17, 2020 at 6:40 PM Arnaud Ferraris arnaud.ferraris@collabora.com wrote:
The current clock selection algorithm might select the same clock for both input and output when, for instance, the output sample rate is a multiple of the input rate.
In that case, both selectable clocks will be multiples of both the input and output sample rates, and therefore the first of these clocks will be selected for both input and output, leading to miscalculation of the dividers for either the input or output side.
Example: Input uses clock A (512kHz) and has a sample rate of 8kHz Output uses clock B (1024kHz) and has a sample rate of 16kHz
In this case, the algorithm will select clock A for both input and output: the input divider will therefore be calculated properly (512 / 8 => 64), but the output divider's value will be only half the expected value (512 / 16 => 32 instead of 1024 / 16 => 64).
(input divider, output divider) = (64, 32) for the same clock source (512kHz) looks no problem. could you explain more detail why (64, 32) can't work?
This patch makes sure it always selects distinct input and output clocks.
There should be no such constraint for this IP, do you have any evidence for we should use distinct input and output clocks?
Signed-off-by: Arnaud Ferraris arnaud.ferraris@collabora.com
sound/soc/fsl/fsl_asrc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index 02c81d2e34ad..de10c208d3c8 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -608,8 +608,8 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, { struct fsl_asrc_pair_priv *pair_priv = pair->private; struct asrc_config *config = pair_priv->config;
int rate[2], select_clk[2]; /* Array size 2 means IN and OUT */int clk_rate, clk_index;
int rate[2], select_clk[2], clk_index[2]; /* Array size 2 means IN and OUT */int clk_rate; int i = 0, j = 0; rate[IN] = in_rate;@@ -618,11 +618,15 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv, /* Select proper clock source for internal ratio mode */ for (j = 0; j < 2; j++) { for (i = 0; i < ASRC_CLK_MAP_LEN; i++) {
clk_index = asrc_priv->clk_map[j][i];clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index]);/* Only match a perfect clock source with no remainder */
clk_index[j] = asrc_priv->clk_map[j][i];clk_rate = clk_get_rate(asrc_priv->asrck_clk[clk_index[j]]);/** Only match a perfect clock source with no remainder* and make sure the input & output clocks are different*/ if (clk_rate != 0 && (clk_rate / rate[j]) <= 1024 &&
(clk_rate % rate[j]) == 0)
(clk_rate % rate[j]) == 0 &&(j == 0 || clk_index[j] != clk_index[j - 1])) break; }-- 2.27.0
On Fri, Jul 17, 2020 at 12:38:56PM +0200, Arnaud Ferraris wrote:
This patch fixes the automatic clock selection so it always selects distinct input and output clocks.
Please don't send new patches in reply to old ones, it buries things and makes it hard to keep track of what the current version of a series looks like. Just send new versions as a completely new thread.
Please don't send cover letters for single patches, if there is anything that needs saying put it in the changelog of the patch or after the --- if it's administrative stuff. This reduces mail volume and ensures that any important information is recorded in the changelog rather than being lost.
Le 17/07/2020 à 13:21, Mark Brown a écrit :
On Fri, Jul 17, 2020 at 12:38:56PM +0200, Arnaud Ferraris wrote:
This patch fixes the automatic clock selection so it always selects distinct input and output clocks.
Please don't send new patches in reply to old ones, it buries things and makes it hard to keep track of what the current version of a series looks like. Just send new versions as a completely new thread.
Please don't send cover letters for single patches, if there is anything that needs saying put it in the changelog of the patch or after the --- if it's administrative stuff. This reduces mail volume and ensures that any important information is recorded in the changelog rather than being lost.
Understood, sorry about that. Should I do a "clean" re-send for this one?
Regards, Arnaud
Even though the current driver calculates the dividers to be used depending on the clocks and sample rates, enabling the internal ratio can lead to noticeable improvements in the audio quality, based on my testing.
As stated in the documentation, "When USRx=1 and IDRx=0, ASRC internal measured ratio will be used", so setting this bit even when not in "Ideal Ratio" mode still makes sense.
Signed-off-by: Arnaud Ferraris arnaud.ferraris@collabora.com --- sound/soc/fsl/fsl_asrc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index 6d43cab6c885..0b79a02d0d76 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -465,7 +465,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate) regmap_update_bits(asrc->regmap, REG_ASRCTR, ASRCTR_ATSi_MASK(index), ASRCTR_ATS(index)); regmap_update_bits(asrc->regmap, REG_ASRCTR, - ASRCTR_USRi_MASK(index), 0); + ASRCTR_USRi_MASK(index), ASRCTR_USR(index));
/* Set the input and output clock sources */ regmap_update_bits(asrc->regmap, REG_ASRCSR, @@ -507,8 +507,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate)
/* Enable Ideal Ratio mode */ regmap_update_bits(asrc->regmap, REG_ASRCTR, - ASRCTR_IDRi_MASK(index) | ASRCTR_USRi_MASK(index), - ASRCTR_IDR(index) | ASRCTR_USR(index)); + ASRCTR_IDRi_MASK(index), ASRCTR_IDR(index);
fsl_asrc_sel_proc(inrate, outrate, &pre_proc, &post_proc);
On Thu, Jul 16, 2020 at 05:13:54PM +0200, Arnaud Ferraris wrote:
Even though the current driver calculates the dividers to be used depending on the clocks and sample rates, enabling the internal ratio can lead to noticeable improvements in the audio quality, based on my testing.
As stated in the documentation, "When USRx=1 and IDRx=0, ASRC internal measured ratio will be used", so setting this bit even when not in "Ideal Ratio" mode still makes sense.
Signed-off-by: Arnaud Ferraris arnaud.ferraris@collabora.com
sound/soc/fsl/fsl_asrc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index 6d43cab6c885..0b79a02d0d76 100644 --- a/sound/soc/fsl/fsl_asrc.c +++ b/sound/soc/fsl/fsl_asrc.c @@ -465,7 +465,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate) regmap_update_bits(asrc->regmap, REG_ASRCTR, ASRCTR_ATSi_MASK(index), ASRCTR_ATS(index)); regmap_update_bits(asrc->regmap, REG_ASRCTR,
ASRCTR_USRi_MASK(index), 0);
ASRCTR_USRi_MASK(index), ASRCTR_USR(index));/* Set the input and output clock sources */ regmap_update_bits(asrc->regmap, REG_ASRCSR,
@@ -507,8 +507,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate)
/* Enable Ideal Ratio mode */
The code is against the comments now -- need to update this line.
regmap_update_bits(asrc->regmap, REG_ASRCTR,
ASRCTR_IDRi_MASK(index) | ASRCTR_USRi_MASK(index),ASRCTR_IDR(index) | ASRCTR_USR(index));
ASRCTR_IDRi_MASK(index), ASRCTR_IDR(index);
The driver falls back to ideal ratio mode if there is no matched clock source. Your change seems to apply internal ratio mode any way? Probably would break the fallback routine.
Le 17/07/2020 à 01:37, Nicolin Chen a écrit :
@@ -507,8 +507,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate)
/* Enable Ideal Ratio mode */
The code is against the comments now -- need to update this line.
It isn't, the following code still enables "Ideal Ratio" mode (see below)
regmap_update_bits(asrc->regmap, REG_ASRCTR,
ASRCTR_IDRi_MASK(index) | ASRCTR_USRi_MASK(index),ASRCTR_IDR(index) | ASRCTR_USR(index));
ASRCTR_IDRi_MASK(index), ASRCTR_IDR(index);The driver falls back to ideal ratio mode if there is no matched clock source. Your change seems to apply internal ratio mode any way? Probably would break the fallback routine.
Strictly speaking, internal ratio is only enabled when we have matched clock sources, and is used in addition to the calculated dividers (allows the ASRC to better adjust to drifting/inaccurate physical clocks). "Ideal Ratio" mode is different, and still enabled as a fallback when no clock source is matched.
Ideal ratio requires both USRi and IDRi bits to be set, and that would still be the case if there is no matched clock source.
The only difference my patch introduces is that USRi is always set (was previously cleared for "normal" mode), and therefore only IDRi needs to be set in order to enable ideal ratio mode.
Regards, Arnaud
On Fri, Jul 17, 2020 at 5:58 PM Arnaud Ferraris arnaud.ferraris@collabora.com wrote:
Le 17/07/2020 à 01:37, Nicolin Chen a écrit :
@@ -507,8 +507,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate)
/* Enable Ideal Ratio mode */The code is against the comments now -- need to update this line.
It isn't, the following code still enables "Ideal Ratio" mode (see below)
regmap_update_bits(asrc->regmap, REG_ASRCTR,
ASRCTR_IDRi_MASK(index) | ASRCTR_USRi_MASK(index),ASRCTR_IDR(index) | ASRCTR_USR(index));
ASRCTR_IDRi_MASK(index), ASRCTR_IDR(index);The driver falls back to ideal ratio mode if there is no matched clock source. Your change seems to apply internal ratio mode any way? Probably would break the fallback routine.
Strictly speaking, internal ratio is only enabled when we have matched clock sources, and is used in addition to the calculated dividers (allows the ASRC to better adjust to drifting/inaccurate physical clocks). "Ideal Ratio" mode is different, and still enabled as a fallback when no clock source is matched.
Ideal ratio requires both USRi and IDRi bits to be set, and that would still be the case if there is no matched clock source.
The only difference my patch introduces is that USRi is always set (was previously cleared for "normal" mode), and therefore only IDRi needs to be set in order to enable ideal ratio mode.
In my experience, the USRi = 0, no matter the value of IDRi, it is internal ratio mode. USRi=1, IDRi=0, it is also internal ratio mode. So original code should be ok for internal ratio mode, no need to add this change. could you please double check it?
best regards wang shengjiu
participants (4)
-
Arnaud Ferraris -
Mark Brown -
Nicolin Chen -
Shengjiu Wang