Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] Add period size constraint for Atom Chromebook
@ 2020-07-29 11:03 Brent Lu
  2020-07-29 11:03 ` [PATCH 1/2] ASoC: intel: atom: Add period size constraint Brent Lu
                   ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: Brent Lu @ 2020-07-29 11:03 UTC (permalink / raw)
  To: alsa-devel
  Cc: Guennadi Liakhovetski, Cezary Rojewski, Kai Vehmanen,
	Kuninori Morimoto, linux-kernel, Takashi Iwai, Jie Yang,
	Pierre-Louis Bossart, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Yu-Hsuan Hsu, Daniel Stuart, Andy Shevchenko,
	Brent Lu, Damian van Soelen

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.

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] ASoC: intel: atom: Add period size constraint
  2020-07-29 11:03 [PATCH 0/2] Add period size constraint for Atom Chromebook Brent Lu
@ 2020-07-29 11:03 ` Brent Lu
  2020-07-29 11:19   ` Andy Shevchenko
  2020-07-29 11:03 ` [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board Brent Lu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 25+ messages in thread
From: Brent Lu @ 2020-07-29 11:03 UTC (permalink / raw)
  To: alsa-devel
  Cc: Guennadi Liakhovetski, Cezary Rojewski, Kai Vehmanen,
	Kuninori Morimoto, linux-kernel, Takashi Iwai, Jie Yang,
	Pierre-Louis Bossart, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Yu-Hsuan Hsu, Daniel Stuart, Andy Shevchenko,
	Brent Lu, Damian van Soelen

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-29 11:03 [PATCH 0/2] Add period size constraint for Atom Chromebook Brent Lu
  2020-07-29 11:03 ` [PATCH 1/2] ASoC: intel: atom: Add period size constraint Brent Lu
@ 2020-07-29 11:03 ` Brent Lu
  2020-07-29 14:08   ` Pierre-Louis Bossart
  2020-07-29 11:20 ` [PATCH 0/2] Add period size constraint for Atom Chromebook Andy Shevchenko
  2020-07-31 12:26 ` [PATCH v3 " Brent Lu
  3 siblings, 1 reply; 25+ messages in thread
From: Brent Lu @ 2020-07-29 11:03 UTC (permalink / raw)
  To: alsa-devel
  Cc: Guennadi Liakhovetski, Cezary Rojewski, Kai Vehmanen,
	Kuninori Morimoto, linux-kernel, Takashi Iwai, Jie Yang,
	Pierre-Louis Bossart, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Yu-Hsuan Hsu, Daniel Stuart, Andy Shevchenko,
	Brent Lu, Damian van Soelen

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] ASoC: intel: atom: Add period size constraint
  2020-07-29 11:03 ` [PATCH 1/2] ASoC: intel: atom: Add period size constraint Brent Lu
@ 2020-07-29 11:19   ` Andy Shevchenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2020-07-29 11:19 UTC (permalink / raw)
  To: Brent Lu
  Cc: Guennadi Liakhovetski, alsa-devel, Kai Vehmanen,
	Kuninori Morimoto, linux-kernel, Takashi Iwai, Jie Yang,
	Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Sam McNally, Mark Brown, Ranjani Sridharan, Daniel Stuart,
	Yu-Hsuan Hsu, Damian van Soelen

On Wed, Jul 29, 2020 at 07:03:04PM +0800, Brent Lu wrote:
> 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..f614651 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

Leave comma at the end.

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

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 0/2] Add period size constraint for Atom Chromebook
  2020-07-29 11:03 [PATCH 0/2] Add period size constraint for Atom Chromebook Brent Lu
  2020-07-29 11:03 ` [PATCH 1/2] ASoC: intel: atom: Add period size constraint Brent Lu
  2020-07-29 11:03 ` [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board Brent Lu
@ 2020-07-29 11:20 ` Andy Shevchenko
  2020-07-31 12:26 ` [PATCH v3 " Brent Lu
  3 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2020-07-29 11:20 UTC (permalink / raw)
  To: Brent Lu
  Cc: Guennadi Liakhovetski, alsa-devel, Kai Vehmanen,
	Kuninori Morimoto, linux-kernel, Takashi Iwai, Jie Yang,
	Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Sam McNally, Mark Brown, Ranjani Sridharan, Daniel Stuart,
	Yu-Hsuan Hsu, Damian van Soelen

On Wed, Jul 29, 2020 at 07:03:03PM +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.

One nit to one patch.
Overall, LGTM and thus FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-29 11:03 ` [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board Brent Lu
@ 2020-07-29 14:08   ` Pierre-Louis Bossart
  2020-07-30  8:02     ` Lu, Brent
  0 siblings, 1 reply; 25+ messages in thread
From: Pierre-Louis Bossart @ 2020-07-29 14:08 UTC (permalink / raw)
  To: Brent Lu, alsa-devel
  Cc: Guennadi Liakhovetski, Cezary Rojewski, Kai Vehmanen,
	Kuninori Morimoto, Jie Yang, Takashi Iwai, linux-kernel,
	Liam Girdwood, Sam McNally, Mark Brown, Ranjani Sridharan,
	Daniel Stuart, Andy Shevchenko, Yu-Hsuan Hsu, Damian van Soelen



On 7/29/20 6:03 AM, 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.

Is this patch required if you've already constrained the period sizes 
for the platform driver in patch1?


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-29 14:08   ` Pierre-Louis Bossart
@ 2020-07-30  8:02     ` Lu, Brent
  2020-07-30 15:27       ` Pierre-Louis Bossart
  0 siblings, 1 reply; 25+ messages in thread
From: Lu, Brent @ 2020-07-30  8:02 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel
  Cc: Guennadi Liakhovetski, Rojewski,  Cezary, Kai Vehmanen,
	Kuninori Morimoto, Jie Yang, Takashi Iwai, linux-kernel,
	Liam Girdwood, Sam McNally, Mark Brown, Ranjani Sridharan,
	Daniel Stuart, Andy Shevchenko, Yu-Hsuan Hsu, Damian van Soelen


> 
> Is this patch required if you've already constrained the period sizes for the
> platform driver in patch1?

Yes or alsa will select 320 as default period size for it.


Regards,
Brent

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-30  8:02     ` Lu, Brent
@ 2020-07-30 15:27       ` Pierre-Louis Bossart
  2020-07-30 15:44         ` Pierre-Louis Bossart
  2020-07-30 16:56         ` Takashi Iwai
  0 siblings, 2 replies; 25+ messages in thread
From: Pierre-Louis Bossart @ 2020-07-30 15:27 UTC (permalink / raw)
  To: Lu, Brent, alsa-devel
  Cc: Guennadi Liakhovetski, Rojewski, Cezary, Kai Vehmanen,
	Kuninori Morimoto, Jie Yang, Takashi Iwai, linux-kernel,
	Liam Girdwood, Sam McNally, Mark Brown, Ranjani Sridharan,
	Daniel Stuart, Andy Shevchenko, Yu-Hsuan Hsu, Damian van Soelen



>> Is this patch required if you've already constrained the period sizes for the
>> platform driver in patch1?
> 
> Yes or alsa will select 320 as default period size for it.

ok, then that's a miss in your patch1. 320 samples is a multiple of 1ms 
for 48kHz rates. I think it was valid only for the 16kHz VoIP paths used 
in some versions of Android, but that we don't support in the upstream code.

To build on Takashi's answer, the real ask here is to require that the 
period be a multiple of 1ms, because that's the fundamental 
design/limitation of firmware. It doesn't matter if it's 48, 96, 192, 
240, 480, 960 samples.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-30 15:27       ` Pierre-Louis Bossart
@ 2020-07-30 15:44         ` Pierre-Louis Bossart
  2020-07-30 16:17           ` Lu, Brent
  2020-07-30 16:56         ` Takashi Iwai
  1 sibling, 1 reply; 25+ messages in thread
From: Pierre-Louis Bossart @ 2020-07-30 15:44 UTC (permalink / raw)
  To: Lu, Brent, alsa-devel
  Cc: Guennadi Liakhovetski, Rojewski, Cezary, Kai Vehmanen,
	Kuninori Morimoto, Jie Yang, Takashi Iwai, linux-kernel,
	Liam Girdwood, Sam McNally, Mark Brown, Ranjani Sridharan,
	Daniel Stuart, Andy Shevchenko, Yu-Hsuan Hsu, Damian van Soelen



On 7/30/20 10:27 AM, Pierre-Louis Bossart wrote:
> 
> 
>>> Is this patch required if you've already constrained the period sizes 
>>> for the
>>> platform driver in patch1?
>>
>> Yes or alsa will select 320 as default period size for it.
> 
> ok, then that's a miss in your patch1. 320 samples is a multiple of 1ms 

typo: is NOT

> for 48kHz rates. I think it was valid only for the 16kHz VoIP paths used 
> in some versions of Android, but that we don't support in the upstream 
> code.
> 
> To build on Takashi's answer, the real ask here is to require that the 
> period be a multiple of 1ms, because that's the fundamental 
> design/limitation of firmware. It doesn't matter if it's 48, 96, 192, 
> 240, 480, 960 samples.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-30 15:44         ` Pierre-Louis Bossart
@ 2020-07-30 16:17           ` Lu, Brent
  0 siblings, 0 replies; 25+ messages in thread
From: Lu, Brent @ 2020-07-30 16:17 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel
  Cc: Guennadi Liakhovetski, Rojewski,  Cezary, Kai Vehmanen,
	Kuninori Morimoto, Jie Yang, Takashi Iwai, linux-kernel,
	Liam Girdwood, Sam McNally, Mark Brown, Ranjani Sridharan,
	Daniel Stuart, Andy Shevchenko, Yu-Hsuan Hsu, Damian van Soelen

> >>
> >> Yes or alsa will select 320 as default period size for it.
> >
> > ok, then that's a miss in your patch1. 320 samples is a multiple of
> > 1ms
> 
> typo: is NOT
> 
> > for 48kHz rates. I think it was valid only for the 16kHz VoIP paths
> > used in some versions of Android, but that we don't support in the
> > upstream code.
> >
> > To build on Takashi's answer, the real ask here is to require that the
> > period be a multiple of 1ms, because that's the fundamental
> > design/limitation of firmware. It doesn't matter if it's 48, 96, 192,
> > 240, 480, 960 samples.

Yes 320 is for VoIP and the rates in CPU DAI are 8/16KHz. It's a mistake.

Regards,
Brent

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-30 15:27       ` Pierre-Louis Bossart
  2020-07-30 15:44         ` Pierre-Louis Bossart
@ 2020-07-30 16:56         ` Takashi Iwai
  2020-07-31 12:28           ` Lu, Brent
  1 sibling, 1 reply; 25+ messages in thread
From: Takashi Iwai @ 2020-07-30 16:56 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Guennadi Liakhovetski, alsa-devel, Kuninori Morimoto,
	Kai Vehmanen, linux-kernel, Jie Yang, Rojewski, Cezary,
	Takashi Iwai, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Yu-Hsuan Hsu, Daniel Stuart, Andy Shevchenko,
	Lu, Brent, Damian van Soelen

On Thu, 30 Jul 2020 17:27:58 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> 
> >> Is this patch required if you've already constrained the period sizes for the
> >> platform driver in patch1?
> >
> > Yes or alsa will select 320 as default period size for it.
> 
> ok, then that's a miss in your patch1. 320 samples is a multiple of
> 1ms for 48kHz rates. I think it was valid only for the 16kHz VoIP
> paths used in some versions of Android, but that we don't support in
> the upstream code.
> 
> To build on Takashi's answer, the real ask here is to require that the
> period be a multiple of 1ms, because that's the fundamental
> design/limitation of firmware. It doesn't matter if it's 48, 96, 192,
> 240, 480, 960 samples.

If the 1ms alignment is the condition, it can be better with a
different hw_params constraint.  We can use
snd_pcm_hw_constraint_step() for such a purpose.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 0/2] Add period size constraint for Atom Chromebook
  2020-07-29 11:03 [PATCH 0/2] Add period size constraint for Atom Chromebook Brent Lu
                   ` (2 preceding siblings ...)
  2020-07-29 11:20 ` [PATCH 0/2] Add period size constraint for Atom Chromebook Andy Shevchenko
@ 2020-07-31 12:26 ` Brent Lu
  2020-07-31 12:26   ` [PATCH v3 1/2] ASoC: intel: atom: Add period size constraint Brent Lu
  2020-07-31 12:26   ` [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board Brent Lu
  3 siblings, 2 replies; 25+ messages in thread
From: Brent Lu @ 2020-07-31 12:26 UTC (permalink / raw)
  To: alsa-devel
  Cc: Guennadi Liakhovetski, Cezary Rojewski, Kai Vehmanen,
	Kuninori Morimoto, linux-kernel, Takashi Iwai, Jie Yang,
	Pierre-Louis Bossart, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Yu-Hsuan Hsu, Daniel Stuart, Andy Shevchenko,
	Brent Lu, Damian van Soelen

Two different constraints are implemented: one is in platform's CPU
DAI to enforce the period to be multiple of 1ms to align with firmware
design. The other is in Atom Chromebook's machine driver to use 240 as
period size which is selected by google.


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

Changes since v2:
-Use snd_pcm_hw_constraint_step to enforce the 1ms period.


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 | 11 +++++++++++
 sound/soc/intel/boards/cht_bsw_max98090_ti.c | 14 +++++++++++++-
 sound/soc/intel/boards/cht_bsw_rt5645.c      | 14 +++++++++++++-
 3 files changed, 37 insertions(+), 2 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/2] ASoC: intel: atom: Add period size constraint
  2020-07-31 12:26 ` [PATCH v3 " Brent Lu
@ 2020-07-31 12:26   ` Brent Lu
  2020-07-31 12:26   ` [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board Brent Lu
  1 sibling, 0 replies; 25+ messages in thread
From: Brent Lu @ 2020-07-31 12:26 UTC (permalink / raw)
  To: alsa-devel
  Cc: Guennadi Liakhovetski, Cezary Rojewski, Kai Vehmanen,
	Kuninori Morimoto, linux-kernel, Takashi Iwai, Jie Yang,
	Pierre-Louis Bossart, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Yu-Hsuan Hsu, Daniel Stuart, Andy Shevchenko,
	Brent Lu, Damian van Soelen

Use constraint to make sure the period size could always be multiple
of 1ms to align with the fundamental design/limitation of firmware.

Signed-off-by: Brent Lu <brent.lu@intel.com>
---
 sound/soc/intel/atom/sst-mfld-platform-pcm.c | 11 +++++++++++
 1 file changed, 11 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..5f89956 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
@@ -333,6 +333,17 @@ static int sst_media_open(struct snd_pcm_substream *substream,
 	if (ret_val < 0)
 		return ret_val;
 
+	/*
+	 * Make sure the period to be multiple of 1ms to align the
+	 * design of firmware. Apply same rule to buffer size to make
+	 * sure alsa could always find a value for period size
+	 * regardless the buffer size given by user space.
+	 */
+	snd_pcm_hw_constraint_step(substream->runtime, 0,
+			   SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 48);
+	snd_pcm_hw_constraint_step(substream->runtime, 0,
+			   SNDRV_PCM_HW_PARAM_BUFFER_SIZE, 48);
+
 	/* 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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-31 12:26 ` [PATCH v3 " Brent Lu
  2020-07-31 12:26   ` [PATCH v3 1/2] ASoC: intel: atom: Add period size constraint Brent Lu
@ 2020-07-31 12:26   ` Brent Lu
  2020-07-31 13:34     ` Takashi Iwai
  1 sibling, 1 reply; 25+ messages in thread
From: Brent Lu @ 2020-07-31 12:26 UTC (permalink / raw)
  To: alsa-devel
  Cc: Guennadi Liakhovetski, Cezary Rojewski, Kai Vehmanen,
	Kuninori Morimoto, linux-kernel, Takashi Iwai, Jie Yang,
	Pierre-Louis Bossart, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Yu-Hsuan Hsu, Daniel Stuart, Andy Shevchenko,
	Brent Lu, Damian van Soelen

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-30 16:56         ` Takashi Iwai
@ 2020-07-31 12:28           ` Lu, Brent
  0 siblings, 0 replies; 25+ messages in thread
From: Lu, Brent @ 2020-07-31 12:28 UTC (permalink / raw)
  To: Takashi Iwai, Pierre-Louis Bossart
  Cc: Guennadi Liakhovetski, alsa-devel, Kuninori Morimoto,
	Kai Vehmanen, linux-kernel, Jie Yang, Rojewski, Cezary,
	Takashi Iwai, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Daniel Stuart, Andy Shevchenko, Yu-Hsuan Hsu,
	Damian van Soelen

> 
> If the 1ms alignment is the condition, it can be better with a different
> hw_params constraint.  We can use
> snd_pcm_hw_constraint_step() for such a purpose.
Will fix. Thanks.

Regards,
Brent
> 
> 
> thanks,
> 
> Takashi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-31 12:26   ` [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board Brent Lu
@ 2020-07-31 13:34     ` Takashi Iwai
  2020-08-01  8:58       ` Lu, Brent
  0 siblings, 1 reply; 25+ messages in thread
From: Takashi Iwai @ 2020-07-31 13:34 UTC (permalink / raw)
  To: Brent Lu
  Cc: Guennadi Liakhovetski, alsa-devel, Kuninori Morimoto,
	Kai Vehmanen, Cezary Rojewski, Jie Yang, linux-kernel,
	Takashi Iwai, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Daniel Stuart, Andy Shevchenko,
	Pierre-Louis Bossart, Yu-Hsuan Hsu, Damian van Soelen

On Fri, 31 Jul 2020 14:26:05 +0200,
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.
> 
> 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;

Again, is this fixed 240 is a must?  Or is this also an alignment
issue?


thanks,

Takashi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-07-31 13:34     ` Takashi Iwai
@ 2020-08-01  8:58       ` Lu, Brent
  2020-08-01  9:26         ` Takashi Iwai
  0 siblings, 1 reply; 25+ messages in thread
From: Lu, Brent @ 2020-08-01  8:58 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Guennadi Liakhovetski, alsa-devel, Kuninori Morimoto,
	Kai Vehmanen, Rojewski, Cezary, Jie Yang, linux-kernel,
	Takashi Iwai, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Daniel Stuart, Andy Shevchenko,
	Pierre-Louis Bossart, Yu-Hsuan Hsu, Damian van Soelen

> 
> Again, is this fixed 240 is a must?  Or is this also an alignment issue?
Hi Takashi,

I think it's a must for Chromebooks. Google found this value works best
with their CRAS server running on their BSW products. They offered this
patch for their own Chromebooks.

> 
> 
> thanks,
> 
> Takashi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-08-01  8:58       ` Lu, Brent
@ 2020-08-01  9:26         ` Takashi Iwai
  2020-08-03 13:00           ` Lu, Brent
  0 siblings, 1 reply; 25+ messages in thread
From: Takashi Iwai @ 2020-08-01  9:26 UTC (permalink / raw)
  To: Lu, Brent
  Cc: Guennadi Liakhovetski, alsa-devel, Kuninori Morimoto,
	Kai Vehmanen, Rojewski, Cezary, Jie Yang, linux-kernel,
	Takashi Iwai, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Daniel Stuart, Andy Shevchenko,
	Pierre-Louis Bossart, Yu-Hsuan Hsu, Damian van Soelen

On Sat, 01 Aug 2020 10:58:16 +0200,
Lu, Brent wrote:
> 
> > 
> > Again, is this fixed 240 is a must?  Or is this also an alignment issue?
> Hi Takashi,
> 
> I think it's a must for Chromebooks. Google found this value works best
> with their CRAS server running on their BSW products. They offered this
> patch for their own Chromebooks.

Hrm, but it's likely a worse choice on other sound backends.

Please double-check whether this fixed small period is a must, or it's
an alignment issue.


Takashi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-08-01  9:26         ` Takashi Iwai
@ 2020-08-03 13:00           ` Lu, Brent
  2020-08-03 15:13             ` Pierre-Louis Bossart
  0 siblings, 1 reply; 25+ messages in thread
From: Lu, Brent @ 2020-08-03 13:00 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Guennadi Liakhovetski, alsa-devel, Kuninori Morimoto,
	Kai Vehmanen, Rojewski, Cezary, Jie Yang, linux-kernel,
	Takashi Iwai, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Daniel Stuart, Andy Shevchenko,
	Pierre-Louis Bossart, Yu-Hsuan Hsu, Damian van Soelen

> > >
> > > Again, is this fixed 240 is a must?  Or is this also an alignment issue?
> > Hi Takashi,
> >
> > I think it's a must for Chromebooks. Google found this value works
> > best with their CRAS server running on their BSW products. They
> > offered this patch for their own Chromebooks.
> 
> Hrm, but it's likely a worse choice on other sound backends.
> 
> Please double-check whether this fixed small period is a must, or it's an
> alignment issue.
Hi Takashi,

I've double checked with google. It's a must for Chromebooks due to low
latency use case.


Regards,
Brent

> 
> 
> Takashi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-08-03 13:00           ` Lu, Brent
@ 2020-08-03 15:13             ` Pierre-Louis Bossart
  2020-08-03 16:45               ` Lu, Brent
  0 siblings, 1 reply; 25+ messages in thread
From: Pierre-Louis Bossart @ 2020-08-03 15:13 UTC (permalink / raw)
  To: Lu, Brent, Takashi Iwai
  Cc: Guennadi Liakhovetski, alsa-devel, Kuninori Morimoto,
	Kai Vehmanen, Takashi Iwai, Jie Yang, Rojewski, Cezary,
	linux-kernel, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Daniel Stuart, Andy Shevchenko, Yu-Hsuan Hsu,
	Damian van Soelen



On 8/3/20 8:00 AM, Lu, Brent wrote:
>>>>
>>>> Again, is this fixed 240 is a must?  Or is this also an alignment issue?
>>> Hi Takashi,
>>>
>>> I think it's a must for Chromebooks. Google found this value works
>>> best with their CRAS server running on their BSW products. They
>>> offered this patch for their own Chromebooks.
>>
>> Hrm, but it's likely a worse choice on other sound backends.
>>
>> Please double-check whether this fixed small period is a must, or it's an
>> alignment issue.
> Hi Takashi,
> 
> I've double checked with google. It's a must for Chromebooks due to low
> latency use case.

I wonder if there's a misunderstanding here?

I believe Takashi's question was "is this a must to ONLY accept 240 
samples for the period size", there was no pushback on the value itself. 
Are those boards broken with e.g. 960 samples?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-08-03 15:13             ` Pierre-Louis Bossart
@ 2020-08-03 16:45               ` Lu, Brent
  2020-08-03 16:56                 ` Takashi Iwai
  0 siblings, 1 reply; 25+ messages in thread
From: Lu, Brent @ 2020-08-03 16:45 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Takashi Iwai
  Cc: Guennadi Liakhovetski, alsa-devel, Kuninori Morimoto,
	Kai Vehmanen, yuhsuan, Takashi Iwai, Jie Yang, Rojewski, Cezary,
	linux-kernel, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Daniel Stuart, Andy Shevchenko, Yu-Hsuan Hsu,
	Damian van Soelen

> > Hi Takashi,
> >
> > I've double checked with google. It's a must for Chromebooks due to
> > low latency use case.
> 
> I wonder if there's a misunderstanding here?
> 
> I believe Takashi's question was "is this a must to ONLY accept 240 samples
> for the period size", there was no pushback on the value itself.
> Are those boards broken with e.g. 960 samples?

I've added google people to discuss directly.

Hi Yuhsuan,
Would you explain why CRAS needs to use such short period size? Thanks.


Regards,
Brent

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-08-03 16:45               ` Lu, Brent
@ 2020-08-03 16:56                 ` Takashi Iwai
  2020-08-04  4:33                   ` Lu, Brent
  0 siblings, 1 reply; 25+ messages in thread
From: Takashi Iwai @ 2020-08-03 16:56 UTC (permalink / raw)
  To: Lu, Brent
  Cc: Guennadi Liakhovetski, alsa-devel, Kai Vehmanen,
	Kuninori Morimoto, linux-kernel, yuhsuan, Takashi Iwai, Jie Yang,
	Rojewski, Cezary, Pierre-Louis Bossart, Liam Girdwood,
	Sam McNally, Mark Brown, Ranjani Sridharan, Daniel Stuart,
	Andy Shevchenko, Yu-Hsuan Hsu, Damian van Soelen

On Mon, 03 Aug 2020 18:45:29 +0200,
Lu, Brent wrote:
> 
> > > Hi Takashi,
> > >
> > > I've double checked with google. It's a must for Chromebooks due to
> > > low latency use case.
> > 
> > I wonder if there's a misunderstanding here?
> > 
> > I believe Takashi's question was "is this a must to ONLY accept 240 samples
> > for the period size", there was no pushback on the value itself.
> > Are those boards broken with e.g. 960 samples?
> 
> I've added google people to discuss directly.
> 
> Hi Yuhsuan,
> Would you explain why CRAS needs to use such short period size? Thanks.

For avoid further misunderstanding: it's fine that CRAS *uses* such a
short period.  It's often required for achieving a short latency.

However, the question is whether the driver can set *only* this value
for making it working.  IOW, if we don't have this constraint, what
actually happens?  If the driver gives the period size alignment,
wouldn't CRAS choose 240?


Takashi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-08-03 16:56                 ` Takashi Iwai
@ 2020-08-04  4:33                   ` Lu, Brent
  2020-08-04 14:24                     ` Pierre-Louis Bossart
  0 siblings, 1 reply; 25+ messages in thread
From: Lu, Brent @ 2020-08-04  4:33 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Guennadi Liakhovetski, alsa-devel, Kai Vehmanen,
	Kuninori Morimoto, linux-kernel, yuhsuan, Takashi Iwai, Jie Yang,
	Rojewski, Cezary, Pierre-Louis Bossart, Liam Girdwood,
	Sam McNally, Mark Brown, Ranjani Sridharan, Daniel Stuart,
	Andy Shevchenko, Yu-Hsuan Hsu, Damian van Soelen

> 
> For avoid further misunderstanding: it's fine that CRAS *uses* such a short
> period.  It's often required for achieving a short latency.
> 
> However, the question is whether the driver can set *only* this value for
> making it working.  IOW, if we don't have this constraint, what actually
> happens?  If the driver gives the period size alignment, wouldn't CRAS
> choose 240?

It won't. Without the constraint it becomes 432. Actually CRAS does not set
period size specifically so the value depends on the constraint rules.

[   52.011146] sound pcmC1D0p: hw_param
[   52.011152] sound pcmC1D0p:   ACCESS 0x1
[   52.011155] sound pcmC1D0p:   FORMAT 0x4
[   52.011158] sound pcmC1D0p:   SUBFORMAT 0x1
[   52.011161] sound pcmC1D0p:   SAMPLE_BITS [16:16]
[   52.011164] sound pcmC1D0p:   FRAME_BITS [32:32]
[   52.011167] sound pcmC1D0p:   CHANNELS [2:2]
[   52.011170] sound pcmC1D0p:   RATE [48000:48000]
[   52.011173] sound pcmC1D0p:   PERIOD_TIME [9000:9000]
[   52.011176] sound pcmC1D0p:   PERIOD_SIZE [432:432]
[   52.011179] sound pcmC1D0p:   PERIOD_BYTES [1728:1728]
[   52.011182] sound pcmC1D0p:   PERIODS [474:474]
[   52.011185] sound pcmC1D0p:   BUFFER_TIME [4266000:4266000]
[   52.011188] sound pcmC1D0p:   BUFFER_SIZE [204768:204768]
[   52.011191] sound pcmC1D0p:   BUFFER_BYTES [819072:819072]
[   52.011194] sound pcmC1D0p:   TICK_TIME [0:0]

Regards,
Brent

> 
> 
> Takashi



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-08-04  4:33                   ` Lu, Brent
@ 2020-08-04 14:24                     ` Pierre-Louis Bossart
  2020-08-06 16:41                       ` Lu, Brent
  0 siblings, 1 reply; 25+ messages in thread
From: Pierre-Louis Bossart @ 2020-08-04 14:24 UTC (permalink / raw)
  To: Lu, Brent, Takashi Iwai
  Cc: Guennadi Liakhovetski, alsa-devel, Kai Vehmanen,
	Kuninori Morimoto, Rojewski, Cezary, Takashi Iwai, Jie Yang,
	linux-kernel, yuhsuan, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Daniel Stuart, Andy Shevchenko, Yu-Hsuan Hsu,
	Damian van Soelen



On 8/3/20 11:33 PM, Lu, Brent wrote:
>>
>> For avoid further misunderstanding: it's fine that CRAS *uses* such a short
>> period.  It's often required for achieving a short latency.
>>
>> However, the question is whether the driver can set *only* this value for
>> making it working.  IOW, if we don't have this constraint, what actually
>> happens?  If the driver gives the period size alignment, wouldn't CRAS
>> choose 240?
> 
> It won't. Without the constraint it becomes 432. Actually CRAS does not set
> period size specifically so the value depends on the constraint rules.

I don't get this. If the platform driver already stated 240 and 960 
samples why would 432 be chosen? Doesn't this mean the constraint is not 
applied?

> [   52.011146] sound pcmC1D0p: hw_param
> [   52.011152] sound pcmC1D0p:   ACCESS 0x1
> [   52.011155] sound pcmC1D0p:   FORMAT 0x4
> [   52.011158] sound pcmC1D0p:   SUBFORMAT 0x1
> [   52.011161] sound pcmC1D0p:   SAMPLE_BITS [16:16]
> [   52.011164] sound pcmC1D0p:   FRAME_BITS [32:32]
> [   52.011167] sound pcmC1D0p:   CHANNELS [2:2]
> [   52.011170] sound pcmC1D0p:   RATE [48000:48000]
> [   52.011173] sound pcmC1D0p:   PERIOD_TIME [9000:9000]
> [   52.011176] sound pcmC1D0p:   PERIOD_SIZE [432:432]
> [   52.011179] sound pcmC1D0p:   PERIOD_BYTES [1728:1728]
> [   52.011182] sound pcmC1D0p:   PERIODS [474:474]
> [   52.011185] sound pcmC1D0p:   BUFFER_TIME [4266000:4266000]
> [   52.011188] sound pcmC1D0p:   BUFFER_SIZE [204768:204768]
> [   52.011191] sound pcmC1D0p:   BUFFER_BYTES [819072:819072]
> [   52.011194] sound pcmC1D0p:   TICK_TIME [0:0]
> 
> Regards,
> Brent
> 
>>
>>
>> Takashi
> 
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board
  2020-08-04 14:24                     ` Pierre-Louis Bossart
@ 2020-08-06 16:41                       ` Lu, Brent
  0 siblings, 0 replies; 25+ messages in thread
From: Lu, Brent @ 2020-08-06 16:41 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Takashi Iwai
  Cc: Guennadi Liakhovetski, alsa-devel, Kai Vehmanen,
	Kuninori Morimoto, Rojewski, Cezary, Takashi Iwai, Jie Yang,
	linux-kernel, yuhsuan, Liam Girdwood, Sam McNally, Mark Brown,
	Ranjani Sridharan, Daniel Stuart, Andy Shevchenko, Yu-Hsuan Hsu,
	Damian van Soelen

> 
> I don't get this. If the platform driver already stated 240 and 960 samples why
> would 432 be chosen? Doesn't this mean the constraint is not applied?

Hi Pierre,

Sorry for late reply. I used following constraints in V3 patch so any period which
aligns 1ms would be accepted.

+	/*
+	 * Make sure the period to be multiple of 1ms to align the
+	 * design of firmware. Apply same rule to buffer size to make
+	 * sure alsa could always find a value for period size
+	 * regardless the buffer size given by user space.
+	 */
+	snd_pcm_hw_constraint_step(substream->runtime, 0,
+			   SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 48);
+	snd_pcm_hw_constraint_step(substream->runtime, 0,
+			   SNDRV_PCM_HW_PARAM_BUFFER_SIZE, 48);

Regards,
Brent

> 
> > [   52.011146] sound pcmC1D0p: hw_param
> > [   52.011152] sound pcmC1D0p:   ACCESS 0x1
> > [   52.011155] sound pcmC1D0p:   FORMAT 0x4
> > [   52.011158] sound pcmC1D0p:   SUBFORMAT 0x1
> > [   52.011161] sound pcmC1D0p:   SAMPLE_BITS [16:16]
> > [   52.011164] sound pcmC1D0p:   FRAME_BITS [32:32]
> > [   52.011167] sound pcmC1D0p:   CHANNELS [2:2]
> > [   52.011170] sound pcmC1D0p:   RATE [48000:48000]
> > [   52.011173] sound pcmC1D0p:   PERIOD_TIME [9000:9000]
> > [   52.011176] sound pcmC1D0p:   PERIOD_SIZE [432:432]
> > [   52.011179] sound pcmC1D0p:   PERIOD_BYTES [1728:1728]
> > [   52.011182] sound pcmC1D0p:   PERIODS [474:474]
> > [   52.011185] sound pcmC1D0p:   BUFFER_TIME [4266000:4266000]
> > [   52.011188] sound pcmC1D0p:   BUFFER_SIZE [204768:204768]
> > [   52.011191] sound pcmC1D0p:   BUFFER_BYTES [819072:819072]
> > [   52.011194] sound pcmC1D0p:   TICK_TIME [0:0]
> >
> > Regards,
> > Brent
> >
> >>
> >>
> >> Takashi
> >
> >

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, back to index

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-29 11:03 [PATCH 0/2] Add period size constraint for Atom Chromebook Brent Lu
2020-07-29 11:03 ` [PATCH 1/2] ASoC: intel: atom: Add period size constraint Brent Lu
2020-07-29 11:19   ` Andy Shevchenko
2020-07-29 11:03 ` [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board Brent Lu
2020-07-29 14:08   ` Pierre-Louis Bossart
2020-07-30  8:02     ` Lu, Brent
2020-07-30 15:27       ` Pierre-Louis Bossart
2020-07-30 15:44         ` Pierre-Louis Bossart
2020-07-30 16:17           ` Lu, Brent
2020-07-30 16:56         ` Takashi Iwai
2020-07-31 12:28           ` Lu, Brent
2020-07-29 11:20 ` [PATCH 0/2] Add period size constraint for Atom Chromebook Andy Shevchenko
2020-07-31 12:26 ` [PATCH v3 " Brent Lu
2020-07-31 12:26   ` [PATCH v3 1/2] ASoC: intel: atom: Add period size constraint Brent Lu
2020-07-31 12:26   ` [PATCH v3 2/2] ASoC: Intel: Add period size constraint on strago board Brent Lu
2020-07-31 13:34     ` Takashi Iwai
2020-08-01  8:58       ` Lu, Brent
2020-08-01  9:26         ` Takashi Iwai
2020-08-03 13:00           ` Lu, Brent
2020-08-03 15:13             ` Pierre-Louis Bossart
2020-08-03 16:45               ` Lu, Brent
2020-08-03 16:56                 ` Takashi Iwai
2020-08-04  4:33                   ` Lu, Brent
2020-08-04 14:24                     ` Pierre-Louis Bossart
2020-08-06 16:41                       ` Lu, Brent

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git