| Message ID | 1475239410-16548-2-git-send-email-subhransu.s.prusty@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 30 Sep 2016 14:43:24 +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. > > Caveat: there is currently no way to query capabilities without opening > a pcm stream, so applications might need to serially open all exposed > devices, check what they support by looking at hw_params->info and close > them (this is what PulseAudio does so might not be an issue) This is a general issue in the current PCM API design, and not specific to this new bit. > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > Signed-off-by: Ramesh Babu <ramesh.babu@intel.com> > Signed-off-by: Vinod Koul <vinod.koul@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 af1fb37..5344c16 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 609cadb..6828ed2 100644 > --- a/include/uapi/sound/asound.h > +++ b/include/uapi/sound/asound.h > @@ -362,6 +362,7 @@ typedef int snd_pcm_hw_param_t; > #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 c61fd50..be8617b 100644 > --- a/sound/core/pcm_native.c > +++ b/sound/core/pcm_native.c > @@ -565,6 +565,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; > @@ -2438,6 +2440,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; Better to return an error instead? > snd_pcm_stream_lock_irq(substream); > switch (runtime->status->state) { > case SNDRV_PCM_STATE_PREPARED: > @@ -2486,6 +2491,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; Ditto. Takashi
On Fri, Sep 30, 2016 at 03:22:40PM +0200, Takashi Iwai wrote: > On Fri, 30 Sep 2016 14:43:24 +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. > > > > Caveat: there is currently no way to query capabilities without opening > > a pcm stream, so applications might need to serially open all exposed > > devices, check what they support by looking at hw_params->info and close > > them (this is what PulseAudio does so might not be an issue) > > This is a general issue in the current PCM API design, and not > specific to this new bit. Will remove this from commit message. > > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > > Signed-off-by: Ramesh Babu <ramesh.babu@intel.com> > > Signed-off-by: Vinod Koul <vinod.koul@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 af1fb37..5344c16 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 609cadb..6828ed2 100644 > > --- a/include/uapi/sound/asound.h > > +++ b/include/uapi/sound/asound.h > > @@ -362,6 +362,7 @@ typedef int snd_pcm_hw_param_t; > > #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 c61fd50..be8617b 100644 > > --- a/sound/core/pcm_native.c > > +++ b/sound/core/pcm_native.c > > @@ -565,6 +565,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; > > @@ -2438,6 +2440,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; > > Better to return an error instead? As the number of frames rewinded is zero, it looks appropriate. Any reason why returning an error code would help? Regards, Subhransu
>>> @@ -2438,6 +2440,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; >> >> Better to return an error instead? > > As the number of frames rewinded is zero, it looks appropriate. Any reason > why returning an error code would help? The return type is also snd_sframes_t, not sure how a error code would be returned without additional changes.
Hi, sorry for the late response. I've been slowly catching up the pending stuff... On Mon, 03 Oct 2016 16:39:41 +0200, Pierre-Louis Bossart wrote: > > > >>> @@ -2438,6 +2440,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; > >> > >> Better to return an error instead? > > > > As the number of frames rewinded is zero, it looks appropriate. Any reason > > why returning an error code would help? Well, this is an operation that is not supposed to work. The configuration declares that there shall be no rewind, right? > The return type is also snd_sframes_t, not sure how a error code would > be returned without additional changes. snd_pcm_sframes_t is a signed type, and a negative value is taken as an error by ioctl. thanks, Takashi
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index af1fb37..5344c16 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 609cadb..6828ed2 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -362,6 +362,7 @@ typedef int snd_pcm_hw_param_t; #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 c61fd50..be8617b 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -565,6 +565,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; @@ -2438,6 +2440,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: @@ -2486,6 +2491,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: