[PATCH v2 0/3] Calculate BCLK using TDM slots and remove channels rule

[PATCH v2 0/3] Calculate BCLK using TDM slots and remove channels rule

[Jyri Sarha]

The first and the last patch are excatly the same as in the first
series, but in the middle thee is a new patch to constraint out
unsupported channel counts in multiple serializers case.

- The first patch is a bug fix and also fixes Dan Carpenter's concern
  about stack usage in davinci_mcasp_hw_rule_channels()

- The second is also a bug fix, even thou the bug has existed in the
  dirver for a very long time. It is just that nobody has used
  multiple serializers before.

- The last patch is a simple optimization

All the patches have been tested are internally reviewed.

Jyri Sarha (3):
  ASoC: davinci-mcasp: Calculate BCLK using TDM slots and remove
    channels rule
  ASoC: davinci-mcasp: Channel count constraints for multi-serializer
    case
  ASoC: davinci-macsp: Optimize implicit BLCK sample-rate rule

 sound/soc/davinci/davinci-mcasp.c | 167 ++++++++++++++++++++------------------
 1 file changed, 90 insertions(+), 77 deletions(-)

-- 
1.9.1

[PATCH v2 1/3] ASoC: davinci-mcasp: Calculate BCLK using TDM slots and remove channels rule

[Jyri Sarha]

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;
 	}
-- 
1.9.1

[PATCH v2 2/3] ASoC: davinci-mcasp: Channel count constraints for multi-serializer case

[Jyri Sarha]

Set channel count constraints for multiple serializers case. On McASP
the active channels mask is the same for all the serializers. With the
current implementation this means that if more than one serializers is
used, all TDM slots have to be active on all serializers. The patch
sets the channel count constraints according to number of RX and TX
serializers.

Reported-by: Misael Lopez Cruz <misael.lopez@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 63 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index a01c6db..f841707 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -107,6 +107,7 @@ struct davinci_mcasp {
 #endif
 
 	struct davinci_mcasp_ruledata ruledata[2];
+	struct snd_pcm_hw_constraint_list chconstr[2];
 };
 
 static inline void mcasp_set_bits(struct davinci_mcasp *mcasp, u32 offset,
@@ -1119,6 +1120,11 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream,
 				     SNDRV_PCM_HW_PARAM_CHANNELS,
 				     2, max_channels);
 
+	if (mcasp->chconstr[substream->stream].count)
+		snd_pcm_hw_constraint_list(substream->runtime,
+					   0, SNDRV_PCM_HW_PARAM_CHANNELS,
+					   &mcasp->chconstr[substream->stream]);
+
 	/*
 	 * If we rely on implicit BCLK divider setting we should
 	 * set constraints based on what we can provide.
@@ -1498,6 +1504,59 @@ nodata:
 	return  pdata;
 }
 
+/* All serializers must have equal number of channels */
+static int davinci_mcasp_ch_constraint(struct davinci_mcasp *mcasp,
+				       struct snd_pcm_hw_constraint_list *cl,
+				       int serializers)
+{
+	unsigned int *list;
+	int i, count = 0;
+
+	if (serializers <= 1)
+		return 0;
+
+	list = devm_kzalloc(mcasp->dev, sizeof(unsigned int) *
+			    (mcasp->tdm_slots + serializers - 2),
+			    GFP_KERNEL);
+	if (!list)
+		return -ENOMEM;
+
+	for (i = 2; i <= mcasp->tdm_slots; i++)
+		list[count++] = i;
+
+	for (i = 2; i <= serializers; i++)
+		list[count++] = i*mcasp->tdm_slots;
+
+	cl->count = count;
+	cl->list = list;
+
+	return 0;
+}
+
+
+static int davinci_mcasp_init_ch_constraints(struct davinci_mcasp *mcasp)
+{
+	int rx_serializers = 0, tx_serializers = 0, ret, i;
+
+	for (i = 0; i < mcasp->num_serializer; i++)
+		if (mcasp->serial_dir[i] == TX_MODE)
+			tx_serializers++;
+		else if (mcasp->serial_dir[i] == RX_MODE)
+			rx_serializers++;
+
+	ret = davinci_mcasp_ch_constraint(mcasp, &mcasp->chconstr[
+						  SNDRV_PCM_STREAM_PLAYBACK],
+					  tx_serializers);
+	if (ret)
+		return ret;
+
+	ret = davinci_mcasp_ch_constraint(mcasp, &mcasp->chconstr[
+						  SNDRV_PCM_STREAM_CAPTURE],
+					  rx_serializers);
+
+	return ret;
+}
+
 static int davinci_mcasp_probe(struct platform_device *pdev)
 {
 	struct snd_dmaengine_dai_dma_data *dma_data;
@@ -1681,6 +1740,10 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 		mcasp->fifo_base = DAVINCI_MCASP_V3_AFIFO_BASE;
 	}
 
+	ret = davinci_mcasp_init_ch_constraints(mcasp);
+	if (ret)
+		goto err;
+
 	dev_set_drvdata(&pdev->dev, mcasp);
 
 	mcasp_reparent_fck(pdev);
-- 
1.9.1

[PATCH v2 3/3] ASoC: davinci-macsp: Optimize implicit BLCK sample-rate rule

[Jyri Sarha]

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 f841707..56da8ce 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -1023,27 +1023,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,
-- 
1.9.1

Re: [PATCH v2 0/3] Calculate BCLK using TDM slots and remove channels rule

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 275 bytes --]

On Thu, Apr 23, 2015 at 04:16:03PM +0300, Jyri Sarha wrote:
> The first and the last patch are excatly the same as in the first
> series, but in the middle thee is a new patch to constraint out
> unsupported channel counts in multiple serializers case.

Applied all, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]