[alsa-devel] [PATCH RFC 00/11] ALSA: pcm: introduce copy_frame operation and SND_PCM_PROXY_DRIVER_SUPPORT

[Takashi Iwai]

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