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

[Keyon Jie]

On 2019/4/4 下午6:37, Takashi Iwai wrote:
> 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.

Yep, you are right, we should use SNDRV_PCM_HW_PARAM_BUFFER_BYTES and 
SNDRV_PCM_HW_PARAM_PERIOD_BYTES here, thanks for pointing out.

They are configured(in tplg file) following comments in 
definition(somewhat confusion comparing to ALSA notion where 'size' 
means in frames?) of struct snd_soc_tplg_stream_caps:
	__le32 period_size_min;	/* min period size bytes */
	__le32 period_size_max;	/* max period size bytes */
	__le32 buffer_size_min;	/* min buffer size bytes */
	__le32 buffer_size_max;	/* max buffer size bytes */

Thanks,
~Keyon

> 
>> +
>> +	/* 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
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>