Hi,
On Jun 23 2017 01:06, Takashi Iwai wrote:
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-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.
Thanks for your positive comment. I'm preparing for new patchset with my
further self-check.
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?
I did it just now. Let's merge it.
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
I'm OK to apply the cleanups. I already reviewed the first patch so
let's merge it. I'll post the rest of two without RFC rebasing to
current master a few hours later.
Thanks
Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
