On Wed, 24 May 2017 02:52:44 +0200, Takashi Sakamoto wrote:
Hi,
This RFC patchset is my concept work of another solution against a series of issues in ALSA PCM core, for which Iwai Takashi work in his hasty proposals[2][3]. Included changes are also available in my personal repository on github.com[4].
The aim of this work is to declare existent issues and to propose solutions as moderated as possible. Code refactoring is done to assist it.
Patch 01-05: code refactoring phase, with an alias of function prototype. Patch 06-09: driver API changing phase. The .copy_frames is introduced. Patch 10-11: integration phase for proxy drivers, to get new configuration.
I need to do this work with a little time (half a day), thus changes are as minimal as possible, especially two drivers are just modified even if drivers in below list should be changed:
- drivers/media/pci/solo6x10/solo6x10-g723.c
- sound/drivers/dummy.c
- sound/isa/gus/gus_pcm.c
- sound/isa/sb/emu8000_pcm.c
- sound/pci/es1938.c
- sound/pci/korg1212/korg1212.c
- sound/pci/nm256/nm256.c
- sound/pci/rme32.c
- sound/pci/rme96.c
- sound/pci/rme9652/hdsp.c
- sound/pci/rme9652/rme9652.c
- sound/sh/sh_dac_audio.c
- sound/soc/blackfin/bf5xx-ac97-pcm.c
- sound/soc/blackfin/bf5xx-i2s-pcm.c
Furthermore, I apologize that this is untested. I wish you to check the concept.
OK, I now understand your idea. Actually the word "proxy" was misleading. "proxy" implies the action, but PCM core can't know the action the caller intended. From PCM core POV, it's merely an in-kernel buffer copy. We don't need to invent new terms here (who is the client?).
Apart from that, basically your idea is: - Keep in-kernel buffer copy flag into substream->runtime. - The ops handles both copy and silence. - Move the whole transfer action to each driver instead of looping in PCM core.
I find the first point is acceptable, from the current usage pattern, as there is little chance to mix up both user-space buffer and kernel-space buffer.
Though, another side of coin is that, by embedding in runtime, you can easily overlook the in-kernel copy case, in comparison with passing via the argument. So it's not always plus.
The second point is as same as mine.
The potential problem is the third point. It'd require a larger change, and it's error-prone, because you'll have to translate the passed pointer in each driver to morph it to different values, either the linear buffer pointer or the vector, in addition to the user-space and kernel-space check.
In the PCM core code, we did that in that way, but it's OK since it's done only there thus it's manageable. However, forcing the complex cast in each driver implementation is better to avoid.
How to make each driver implementing less error-prone codes -- that's what we need to reconsider further.
thanks,
Takashi
[1] [alsa-devel] [PATCH RFC 00/26] Kill set_fs() in ALSA codes http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/120542 [2] [alsa-devel] [PATCH 00/16] ALSA: Convert to new copy_silence PCM ops http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/120895.html [3] [alsa-devel] [PATCH 0/4] ALSA: Extend PCM buffer-copy helpers http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/120957.html [4] takaswie/sound https://github.com/takaswie/sound/tree/topic/add-pcm-frame-ops
Takashi Sakamoto (11): ALSA: pcm: introduce an alias of prototype to copy PCM frames ALSA: pcm: use new function alias instead of local one ALSA: pcm: refactoring silence operation ALSA: pcm: add alternative calls ALSA: core: remove duplicated codes to handle PCM frames ALSA: pcm: add new operation; copy_frames ALSA: rme9652: a sample for drivers which support interleaved buffer ALSA: gus: a sample for drivers which support non-interleaved buffer ALSA: pcm: remove copy and silence callbacks ALSA: pcm: add client_space parameter to runtime of PCM substream for PCM proxy drivers ALSA: pcm: add new configuration for PCM proxy drivers
drivers/usb/gadget/Kconfig | 3 +- include/sound/pcm.h | 13 +- sound/core/Kconfig | 15 +- sound/core/pcm_lib.c | 448 +++++++++++++++++++++++++++++---------------- sound/core/pcm_native.c | 10 + sound/isa/gus/gus_pcm.c | 100 +++++----- sound/pci/rme9652/hdsp.c | 59 +++--- 7 files changed, 398 insertions(+), 250 deletions(-)
-- 2.11.0