On Fri, Oct 25, 2019 at 03:13:22PM +0800, Shengjiu Wang wrote:
> The output divider should align with the output sample
> rate, if use ideal sample rate, there will be a lot of overload,
> which would cause underrun.
>
> The maximum divider of asrc clock is 1024, but there is no
> judgement for this limitaion in driver, which may cause the divider
typo: "limitaion" => "limitation"
> setting not correct.
>
> For non-ideal ratio mode, the clock rate should divide the sample
> rate with no remainder, and the quotient should be less than 1024.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@xxxxxxx>
And some comments inline. Please add my ack once they are fixed:
Acked-by: Nicolin Chen <nicoleotsuka@xxxxxxxxx>
Thanks
> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 0bf91a6f54b9..89cf333154c7 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -259,8 +259,11 @@ static int fsl_asrc_set_ideal_ratio(struct fsl_asrc_pair *pair,
> * It configures those ASRC registers according to a configuration instance
> * of struct asrc_config which includes in/output sample rate, width, channel
> * and clock settings.
> + *
> + * Note:
> + * use_ideal_rate = true is need by some case which need higher performance.
I feel we can have a detailed one here and drop those inline comments, e.g.:
+ * Note:
+ * The ideal ratio configuration can work with a flexible clock rate setting.
+ * Using IDEAL_RATIO_RATE gives a faster converting speed but overloads ASRC.
+ * For a regular audio playback, the clock rate should not be slower than an
+ * clock rate aligning with the output sample rate; For a use case requiring
+ * faster conversion, set use_ideal_rate to have the faster speed.
> @@ -351,8 +355,10 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair)
> /* We only have output clock for ideal ratio mode */
> clk = asrc_priv->asrck_clk[clk_index[ideal ? OUT : IN]];
>
> - div[IN] = clk_get_rate(clk) / inrate;
> - if (div[IN] == 0) {
> + clk_rate = clk_get_rate(clk);
> + rem[IN] = do_div(clk_rate, inrate);
> + div[IN] = (u32)clk_rate;
> + if (div[IN] == 0 || (!ideal && (div[IN] > 1024 || rem[IN] != 0))) {
Should have some comments to explain this like:
/*
* The divider range is [1, 1024], defined by the hardware. For non-
* ideal ratio configuration, clock rate has to be strictly aligned
* with the sample rate. For ideal ratio configuration, clock rates
* only result in different converting speeds. So remainder does not
* matter, as long as we keep the divider within its valid range.
*/
> pair_err("failed to support input sample rate %dHz by asrck_%x\n",
> inrate, clk_index[ideal ? OUT : IN]);
> return -EINVAL;
And move the min() behind this if-condition with no more comments:
+ div[IN] = min_t(u32, 1024, div[IN]);
> @@ -360,18 +366,29 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair)
>
> clk = asrc_priv->asrck_clk[clk_index[OUT]];
>
> - /* Use fixed output rate for Ideal Ratio mode (INCLK_NONE) */
> - if (ideal)
> - div[OUT] = clk_get_rate(clk) / IDEAL_RATIO_RATE;
> + /*
> + * Output rate should be align with the out samplerate. If set too
> + * high output rate, there will be lots of Overload.
> + * But some case need higher performance, then we can use
> + * IDEAL_RATIO_RATE specifically for such case.
> + */
Can drop this since we have the detailed comments at the top.
> + clk_rate = clk_get_rate(clk);
> + if (ideal && use_ideal_rate)
> + rem[OUT] = do_div(clk_rate, IDEAL_RATIO_RATE);
> else
> - div[OUT] = clk_get_rate(clk) / outrate;
> + rem[OUT] = do_div(clk_rate, outrate);
> + div[OUT] = clk_rate;
>
> - if (div[OUT] == 0) {
And add before this if-condition:
/* Output divider has the same limitation as the input one */
> + if (div[OUT] == 0 || (!ideal && (div[OUT] > 1024 || rem[OUT] != 0))) {
> pair_err("failed to support output sample rate %dHz by asrck_%x\n",
> outrate, clk_index[OUT]);
> return -EINVAL;
> }
>
> + /* Divider range is [1, 1024] */
Can drop this too.
> + div[IN] = min_t(u32, 1024, div[IN]);
> + div[OUT] = min_t(u32, 1024, div[OUT]);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
