[PATCH v3] ALSA: core: let low-level driver or userspace disable rewinds

[PATCH v3] ALSA: core: let low-level driver or userspace disable rewinds

[Subhransu S. Prusty]

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Add new hw_params flag to explicitly tell driver that rewinds will never
be used. This can be used by low-level driver to optimize DMA operations
and reduce power consumption. Use this flag only when data written in
ring buffer will never be invalidated, e.g. any update of appl_ptr is
final.

Note that the update of appl_ptr include both a read/write data
operation as well as snd_pcm_forward() whose behavior is not modified.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Ramesh Babu <ramesh.babu@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---

v2 -> v3
	return error for rewind operation if no_rewinds set.

 include/sound/pcm.h         | 1 +
 include/uapi/sound/asound.h | 1 +
 sound/core/pcm_native.c     | 8 ++++++++
 3 files changed, 10 insertions(+)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 79fedf517070..c1e2b87cd409 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -368,6 +368,7 @@ struct snd_pcm_runtime {
 	unsigned int rate_num;
 	unsigned int rate_den;
 	unsigned int no_period_wakeup: 1;
+	unsigned int no_rewinds:1;
 
 	/* -- SW params -- */
 	int tstamp_mode;		/* mmap timestamp is updated */
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index fd41697cb4d3..c697ff90450d 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -365,6 +365,7 @@ struct snd_pcm_info {
 #define SNDRV_PCM_HW_PARAMS_NORESAMPLE	(1<<0)	/* avoid rate resampling */
 #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER	(1<<1)	/* export buffer */
 #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP	(1<<2)	/* disable period wakeups */
+#define SNDRV_PCM_HW_PARAMS_NO_REWINDS	        (1<<3)	/* disable rewinds */
 
 struct snd_interval {
 	unsigned int min, max;
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index bf5d0f2acfb9..6769f4751fa0 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -554,6 +554,8 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 	runtime->no_period_wakeup =
 			(params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) &&
 			(params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP);
+	runtime->no_rewinds =
+		(params->flags & SNDRV_PCM_HW_PARAMS_NO_REWINDS) ? 1 : 0;
 
 	bits = snd_pcm_format_physical_width(runtime->format);
 	runtime->sample_bits = bits;
@@ -2521,6 +2523,9 @@ static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *subst
 	if (frames == 0)
 		return 0;
 
+	if (runtime->no_rewinds)
+		return -ENODEV;
+
 	snd_pcm_stream_lock_irq(substream);
 	ret = do_pcm_hwsync(substream);
 	if (!ret)
@@ -2539,6 +2544,9 @@ static snd_pcm_sframes_t snd_pcm_capture_rewind(struct snd_pcm_substream *substr
 	if (frames == 0)
 		return 0;
 
+	if (runtime->no_rewinds)
+		return -ENODEV;
+
 	snd_pcm_stream_lock_irq(substream);
 	ret = do_pcm_hwsync(substream);
 	if (!ret)
-- 
1.9.1

Re: [PATCH v3] ALSA: core: let low-level driver or userspace disable rewinds

[Vinod Koul]

On Mon, Jun 19, 2017 at 11:52:37AM +0530, Subhransu S. Prusty wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> Add new hw_params flag to explicitly tell driver that rewinds will never
> be used. This can be used by low-level driver to optimize DMA operations
> and reduce power consumption. Use this flag only when data written in
> ring buffer will never be invalidated, e.g. any update of appl_ptr is
> final.
> 
> Note that the update of appl_ptr include both a read/write data
> operation as well as snd_pcm_forward() whose behavior is not modified.

Reviewed-By: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod

Re: [PATCH v3] ALSA: core: let low-level driver or userspace disable rewinds

[Takashi Iwai]

On Mon, 19 Jun 2017 08:22:37 +0200,
Subhransu S. Prusty wrote:
> 
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> Add new hw_params flag to explicitly tell driver that rewinds will never
> be used. This can be used by low-level driver to optimize DMA operations
> and reduce power consumption. Use this flag only when data written in
> ring buffer will never be invalidated, e.g. any update of appl_ptr is
> final.
> 
> Note that the update of appl_ptr include both a read/write data
> operation as well as snd_pcm_forward() whose behavior is not modified.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Signed-off-by: Ramesh Babu <ramesh.babu@intel.com>
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> ---
> 
> v2 -> v3
> 	return error for rewind operation if no_rewinds set.

Now I'm thinking whether this change is really needed or in the right
direction.  I suppose that the driver would change the behavior
depending on the flag of this flag.  But then how can application know
that it should not rewind on a certain device and set the flag?

We may, instead, let the driver behaving in the deep buffer mode as
default (or turned on/off via the kcontrol).  Then you can simply
return an error for the negative appl_ptr change in ack callback --
which effectively disables the rewind.


thanks,

Takashi

>  include/sound/pcm.h         | 1 +
>  include/uapi/sound/asound.h | 1 +
>  sound/core/pcm_native.c     | 8 ++++++++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 79fedf517070..c1e2b87cd409 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -368,6 +368,7 @@ struct snd_pcm_runtime {
>  	unsigned int rate_num;
>  	unsigned int rate_den;
>  	unsigned int no_period_wakeup: 1;
> +	unsigned int no_rewinds:1;
>  
>  	/* -- SW params -- */
>  	int tstamp_mode;		/* mmap timestamp is updated */
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index fd41697cb4d3..c697ff90450d 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -365,6 +365,7 @@ struct snd_pcm_info {
>  #define SNDRV_PCM_HW_PARAMS_NORESAMPLE	(1<<0)	/* avoid rate resampling */
>  #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER	(1<<1)	/* export buffer */
>  #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP	(1<<2)	/* disable period wakeups */
> +#define SNDRV_PCM_HW_PARAMS_NO_REWINDS	        (1<<3)	/* disable rewinds */
>  
>  struct snd_interval {
>  	unsigned int min, max;
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index bf5d0f2acfb9..6769f4751fa0 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -554,6 +554,8 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
>  	runtime->no_period_wakeup =
>  			(params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) &&
>  			(params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP);
> +	runtime->no_rewinds =
> +		(params->flags & SNDRV_PCM_HW_PARAMS_NO_REWINDS) ? 1 : 0;
>  
>  	bits = snd_pcm_format_physical_width(runtime->format);
>  	runtime->sample_bits = bits;
> @@ -2521,6 +2523,9 @@ static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *subst
>  	if (frames == 0)
>  		return 0;
>  
> +	if (runtime->no_rewinds)
> +		return -ENODEV;
> +
>  	snd_pcm_stream_lock_irq(substream);
>  	ret = do_pcm_hwsync(substream);
>  	if (!ret)
> @@ -2539,6 +2544,9 @@ static snd_pcm_sframes_t snd_pcm_capture_rewind(struct snd_pcm_substream *substr
>  	if (frames == 0)
>  		return 0;
>  
> +	if (runtime->no_rewinds)
> +		return -ENODEV;
> +
>  	snd_pcm_stream_lock_irq(substream);
>  	ret = do_pcm_hwsync(substream);
>  	if (!ret)
> -- 
> 1.9.1
> 

Re: [PATCH v3] ALSA: core: let low-level driver or userspace disable rewinds

[Pierre-Louis Bossart]

On 6/19/17 3:44 AM, Takashi Iwai wrote:
> On Mon, 19 Jun 2017 08:22:37 +0200,
> Subhransu S. Prusty wrote:
>>
>> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>
>> Add new hw_params flag to explicitly tell driver that rewinds will never
>> be used. This can be used by low-level driver to optimize DMA operations
>> and reduce power consumption. Use this flag only when data written in
>> ring buffer will never be invalidated, e.g. any update of appl_ptr is
>> final.
>>
>> Note that the update of appl_ptr include both a read/write data
>> operation as well as snd_pcm_forward() whose behavior is not modified.
>>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> Signed-off-by: Ramesh Babu <ramesh.babu@intel.com>
>> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
>> ---
>>
>> v2 -> v3
>> 	return error for rewind operation if no_rewinds set.
>
> Now I'm thinking whether this change is really needed or in the right
> direction.  I suppose that the driver would change the behavior
> depending on the flag of this flag.  But then how can application know
> that it should not rewind on a certain device and set the flag?

The application (which is in most cases an audio server) *knows* if it 
requires rewinds or not. It's part of its design, with rewinds typically 
disabled if period interrupts are required. It's been that way for a 
number of years now. The use of rewinds is typically associated with the 
combination of a large buffer and no interrupts (having either of the 
two would not require rewinds).

So the idea is that the application makes a statement that rewinds will 
not be used, and the low-level driver makes use of the information to 
enable whatever optimizations are available at the hardware level.

Exposing more information to userspace would quickly lead to a confusing 
decision-making and would require more than just a flag.

>
> We may, instead, let the driver behaving in the deep buffer mode as
> default (or turned on/off via the kcontrol).  Then you can simply
> return an error for the negative appl_ptr change in ack callback --
> which effectively disables the rewind.
>
>
> thanks,
>
> Takashi
>
>>  include/sound/pcm.h         | 1 +
>>  include/uapi/sound/asound.h | 1 +
>>  sound/core/pcm_native.c     | 8 ++++++++
>>  3 files changed, 10 insertions(+)
>>
>> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
>> index 79fedf517070..c1e2b87cd409 100644
>> --- a/include/sound/pcm.h
>> +++ b/include/sound/pcm.h
>> @@ -368,6 +368,7 @@ struct snd_pcm_runtime {
>>  	unsigned int rate_num;
>>  	unsigned int rate_den;
>>  	unsigned int no_period_wakeup: 1;
>> +	unsigned int no_rewinds:1;
>>
>>  	/* -- SW params -- */
>>  	int tstamp_mode;		/* mmap timestamp is updated */
>> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
>> index fd41697cb4d3..c697ff90450d 100644
>> --- a/include/uapi/sound/asound.h
>> +++ b/include/uapi/sound/asound.h
>> @@ -365,6 +365,7 @@ struct snd_pcm_info {
>>  #define SNDRV_PCM_HW_PARAMS_NORESAMPLE	(1<<0)	/* avoid rate resampling */
>>  #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER	(1<<1)	/* export buffer */
>>  #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP	(1<<2)	/* disable period wakeups */
>> +#define SNDRV_PCM_HW_PARAMS_NO_REWINDS	        (1<<3)	/* disable rewinds */
>>
>>  struct snd_interval {
>>  	unsigned int min, max;
>> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
>> index bf5d0f2acfb9..6769f4751fa0 100644
>> --- a/sound/core/pcm_native.c
>> +++ b/sound/core/pcm_native.c
>> @@ -554,6 +554,8 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
>>  	runtime->no_period_wakeup =
>>  			(params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) &&
>>  			(params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP);
>> +	runtime->no_rewinds =
>> +		(params->flags & SNDRV_PCM_HW_PARAMS_NO_REWINDS) ? 1 : 0;
>>
>>  	bits = snd_pcm_format_physical_width(runtime->format);
>>  	runtime->sample_bits = bits;
>> @@ -2521,6 +2523,9 @@ static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *subst
>>  	if (frames == 0)
>>  		return 0;
>>
>> +	if (runtime->no_rewinds)
>> +		return -ENODEV;
>> +
>>  	snd_pcm_stream_lock_irq(substream);
>>  	ret = do_pcm_hwsync(substream);
>>  	if (!ret)
>> @@ -2539,6 +2544,9 @@ static snd_pcm_sframes_t snd_pcm_capture_rewind(struct snd_pcm_substream *substr
>>  	if (frames == 0)
>>  		return 0;
>>
>> +	if (runtime->no_rewinds)
>> +		return -ENODEV;
>> +
>>  	snd_pcm_stream_lock_irq(substream);
>>  	ret = do_pcm_hwsync(substream);
>>  	if (!ret)
>> --
>> 1.9.1
>>

Re: [PATCH v3] ALSA: core: let low-level driver or userspace disable rewinds

[Takashi Iwai]

On Mon, 19 Jun 2017 17:16:28 +0200,
Pierre-Louis Bossart wrote:
> 
> On 6/19/17 3:44 AM, Takashi Iwai wrote:
> > On Mon, 19 Jun 2017 08:22:37 +0200,
> > Subhransu S. Prusty wrote:
> >>
> >> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >>
> >> Add new hw_params flag to explicitly tell driver that rewinds will never
> >> be used. This can be used by low-level driver to optimize DMA operations
> >> and reduce power consumption. Use this flag only when data written in
> >> ring buffer will never be invalidated, e.g. any update of appl_ptr is
> >> final.
> >>
> >> Note that the update of appl_ptr include both a read/write data
> >> operation as well as snd_pcm_forward() whose behavior is not modified.
> >>
> >> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >> Signed-off-by: Ramesh Babu <ramesh.babu@intel.com>
> >> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> >> ---
> >>
> >> v2 -> v3
> >> 	return error for rewind operation if no_rewinds set.
> >
> > Now I'm thinking whether this change is really needed or in the right
> > direction.  I suppose that the driver would change the behavior
> > depending on the flag of this flag.  But then how can application know
> > that it should not rewind on a certain device and set the flag?
> 
> The application (which is in most cases an audio server) *knows* if it
> requires rewinds or not. It's part of its design, with rewinds
> typically disabled if period interrupts are required. It's been that
> way for a number of years now. The use of rewinds is typically
> associated with the combination of a large buffer and no interrupts
> (having either of the two would not require rewinds).
> 
> So the idea is that the application makes a statement that rewinds
> will not be used, and the low-level driver makes use of the
> information to enable whatever optimizations are available at the
> hardware level.

I could imagine that, but the usefulness of such a flag is shown only
with the actual usage.  So I'd say, let's include this together with
the actual user of this flag in the driver side.

The only question at this point is whether this addition of the flag
is appropriate.  Someone may think whether it's only about rewind?
What about forward?  Is hw_params flags the best?

I find it "OK, not very sexy but acceptable" for now.  But I'd like to
hear from other opinions.


> Exposing more information to userspace would quickly lead to a
> confusing decision-making and would require more than just a flag.

Yes, that's another side of the coin, too.


thanks,

Takashi

> > We may, instead, let the driver behaving in the deep buffer mode as
> > default (or turned on/off via the kcontrol).  Then you can simply
> > return an error for the negative appl_ptr change in ack callback --
> > which effectively disables the rewind.
> >
> >
> > thanks,
> >
> > Takashi
> >
> >>  include/sound/pcm.h         | 1 +
> >>  include/uapi/sound/asound.h | 1 +
> >>  sound/core/pcm_native.c     | 8 ++++++++
> >>  3 files changed, 10 insertions(+)
> >>
> >> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> >> index 79fedf517070..c1e2b87cd409 100644
> >> --- a/include/sound/pcm.h
> >> +++ b/include/sound/pcm.h
> >> @@ -368,6 +368,7 @@ struct snd_pcm_runtime {
> >>  	unsigned int rate_num;
> >>  	unsigned int rate_den;
> >>  	unsigned int no_period_wakeup: 1;
> >> +	unsigned int no_rewinds:1;
> >>
> >>  	/* -- SW params -- */
> >>  	int tstamp_mode;		/* mmap timestamp is updated */
> >> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> >> index fd41697cb4d3..c697ff90450d 100644
> >> --- a/include/uapi/sound/asound.h
> >> +++ b/include/uapi/sound/asound.h
> >> @@ -365,6 +365,7 @@ struct snd_pcm_info {
> >>  #define SNDRV_PCM_HW_PARAMS_NORESAMPLE	(1<<0)	/* avoid rate resampling */
> >>  #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER	(1<<1)	/* export buffer */
> >>  #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP	(1<<2)	/* disable period wakeups */
> >> +#define SNDRV_PCM_HW_PARAMS_NO_REWINDS	        (1<<3)	/* disable rewinds */
> >>
> >>  struct snd_interval {
> >>  	unsigned int min, max;
> >> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> >> index bf5d0f2acfb9..6769f4751fa0 100644
> >> --- a/sound/core/pcm_native.c
> >> +++ b/sound/core/pcm_native.c
> >> @@ -554,6 +554,8 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
> >>  	runtime->no_period_wakeup =
> >>  			(params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) &&
> >>  			(params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP);
> >> +	runtime->no_rewinds =
> >> +		(params->flags & SNDRV_PCM_HW_PARAMS_NO_REWINDS) ? 1 : 0;
> >>
> >>  	bits = snd_pcm_format_physical_width(runtime->format);
> >>  	runtime->sample_bits = bits;
> >> @@ -2521,6 +2523,9 @@ static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *subst
> >>  	if (frames == 0)
> >>  		return 0;
> >>
> >> +	if (runtime->no_rewinds)
> >> +		return -ENODEV;
> >> +
> >>  	snd_pcm_stream_lock_irq(substream);
> >>  	ret = do_pcm_hwsync(substream);
> >>  	if (!ret)
> >> @@ -2539,6 +2544,9 @@ static snd_pcm_sframes_t snd_pcm_capture_rewind(struct snd_pcm_substream *substr
> >>  	if (frames == 0)
> >>  		return 0;
> >>
> >> +	if (runtime->no_rewinds)
> >> +		return -ENODEV;
> >> +
> >>  	snd_pcm_stream_lock_irq(substream);
> >>  	ret = do_pcm_hwsync(substream);
> >>  	if (!ret)
> >> --
> >> 1.9.1
> >>
> 

Re: [PATCH v3] ALSA: core: let low-level driver or userspace disable rewinds

[Pierre-Louis Bossart]

On 6/19/17 10:25 AM, Takashi Iwai wrote:
> On Mon, 19 Jun 2017 17:16:28 +0200,
> Pierre-Louis Bossart wrote:
>>
>> On 6/19/17 3:44 AM, Takashi Iwai wrote:
>>> On Mon, 19 Jun 2017 08:22:37 +0200,
>>> Subhransu S. Prusty wrote:
>>>>
>>>> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>>
>>>> Add new hw_params flag to explicitly tell driver that rewinds will never
>>>> be used. This can be used by low-level driver to optimize DMA operations
>>>> and reduce power consumption. Use this flag only when data written in
>>>> ring buffer will never be invalidated, e.g. any update of appl_ptr is
>>>> final.
>>>>
>>>> Note that the update of appl_ptr include both a read/write data
>>>> operation as well as snd_pcm_forward() whose behavior is not modified.
>>>>
>>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>> Signed-off-by: Ramesh Babu <ramesh.babu@intel.com>
>>>> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
>>>> ---
>>>>
>>>> v2 -> v3
>>>> 	return error for rewind operation if no_rewinds set.
>>>
>>> Now I'm thinking whether this change is really needed or in the right
>>> direction.  I suppose that the driver would change the behavior
>>> depending on the flag of this flag.  But then how can application know
>>> that it should not rewind on a certain device and set the flag?
>>
>> The application (which is in most cases an audio server) *knows* if it
>> requires rewinds or not. It's part of its design, with rewinds
>> typically disabled if period interrupts are required. It's been that
>> way for a number of years now. The use of rewinds is typically
>> associated with the combination of a large buffer and no interrupts
>> (having either of the two would not require rewinds).
>>
>> So the idea is that the application makes a statement that rewinds
>> will not be used, and the low-level driver makes use of the
>> information to enable whatever optimizations are available at the
>> hardware level.
>
> I could imagine that, but the usefulness of such a flag is shown only
> with the actual usage.  So I'd say, let's include this together with
> the actual user of this flag in the driver side.
>
> The only question at this point is whether this addition of the flag
> is appropriate.  Someone may think whether it's only about rewind?
> What about forward?  Is hw_params flags the best?

Rewinds is already a problematic feature, e.g. in PulseAudio we had to 
introduce a notion of 'guard band' to avoid rewinding too close to the 
actual hardware pointer and update parts of the ring buffer that have 
already been fetched by a DMA transaction.
Disabling rewinds brings two main benefits
- if you work with small buffers, the DMA/DSP has information on how 
much information it can fetch in each transaction. This was always a 
nightmare for DSP firmware.
- if you work with large buffers, the DMA/DSP can fetch data whenever 
the path to memory is enabled at the system level due to other non-audio 
transfers, thus reducing unnecessary partial wakes in an SoC.
HW flags were chosen mainly because you would not want to change this 
dynamically (which excludes sw_params) and it follows the same logic as 
the no-interrupt flag.


Forward is just fine, it moves the application pointer away from the 
hardware one. What to do with the data in the ring buffer is a different 
story though.


>
> I find it "OK, not very sexy but acceptable" for now.  But I'd like to
> hear from other opinions.
>
>
>> Exposing more information to userspace would quickly lead to a
>> confusing decision-making and would require more than just a flag.
>
> Yes, that's another side of the coin, too.
>
>
> thanks,
>
> Takashi
>
>>> We may, instead, let the driver behaving in the deep buffer mode as
>>> default (or turned on/off via the kcontrol).  Then you can simply
>>> return an error for the negative appl_ptr change in ack callback --
>>> which effectively disables the rewind.
>>>
>>>
>>> thanks,
>>>
>>> Takashi
>>>
>>>>  include/sound/pcm.h         | 1 +
>>>>  include/uapi/sound/asound.h | 1 +
>>>>  sound/core/pcm_native.c     | 8 ++++++++
>>>>  3 files changed, 10 insertions(+)
>>>>
>>>> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
>>>> index 79fedf517070..c1e2b87cd409 100644
>>>> --- a/include/sound/pcm.h
>>>> +++ b/include/sound/pcm.h
>>>> @@ -368,6 +368,7 @@ struct snd_pcm_runtime {
>>>>  	unsigned int rate_num;
>>>>  	unsigned int rate_den;
>>>>  	unsigned int no_period_wakeup: 1;
>>>> +	unsigned int no_rewinds:1;
>>>>
>>>>  	/* -- SW params -- */
>>>>  	int tstamp_mode;		/* mmap timestamp is updated */
>>>> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
>>>> index fd41697cb4d3..c697ff90450d 100644
>>>> --- a/include/uapi/sound/asound.h
>>>> +++ b/include/uapi/sound/asound.h
>>>> @@ -365,6 +365,7 @@ struct snd_pcm_info {
>>>>  #define SNDRV_PCM_HW_PARAMS_NORESAMPLE	(1<<0)	/* avoid rate resampling */
>>>>  #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER	(1<<1)	/* export buffer */
>>>>  #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP	(1<<2)	/* disable period wakeups */
>>>> +#define SNDRV_PCM_HW_PARAMS_NO_REWINDS	        (1<<3)	/* disable rewinds */
>>>>
>>>>  struct snd_interval {
>>>>  	unsigned int min, max;
>>>> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
>>>> index bf5d0f2acfb9..6769f4751fa0 100644
>>>> --- a/sound/core/pcm_native.c
>>>> +++ b/sound/core/pcm_native.c
>>>> @@ -554,6 +554,8 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
>>>>  	runtime->no_period_wakeup =
>>>>  			(params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) &&
>>>>  			(params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP);
>>>> +	runtime->no_rewinds =
>>>> +		(params->flags & SNDRV_PCM_HW_PARAMS_NO_REWINDS) ? 1 : 0;
>>>>
>>>>  	bits = snd_pcm_format_physical_width(runtime->format);
>>>>  	runtime->sample_bits = bits;
>>>> @@ -2521,6 +2523,9 @@ static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *subst
>>>>  	if (frames == 0)
>>>>  		return 0;
>>>>
>>>> +	if (runtime->no_rewinds)
>>>> +		return -ENODEV;
>>>> +
>>>>  	snd_pcm_stream_lock_irq(substream);
>>>>  	ret = do_pcm_hwsync(substream);
>>>>  	if (!ret)
>>>> @@ -2539,6 +2544,9 @@ static snd_pcm_sframes_t snd_pcm_capture_rewind(struct snd_pcm_substream *substr
>>>>  	if (frames == 0)
>>>>  		return 0;
>>>>
>>>> +	if (runtime->no_rewinds)
>>>> +		return -ENODEV;
>>>> +
>>>>  	snd_pcm_stream_lock_irq(substream);
>>>>  	ret = do_pcm_hwsync(substream);
>>>>  	if (!ret)
>>>> --
>>>> 1.9.1
>>>>
>>