diff mbox series

[v2,2/2] ASoC: Intel: Add period size constraint on strago board

Message ID 1596096815-32043-3-git-send-email-brent.lu@intel.com (mailing list archive)
State New, archived
Headers show
Series Add period size constraint for Atom Chromebook | expand

Commit Message

Brent Lu July 30, 2020, 8:13 a.m. UTC 
From: Yu-Hsuan Hsu <yuhsuan@chromium.org>

The CRAS server does not set the period size in hw_param so ALSA will
calculate a value for period size which is based on the buffer size
and other parameters. The value may not always be aligned with Atom's
dsp design so a constraint is added to make sure the board always has
a good value.

Cyan uses chtmax98090 and others(banon, celes, edgar, kefka...) use
rt5650.

Signed-off-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Signed-off-by: Brent Lu <brent.lu@intel.com>
---
 sound/soc/intel/boards/cht_bsw_max98090_ti.c | 14 +++++++++++++-
 sound/soc/intel/boards/cht_bsw_rt5645.c      | 14 +++++++++++++-
 2 files changed, 26 insertions(+), 2 deletions(-)

Comments

Andy Shevchenko July 30, 2020, 8:42 a.m. UTC | #1 
On Thu, Jul 30, 2020 at 04:13:35PM +0800, Brent Lu wrote:
> From: Yu-Hsuan Hsu <yuhsuan@chromium.org>
> 
> The CRAS server does not set the period size in hw_param so ALSA will
> calculate a value for period size which is based on the buffer size
> and other parameters. The value may not always be aligned with Atom's
> dsp design so a constraint is added to make sure the board always has
> a good value.
> 
> Cyan uses chtmax98090 and others(banon, celes, edgar, kefka...) use
> rt5650.

Actually one more comment here.
Can you split per machine driver?

>  sound/soc/intel/boards/cht_bsw_max98090_ti.c | 14 +++++++++++++-
>  sound/soc/intel/boards/cht_bsw_rt5645.c      | 14 +++++++++++++-
Brent Lu July 30, 2020, 1:23 p.m. UTC | #2 
> On Thu, Jul 30, 2020 at 04:13:35PM +0800, Brent Lu wrote:
> > From: Yu-Hsuan Hsu <yuhsuan@chromium.org>
> >
> > The CRAS server does not set the period size in hw_param so ALSA will
> > calculate a value for period size which is based on the buffer size
> > and other parameters. The value may not always be aligned with Atom's
> > dsp design so a constraint is added to make sure the board always has
> > a good value.
> >
> > Cyan uses chtmax98090 and others(banon, celes, edgar, kefka...) use
> > rt5650.
> 
> Actually one more comment here.
> Can you split per machine driver?
> 

It adds constraints on BSW Chromebooks for same purpose. I don't see the
benefit to split it.

Regards,
Brent
> >  sound/soc/intel/boards/cht_bsw_max98090_ti.c | 14 +++++++++++++-
> >  sound/soc/intel/boards/cht_bsw_rt5645.c      | 14 +++++++++++++-
> 
> --
> With Best Regards,
> Andy Shevchenko
>
Andy Shevchenko July 30, 2020, 1:49 p.m. UTC | #3 
On Thu, Jul 30, 2020 at 01:23:57PM +0000, Lu, Brent wrote:
> > On Thu, Jul 30, 2020 at 04:13:35PM +0800, Brent Lu wrote:
> > > From: Yu-Hsuan Hsu <yuhsuan@chromium.org>
> > >
> > > The CRAS server does not set the period size in hw_param so ALSA will
> > > calculate a value for period size which is based on the buffer size
> > > and other parameters. The value may not always be aligned with Atom's
> > > dsp design so a constraint is added to make sure the board always has
> > > a good value.
> > >
> > > Cyan uses chtmax98090 and others(banon, celes, edgar, kefka...) use
> > > rt5650.
> > 
> > Actually one more comment here.
> > Can you split per machine driver?
> > 
> 
> It adds constraints on BSW Chromebooks for same purpose. I don't see the
> benefit to split it.

I didn't get this.

Purpose of splitting this to two is to keep track on per driver basis what has
had happen there.

But it's minor and up to maintainers, of course.
diff mbox series

Patch

diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
index 835e9bd..bf67254 100644
--- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c
+++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
@@ -283,8 +283,20 @@  static int cht_codec_fixup(struct snd_soc_pcm_runtime *rtd,
 
 static int cht_aif1_startup(struct snd_pcm_substream *substream)
 {
-	return snd_pcm_hw_constraint_single(substream->runtime,
+	int err;
+
+	/* Set period size to 240 to align with Atom design */
+	err = snd_pcm_hw_constraint_minmax(substream->runtime,
+			SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 240, 240);
+	if (err < 0)
+		return err;
+
+	err = snd_pcm_hw_constraint_single(substream->runtime,
 			SNDRV_PCM_HW_PARAM_RATE, 48000);
+	if (err < 0)
+		return err;
+
+	return 0;
 }
 
 static int cht_max98090_headset_init(struct snd_soc_component *component)
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index b53c024..6e62f0d 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -414,8 +414,20 @@  static int cht_codec_fixup(struct snd_soc_pcm_runtime *rtd,
 
 static int cht_aif1_startup(struct snd_pcm_substream *substream)
 {
-	return snd_pcm_hw_constraint_single(substream->runtime,
+	int err;
+
+	/* Set period size to 240 to align with Atom design */
+	err = snd_pcm_hw_constraint_minmax(substream->runtime,
+			SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 240, 240);
+	if (err < 0)
+		return err;
+
+	err = snd_pcm_hw_constraint_single(substream->runtime,
 			SNDRV_PCM_HW_PARAM_RATE, 48000);
+	if (err < 0)
+		return err;
+
+	return 0;
 }
 
 static const struct snd_soc_ops cht_aif1_ops = {