[alsa-devel] [PATCH v5 05/14] ASoC: SOF: Add PCM operations support

[Takashi Iwai]

On Thu, 21 Mar 2019 17:10:07 +0100,
Pierre-Louis Bossart wrote:
> 
> +/* this may get called several times by oss emulation */
> +static int sof_pcm_hw_params(struct snd_pcm_substream *substream,
> +			     struct snd_pcm_hw_params *params)
> +{
....
> +	/* container size */
> +	switch (params_width(params)) {
> +	case 16:
> +		pcm.params.sample_container_bytes = 2;
> +		break;
> +	case 24:
> +		pcm.params.sample_container_bytes = 4;
> +		break;
> +	case 32:
> +		pcm.params.sample_container_bytes = 4;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}

This can be simply snd_pcm_format_physical_width() / 8.

> +static int sof_pcm_open(struct snd_pcm_substream *substream)
> +{
....
> +	/* set any runtime constraints based on topology */
> +	snd_pcm_hw_constraint_step(substream->runtime, 0,
> +				   SNDRV_PCM_HW_PARAM_BUFFER_SIZE,
> +				   le32_to_cpu(caps->period_size_min));
> +	snd_pcm_hw_constraint_step(substream->runtime, 0,
> +				   SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
> +				   le32_to_cpu(caps->period_size_min));

Is period_size_min in frames or in bytes?  The code below refers as
byte size while this refers as frames, so they look inconsistent.

> +
> +	/* set runtime config */
> +	runtime->hw.info = SNDRV_PCM_INFO_MMAP |
> +			  SNDRV_PCM_INFO_MMAP_VALID |
> +			  SNDRV_PCM_INFO_INTERLEAVED |
> +			  SNDRV_PCM_INFO_PAUSE |
> +			  SNDRV_PCM_INFO_RESUME |
> +			  SNDRV_PCM_INFO_NO_PERIOD_WAKEUP;

Does SOF support the full resume?  That is, the stream gets resumed at
the exact position that was suspended.  Most devices can't, hence they
don't set *_INFO_RESUME flag and let user-space restarting.

> +	runtime->hw.period_bytes_min = le32_to_cpu(caps->period_size_min);
> +	runtime->hw.period_bytes_max = le32_to_cpu(caps->period_size_max);
> +	runtime->hw.periods_min = le32_to_cpu(caps->periods_min);
> +	runtime->hw.periods_max = le32_to_cpu(caps->periods_max);
> +	runtime->hw.buffer_bytes_max = le32_to_cpu(caps->buffer_size_max);

These refer as bytes...


thanks,

Takashi