[alsa-devel] [PATCH 0/3] ALSA: Add the explicit appl_ptr sync support

[Takashi Iwai]

On Thu, 22 Jun 2017 17:35:34 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Jun 22 2017 17:44, Takashi Iwai wrote:
> >>> Currently alsa-lib issues sync_ptr() also for obtaining the PCM state
> >>> from the kernel, and this also triggers ack unless SYNC_APPL_PTR flag
> >>> is set.  The commit f8ff2f28ba49 tried to reduce the ack calls  by
> >>> filtering in pcm_lib_apply_appl_ptr() side, but it has a side-effect
> >>> as you noticed.
> >>>
> >>> There can be a few options to cover this for the newer alsa-lib, for
> >>> example:
> >>>
> >>> A. Add a new bit flag to sync_ptr() allowing only the state update
> >>>      and/or the explicit applptr update.
> >>>
> >>> B. Introduce a completely new ioctl for the appl_ptr sync.
> >>>
> >>> C. Keep disabling control mmap while allowing state mmap.
> >>>
> >>> D. Track the last appl_ptr individually from runtime->control->appl_ptr
> >>>      and compare/update with this value in pcm_lib_apply_appl_ptr().
> >>>
> >>> E. Remove the filtering from pcm_lib_apply_appl_ptr(), let each driver
> >>>      optimize if needed.
> >>>
> >>> Currently I'm thinking of (A), but open for all and other possible
> >>> options.  All these may need the check of user_pversion so that the
> >>> old code still works as is.
> >>
> >> I prefer C) and I'm working for it, because protocol validation of
> >> userland is not needed in kernel land, as long as I understand. But if
> >> allowed to add more changes to protocol/interface, A) and B) could be
> >> within my candidates.
> >
> > (C) was my original idea, too, so it doesn't sound bad to me, too :)
> > But, we're bumping the protocol version, so more intensive changes
> > should be OK.  Let's see.
> 
> I posted patchset for alsa-lib for your C) idea. In this patchset,
> mapping for status/control data is processed independently,
> 
> [alsa-devel] [alsa-lib][RFC][PATCH 0/9] pcm: handle status/control
> mapping independently
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/122130.html

The patchset looks good through a quick glance.
I'd need to test it more intensively later, but I'm basically fine to
go in that direction.  In general, this change wouldn't conflict with
other improvements.


> Additionally, some helper functions to call ioctl(2) with
> SNDRV_PCM_IOCTL_SYNC_PTR for several purposes. I think to get help
> from it for our arrangement to A) and B).
> 
> I agree with the idea in A) to add a flag to update state only. For
> the other probabilities, I need time to consider about it.

Yeah, let's take some time.

Meanwhile, I'd like to merge the basic part in order to make
maintenance easier.  Let's sort out which ones to be applied right
now.

At least, the following one should be OK:

  [PATCH v2 1/3] ALSA: pcm: Add the explicit appl_ptr sync support

May I re-apply your reviewed-by tag for this?

For alsa-lib side, a few of yours and my patches are merely cleanups,
so we should apply them beforehand, too.

  [PATCH alsa-lib 3/4] pcm: hw: Remove superfluous call of snd_pcm_set_appl_ptr()
  [alsa-lib][RFC][PATCH 1/9] pcm: obsolete 'mmap_emulation' parameter of snd_pcm_hw_open_fd()
  [alsa-lib][RFC][PATCH 2/9] pcm: minor code refactoring for ioctl call


thanks,

Takashi