[alsa-devel] [PATCH 0/2] Calculate BCLK using TDM slots and remove channels rule
The first patch is a bugfix. I did not have the HW see the problem myself, but reading from the code the problem is evident. This should also fix Dan Carpenter's concern about stack usage in davinci_mcasp_hw_rule_channels(), as the whole function is removed.
The second patch is just an optimization of the sample-rate rule. In effect I put in use Takashi Iwai's suggestion for fixing the stack usage issue in the channels rule.
Jyri Sarha (2): ASoC: davinci-mcasp: Calculate BCLK using TDM slots and remove channels rule ASoC: davinci-macsp: Optimize implicit BLCK sample-rate rule
sound/soc/davinci/davinci-mcasp.c | 104 ++++++++++---------------------------- 1 file changed, 27 insertions(+), 77 deletions(-)
The McASP driver currently always sends as many slots or channels to a i2s-wire as there are configured tdm_slots (see mcasp_i2s_hw_param()). Thus the BLCK rate does not depend on the amount of channels, just the configure amount of tdm-slots.
Reported-by: Misael Lopez Cruz misael.lopez@ti.com Signed-off-by: Jyri Sarha jsarha@ti.com --- sound/soc/davinci/davinci-mcasp.c | 82 ++++++--------------------------------- 1 file changed, 12 insertions(+), 70 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index bb4b78e..a01c6db 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -915,15 +915,12 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream, * the machine driver, we need to calculate the ratio. */ if (mcasp->bclk_master && mcasp->bclk_div == 0 && mcasp->sysclk_freq) { - int channels = params_channels(params); + int slots = mcasp->tdm_slots; int rate = params_rate(params); int sbits = params_width(params); int ppm, div;
- if (channels > mcasp->tdm_slots) - channels = mcasp->tdm_slots; - - div = davinci_mcasp_calc_clk_div(mcasp, rate*sbits*channels, + div = davinci_mcasp_calc_clk_div(mcasp, rate*sbits*slots, &ppm); if (ppm) dev_info(mcasp->dev, "Sample-rate is off by %d PPM\n", @@ -1024,17 +1021,14 @@ static int davinci_mcasp_hw_rule_rate(struct snd_pcm_hw_params *params, struct snd_interval *ri = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); int sbits = params_width(params); - int channels = params_channels(params); + int slots = rd->mcasp->tdm_slots; unsigned int list[ARRAY_SIZE(davinci_mcasp_dai_rates)]; int i, count = 0;
- if (channels > rd->mcasp->tdm_slots) - channels = rd->mcasp->tdm_slots; - for (i = 0; i < ARRAY_SIZE(davinci_mcasp_dai_rates); i++) { if (ri->min <= davinci_mcasp_dai_rates[i] && ri->max >= davinci_mcasp_dai_rates[i]) { - uint bclk_freq = sbits*channels* + uint bclk_freq = sbits*slots* davinci_mcasp_dai_rates[i]; int ppm;
@@ -1044,8 +1038,8 @@ static int davinci_mcasp_hw_rule_rate(struct snd_pcm_hw_params *params, } } dev_dbg(rd->mcasp->dev, - "%d frequencies (%d-%d) for %d sbits and %d channels\n", - count, ri->min, ri->max, sbits, channels); + "%d frequencies (%d-%d) for %d sbits and %d tdm slots\n", + count, ri->min, ri->max, sbits, slots);
return snd_interval_list(hw_param_interval(params, rule->var), count, list, 0); @@ -1058,17 +1052,14 @@ static int davinci_mcasp_hw_rule_format(struct snd_pcm_hw_params *params, struct snd_mask *fmt = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); struct snd_mask nfmt; int rate = params_rate(params); - int channels = params_channels(params); + int slots = rd->mcasp->tdm_slots; int i, count = 0;
snd_mask_none(&nfmt);
- if (channels > rd->mcasp->tdm_slots) - channels = rd->mcasp->tdm_slots; - for (i = 0; i < SNDRV_PCM_FORMAT_LAST; i++) { if (snd_mask_test(fmt, i)) { - uint bclk_freq = snd_pcm_format_width(i)*channels*rate; + uint bclk_freq = snd_pcm_format_width(i)*slots*rate; int ppm;
davinci_mcasp_calc_clk_div(rd->mcasp, bclk_freq, &ppm); @@ -1079,51 +1070,12 @@ static int davinci_mcasp_hw_rule_format(struct snd_pcm_hw_params *params, } } dev_dbg(rd->mcasp->dev, - "%d possible sample format for %d Hz and %d channels\n", - count, rate, channels); + "%d possible sample format for %d Hz and %d tdm slots\n", + count, rate, slots);
return snd_mask_refine(fmt, &nfmt); }
-static int davinci_mcasp_hw_rule_channels(struct snd_pcm_hw_params *params, - struct snd_pcm_hw_rule *rule) -{ - struct davinci_mcasp_ruledata *rd = rule->private; - struct snd_interval *ci = - hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); - int sbits = params_width(params); - int rate = params_rate(params); - int max_chan_per_wire = rd->mcasp->tdm_slots < ci->max ? - rd->mcasp->tdm_slots : ci->max; - unsigned int list[ci->max - ci->min + 1]; - int c1, c, count = 0; - - for (c1 = ci->min; c1 <= max_chan_per_wire; c1++) { - uint bclk_freq = c1*sbits*rate; - int ppm; - - davinci_mcasp_calc_clk_div(rd->mcasp, bclk_freq, &ppm); - if (abs(ppm) < DAVINCI_MAX_RATE_ERROR_PPM) { - /* If we can use all tdm_slots, we can put any - amount of channels to remaining wires as - long as they fit in. */ - if (c1 == rd->mcasp->tdm_slots) { - for (c = c1; c <= rd->serializers*c1 && - c <= ci->max; c++) - list[count++] = c; - } else { - list[count++] = c1; - } - } - } - dev_dbg(rd->mcasp->dev, - "%d possible channel counts (%d-%d) for %d Hz and %d sbits\n", - count, ci->min, ci->max, rate, sbits); - - return snd_interval_list(hw_param_interval(params, rule->var), - count, list, 0); -} - static int davinci_mcasp_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai) { @@ -1180,24 +1132,14 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream, SNDRV_PCM_HW_PARAM_RATE, davinci_mcasp_hw_rule_rate, ruledata, - SNDRV_PCM_HW_PARAM_FORMAT, - SNDRV_PCM_HW_PARAM_CHANNELS, -1); + SNDRV_PCM_HW_PARAM_FORMAT, -1); if (ret) return ret; ret = snd_pcm_hw_rule_add(substream->runtime, 0, SNDRV_PCM_HW_PARAM_FORMAT, davinci_mcasp_hw_rule_format, ruledata, - SNDRV_PCM_HW_PARAM_RATE, - SNDRV_PCM_HW_PARAM_CHANNELS, -1); - if (ret) - return ret; - ret = snd_pcm_hw_rule_add(substream->runtime, 0, - SNDRV_PCM_HW_PARAM_CHANNELS, - davinci_mcasp_hw_rule_channels, - ruledata, - SNDRV_PCM_HW_PARAM_RATE, - SNDRV_PCM_HW_PARAM_FORMAT, -1); + SNDRV_PCM_HW_PARAM_RATE, -1); if (ret) return ret; }
There is no need to copy the list of all supported sample-rates. Finding the supported endpoints within the current range is enough (see snd_interval_list()).
Signed-off-by: Jyri Sarha jsarha@ti.com --- sound/soc/davinci/davinci-mcasp.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index a01c6db..71127fe 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -1022,27 +1022,35 @@ static int davinci_mcasp_hw_rule_rate(struct snd_pcm_hw_params *params, hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); int sbits = params_width(params); int slots = rd->mcasp->tdm_slots; - unsigned int list[ARRAY_SIZE(davinci_mcasp_dai_rates)]; - int i, count = 0; + struct snd_interval range; + int i; + + snd_interval_any(&range); + range.empty = 1;
for (i = 0; i < ARRAY_SIZE(davinci_mcasp_dai_rates); i++) { - if (ri->min <= davinci_mcasp_dai_rates[i] && - ri->max >= davinci_mcasp_dai_rates[i]) { + if (snd_interval_test(ri, davinci_mcasp_dai_rates[i])) { uint bclk_freq = sbits*slots* davinci_mcasp_dai_rates[i]; int ppm;
davinci_mcasp_calc_clk_div(rd->mcasp, bclk_freq, &ppm); - if (abs(ppm) < DAVINCI_MAX_RATE_ERROR_PPM) - list[count++] = davinci_mcasp_dai_rates[i]; + if (abs(ppm) < DAVINCI_MAX_RATE_ERROR_PPM) { + if (range.empty) { + range.min = davinci_mcasp_dai_rates[i]; + range.empty = 0; + } + range.max = davinci_mcasp_dai_rates[i]; + } } } + dev_dbg(rd->mcasp->dev, - "%d frequencies (%d-%d) for %d sbits and %d tdm slots\n", - count, ri->min, ri->max, sbits, slots); + "Frequencies %d-%d -> %d-%d for %d sbits and %d tdm slots\n", + ri->min, ri->max, range.min, range.max, sbits, slots);
- return snd_interval_list(hw_param_interval(params, rule->var), - count, list, 0); + return snd_interval_refine(hw_param_interval(params, rule->var), + &range); }
static int davinci_mcasp_hw_rule_format(struct snd_pcm_hw_params *params,
On 04/20/2015 04:58 PM, Jyri Sarha wrote:
The first patch is a bugfix. I did not have the HW see the problem myself, but reading from the code the problem is evident. This should also fix Dan Carpenter's concern about stack usage in davinci_mcasp_hw_rule_channels(), as the whole function is removed.
The second patch is just an optimization of the sample-rate rule. In effect I put in use Takashi Iwai's suggestion for fixing the stack usage issue in the channels rule.
Both: Acked-by: Peter Ujfalusi peter.ujfalusi@ti.com
Jyri Sarha (2): ASoC: davinci-mcasp: Calculate BCLK using TDM slots and remove channels rule ASoC: davinci-macsp: Optimize implicit BLCK sample-rate rule
sound/soc/davinci/davinci-mcasp.c | 104 ++++++++++---------------------------- 1 file changed, 27 insertions(+), 77 deletions(-)
There still something fishy with multi channel multi serializer case, so please don't take these patches yet.
Best regards, Jyri
On 04/20/15 16:58, Jyri Sarha wrote:
The first patch is a bugfix. I did not have the HW see the problem myself, but reading from the code the problem is evident. This should also fix Dan Carpenter's concern about stack usage in davinci_mcasp_hw_rule_channels(), as the whole function is removed.
The second patch is just an optimization of the sample-rate rule. In effect I put in use Takashi Iwai's suggestion for fixing the stack usage issue in the channels rule.
Jyri Sarha (2): ASoC: davinci-mcasp: Calculate BCLK using TDM slots and remove channels rule ASoC: davinci-macsp: Optimize implicit BLCK sample-rate rule
sound/soc/davinci/davinci-mcasp.c | 104 ++++++++++---------------------------- 1 file changed, 27 insertions(+), 77 deletions(-)
participants (2)
-
Jyri Sarha -
Peter Ujfalusi