On Mon, 18 Nov 2019 17:24:45 +0100, Pierre-Louis Bossart wrote:
On 11/17/19 2:53 AM, Takashi Iwai wrote:
This patch adds the support for the feature to automatically allocate and free PCM buffers, so called "managed buffer allocation" mode. It's set up via new PCM helpers, snd_pcm_set_managed_buffer() and snd_pcm_set_managed_buffer_all(), both of which correspond to the existing preallocator helpers, snd_pcm_lib_preallocate_pages() and snd_pcm_lib_preallocate_pages_for_all(). When the new helper is used, it not only performs the pre-allocation of buffers, but also it manages to call snd_pcm_lib_malloc_pages() before the PCM hw_params ops and snd_lib_pcm_free() after the PCM hw_free ops inside PCM core, respectively. This allows drivers to drop the explicit calls of the memory allocation / release functions, and it will be a good amount of code reduction in the end of this patch series.
When the PCM substream is set to the managed buffer allocation mode, the managed_buffer_alloc flag is set in the substream object. Since some drivers want to know when a buffer is newly allocated or re-allocated at hw_params callback (e.g. want to set up the additional stuff for the given buffer only at allocation time), now PCM core turns on buffer_changed flag when the buffer has changed.
I am a bit lost on the directions: a) is this introducing a new API that will eventually replace the existing one? b) or are we going to have two options, managed and non-managed buffers? in this case, what would drive an implementer to keep using non-managed buffers? It'd be useful to provide examples.
In the cover letter, the wording 'almost all drivers' is used, which leads be to think option b) it is, but the exceptions are not clear to me.
Yes, it's currently it's (b), the new API is completely optional and the driver may keep the old API. Actually in my upcoming patchset (see below), only three drivers are left with the old API due to the special buffer handling and all the rest are replaced with the new API.
You can find the conversion results in topic/pcm-managed branch. As seen in the patch diff below, it's a long series.
thanks,
Takashi
.../sound/kernel-api/writing-an-alsa-driver.rst | 148 +++++++++++++++------ .../gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 48 ++++--- drivers/media/pci/cobalt/cobalt-alsa-pcm.c | 61 +-------- drivers/media/pci/cx18/cx18-alsa-pcm.c | 62 +-------- drivers/media/pci/ivtv/ivtv-alsa-pcm.c | 63 +-------- drivers/media/pci/solo6x10/solo6x10-g723.c | 23 +--- drivers/media/pci/tw686x/tw686x-audio.c | 15 +-- drivers/media/usb/cx231xx/cx231xx-audio.c | 78 +---------- drivers/media/usb/em28xx/em28xx-audio.c | 86 +----------- drivers/media/usb/go7007/snd-go7007.c | 24 +--- drivers/media/usb/tm6000/tm6000-alsa.c | 81 +---------- drivers/media/usb/usbtv/usbtv-audio.c | 28 +--- drivers/staging/most/sound/sound.c | 44 +----- .../vc04_services/bcm2835-audio/bcm2835-pcm.c | 17 +-- drivers/usb/gadget/function/u_audio.c | 18 +-- include/sound/core.h | 1 + include/sound/pcm.h | 52 ++------ sound/aoa/soundbus/i2sbus/pcm.c | 11 +- sound/arm/aaci.c | 40 +++--- sound/atmel/ac97c.c | 20 +-- sound/core/init.c | 1 + sound/core/pcm_local.h | 2 + sound/core/pcm_memory.c | 143 ++++++++++---------- sound/core/pcm_native.c | 48 ++++++- sound/drivers/aloop.c | 12 +- sound/drivers/dummy.c | 14 +- sound/drivers/ml403-ac97cr.c | 29 +--- sound/drivers/pcsp/pcsp_lib.c | 17 +-- sound/drivers/vx/vx_pcm.c | 27 +--- sound/firewire/bebob/bebob_pcm.c | 11 +- sound/firewire/dice/dice-pcm.c | 13 +- sound/firewire/digi00x/digi00x-pcm.c | 11 +- sound/firewire/fireface/ff-pcm.c | 9 +- sound/firewire/fireworks/fireworks_pcm.c | 11 +- sound/firewire/isight.c | 10 +- sound/firewire/motu/motu-pcm.c | 11 +- sound/firewire/oxfw/oxfw-pcm.c | 17 +-- sound/firewire/tascam/tascam-pcm.c | 11 +- sound/isa/ad1816a/ad1816a_lib.c | 20 +-- sound/isa/cmi8330.c | 5 +- sound/isa/es1688/es1688_lib.c | 20 +-- sound/isa/es18xx.c | 24 +--- sound/isa/gus/gus_pcm.c | 28 ++-- sound/isa/sb/sb16_main.c | 21 +-- sound/isa/sb/sb8_main.c | 21 +-- sound/isa/wss/wss_lib.c | 23 +--- sound/mips/hal2.c | 25 +--- sound/mips/sgio2audio.c | 20 +-- sound/parisc/harmony.c | 18 +-- sound/pci/ad1889.c | 24 +--- sound/pci/ali5451/ali5451.c | 30 +---- sound/pci/als300.c | 23 +--- sound/pci/als4000.c | 23 +--- sound/pci/asihpi/asihpi.c | 10 +- sound/pci/atiixp.c | 14 +- sound/pci/atiixp_modem.c | 9 +- sound/pci/au88x0/au88x0_pcm.c | 15 +-- sound/pci/aw2/aw2-alsa.c | 45 ++----- sound/pci/azt3328.c | 30 +---- sound/pci/bt87x.c | 14 +- sound/pci/ca0106/ca0106_main.c | 56 +------- sound/pci/cmipci.c | 36 ++--- sound/pci/cs4281.c | 20 +-- sound/pci/cs5535audio/cs5535audio_pcm.c | 12 +- sound/pci/ctxfi/ctpcm.c | 15 +-- sound/pci/echoaudio/echoaudio.c | 19 +-- sound/pci/emu10k1/emu10k1x.c | 15 +-- sound/pci/emu10k1/emupcm.c | 41 +----- sound/pci/emu10k1/p16v.c | 48 +------ sound/pci/ens1370.c | 27 +--- sound/pci/es1938.c | 28 +--- sound/pci/fm801.c | 20 +-- sound/pci/hda/hda_controller.c | 13 +- sound/pci/ice1712/ice1712.c | 42 ++---- sound/pci/ice1712/ice1724.c | 25 ++-- sound/pci/intel8x0.c | 11 +- sound/pci/intel8x0m.c | 23 +--- sound/pci/lola/lola_pcm.c | 11 +- sound/pci/lx6464es/lx6464es.c | 14 +- sound/pci/maestro3.c | 9 +- sound/pci/mixart/mixart.c | 14 +- sound/pci/oxygen/oxygen_pcm.c | 52 ++++---- sound/pci/pcxhr/pcxhr.c | 31 +---- sound/pci/riptide/riptide.c | 10 +- sound/pci/rme32.c | 35 +---- sound/pci/sis7019.c | 25 +--- sound/pci/sonicvibes.c | 20 +-- sound/pci/trident/trident_main.c | 49 +++---- sound/pci/via82xx.c | 45 +++---- sound/pci/via82xx_modem.c | 9 +- sound/pci/ymfpci/ymfpci_main.c | 33 ++--- sound/pcmcia/pdaudiocf/pdaudiocf_pcm.c | 25 +--- sound/ppc/pmac.c | 28 +--- sound/ppc/snd_ps3.c | 28 +--- sound/sh/aica.c | 29 +--- sound/sh/sh_dac_audio.c | 20 +-- sound/soc/amd/acp-pcm-dma.c | 58 +++----- sound/soc/amd/raven/acp3x-pcm-dma.c | 30 +---- sound/soc/au1x/dbdma2.c | 14 +- sound/soc/au1x/dma.c | 21 +-- sound/soc/codecs/cros_ec_codec.c | 8 +- sound/soc/codecs/rt5514-spi.c | 10 +- sound/soc/codecs/rt5677-spi.c | 10 +- sound/soc/dwc/dwc-pcm.c | 24 +--- sound/soc/intel/atom/sst-mfld-platform-pcm.c | 25 +--- sound/soc/intel/baytrail/sst-baytrail-pcm.c | 19 +-- sound/soc/intel/haswell/sst-haswell-pcm.c | 17 +-- sound/soc/intel/skylake/skl-pcm.c | 26 +--- sound/soc/mediatek/common/mtk-afe-fe-dai.c | 14 +- sound/soc/mediatek/common/mtk-afe-fe-dai.h | 2 - .../soc/mediatek/common/mtk-afe-platform-driver.c | 12 +- .../soc/mediatek/common/mtk-afe-platform-driver.h | 2 - sound/soc/mediatek/mt2701/mt2701-afe-pcm.c | 2 - sound/soc/mediatek/mt6797/mt6797-afe-pcm.c | 1 - sound/soc/mediatek/mt8183/mt8183-afe-pcm.c | 1 - sound/soc/meson/axg-fifo.c | 13 +- sound/soc/sh/dma-sh7760.c | 14 +- sound/soc/sh/fsi.c | 18 +-- sound/soc/sh/rcar/core.c | 23 +--- sound/soc/sh/siu_pcm.c | 39 +----- sound/soc/soc-generic-dmaengine-pcm.c | 12 +- sound/soc/sof/pcm.c | 34 ++--- sound/soc/stm/stm32_adfsdm.c | 29 +--- sound/soc/txx9/txx9aclc.c | 14 +- sound/soc/uniphier/aio-dma.c | 30 +---- sound/soc/xilinx/xlnx_formatter_pcm.c | 13 +- sound/soc/xtensa/xtfpga-i2s.c | 9 +- sound/sparc/amd7930.c | 20 +-- sound/sparc/cs4231.c | 17 +-- sound/sparc/dbri.c | 13 +- sound/spi/at73c213.c | 11 +- sound/usb/6fire/pcm.c | 17 +-- sound/usb/caiaq/audio.c | 13 +- sound/usb/hiface/pcm.c | 18 +-- sound/usb/line6/pcm.c | 13 +- sound/usb/misc/ua101.c | 23 +--- sound/usb/pcm.c | 15 +-- sound/usb/usx2y/usbusx2yaudio.c | 26 ++-- sound/usb/usx2y/usx2yhwdeppcm.c | 18 +-- sound/x86/intel_hdmi_audio.c | 16 +-- 140 files changed, 781 insertions(+), 2700 deletions(-)