Re: [PATCH 8/8] ASoC: SOF: Intel: add .ack support for HDaudio platforms

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

>> +/* update SPIB register with appl position */
>> +int hda_dsp_pcm_ack(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream)
>> +{
>> +	struct hdac_stream *hstream = substream->runtime->private_data;
>> +	struct hdac_ext_stream *stream = stream_to_hdac_ext_stream(hstream);
>> +	struct snd_pcm_runtime *runtime = substream->runtime;
>> +	ssize_t appl_pos, buf_size;
>> +	u32 spib;
>> +
>> +	if (!hda_disable_rewinds)
>> +		return 0;
>> +
>> +	/*
>> +	 * paranoia check: if a rewind request took place after the RUN bits were programmed,
>> +	 * deny it since hardware only supports monotonic (modulo) increments for SPIB.
>> +	 */
>> +	if (hstream->running) {
>> +		if (runtime->control->appl_ptr < stream->old_appl_ptr)
>> +			return -EINVAL;
> 
> This condition won't be enough when the appl_ptr overlap the buffer
> boundary.  It's still possible on 32bit architecture.

And I missed this feedback as well...I only replied at the comments on module parameters/KConfig/controls/new API.

Takashi, is this saying that on 32-bit architectures there's no way to make the difference in the .ack implementation between
- regular rewind and forward after the buffer max boundary
- regular forward and rewind before the buffer zero boundary

If that was the case, the proposal made in this patch to validate the rewind with the .ack wouldn't work, we would have to go back to a filter in snd_pcm_rewind similar to what was initially suggested in [1]

[1] https://lore.kernel.org/alsa-devel/1494896518-23399-2-git-send-email-subhransu.s.prusty@intel.com/