From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [RFC PATCH 2/4] ALSA: core: add .notify callback for pcm ops Date: Wed, 08 Jul 2015 16:27:09 +0200 Message-ID: References: <1436350236-17509-1-git-send-email-pierre-louis.bossart@linux.intel.com> <1436350236-17509-3-git-send-email-pierre-louis.bossart@linux.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 mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 253D926154E for ; Wed, 8 Jul 2015 16:27:10 +0200 (CEST) In-Reply-To: <1436350236-17509-3-git-send-email-pierre-louis.bossart@linux.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: Pierre-Louis Bossart Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org On Wed, 08 Jul 2015 12:10:34 +0200, Pierre-Louis Bossart wrote: > > When appl_ptr is updated let low-level driver know. > > This is only enabled when the NO_REWIND hardware flag is used, > so that the low-level driver/hardware to opportunistically pre-fetch > data. > > FIXME: should we rely on .ack for this? > Signed-off-by: Pierre-Louis Bossart Hmm, OK, so the forward is allowed but with workarounds... But then why rewind won't work in a similar way? DSP might be able to cancel some of inflight data. In other words, I see no reason to strict notify callback only for no_rewinds. This is an optional ops in anyway. Also, I find the name "notify" a bit too ambiguous. In this case, it's notifying the applptr change. So, a name related with the function would be more understandable. thanks, Takashi > --- > include/sound/pcm.h | 2 ++ > sound/core/pcm_native.c | 18 +++++++++++++++++- > 2 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/include/sound/pcm.h b/include/sound/pcm.h > index 25310b7..d5eff03 100644 > --- a/include/sound/pcm.h > +++ b/include/sound/pcm.h > @@ -87,6 +87,8 @@ struct snd_pcm_ops { > unsigned long offset); > int (*mmap)(struct snd_pcm_substream *substream, struct vm_area_struct *vma); > int (*ack)(struct snd_pcm_substream *substream); > + int (*notify)(struct snd_pcm_substream *substream); > + /* FIXME: what's the difference between ack and notify ? */ > }; > > /* > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c > index a70e52d..dd519b8 100644 > --- a/sound/core/pcm_native.c > +++ b/sound/core/pcm_native.c > @@ -2565,6 +2565,10 @@ static snd_pcm_sframes_t snd_pcm_playback_forward(struct snd_pcm_substream *subs > appl_ptr -= runtime->boundary; > runtime->control->appl_ptr = appl_ptr; > ret = frames; > + > + if (runtime->no_rewinds && substream->ops->notify) > + substream->ops->notify(substream); > + > __end: > snd_pcm_stream_unlock_irq(substream); > return ret; > @@ -2614,6 +2618,10 @@ static snd_pcm_sframes_t snd_pcm_capture_forward(struct snd_pcm_substream *subst > appl_ptr -= runtime->boundary; > runtime->control->appl_ptr = appl_ptr; > ret = frames; > + > + if (runtime->no_rewinds && substream->ops->notify) > + substream->ops->notify(substream); > + > __end: > snd_pcm_stream_unlock_irq(substream); > return ret; > @@ -2713,8 +2721,16 @@ static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream, > return err; > } > snd_pcm_stream_lock_irq(substream); > - if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) > + if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) { > + /* FIXME: this code is used by mmap_commit, should it handle boundary > + * wrap-around as done for read/write in pcm_lib.c > + */ > + > control->appl_ptr = sync_ptr.c.control.appl_ptr; > + /* if supported, let low-level driver know about appl_ptr change */ > + if (runtime->no_rewinds && substream->ops->notify) > + substream->ops->notify(substream); > + } > else > sync_ptr.c.control.appl_ptr = control->appl_ptr; > if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_AVAIL_MIN)) > -- > 1.9.1 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >