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
Use constraint to enforce the period sizes which are validated in
Android BSP.
Signed-off-by: Brent Lu <[email protected]>
---
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
From: Yu-Hsuan Hsu <[email protected]>
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 <[email protected]>
Signed-off-by: Brent Lu <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> 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
On 7/29/20 6:03 AM, Brent Lu wrote:
> From: Yu-Hsuan Hsu <[email protected]>
>
> 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?
>
> 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
>> 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.
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.
> >>
> >> 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
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
>
> 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
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
On Fri, 31 Jul 2020 20:26:03 +0800, Brent Lu wrote:
> 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.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: intel: atom: Add period size constraint
commit: 5e7820e369248f880767c4c4079b414529bc2125
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark