| Message ID | 1494896518-23399-2-git-send-email-subhransu.s.prusty@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 16 May 2017 03:01:56 +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> > --- > 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 361749e60799..a2682c5f5b72 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 13dec5ec93f2..35dd4ca93f84 100644 > --- a/sound/core/pcm_native.c > +++ b/sound/core/pcm_native.c > @@ -566,6 +566,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; > @@ -2439,6 +2441,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 0; I'd return an error here, because it is a clear error -- you declared that you won't do it but you did. Also, I wonder whether we should add FORWARD disablement flag as well. It's not needed in your case, yes, but FORWARD and REWIND operations are usually paired. thanks, Takashi
On Tue, May 16, 2017 at 07:53:38AM +0200, Takashi Iwai wrote: > On Tue, 16 May 2017 03:01:56 +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> > > --- > > 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 361749e60799..a2682c5f5b72 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 13dec5ec93f2..35dd4ca93f84 100644 > > --- a/sound/core/pcm_native.c > > +++ b/sound/core/pcm_native.c > > @@ -566,6 +566,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; > > @@ -2439,6 +2441,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 0; > > I'd return an error here, because it is a clear error -- you declared > that you won't do it but you did. This is done based on the suggestion from Pierre: "This forces the application to both set the NOREWIND flag and change the error handling code on a rewind, when the latter point is unnecessary. if no rewind is performed then there is no harm." > > Also, I wonder whether we should add FORWARD disablement flag as > well. It's not needed in your case, yes, but FORWARD and REWIND > operations are usually paired. Either ways is ok for us. Please suggest the approach. Regards, Subhransu > > > thanks, > > Takashi
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 361749e60799..a2682c5f5b72 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 13dec5ec93f2..35dd4ca93f84 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -566,6 +566,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; @@ -2439,6 +2441,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 0; + snd_pcm_stream_lock_irq(substream); switch (runtime->status->state) { case SNDRV_PCM_STATE_PREPARED: @@ -2487,6 +2492,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 0; + snd_pcm_stream_lock_irq(substream); switch (runtime->status->state) { case SNDRV_PCM_STATE_PREPARED: