[PATCH v2 0/2] Add period size constraint for Atom Chromebook

[PATCH v2 0/2] Add period size constraint for Atom Chromebook

[Brent Lu]

Two different constraints are implemented: one is in platform's CPU
DAI to enforce period sizes which are already used in Android BSP. The
other is in Atom Chromebook's machine driver to use 240 as period size.

Changes since v1:
-Add comma at the end of media_period_size array declaration.

Brent Lu (1):
  ASoC: intel: atom: Add period size constraint

Yu-Hsuan Hsu (1):
  ASoC: Intel: Add period size constraint on strago board

 sound/soc/intel/atom/sst-mfld-platform-pcm.c | 15 +++++++++++++++
 sound/soc/intel/boards/cht_bsw_max98090_ti.c | 14 +++++++++++++-
 sound/soc/intel/boards/cht_bsw_rt5645.c      | 14 +++++++++++++-
 3 files changed, 41 insertions(+), 2 deletions(-)

-- 
2.7.4

[PATCH v2 1/2] ASoC: intel: atom: Add period size constraint

[Brent Lu]

Use constraint to enforce the period sizes which are validated in
Android BSP.

Signed-off-by: Brent Lu <brent.lu@intel.com>
---
 sound/soc/intel/atom/sst-mfld-platform-pcm.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/soc/intel/atom/sst-mfld-platform-pcm.c b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
index 49b9f18..70100ef 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
@@ -300,6 +300,16 @@ static void power_down_sst(struct sst_runtime_stream *stream)
 	stream->ops->power(sst->dev, false);
 }
 
+static const unsigned int media_period_size[] = {
+	/* sizes validated on Android platform */
+	240, 320, 960, 3072,
+};
+
+static const struct snd_pcm_hw_constraint_list constraints_media_period_size = {
+	.count = ARRAY_SIZE(media_period_size),
+	.list  = media_period_size,
+};
+
 static int sst_media_open(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
@@ -333,6 +343,11 @@ static int sst_media_open(struct snd_pcm_substream *substream,
 	if (ret_val < 0)
 		return ret_val;
 
+	/* Avoid using period size which is not validated */
+	snd_pcm_hw_constraint_list(substream->runtime, 0,
+			SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
+			&constraints_media_period_size);
+
 	/* Make sure, that the period size is always even */
 	snd_pcm_hw_constraint_step(substream->runtime, 0,
 			   SNDRV_PCM_HW_PARAM_PERIODS, 2);
-- 
2.7.4

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

[Brent Lu]

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(-)

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 = {
-- 
2.7.4

Re: [PATCH v2 0/2] Add period size constraint for Atom Chromebook

[Andy Shevchenko]

On Thu, Jul 30, 2020 at 04:13:33PM +0800, Brent Lu wrote:
> Two different constraints are implemented: one is in platform's CPU
> DAI to enforce period sizes which are already used in Android BSP. The
> other is in Atom Chromebook's machine driver to use 240 as period size.
> 
> Changes since v1:
> -Add comma at the end of media_period_size array declaration.

You missed my tags, any reason why?

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

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 +++++++++++++-

-- 
With Best Regards,
Andy Shevchenko

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

[Lu, Brent]

> 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
> 

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

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] Add period size constraint for Atom Chromebook

[Takashi Iwai]

On Thu, 30 Jul 2020 10:13:33 +0200,
Brent Lu wrote:
> 
> Two different constraints are implemented: one is in platform's CPU
> DAI to enforce period sizes which are already used in Android BSP. The
> other is in Atom Chromebook's machine driver to use 240 as period size.
> 
> Changes since v1:
> -Add comma at the end of media_period_size array declaration.

Is it a hardware restriction?  Unless it's a must for some hardware
issues, enforcing such a small period size is nothing but a drawback
for a system without CRAS.


thanks,

Takashi

> 
> Brent Lu (1):
>   ASoC: intel: atom: Add period size constraint
> 
> Yu-Hsuan Hsu (1):
>   ASoC: Intel: Add period size constraint on strago board
> 
>  sound/soc/intel/atom/sst-mfld-platform-pcm.c | 15 +++++++++++++++
>  sound/soc/intel/boards/cht_bsw_max98090_ti.c | 14 +++++++++++++-
>  sound/soc/intel/boards/cht_bsw_rt5645.c      | 14 +++++++++++++-
>  3 files changed, 41 insertions(+), 2 deletions(-)
> 
> -- 
> 2.7.4
> 

RE: [PATCH v2 0/2] Add period size constraint for Atom Chromebook

[Lu, Brent]

> >
> > Two different constraints are implemented: one is in platform's CPU
> > DAI to enforce period sizes which are already used in Android BSP. The
> > other is in Atom Chromebook's machine driver to use 240 as period size.
> >
> > Changes since v1:
> > -Add comma at the end of media_period_size array declaration.
> 
> Is it a hardware restriction?  Unless it's a must for some hardware issues,
> enforcing such a small period size is nothing but a drawback for a system
> without CRAS.

Hi Takashi,

This patch is cherry-picked from Chrome's branch which is originally implemented
by google. They found this value works best with CRAS and these two machine
drivers are for Chromebooks. Other BSW machine drivers are untouched.


Regards,
Brent

> 
> 
> thanks,
> 
> Takashi
> 
> >
> > Brent Lu (1):
> >   ASoC: intel: atom: Add period size constraint
> >
> > Yu-Hsuan Hsu (1):
> >   ASoC: Intel: Add period size constraint on strago board
> >
> >  sound/soc/intel/atom/sst-mfld-platform-pcm.c | 15 +++++++++++++++
> > sound/soc/intel/boards/cht_bsw_max98090_ti.c | 14 +++++++++++++-
> >  sound/soc/intel/boards/cht_bsw_rt5645.c      | 14 +++++++++++++-
> >  3 files changed, 41 insertions(+), 2 deletions(-)
> >
> > --
> > 2.7.4
> >