On Wed, 24 May 2017 08:29:28 +0200, Takashi Iwai wrote:
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.
Actually it's even dynamically switched in PCM OSS emulation at each read/write. Having the flag in runtime field would still work because the OSS read/write takes the mutex, but you see that it can be more volatile than one thinks.
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.
... and now reading back your patches, I still don't figure out how you achieve the in-kernel data transfer via your new copy_frames ops. For example, with your patches gus_pcm.c, how OSS or UAC1 gadget would work?
What am I missing?
thanks,
Takashi