Re: [RFC PATCH] ASoC: omap-mcbsp: Add support for highter rate up to 384000

[Michael Trimarchi <michael@amarulasolutions.com>]

Hi Peter

On Fri, Aug 09, 2013 at 08:18:23AM +0300, Peter Ujfalusi wrote:
> On 08/08/2013 02:04 PM, Michael Trimarchi wrote:
> > On Thu, Aug 08, 2013 at 01:51:25PM +0300, Peter Ujfalusi wrote:
> >> On 08/07/2013 09:28 PM, Michael Trimarchi wrote:
> >>> Add support for highter rate up to 384000Khz. There are codecs in the
> >>> market that can play up to this frequency.
> >>> Test was done just with:
> >>>
> >>> #define OMAP_MCBSP_RATES	(SNDRV_PCM_RATE_8000_192000)
> >>>
> >>> playing wav file 32bit/192000Khz stereo. Some glitch has observed
> >>> but this should depend on the selected min buffer bytes that was
> >>> left untouched in aplay and in the omap-pcm driver.
> >>>
> >>> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> >>> ---
> >>>  sound/soc/omap/omap-mcbsp.c |    8 +++++++-
> >>>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
> >>> index 7483efb..1e95f6a 100644
> >>> --- a/sound/soc/omap/omap-mcbsp.c
> >>> +++ b/sound/soc/omap/omap-mcbsp.c
> >>> @@ -39,7 +39,9 @@
> >>>  #include "mcbsp.h"
> >>>  #include "omap-mcbsp.h"
> >>>  
> >>> -#define OMAP_MCBSP_RATES	(SNDRV_PCM_RATE_8000_96000)
> >>> +#define OMAP_MCBSP_RATES	(SNDRV_PCM_RATE_8000_192000 |		\
> >>
> >> I think only this change is enough for what you want to achieve.
> >>
> >>> +				 SNDRV_PCM_RATE_CONTINUOUS |		\
> >>> +				 SNDRV_PCM_RATE_KNOT)
> >>
> >> The _KNOT is for sure not needed and I would not set the CONTINUOUS either.
> > 
> > pcm5102pw is a codec that support 384000 rate so I think that PCM_RATE_KNOT
> > is requested for such rate. Correct?
> 
> Yes, looks that way. I had overlooked the 384000 KHz...
> 
> >>
> >>>  
> >>>  #define OMAP_MCBSP_SOC_SINGLE_S16_EXT(xname, xmin, xmax, \
> >>>  	xhandler_get, xhandler_put) \
> >>> @@ -574,11 +576,15 @@ static struct snd_soc_dai_driver omap_mcbsp_dai = {
> >>>  		.channels_max = 16,
> >>>  		.rates = OMAP_MCBSP_RATES,
> >>>  		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE,
> >>> +		.rate_min = 8000,
> >>> +		.rate_max = 384000,
> >>
> >> Why you want to limit the rates here?
> >>
> > 
> > I didn't find in the market somenthing up to 384000.
> 
> Sure, but it is not correct to say that McBSP supports rates up to 384000.
> McBSP have constraint on the BCLK speed up to 48MHz. So while
> 32bit/stereo/384000 is OK, 32bit/4channel/384000 is not, but
> 16bit/4channel/384000 is again good.
> When McBSP is master we have a check for the clock speed. We might need to
> have similar for McBSP slave mode, but I don't think we are going to hit the
> 48MHz limit anyways.
> 

I found some time but I can't not test this patch now ;) too far from
my office. The idea is to check with MAX_BCLK. There are two points
that should cover slave and master mode. Is it the right direction?
BTW Do you think that is better to have at soc level as a new constraint?

Michael

diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
index 7483efb..ea95ad2 100644
--- a/sound/soc/omap/omap-mcbsp.c
+++ b/sound/soc/omap/omap-mcbsp.c
@@ -39,7 +39,9 @@
 #include "mcbsp.h"
 #include "omap-mcbsp.h"
 
-#define OMAP_MCBSP_RATES	(SNDRV_PCM_RATE_8000_96000)
+#define OMAP_MCBSP_MAX_BCLK	48000000
+#define OMAP_MCBSP_RATES	(SNDRV_PCM_RATE_8000_192000 |		\
+				 SNDRV_PCM_RATE_KNOT)
 
 #define OMAP_MCBSP_SOC_SINGLE_S16_EXT(xname, xmin, xmax, \
 	xhandler_get, xhandler_put) \
@@ -225,10 +227,14 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
 	struct omap_mcbsp *mcbsp = snd_soc_dai_get_drvdata(cpu_dai);
 	struct omap_mcbsp_reg_cfg *regs = &mcbsp->cfg_regs;
 	struct snd_dmaengine_dai_dma_data *dma_data;
-	int wlen, channels, wpf;
+	int wlen, channels, wpf, bclk;
 	int pkt_size = 0;
 	unsigned int format, div, framesize, master;
 
+	bclk = snd_soc_params_to_bclk(params);
+	if (bclk < 0 || bclk > OMAP_MCBSP_MAX_BCLK)
+		return -EINVAL;
+
 	dma_data = snd_soc_dai_get_dma_data(cpu_dai, substream);
 	channels = params_channels(params);
 
@@ -326,7 +332,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
 	/* In McBSP master modes, FRAME (i.e. sample rate) is generated
 	 * by _counting_ BCLKs. Calculate frame size in BCLKs */
 	master = mcbsp->fmt & SND_SOC_DAIFMT_MASTER_MASK;
-	if (master ==	SND_SOC_DAIFMT_CBS_CFS) {
+	if (master == SND_SOC_DAIFMT_CBS_CFS) {
 		div = mcbsp->clk_div ? mcbsp->clk_div : 1;
 		framesize = (mcbsp->in_freq / div) / params_rate(params);
 
@@ -500,6 +506,9 @@ static int omap_mcbsp_dai_set_dai_sysclk(struct snd_soc_dai *cpu_dai,
 			return -EBUSY;
 	}
 
+	if (freq > OMAP_MCBSP_MAX_BCLK)
+		return -EINVAL;
+
 	mcbsp->in_freq = freq;
 	regs->srgr2 &= ~CLKSM;
 	regs->pcr0 &= ~SCLKME;
> 
> > 
> > Michael
> > 
> >>>  	},
> >>>  	.capture = {
> >>>  		.channels_min = 1,
> >>>  		.channels_max = 16,
> >>>  		.rates = OMAP_MCBSP_RATES,
> >>> +		.rate_min = 8000,
> >>> +		.rate_max = 384000,
> >>
> >> Same here, do not set these.
> >>
> >>>  		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE,
> >>>  	},
> >>>  	.ops = &mcbsp_dai_ops,
> >>>
> >>
> >>
> >> -- 
> >> Péter
> > 
> 
> 
> -- 
> Péter