From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH v3] ALSA: core: let low-level driver or userspace disable rewinds Date: Mon, 19 Jun 2017 10:44:15 +0200 Message-ID: References: <1497853357-2931-1-git-send-email-subhransu.s.prusty@intel.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 6E3A9266851 for ; Mon, 19 Jun 2017 10:44:16 +0200 (CEST) In-Reply-To: <1497853357-2931-1-git-send-email-subhransu.s.prusty@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: "Subhransu S. Prusty" Cc: alsa-devel@alsa-project.org, patches.audio@intel.com, lgirdwood@gmail.com, Ramesh Babu , Pierre-Louis Bossart , broonie@kernel.org List-Id: alsa-devel@alsa-project.org On Mon, 19 Jun 2017 08:22:37 +0200, Subhransu S. Prusty wrote: > > From: Pierre-Louis Bossart > > 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 > Signed-off-by: Ramesh Babu > Signed-off-by: Subhransu S. Prusty > --- > > 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 >