From f02f2f1bf9d154148325eb60d74bdf199022ea52 Mon Sep 17 00:00:00 2001 From: Len Baker Date: Sun, 19 Sep 2021 15:37:27 +0200 Subject: ALSA: usx2y: Prefer struct_size over open coded arithmetic As noted in the "Deprecated Interfaces, Language Features, Attributes, and Conventions" documentation [1], size calculations (especially multiplication) should not be performed in memory allocator (or similar) function arguments due to the risk of them overflowing. This could lead to values wrapping around and a smaller allocation being made than the caller was expecting. Using those allocations could lead to linear overflows of heap memory and other misbehaviors. In this case this is not actually dynamic size: all the operands involved in the calculation are constant values. However it is better to refactor this anyway, just to keep the open-coded math idiom out of code. So, use the struct_size() helper to do the arithmetic instead of the argument "size + size * count" in the kzalloc() function. Also, take the opportunity to refactor the declaration variables to make it more easy to read. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments Signed-off-by: Len Baker Link: https://lore.kernel.org/r/20210919133727.44694-1-len.baker@gmx.com Signed-off-by: Takashi Iwai --- sound/usb/usx2y/usbusx2yaudio.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c index c39cc6851e2d..cfc1ea53978d 100644 --- a/sound/usb/usx2y/usbusx2yaudio.c +++ b/sound/usb/usx2y/usbusx2yaudio.c @@ -668,14 +668,15 @@ static void i_usx2y_04int(struct urb *urb) static int usx2y_rate_set(struct usx2ydev *usx2y, int rate) { - int err = 0, i; - struct snd_usx2y_urb_seq *us = NULL; - int *usbdata = NULL; - const struct s_c2 *ra = rate == 48000 ? setrate_48000 : setrate_44100; + int err = 0, i; + struct snd_usx2y_urb_seq *us = NULL; + int *usbdata = NULL; + const struct s_c2 *ra = rate == 48000 ? setrate_48000 : setrate_44100; struct urb *urb; if (usx2y->rate != rate) { - us = kzalloc(sizeof(*us) + sizeof(struct urb *) * NOOF_SETRATE_URBS, GFP_KERNEL); + us = kzalloc(struct_size(us, urb, NOOF_SETRATE_URBS), + GFP_KERNEL); if (!us) { err = -ENOMEM; goto cleanup; -- cgit v1.2.3 From 882e013a32ecdad7877bfb1669cd51ee052f3369 Mon Sep 17 00:00:00 2001 From: Geraldo Nascimento Date: Fri, 24 Sep 2021 23:33:51 -0300 Subject: ALSA: usb-audio: fix comment reference in __uac_clock_find_source snd_usb_find_clock_source and snd_usb_find_clock_selector are helper macros that look at an entity id and validate that this entity id is in fact a clock source or a clock selector. The present comments inside __uac_clock_find_source give the reader the impression we're looking for an entity id. We're looking for an entity id indeed, the clock source, but since __uac_clock_find_source is recursive, we're also looking *at* the entity ids, in the search for the one clock source. Fix the comment so we don't give readers a wrong idea. Signed-off-by: Geraldo Nascimento Link: https://lore.kernel.org/r/YU6Kj05oOqRmhJDf@geday Signed-off-by: Takashi Iwai --- sound/usb/clock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 81d5ce07d548..7dd71d342443 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -271,7 +271,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, return -EINVAL; } - /* first, see if the ID we're looking for is a clock source already */ + /* first, see if the ID we're looking at is a clock source already */ source = snd_usb_find_clock_source(chip, entity_id, proto); if (source) { entity_id = GET_VAL(source, proto, bClockID); @@ -297,7 +297,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, goto find_source; } - /* the entity ID we are looking for is a selector. + /* the entity ID we are looking at is a selector. * find out what it currently selects */ ret = uac_clock_selector_get_val(chip, clock_id); if (ret < 0) { -- cgit v1.2.3 From 1465d06a6d8580e73ae65f8590392df58c5ed2fd Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 24 Sep 2021 14:24:14 -0500 Subject: ALSA: hda: hdac_stream: fix potential locking issue in snd_hdac_stream_assign() The fields 'opened', 'running', 'assigned_key' are all protected by a spinlock, but the spinlock is not taken when looking for a stream. This can result in a possible race between assign() and release(). Fix by taking the spinlock before walking through the bus stream list. Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20210924192417.169243-2-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/hda/hdac_stream.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c index 1eb8563db2df..9867555883c3 100644 --- a/sound/hda/hdac_stream.c +++ b/sound/hda/hdac_stream.c @@ -296,6 +296,7 @@ struct hdac_stream *snd_hdac_stream_assign(struct hdac_bus *bus, int key = (substream->pcm->device << 16) | (substream->number << 2) | (substream->stream + 1); + spin_lock_irq(&bus->reg_lock); list_for_each_entry(azx_dev, &bus->stream_list, list) { if (azx_dev->direction != substream->stream) continue; @@ -309,13 +310,12 @@ struct hdac_stream *snd_hdac_stream_assign(struct hdac_bus *bus, res = azx_dev; } if (res) { - spin_lock_irq(&bus->reg_lock); res->opened = 1; res->running = 0; res->assigned_key = key; res->substream = substream; - spin_unlock_irq(&bus->reg_lock); } + spin_unlock_irq(&bus->reg_lock); return res; } EXPORT_SYMBOL_GPL(snd_hdac_stream_assign); -- cgit v1.2.3 From 868ddfcef31ff93ea8961b2e81ea7fe12f6f144b Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 24 Sep 2021 14:24:16 -0500 Subject: ALSA: hda: hdac_ext_stream: fix potential locking issues The code for hdac_ext_stream seems inherited from hdac_stream, and similar locking issues are present: the use of the bus->reg_lock spinlock is inconsistent, with only writes to specific fields being protected. Apply similar fix as in hdac_stream by protecting all accesses to 'link_locked' and 'decoupled' fields, with a new helper snd_hdac_ext_stream_decouple_locked() added to simplify code changes. Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20210924192417.169243-4-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- include/sound/hdaudio_ext.h | 2 ++ sound/hda/ext/hdac_ext_stream.c | 46 +++++++++++++++++++++++++---------------- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 375581634143..d4e31ea16aba 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -88,6 +88,8 @@ struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_bus *bus, struct snd_pcm_substream *substream, int type); void snd_hdac_ext_stream_release(struct hdac_ext_stream *azx_dev, int type); +void snd_hdac_ext_stream_decouple_locked(struct hdac_bus *bus, + struct hdac_ext_stream *azx_dev, bool decouple); void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, struct hdac_ext_stream *azx_dev, bool decouple); void snd_hdac_ext_stop_streams(struct hdac_bus *bus); diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c index 0c005d67fa89..37154ed43bd5 100644 --- a/sound/hda/ext/hdac_ext_stream.c +++ b/sound/hda/ext/hdac_ext_stream.c @@ -106,20 +106,14 @@ void snd_hdac_stream_free_all(struct hdac_bus *bus) } EXPORT_SYMBOL_GPL(snd_hdac_stream_free_all); -/** - * snd_hdac_ext_stream_decouple - decouple the hdac stream - * @bus: HD-audio core bus - * @stream: HD-audio ext core stream object to initialize - * @decouple: flag to decouple - */ -void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, - struct hdac_ext_stream *stream, bool decouple) +void snd_hdac_ext_stream_decouple_locked(struct hdac_bus *bus, + struct hdac_ext_stream *stream, + bool decouple) { struct hdac_stream *hstream = &stream->hstream; u32 val; int mask = AZX_PPCTL_PROCEN(hstream->index); - spin_lock_irq(&bus->reg_lock); val = readw(bus->ppcap + AZX_REG_PP_PPCTL) & mask; if (decouple && !val) @@ -128,6 +122,20 @@ void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, snd_hdac_updatel(bus->ppcap, AZX_REG_PP_PPCTL, mask, 0); stream->decoupled = decouple; +} +EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_decouple_locked); + +/** + * snd_hdac_ext_stream_decouple - decouple the hdac stream + * @bus: HD-audio core bus + * @stream: HD-audio ext core stream object to initialize + * @decouple: flag to decouple + */ +void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, + struct hdac_ext_stream *stream, bool decouple) +{ + spin_lock_irq(&bus->reg_lock); + snd_hdac_ext_stream_decouple_locked(bus, stream, decouple); spin_unlock_irq(&bus->reg_lock); } EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_decouple); @@ -252,6 +260,7 @@ hdac_ext_link_stream_assign(struct hdac_bus *bus, return NULL; } + spin_lock_irq(&bus->reg_lock); list_for_each_entry(stream, &bus->stream_list, list) { struct hdac_ext_stream *hstream = container_of(stream, struct hdac_ext_stream, @@ -266,17 +275,16 @@ hdac_ext_link_stream_assign(struct hdac_bus *bus, } if (!hstream->link_locked) { - snd_hdac_ext_stream_decouple(bus, hstream, true); + snd_hdac_ext_stream_decouple_locked(bus, hstream, true); res = hstream; break; } } if (res) { - spin_lock_irq(&bus->reg_lock); res->link_locked = 1; res->link_substream = substream; - spin_unlock_irq(&bus->reg_lock); } + spin_unlock_irq(&bus->reg_lock); return res; } @@ -292,6 +300,7 @@ hdac_ext_host_stream_assign(struct hdac_bus *bus, return NULL; } + spin_lock_irq(&bus->reg_lock); list_for_each_entry(stream, &bus->stream_list, list) { struct hdac_ext_stream *hstream = container_of(stream, struct hdac_ext_stream, @@ -301,18 +310,17 @@ hdac_ext_host_stream_assign(struct hdac_bus *bus, if (!stream->opened) { if (!hstream->decoupled) - snd_hdac_ext_stream_decouple(bus, hstream, true); + snd_hdac_ext_stream_decouple_locked(bus, hstream, true); res = hstream; break; } } if (res) { - spin_lock_irq(&bus->reg_lock); res->hstream.opened = 1; res->hstream.running = 0; res->hstream.substream = substream; - spin_unlock_irq(&bus->reg_lock); } + spin_unlock_irq(&bus->reg_lock); return res; } @@ -378,15 +386,17 @@ void snd_hdac_ext_stream_release(struct hdac_ext_stream *stream, int type) break; case HDAC_EXT_STREAM_TYPE_HOST: + spin_lock_irq(&bus->reg_lock); if (stream->decoupled && !stream->link_locked) - snd_hdac_ext_stream_decouple(bus, stream, false); + snd_hdac_ext_stream_decouple_locked(bus, stream, false); + spin_unlock_irq(&bus->reg_lock); snd_hdac_stream_release(&stream->hstream); break; case HDAC_EXT_STREAM_TYPE_LINK: - if (stream->decoupled && !stream->hstream.opened) - snd_hdac_ext_stream_decouple(bus, stream, false); spin_lock_irq(&bus->reg_lock); + if (stream->decoupled && !stream->hstream.opened) + snd_hdac_ext_stream_decouple_locked(bus, stream, false); stream->link_locked = 0; stream->link_substream = NULL; spin_unlock_irq(&bus->reg_lock); -- cgit v1.2.3 From a20f3b10de61add5e14b6ce4df982f4df2a4cbbc Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 24 Sep 2021 14:24:17 -0500 Subject: ASoC: SOF: Intel: hda-dai: fix potential locking issue The initial hdac_stream code was adapted a third time with the same locking issues. Move the spin_lock outside the loops and make sure the fields are protected on read/write. Signed-off-by: Pierre-Louis Bossart Acked-by: Mark Brown Link: https://lore.kernel.org/r/20210924192417.169243-5-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai --- sound/soc/sof/intel/hda-dai.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index c1f9f0f58464..6704dbcd101c 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -68,6 +68,7 @@ static struct hdac_ext_stream * return NULL; } + spin_lock_irq(&bus->reg_lock); list_for_each_entry(stream, &bus->stream_list, list) { struct hdac_ext_stream *hstream = stream_to_hdac_ext_stream(stream); @@ -107,12 +108,12 @@ static struct hdac_ext_stream * * is updated in snd_hdac_ext_stream_decouple(). */ if (!res->decoupled) - snd_hdac_ext_stream_decouple(bus, res, true); - spin_lock_irq(&bus->reg_lock); + snd_hdac_ext_stream_decouple_locked(bus, res, true); + res->link_locked = 1; res->link_substream = substream; - spin_unlock_irq(&bus->reg_lock); } + spin_unlock_irq(&bus->reg_lock); return res; } -- cgit v1.2.3 From b38269ecd2b2bdd63780b3f7d43c39f924ac515a Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Wed, 29 Sep 2021 14:15:04 -0500 Subject: ALSA: virtio: Replace zero-length array with flexible-array member MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is a regular need in the kernel to provide a way to declare having a dynamically sized set of trailing elements in a structure. Kernel code should always use “flexible array members”[1] for these cases. The older style of one-element or zero-length arrays should no longer be used[2]. Also, make use of the struct_size() helper in kzalloc(). [1] https://en.wikipedia.org/wiki/Flexible_array_member [2] https://www.kernel.org/doc/html/v5.10/process/deprecated.html#zero-length-and-one-element-arrays Link: https://github.com/KSPP/linux/issues/78 Signed-off-by: Gustavo A. R. Silva Link: https://lore.kernel.org/r/20210929191504.GA337268@embeddedor Signed-off-by: Takashi Iwai --- sound/virtio/virtio_pcm_msg.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c index f88c8f29cbd8..aca2dc1989ba 100644 --- a/sound/virtio/virtio_pcm_msg.c +++ b/sound/virtio/virtio_pcm_msg.c @@ -20,7 +20,7 @@ struct virtio_pcm_msg { struct virtio_snd_pcm_xfer xfer; struct virtio_snd_pcm_status status; size_t length; - struct scatterlist sgs[0]; + struct scatterlist sgs[]; }; /** @@ -146,8 +146,7 @@ int virtsnd_pcm_msg_alloc(struct virtio_pcm_substream *vss, int sg_num = virtsnd_pcm_sg_num(data, period_bytes); struct virtio_pcm_msg *msg; - msg = kzalloc(sizeof(*msg) + sizeof(*msg->sgs) * (sg_num + 2), - GFP_KERNEL); + msg = kzalloc(struct_size(msg, sgs, sg_num + 2), GFP_KERNEL); if (!msg) return -ENOMEM; -- cgit v1.2.3 From 46243b85b0ec5d2cee7545e5ce18c015ce91957e Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 09:29:33 +0200 Subject: ALSA: hda: Reduce udelay() at SKL+ position reporting The position reporting on Intel Skylake and later chips via azx_get_pos_skl() contains a udelay(20) call for the capture streams. A call for this alone doesn't sound too harmful. However, as the pointer PCM ops is one of the hottest path in the PCM operations -- especially for the timer-scheduled operations like PulseAudio -- such a delay hogs CPU usage significantly in the total performance. The code there was taken from the original code in ASoC SST Skylake driver blindly. The udelay() is a workaround for the case where the reported position is behind the period boundary at the timing triggered from interrupts; applications often expect that the full data is available for the whole period when returned (and also that's the definition of the ALSA PCM period). OTOH, HD-audio (legacy) driver has already some workarounds for the delayed position reporting due to its relatively large FIFO, such as the BDL position adjustment and the delayed period-elapsed call in the work. That said, the udelay() is almost superfluous for HD-audio driver unlike SST, and we can drop the udelay(). Though, the current code doesn't guarantee the full period readiness as mentioned in the above, but rather it checks the wallclock and detects the unexpected jump. That's one missing piece, and the drop of udelay() needs a bit more sanity checks for the delayed handling. This patch implements those: the drop of udelay() call in azx_get_pos_skl() and the more proper check of hwptr in azx_position_ok(). The latter change is applied only for the case where the stream is running in the normal mode without no_period_wakeup flag. When no_period_wakeup is set, it essentially ignores the period handling and rather concentrates only on the current position; which implies that we don't need to care about the period boundary at all. Fixes: f87e7f25893d ("ALSA: hda - Improved position reporting on SKL+") Reported-by: Jens Axboe Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20210929072934.6809-2-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/pci/hda/hda_intel.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 47777439961c..9989ec4dc324 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -637,13 +637,17 @@ static int azx_position_check(struct azx *chip, struct azx_dev *azx_dev) * the update-IRQ timing. The IRQ is issued before actually the * data is processed. So, we need to process it afterwords in a * workqueue. + * + * Returns 1 if OK to proceed, 0 for delay handling, -1 for skipping update */ static int azx_position_ok(struct azx *chip, struct azx_dev *azx_dev) { struct snd_pcm_substream *substream = azx_dev->core.substream; + struct snd_pcm_runtime *runtime = substream->runtime; int stream = substream->stream; u32 wallclk; unsigned int pos; + snd_pcm_uframes_t hwptr, target; wallclk = azx_readl(chip, WALLCLK) - azx_dev->core.start_wallclk; if (wallclk < (azx_dev->core.period_wallclk * 2) / 3) @@ -680,6 +684,24 @@ static int azx_position_ok(struct azx *chip, struct azx_dev *azx_dev) /* NG - it's below the first next period boundary */ return chip->bdl_pos_adj ? 0 : -1; azx_dev->core.start_wallclk += wallclk; + + if (azx_dev->core.no_period_wakeup) + return 1; /* OK, no need to check period boundary */ + + if (runtime->hw_ptr_base != runtime->hw_ptr_interrupt) + return 1; /* OK, already in hwptr updating process */ + + /* check whether the period gets really elapsed */ + pos = bytes_to_frames(runtime, pos); + hwptr = runtime->hw_ptr_base + pos; + if (hwptr < runtime->status->hw_ptr) + hwptr += runtime->buffer_size; + target = runtime->hw_ptr_interrupt + runtime->period_size; + if (hwptr < target) { + /* too early wakeup, process it later */ + return chip->bdl_pos_adj ? 0 : -1; + } + return 1; /* OK, it's fine */ } @@ -874,11 +896,7 @@ static unsigned int azx_get_pos_skl(struct azx *chip, struct azx_dev *azx_dev) if (azx_dev->core.substream->stream == SNDRV_PCM_STREAM_PLAYBACK) return azx_skl_get_dpib_pos(chip, azx_dev); - /* For capture, we need to read posbuf, but it requires a delay - * for the possible boundary overlap; the read of DPIB fetches the - * actual posbuf - */ - udelay(20); + /* read of DPIB fetches the actual posbuf */ azx_skl_get_dpib_pos(chip, azx_dev); return azx_get_pos_posbuf(chip, azx_dev); } -- cgit v1.2.3 From c4ca3871e21fa085096316f5f8d9975cf3dfde1d Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 09:29:34 +0200 Subject: ALSA: hda: Use position buffer for SKL+ again The commit f87e7f25893d ("ALSA: hda - Improved position reporting on SKL+") changed the PCM position report for SKL+ chips to use DPIB, but according to Pierre, DPIB is no best choice for the accurate position reports and it often reports too early. The recommended method is rather the classical position buffer. This patch makes the PCM position reporting on SKL+ back to the position buffer again. Fixes: f87e7f25893d ("ALSA: hda - Improved position reporting on SKL+") Suggested-by: Pierre-Louis Bossart Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20210929072934.6809-3-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/pci/hda/hda_intel.c | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 9989ec4dc324..14298f015fba 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -880,27 +880,6 @@ static int azx_get_delay_from_fifo(struct azx *chip, struct azx_dev *azx_dev, return substream->runtime->delay; } -static unsigned int azx_skl_get_dpib_pos(struct azx *chip, - struct azx_dev *azx_dev) -{ - return _snd_hdac_chip_readl(azx_bus(chip), - AZX_REG_VS_SDXDPIB_XBASE + - (AZX_REG_VS_SDXDPIB_XINTERVAL * - azx_dev->core.index)); -} - -/* get the current DMA position with correction on SKL+ chips */ -static unsigned int azx_get_pos_skl(struct azx *chip, struct azx_dev *azx_dev) -{ - /* DPIB register gives a more accurate position for playback */ - if (azx_dev->core.substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - return azx_skl_get_dpib_pos(chip, azx_dev); - - /* read of DPIB fetches the actual posbuf */ - azx_skl_get_dpib_pos(chip, azx_dev); - return azx_get_pos_posbuf(chip, azx_dev); -} - static void __azx_shutdown_chip(struct azx *chip, bool skip_link_reset) { azx_stop_chip(chip); @@ -1590,7 +1569,7 @@ static void assign_position_fix(struct azx *chip, int fix) [POS_FIX_POSBUF] = azx_get_pos_posbuf, [POS_FIX_VIACOMBO] = azx_via_get_position, [POS_FIX_COMBO] = azx_get_pos_lpib, - [POS_FIX_SKL] = azx_get_pos_skl, + [POS_FIX_SKL] = azx_get_pos_posbuf, [POS_FIX_FIFO] = azx_get_pos_fifo, }; -- cgit v1.2.3 From 4e7cf1fbb34ecb472c073980458cbe413afd4d64 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:36 +0200 Subject: ALSA: usb-audio: Restrict rates for the shared clocks When a single clock source is shared among several endpoints, we have to keep the same rate on all active endpoints as long as the clock is being used. For dealing with such a case, this patch adds one more check in the hw params constraint for the rate to take the shared clocks into account. The current rate is evaluated from the endpoint list that applies the same clock source. BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1190418 Link: https://lore.kernel.org/r/20210929080844.11583-2-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/card.h | 1 + sound/usb/endpoint.c | 21 +++++++++++++++++++++ sound/usb/endpoint.h | 1 + sound/usb/pcm.c | 9 +++++++++ 4 files changed, 32 insertions(+) diff --git a/sound/usb/card.h b/sound/usb/card.h index 5b19901f305a..3329ce710cb9 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -136,6 +136,7 @@ struct snd_usb_endpoint { unsigned int cur_period_frames; unsigned int cur_period_bytes; unsigned int cur_buffer_periods; + unsigned char cur_clock; spinlock_t lock; struct list_head list; diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 533919a28856..29c4865966f5 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -722,6 +722,7 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip, ep->cur_period_frames = params_period_size(params); ep->cur_period_bytes = ep->cur_period_frames * ep->cur_frame_bytes; ep->cur_buffer_periods = params_periods(params); + ep->cur_clock = fp->clock; if (ep->type == SND_USB_ENDPOINT_TYPE_SYNC) endpoint_set_syncinterval(chip, ep); @@ -833,6 +834,7 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip, ep->altsetting = 0; ep->cur_audiofmt = NULL; ep->cur_rate = 0; + ep->cur_clock = 0; ep->iface_ref = NULL; usb_audio_dbg(chip, "EP 0x%x closed\n", ep->ep_num); } @@ -1340,6 +1342,25 @@ unlock: return err; } +/* get the current rate set to the given clock by any endpoint */ +int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock) +{ + struct snd_usb_endpoint *ep; + int rate = 0; + + if (!clock) + return 0; + mutex_lock(&chip->mutex); + list_for_each_entry(ep, &chip->ep_list, list) { + if (ep->cur_clock == clock && ep->cur_rate) { + rate = ep->cur_rate; + break; + } + } + mutex_unlock(&chip->mutex); + return rate; +} + /** * snd_usb_endpoint_start: start an snd_usb_endpoint * diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index a668f675b52b..a1099ec37e1c 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -19,6 +19,7 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep); int snd_usb_endpoint_configure(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep); +int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock); bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep, diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 5dc9266180e3..19392117de9e 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -734,6 +734,7 @@ static int hw_rule_rate(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { struct snd_usb_substream *subs = rule->private; + struct snd_usb_audio *chip = subs->stream->chip; const struct audioformat *fp; struct snd_interval *it = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); unsigned int rmin, rmax, r; @@ -745,6 +746,14 @@ static int hw_rule_rate(struct snd_pcm_hw_params *params, list_for_each_entry(fp, &subs->fmt_list, list) { if (!hw_check_valid_format(subs, params, fp)) continue; + r = snd_usb_endpoint_get_clock_rate(chip, fp->clock); + if (r > 0) { + if (!snd_interval_test(it, r)) + continue; + rmin = min(rmin, r); + rmax = max(rmax, r); + continue; + } if (fp->rate_table && fp->nr_rates) { for (i = 0; i < fp->nr_rates; i++) { r = fp->rate_table[i]; -- cgit v1.2.3 From 86a42ad07905110f82648853c0ea3434b4eab173 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:37 +0200 Subject: ALSA: usb-audio: Fix possible race at sync of urb completions USB-audio driver tries to sync with the clear of all pending URBs in wait_clear_urbs(), and it waits for all bits in active_mask getting cleared. This works fine for the normal operations, but when a stream is managed in the implicit feedback mode, there is still a very thin race window: namely, in snd_complete_usb(), the active_mask bit for the current URB is once cleared before re-submitted in queue_pending_output_urbs(). If wait_clear_urbs() is called during that period, it may pass the test and go forward even though there may be a still pending URB. For covering it, this patch adds a new counter to each endpoint to keep the number of in-flight URBs, and changes wait_clear_urbs() checking this number instead. The counter is decremented at the end of URB complete, hence the reference is kept as long as the URB complete is in process. Link: https://lore.kernel.org/r/20210929080844.11583-3-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/card.h | 1 + sound/usb/endpoint.c | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/sound/usb/card.h b/sound/usb/card.h index 3329ce710cb9..746a765b2437 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -97,6 +97,7 @@ struct snd_usb_endpoint { unsigned int nominal_queue_size; /* total buffer sizes in URBs */ unsigned long active_mask; /* bitmask of active urbs */ unsigned long unlink_mask; /* bitmask of unlinked urbs */ + atomic_t submitted_urbs; /* currently submitted urbs */ char *syncbuf; /* sync buffer for all sync URBs */ dma_addr_t sync_dma; /* DMA address of syncbuf */ diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 29c4865966f5..06241568abf7 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -451,6 +451,7 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep) } set_bit(ctx->index, &ep->active_mask); + atomic_inc(&ep->submitted_urbs); } } @@ -488,6 +489,7 @@ static void snd_complete_urb(struct urb *urb) clear_bit(ctx->index, &ep->active_mask); spin_unlock_irqrestore(&ep->lock, flags); queue_pending_output_urbs(ep); + atomic_dec(&ep->submitted_urbs); /* decrement at last */ return; } @@ -513,6 +515,7 @@ static void snd_complete_urb(struct urb *urb) exit_clear: clear_bit(ctx->index, &ep->active_mask); + atomic_dec(&ep->submitted_urbs); } /* @@ -596,6 +599,7 @@ int snd_usb_add_endpoint(struct snd_usb_audio *chip, int ep_num, int type) ep->type = type; ep->ep_num = ep_num; INIT_LIST_HEAD(&ep->ready_playback_urbs); + atomic_set(&ep->submitted_urbs, 0); is_playback = ((ep_num & USB_ENDPOINT_DIR_MASK) == USB_DIR_OUT); ep_num &= USB_ENDPOINT_NUMBER_MASK; @@ -861,7 +865,7 @@ static int wait_clear_urbs(struct snd_usb_endpoint *ep) return 0; do { - alive = bitmap_weight(&ep->active_mask, ep->nurbs); + alive = atomic_read(&ep->submitted_urbs); if (!alive) break; @@ -1441,6 +1445,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) goto __error; } set_bit(i, &ep->active_mask); + atomic_inc(&ep->submitted_urbs); } usb_audio_dbg(ep->chip, "%d URBs submitted for EP 0x%x\n", -- cgit v1.2.3 From 9c9a3b9da891cc70405a544da6855700eddcbb71 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:38 +0200 Subject: ALSA: usb-audio: Rename early_playback_start flag with lowlatency_playback This is a preparation patch for the upcoming low-latency improvement changes. Rename early_playback_start flag with lowlatency_playback as it's more intuitive. The new flag is basically a reverse meaning. Along with the rename, factor out the code to set the flag to a function. This makes the complex condition checks simpler. Also, the same flag is introduced to snd_usb_endpoint, too, that is carried from the snd_usb_substream flag. Currently the endpoint flag isn't still referred, but will be used in later patches. Link: https://lore.kernel.org/r/20210929080844.11583-4-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/card.h | 3 ++- sound/usb/endpoint.c | 4 ++++ sound/usb/pcm.c | 29 ++++++++++++++++++++--------- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/sound/usb/card.h b/sound/usb/card.h index 746a765b2437..a00caa1db37e 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -126,6 +126,7 @@ struct snd_usb_endpoint { int skip_packets; /* quirks for devices to ignore the first n packets in a stream */ bool implicit_fb_sync; /* syncs with implicit feedback */ + bool lowlatency_playback; /* low-latency playback mode */ bool need_setup; /* (re-)need for configure? */ /* for hw constraints */ @@ -190,7 +191,7 @@ struct snd_usb_substream { } dsd_dop; bool trigger_tstamp_pending_update; /* trigger timestamp being updated from initial estimate */ - bool early_playback_start; /* early start needed for playback? */ + bool lowlatency_playback; /* low-latency playback mode */ struct media_ctl *media_ctl; }; diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 06241568abf7..8e164d71d9ac 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -794,6 +794,10 @@ void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep, { ep->prepare_data_urb = prepare; ep->retire_data_urb = retire; + if (data_subs) + ep->lowlatency_playback = data_subs->lowlatency_playback; + else + ep->lowlatency_playback = false; WRITE_ONCE(ep->data_subs, data_subs); } diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 19392117de9e..4dd7f1c9e2af 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -581,6 +581,22 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) return 0; } +/* check whether early start is needed for playback stream */ +static int lowlatency_playback_available(struct snd_usb_substream *subs) +{ + struct snd_usb_audio *chip = subs->stream->chip; + + if (subs->direction == SNDRV_PCM_STREAM_CAPTURE) + return false; + /* disabled via module option? */ + if (!chip->lowlatency) + return false; + /* too short periods? */ + if (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes) + return false; + return true; +} + /* * prepare callback * @@ -614,13 +630,8 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) subs->period_elapsed_pending = 0; runtime->delay = 0; - /* check whether early start is needed for playback stream */ - subs->early_playback_start = - subs->direction == SNDRV_PCM_STREAM_PLAYBACK && - (!chip->lowlatency || - (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes)); - - if (subs->early_playback_start) + subs->lowlatency_playback = lowlatency_playback_available(subs); + if (!subs->lowlatency_playback) ret = start_endpoints(subs); unlock: @@ -1412,7 +1423,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, subs->trigger_tstamp_pending_update = false; } - if (period_elapsed && !subs->running && !subs->early_playback_start) { + if (period_elapsed && !subs->running && subs->lowlatency_playback) { subs->period_elapsed_pending = 1; period_elapsed = 0; } @@ -1466,7 +1477,7 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea prepare_playback_urb, retire_playback_urb, subs); - if (!subs->early_playback_start && + if (subs->lowlatency_playback && cmd == SNDRV_PCM_TRIGGER_START) { err = start_endpoints(subs); if (err < 0) { -- cgit v1.2.3 From e581f1cec4f899f788f6c9477f805b1d5fef25e2 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:39 +0200 Subject: ALSA: usb-audio: Disable low-latency playback for free-wheel mode The free-wheel stream operation like dmix may not update the appl_ptr appropriately, and it doesn't fit with the low-latency playback mode. Disable the low-latency playback operation when the stream is set up in such a mode. Link: https://lore.kernel.org/r/20210929080844.11583-5-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/pcm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 4dd7f1c9e2af..84b03a32ee23 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -582,7 +582,8 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) } /* check whether early start is needed for playback stream */ -static int lowlatency_playback_available(struct snd_usb_substream *subs) +static int lowlatency_playback_available(struct snd_pcm_runtime *runtime, + struct snd_usb_substream *subs) { struct snd_usb_audio *chip = subs->stream->chip; @@ -591,6 +592,9 @@ static int lowlatency_playback_available(struct snd_usb_substream *subs) /* disabled via module option? */ if (!chip->lowlatency) return false; + /* free-wheeling mode? (e.g. dmix) */ + if (runtime->stop_threshold > runtime->buffer_size) + return false; /* too short periods? */ if (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes) return false; @@ -630,7 +634,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) subs->period_elapsed_pending = 0; runtime->delay = 0; - subs->lowlatency_playback = lowlatency_playback_available(subs); + subs->lowlatency_playback = lowlatency_playback_available(runtime, subs); if (!subs->lowlatency_playback) ret = start_endpoints(subs); -- cgit v1.2.3 From bceee75387554f682638e719d1ea60125ea78cea Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:40 +0200 Subject: ALSA: usb-audio: Disable low-latency mode for implicit feedback sync When a playback stream runs in the implicit feedback mode, its operation is passive and won't start unless the capture packet is received. This behavior contradicts with the low-latency playback mode, and we should turn off lowlatency_playback flag accordingly. In theory, we may take the low-latency mode when the playback-first quirk is set, but it still conflicts with the later operation with the fixed packet numbers, so it's disabled all together for now. Link: https://lore.kernel.org/r/20210929080844.11583-6-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/pcm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 84b03a32ee23..ec7eeb1b82b8 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -595,6 +595,9 @@ static int lowlatency_playback_available(struct snd_pcm_runtime *runtime, /* free-wheeling mode? (e.g. dmix) */ if (runtime->stop_threshold > runtime->buffer_size) return false; + /* implicit feedback mode has own operation mode */ + if (snd_usb_endpoint_implicit_feedback_sink(subs->data_endpoint)) + return false; /* too short periods? */ if (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes) return false; -- cgit v1.2.3 From d215f63d49da9a8803af3e81acd6cad743686573 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:41 +0200 Subject: ALSA: usb-audio: Check available frames for the next packet size This is yet more preparation for the upcoming changes. Extend snd_usb_endpoint_next_packet_size() to check the available frames and return -EAGAIN if the next packet size is equal or exceeds the given size. This will be needed for avoiding XRUN during the low latency operation. As of this patch, avail=0 is passed, i.e. the check is skipped and no behavior change. Link: https://lore.kernel.org/r/20210929080844.11583-7-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/endpoint.c | 51 ++++++++++++++++++++++++++++++++++++--------------- sound/usb/endpoint.h | 3 ++- sound/usb/pcm.c | 2 +- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 8e164d71d9ac..1f757a7eeafe 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -148,18 +148,23 @@ int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep) * This won't be used for implicit feedback which takes the packet size * returned from the sync source */ -static int slave_next_packet_size(struct snd_usb_endpoint *ep) +static int slave_next_packet_size(struct snd_usb_endpoint *ep, + unsigned int avail) { unsigned long flags; + unsigned int phase; int ret; if (ep->fill_max) return ep->maxframesize; spin_lock_irqsave(&ep->lock, flags); - ep->phase = (ep->phase & 0xffff) - + (ep->freqm << ep->datainterval); - ret = min(ep->phase >> 16, ep->maxframesize); + phase = (ep->phase & 0xffff) + (ep->freqm << ep->datainterval); + ret = min(phase >> 16, ep->maxframesize); + if (avail && ret >= avail) + ret = -EAGAIN; + else + ep->phase = phase; spin_unlock_irqrestore(&ep->lock, flags); return ret; @@ -169,20 +174,25 @@ static int slave_next_packet_size(struct snd_usb_endpoint *ep) * Return the number of samples to be sent in the next packet * for adaptive and synchronous endpoints */ -static int next_packet_size(struct snd_usb_endpoint *ep) +static int next_packet_size(struct snd_usb_endpoint *ep, unsigned int avail) { + unsigned int sample_accum; int ret; if (ep->fill_max) return ep->maxframesize; - ep->sample_accum += ep->sample_rem; - if (ep->sample_accum >= ep->pps) { - ep->sample_accum -= ep->pps; + sample_accum += ep->sample_rem; + if (sample_accum >= ep->pps) { + sample_accum -= ep->pps; ret = ep->packsize[1]; } else { ret = ep->packsize[0]; } + if (avail && ret >= avail) + ret = -EAGAIN; + else + ep->sample_accum = sample_accum; return ret; } @@ -190,16 +200,27 @@ static int next_packet_size(struct snd_usb_endpoint *ep) /* * snd_usb_endpoint_next_packet_size: Return the number of samples to be sent * in the next packet + * + * If the size is equal or exceeds @avail, don't proceed but return -EAGAIN + * Exception: @avail = 0 for skipping the check. */ int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep, - struct snd_urb_ctx *ctx, int idx) + struct snd_urb_ctx *ctx, int idx, + unsigned int avail) { - if (ctx->packet_size[idx]) - return ctx->packet_size[idx]; - else if (ep->sync_source) - return slave_next_packet_size(ep); + unsigned int packet; + + packet = ctx->packet_size[idx]; + if (packet) { + if (avail && packet >= avail) + return -EAGAIN; + return packet; + } + + if (ep->sync_source) + return slave_next_packet_size(ep, avail); else - return next_packet_size(ep); + return next_packet_size(ep, avail); } static void call_retire_callback(struct snd_usb_endpoint *ep, @@ -263,7 +284,7 @@ static void prepare_silent_urb(struct snd_usb_endpoint *ep, unsigned int length; int counts; - counts = snd_usb_endpoint_next_packet_size(ep, ctx, i); + counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, 0); length = counts * ep->stride; /* number of silent bytes */ offset = offs * ep->stride + extra * i; urb->iso_frame_desc[i].offset = offset; diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index a1099ec37e1c..1f1a72535a64 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -46,6 +46,7 @@ void snd_usb_endpoint_free_all(struct snd_usb_audio *chip); int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep); int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep, - struct snd_urb_ctx *ctx, int idx); + struct snd_urb_ctx *ctx, int idx, + unsigned int avail); #endif /* __USBAUDIO_ENDPOINT_H */ diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index ec7eeb1b82b8..8ad48c35c559 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1365,7 +1365,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, spin_lock_irqsave(&subs->lock, flags); subs->frame_limit += ep->max_urb_frames; for (i = 0; i < ctx->packets; i++) { - counts = snd_usb_endpoint_next_packet_size(ep, ctx, i); + counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, 0); /* set up descriptor */ urb->iso_frame_desc[i].offset = frames * stride; urb->iso_frame_desc[i].length = counts * stride; -- cgit v1.2.3 From 0ef74366bc150dda4f53c546dfa6e8f7c707e087 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:42 +0200 Subject: ALSA: usb-audio: Add spinlock to stop_urbs() In theory, stop_urbs() may be called concurrently. Although we have the state check beforehand, it's safer to apply ep->lock during the critical list head manipulations. Link: https://lore.kernel.org/r/20210929080844.11583-8-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/endpoint.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 1f757a7eeafe..c32022672319 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -927,6 +927,7 @@ void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep) static int stop_urbs(struct snd_usb_endpoint *ep, bool force) { unsigned int i; + unsigned long flags; if (!force && atomic_read(&ep->running)) return -EBUSY; @@ -934,9 +935,11 @@ static int stop_urbs(struct snd_usb_endpoint *ep, bool force) if (!ep_state_update(ep, EP_STATE_RUNNING, EP_STATE_STOPPING)) return 0; + spin_lock_irqsave(&ep->lock, flags); INIT_LIST_HEAD(&ep->ready_playback_urbs); ep->next_packet_head = 0; ep->next_packet_queued = 0; + spin_unlock_irqrestore(&ep->lock, flags); for (i = 0; i < ep->nurbs; i++) { if (test_bit(i, &ep->active_mask)) { -- cgit v1.2.3 From d5f871f89e21bb71827ea57bd484eedea85839a0 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:43 +0200 Subject: ALSA: usb-audio: Improved lowlatency playback support This is another attempt to improve further the handling of playback stream in the low latency mode. The latest workaround in commit 4267c5a8f313 ("ALSA: usb-audio: Work around for XRUN with low latency playback") revealed that submitting URBs forcibly in advance may trigger XRUN easily. In the classical mode, this problem was avoided by practically delaying the submission of the actual data with the pre-submissions of silent data before triggering the stream start. But that is exactly what we want to avoid. Now, in this patch, instead of the previous workaround, we take a similar approach as used in the implicit feedback mode. The URBs are queued at the PCM trigger start like before, but we check whether the buffer has been already filled enough before each submission, and stop queuing if the data overcomes the threshold. The remaining URBs are kept in the ready list, and they will be retrieved in the URB complete callback of other (already queued) URBs. In the complete callback, we try to fill the data and submit as much as possible again. When there is no more available in-flight URBs that may handle the pending data, we'll check in PCM ack callback and submit and process URBs there in addition. In this way, the amount of in-flight URBs may vary dynamically and flexibly depending on the available data without hitting XRUN. The following things are changed to achieve the behavior above: * The endpoint prepare callback is changed to return an error code; when there is no enough data available, it may return -EAGAIN. Currently only prepare_playback_urb() returns the error. The evaluation of the available data is a bit messy here; we can't check with snd_pcm_avail() at the point of prepare callback (as runtime->status->hwptr hasn't been updated yet), hence we manually estimate the appl_ptr and compare with the internal hwptr_done to calculate the available frames. * snd_usb_endpoint_start() doesn't submit full URBs if the prepare callback returns -EAGAIN, and puts the remaining URBs to the ready list for the later submission. * snd_complete_urb() treats the URBs in the low-latency mode similarly like the implicit feedback mode, and submissions are done in (now exported) snd_usb_queue_pending_output_urbs(). * snd_usb_queue_pending_output_urbs() again checks the error value from the prepare callback. If it's -EAGAIN for the normal stream (i.e. not implicit feedback mode), we push it back to the ready list again. * PCM ack callback is introduced for the playback stream, and it calls snd_usb_queue_pending_output_urbs() if there is no in-flight URB while the stream is running. This corresponds to the case where the system needs the appl_ptr update for re-submitting a new URB. * snd_usb_queue_pending_output_urbs() and the prepare EP callback receive in_stream_lock argument, which is a bool flag indicating the call path from PCM ack. It's needed for avoiding the deadlock of snd_pcm_period_elapsed() calls. * Set the new SNDRV_PCM_INFO_EXPLICIT_SYNC flag when the new low-latency mode is deployed. This assures catching each applptr update even in the mmap mode. Fixes: 4267c5a8f313 ("ALSA: usb-audio: Work around for XRUN with low latency playback") Link: https://lore.kernel.org/r/20210929080844.11583-9-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/card.h | 6 +-- sound/usb/endpoint.c | 130 ++++++++++++++++++++++++++++++++++----------------- sound/usb/endpoint.h | 7 ++- sound/usb/pcm.c | 102 +++++++++++++++++++++++++++++++--------- 4 files changed, 177 insertions(+), 68 deletions(-) diff --git a/sound/usb/card.h b/sound/usb/card.h index a00caa1db37e..87f042d06ce0 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -74,8 +74,9 @@ struct snd_usb_endpoint { atomic_t state; /* running state */ - void (*prepare_data_urb) (struct snd_usb_substream *subs, - struct urb *urb); + int (*prepare_data_urb) (struct snd_usb_substream *subs, + struct urb *urb, + bool in_stream_lock); void (*retire_data_urb) (struct snd_usb_substream *subs, struct urb *urb); @@ -94,7 +95,6 @@ struct snd_usb_endpoint { struct list_head ready_playback_urbs; /* playback URB FIFO for implicit fb */ unsigned int nurbs; /* # urbs */ - unsigned int nominal_queue_size; /* total buffer sizes in URBs */ unsigned long active_mask; /* bitmask of active urbs */ unsigned long unlink_mask; /* bitmask of unlinked urbs */ atomic_t submitted_urbs; /* currently submitted urbs */ diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index c32022672319..0b336876e36d 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -307,8 +307,9 @@ static void prepare_silent_urb(struct snd_usb_endpoint *ep, /* * Prepare a PLAYBACK urb for submission to the bus. */ -static void prepare_outbound_urb(struct snd_usb_endpoint *ep, - struct snd_urb_ctx *ctx) +static int prepare_outbound_urb(struct snd_usb_endpoint *ep, + struct snd_urb_ctx *ctx, + bool in_stream_lock) { struct urb *urb = ctx->urb; unsigned char *cp = urb->transfer_buffer; @@ -320,9 +321,9 @@ static void prepare_outbound_urb(struct snd_usb_endpoint *ep, case SND_USB_ENDPOINT_TYPE_DATA: data_subs = READ_ONCE(ep->data_subs); if (data_subs && ep->prepare_data_urb) - ep->prepare_data_urb(data_subs, urb); - else /* no data provider, so send silence */ - prepare_silent_urb(ep, ctx); + return ep->prepare_data_urb(data_subs, urb, in_stream_lock); + /* no data provider, so send silence */ + prepare_silent_urb(ep, ctx); break; case SND_USB_ENDPOINT_TYPE_SYNC: @@ -351,13 +352,14 @@ static void prepare_outbound_urb(struct snd_usb_endpoint *ep, break; } + return 0; } /* * Prepare a CAPTURE or SYNC urb for submission to the bus. */ -static inline void prepare_inbound_urb(struct snd_usb_endpoint *ep, - struct snd_urb_ctx *urb_ctx) +static int prepare_inbound_urb(struct snd_usb_endpoint *ep, + struct snd_urb_ctx *urb_ctx) { int i, offs; struct urb *urb = urb_ctx->urb; @@ -382,6 +384,7 @@ static inline void prepare_inbound_urb(struct snd_usb_endpoint *ep, urb->iso_frame_desc[0].offset = 0; break; } + return 0; } /* notify an error as XRUN to the assigned PCM data substream */ @@ -417,6 +420,16 @@ next_packet_fifo_dequeue(struct snd_usb_endpoint *ep) return p; } +static void push_back_to_ready_list(struct snd_usb_endpoint *ep, + struct snd_urb_ctx *ctx) +{ + unsigned long flags; + + spin_lock_irqsave(&ep->lock, flags); + list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs); + spin_unlock_irqrestore(&ep->lock, flags); +} + /* * Send output urbs that have been prepared previously. URBs are dequeued * from ep->ready_playback_urbs and in case there aren't any available @@ -427,12 +440,14 @@ next_packet_fifo_dequeue(struct snd_usb_endpoint *ep) * is that host controllers don't guarantee the order in which they return * inbound and outbound packets to their submitters. * - * This function is only used for implicit feedback endpoints. For endpoints - * driven by dedicated sync endpoints, URBs are immediately re-submitted - * from their completion handler. + * This function is used both for implicit feedback endpoints and in low- + * latency playback mode. */ -static void queue_pending_output_urbs(struct snd_usb_endpoint *ep) +void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, + bool in_stream_lock) { + bool implicit_fb = snd_usb_endpoint_implicit_feedback_sink(ep); + while (ep_state_running(ep)) { unsigned long flags; @@ -441,14 +456,14 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep) int err, i; spin_lock_irqsave(&ep->lock, flags); - if (ep->next_packet_queued > 0 && + if ((!implicit_fb || ep->next_packet_queued > 0) && !list_empty(&ep->ready_playback_urbs)) { /* take URB out of FIFO */ ctx = list_first_entry(&ep->ready_playback_urbs, struct snd_urb_ctx, ready_list); list_del_init(&ctx->ready_list); - - packet = next_packet_fifo_dequeue(ep); + if (implicit_fb) + packet = next_packet_fifo_dequeue(ep); } spin_unlock_irqrestore(&ep->lock, flags); @@ -456,11 +471,24 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep) return; /* copy over the length information */ - for (i = 0; i < packet->packets; i++) - ctx->packet_size[i] = packet->packet_size[i]; + if (implicit_fb) { + for (i = 0; i < packet->packets; i++) + ctx->packet_size[i] = packet->packet_size[i]; + } /* call the data handler to fill in playback data */ - prepare_outbound_urb(ep, ctx); + err = prepare_outbound_urb(ep, ctx, in_stream_lock); + /* can be stopped during prepare callback */ + if (unlikely(!ep_state_running(ep))) + break; + if (err < 0) { + /* push back to ready list again for -EAGAIN */ + if (err == -EAGAIN) + push_back_to_ready_list(ep, ctx); + else + notify_xrun(ep); + return; + } err = usb_submit_urb(ctx->urb, GFP_ATOMIC); if (err < 0) { @@ -483,7 +511,6 @@ static void snd_complete_urb(struct urb *urb) { struct snd_urb_ctx *ctx = urb->context; struct snd_usb_endpoint *ep = ctx->ep; - unsigned long flags; int err; if (unlikely(urb->status == -ENOENT || /* unlinked */ @@ -504,17 +531,20 @@ static void snd_complete_urb(struct urb *urb) if (unlikely(!ep_state_running(ep))) goto exit_clear; - if (snd_usb_endpoint_implicit_feedback_sink(ep)) { - spin_lock_irqsave(&ep->lock, flags); - list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs); + /* in low-latency and implicit-feedback modes, push back the + * URB to ready list at first, then process as much as possible + */ + if (ep->lowlatency_playback || + snd_usb_endpoint_implicit_feedback_sink(ep)) { + push_back_to_ready_list(ep, ctx); clear_bit(ctx->index, &ep->active_mask); - spin_unlock_irqrestore(&ep->lock, flags); - queue_pending_output_urbs(ep); + snd_usb_queue_pending_output_urbs(ep, false); atomic_dec(&ep->submitted_urbs); /* decrement at last */ return; } - prepare_outbound_urb(ep, ctx); + /* in non-lowlatency mode, no error handling for prepare */ + prepare_outbound_urb(ep, ctx, false); /* can be stopped during prepare callback */ if (unlikely(!ep_state_running(ep))) goto exit_clear; @@ -807,8 +837,9 @@ void snd_usb_endpoint_set_sync(struct snd_usb_audio *chip, * Pass NULL to deactivate each callback. */ void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep, - void (*prepare)(struct snd_usb_substream *subs, - struct urb *urb), + int (*prepare)(struct snd_usb_substream *subs, + struct urb *urb, + bool in_stream_lock), void (*retire)(struct snd_usb_substream *subs, struct urb *urb), struct snd_usb_substream *data_subs) @@ -1166,10 +1197,6 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep) INIT_LIST_HEAD(&u->ready_list); } - /* total buffer bytes of all URBs plus the next queue; - * referred in pcm.c - */ - ep->nominal_queue_size = maxsize * urb_packs * (ep->nurbs + 1); return 0; out_of_memory: @@ -1408,6 +1435,7 @@ int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock) */ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) { + bool is_playback = usb_pipeout(ep->pipe); int err; unsigned int i; @@ -1444,13 +1472,9 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) if (snd_usb_endpoint_implicit_feedback_sink(ep) && !(ep->chip->quirk_flags & QUIRK_FLAG_PLAYBACK_FIRST)) { - for (i = 0; i < ep->nurbs; i++) { - struct snd_urb_ctx *ctx = ep->urb + i; - list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs); - } - usb_audio_dbg(ep->chip, "No URB submission due to implicit fb sync\n"); - return 0; + i = 0; + goto fill_rest; } for (i = 0; i < ep->nurbs; i++) { @@ -1459,10 +1483,18 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) if (snd_BUG_ON(!urb)) goto __error; - if (usb_pipeout(ep->pipe)) { - prepare_outbound_urb(ep, urb->context); - } else { - prepare_inbound_urb(ep, urb->context); + if (is_playback) + err = prepare_outbound_urb(ep, urb->context, true); + else + err = prepare_inbound_urb(ep, urb->context); + if (err < 0) { + /* stop filling at applptr */ + if (err == -EAGAIN) + break; + usb_audio_dbg(ep->chip, + "EP 0x%x: failed to prepare urb: %d\n", + ep->ep_num, err); + goto __error; } err = usb_submit_urb(urb, GFP_ATOMIC); @@ -1476,8 +1508,22 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) atomic_inc(&ep->submitted_urbs); } + if (!i) { + usb_audio_dbg(ep->chip, "XRUN at starting EP 0x%x\n", + ep->ep_num); + goto __error; + } + usb_audio_dbg(ep->chip, "%d URBs submitted for EP 0x%x\n", - ep->nurbs, ep->ep_num); + i, ep->ep_num); + + fill_rest: + /* put the remaining URBs to ready list */ + if (is_playback) { + for (; i < ep->nurbs; i++) + push_back_to_ready_list(ep, ep->urb + i); + } + return 0; __error: @@ -1629,7 +1675,7 @@ static void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep, } spin_unlock_irqrestore(&ep->lock, flags); - queue_pending_output_urbs(ep); + snd_usb_queue_pending_output_urbs(ep, false); return; } diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 1f1a72535a64..6895d50d14d1 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -30,8 +30,9 @@ void snd_usb_endpoint_set_sync(struct snd_usb_audio *chip, struct snd_usb_endpoint *data_ep, struct snd_usb_endpoint *sync_ep); void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep, - void (*prepare)(struct snd_usb_substream *subs, - struct urb *urb), + int (*prepare)(struct snd_usb_substream *subs, + struct urb *urb, + bool in_stream_lock), void (*retire)(struct snd_usb_substream *subs, struct urb *urb), struct snd_usb_substream *data_subs); @@ -48,5 +49,7 @@ int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep); int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep, struct snd_urb_ctx *ctx, int idx, unsigned int avail); +void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, + bool in_stream_lock); #endif /* __USBAUDIO_ENDPOINT_H */ diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 8ad48c35c559..d5a14e5b9ad3 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -598,9 +598,6 @@ static int lowlatency_playback_available(struct snd_pcm_runtime *runtime, /* implicit feedback mode has own operation mode */ if (snd_usb_endpoint_implicit_feedback_sink(subs->data_endpoint)) return false; - /* too short periods? */ - if (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes) - return false; return true; } @@ -1095,6 +1092,10 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream) int ret; runtime->hw = snd_usb_hardware; + /* need an explicit sync to catch applptr update in low-latency mode */ + if (direction == SNDRV_PCM_STREAM_PLAYBACK && + as->chip->lowlatency) + runtime->hw.info |= SNDRV_PCM_INFO_EXPLICIT_SYNC; runtime->private_data = subs; subs->pcm_substream = substream; /* runtime PM is also done there */ @@ -1347,44 +1348,66 @@ static unsigned int copy_to_urb_quirk(struct snd_usb_substream *subs, return bytes; } -static void prepare_playback_urb(struct snd_usb_substream *subs, - struct urb *urb) +static int prepare_playback_urb(struct snd_usb_substream *subs, + struct urb *urb, + bool in_stream_lock) { struct snd_pcm_runtime *runtime = subs->pcm_substream->runtime; struct snd_usb_endpoint *ep = subs->data_endpoint; struct snd_urb_ctx *ctx = urb->context; - unsigned int counts, frames, bytes; + unsigned int frames, bytes; + int counts; + unsigned int transfer_done, frame_limit, avail = 0; int i, stride, period_elapsed = 0; unsigned long flags; + int err = 0; stride = ep->stride; frames = 0; ctx->queued = 0; urb->number_of_packets = 0; + spin_lock_irqsave(&subs->lock, flags); - subs->frame_limit += ep->max_urb_frames; + frame_limit = subs->frame_limit + ep->max_urb_frames; + transfer_done = subs->transfer_done; + + if (subs->lowlatency_playback && + runtime->status->state != SNDRV_PCM_STATE_DRAINING) { + unsigned int hwptr = subs->hwptr_done / stride; + + /* calculate the byte offset-in-buffer of the appl_ptr */ + avail = (runtime->control->appl_ptr - runtime->hw_ptr_base) + % runtime->buffer_size; + if (avail <= hwptr) + avail += runtime->buffer_size; + avail -= hwptr; + } + for (i = 0; i < ctx->packets; i++) { - counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, 0); + counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, avail); + if (counts < 0) + break; /* set up descriptor */ urb->iso_frame_desc[i].offset = frames * stride; urb->iso_frame_desc[i].length = counts * stride; frames += counts; + avail -= counts; urb->number_of_packets++; - subs->transfer_done += counts; - if (subs->transfer_done >= runtime->period_size) { - subs->transfer_done -= runtime->period_size; - subs->frame_limit = 0; + transfer_done += counts; + if (transfer_done >= runtime->period_size) { + transfer_done -= runtime->period_size; + frame_limit = 0; period_elapsed = 1; if (subs->fmt_type == UAC_FORMAT_TYPE_II) { - if (subs->transfer_done > 0) { + if (transfer_done > 0) { /* FIXME: fill-max mode is not * supported yet */ - frames -= subs->transfer_done; - counts -= subs->transfer_done; + frames -= transfer_done; + counts -= transfer_done; urb->iso_frame_desc[i].length = counts * stride; - subs->transfer_done = 0; + transfer_done = 0; } i++; if (i < ctx->packets) { @@ -1398,13 +1421,19 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, } } /* finish at the period boundary or after enough frames */ - if ((period_elapsed || - subs->transfer_done >= subs->frame_limit) && + if ((period_elapsed || transfer_done >= frame_limit) && !snd_usb_endpoint_implicit_feedback_sink(ep)) break; } - bytes = frames * stride; + if (!frames) { + err = -EAGAIN; + goto unlock; + } + + bytes = frames * stride; + subs->transfer_done = transfer_done; + subs->frame_limit = frame_limit; if (unlikely(ep->cur_format == SNDRV_PCM_FORMAT_DSD_U16_LE && subs->cur_audiofmt->dsd_dop)) { fill_playback_urb_dsd_dop(subs, urb, bytes); @@ -1434,10 +1463,19 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, subs->period_elapsed_pending = 1; period_elapsed = 0; } + + unlock: spin_unlock_irqrestore(&subs->lock, flags); + if (err < 0) + return err; urb->transfer_buffer_length = bytes; - if (period_elapsed) - snd_pcm_period_elapsed(subs->pcm_substream); + if (period_elapsed) { + if (in_stream_lock) + snd_pcm_period_elapsed_under_stream_lock(subs->pcm_substream); + else + snd_pcm_period_elapsed(subs->pcm_substream); + } + return 0; } /* @@ -1469,6 +1507,27 @@ static void retire_playback_urb(struct snd_usb_substream *subs, snd_pcm_period_elapsed(subs->pcm_substream); } +/* PCM ack callback for the playback stream; + * this plays a role only when the stream is running in low-latency mode. + */ +static int snd_usb_pcm_playback_ack(struct snd_pcm_substream *substream) +{ + struct snd_usb_substream *subs = substream->runtime->private_data; + struct snd_usb_endpoint *ep; + + if (!subs->lowlatency_playback || !subs->running) + return 0; + ep = subs->data_endpoint; + if (!ep) + return 0; + /* When no more in-flight URBs available, try to process the pending + * outputs here + */ + if (!ep->active_mask) + snd_usb_queue_pending_output_urbs(ep, true); + return 0; +} + static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substream, int cmd) { @@ -1572,6 +1631,7 @@ static const struct snd_pcm_ops snd_usb_playback_ops = { .trigger = snd_usb_substream_playback_trigger, .sync_stop = snd_usb_pcm_sync_stop, .pointer = snd_usb_pcm_pointer, + .ack = snd_usb_pcm_playback_ack, }; static const struct snd_pcm_ops snd_usb_capture_ops = { -- cgit v1.2.3 From 813a17cab9b708bbb1e0db8902e19857b57196ec Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:44 +0200 Subject: ALSA: usb-audio: Avoid killing in-flight URBs during draining While draining a stream, ALSA PCM core stops the stream by issuing snd_pcm_stop() after all data has been sent out. And, at PCM trigger stop, currently USB-audio driver kills the in-flight URBs explicitly, then at sync-stop ops, sync with the finish of all remaining URBs. This might result in a drop of the drained samples as most of USB-audio devices / hosts allow relatively long in-flight samples (as a sort of FIFO). For avoiding the trimming, this patch changes the stream-stop behavior during PCM draining state. Under that condition, the pending URBs won't be killed. The leftover in-flight URBs are caught by the sync-stop operation that shall be performed after the trigger-stop operation. Link: https://lore.kernel.org/r/20210929080844.11583-10-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/endpoint.c | 14 +++++++++----- sound/usb/endpoint.h | 2 +- sound/usb/pcm.c | 16 ++++++++-------- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 0b336876e36d..42c0d2db8ba8 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -955,7 +955,7 @@ void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep) * * This function moves the EP to STOPPING state if it's being RUNNING. */ -static int stop_urbs(struct snd_usb_endpoint *ep, bool force) +static int stop_urbs(struct snd_usb_endpoint *ep, bool force, bool keep_pending) { unsigned int i; unsigned long flags; @@ -972,6 +972,9 @@ static int stop_urbs(struct snd_usb_endpoint *ep, bool force) ep->next_packet_queued = 0; spin_unlock_irqrestore(&ep->lock, flags); + if (keep_pending) + return 0; + for (i = 0; i < ep->nurbs; i++) { if (test_bit(i, &ep->active_mask)) { if (!test_and_set_bit(i, &ep->unlink_mask)) { @@ -995,7 +998,7 @@ static int release_urbs(struct snd_usb_endpoint *ep, bool force) snd_usb_endpoint_set_callback(ep, NULL, NULL, NULL); /* stop and unlink urbs */ - err = stop_urbs(ep, force); + err = stop_urbs(ep, force, false); if (err) return err; @@ -1527,7 +1530,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) return 0; __error: - snd_usb_endpoint_stop(ep); + snd_usb_endpoint_stop(ep, false); return -EPIPE; } @@ -1535,6 +1538,7 @@ __error: * snd_usb_endpoint_stop: stop an snd_usb_endpoint * * @ep: the endpoint to stop (may be NULL) + * @keep_pending: keep in-flight URBs * * A call to this function will decrement the running count of the endpoint. * In case the last user has requested the endpoint stop, the URBs will @@ -1545,7 +1549,7 @@ __error: * The caller needs to synchronize the pending stop operation via * snd_usb_endpoint_sync_pending_stop(). */ -void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep) +void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, bool keep_pending) { if (!ep) return; @@ -1560,7 +1564,7 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep) if (!atomic_dec_return(&ep->running)) { if (ep->sync_source) WRITE_ONCE(ep->sync_source->sync_sink, NULL); - stop_urbs(ep, false); + stop_urbs(ep, false, keep_pending); } } diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 6895d50d14d1..6a9af04cf175 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -38,7 +38,7 @@ void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep, struct snd_usb_substream *data_subs); int snd_usb_endpoint_start(struct snd_usb_endpoint *ep); -void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep); +void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, bool keep_pending); void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep); void snd_usb_endpoint_suspend(struct snd_usb_endpoint *ep); int snd_usb_endpoint_activate(struct snd_usb_endpoint *ep); diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index d5a14e5b9ad3..f09c7380a923 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -219,16 +219,16 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, return 0; } -static bool stop_endpoints(struct snd_usb_substream *subs) +static bool stop_endpoints(struct snd_usb_substream *subs, bool keep_pending) { bool stopped = 0; if (test_and_clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) { - snd_usb_endpoint_stop(subs->sync_endpoint); + snd_usb_endpoint_stop(subs->sync_endpoint, keep_pending); stopped = true; } if (test_and_clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) { - snd_usb_endpoint_stop(subs->data_endpoint); + snd_usb_endpoint_stop(subs->data_endpoint, keep_pending); stopped = true; } return stopped; @@ -261,7 +261,7 @@ static int start_endpoints(struct snd_usb_substream *subs) return 0; error: - stop_endpoints(subs); + stop_endpoints(subs, false); return err; } @@ -437,7 +437,7 @@ static int configure_endpoints(struct snd_usb_audio *chip, if (subs->data_endpoint->need_setup) { /* stop any running stream beforehand */ - if (stop_endpoints(subs)) + if (stop_endpoints(subs, false)) sync_pending_stops(subs); err = snd_usb_endpoint_configure(chip, subs->data_endpoint); if (err < 0) @@ -572,7 +572,7 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) subs->cur_audiofmt = NULL; mutex_unlock(&chip->mutex); if (!snd_usb_lock_shutdown(chip)) { - if (stop_endpoints(subs)) + if (stop_endpoints(subs, false)) sync_pending_stops(subs); close_endpoints(chip, subs); snd_usb_unlock_shutdown(chip); @@ -1559,7 +1559,7 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea return 0; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: - stop_endpoints(subs); + stop_endpoints(subs, substream->runtime->status->state == SNDRV_PCM_STATE_DRAINING); snd_usb_endpoint_set_callback(subs->data_endpoint, NULL, NULL, NULL); subs->running = 0; @@ -1607,7 +1607,7 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream return 0; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: - stop_endpoints(subs); + stop_endpoints(subs, false); fallthrough; case SNDRV_PCM_TRIGGER_PAUSE_PUSH: snd_usb_endpoint_set_callback(subs->data_endpoint, -- cgit v1.2.3 From 28c369e60827f706cef4604a3e2848198f25bd26 Mon Sep 17 00:00:00 2001 From: Geraldo Nascimento Date: Thu, 30 Sep 2021 16:40:14 -0300 Subject: ALSA: usb-audio: disable implicit feedback sync for Behringer UFX1204 and UFX1604 Behringer UFX1204 and UFX1604 have Synchronous endpoints to which current ALSA code applies implicit feedback sync as if they were Asynchronous endpoints. This breaks UAC compliance and is unneeded. The commit 5e35dc0338d85ccebacf3f77eca1e5dea73155e8 and subsequent 1a15718b41df026cffd0e42cfdc38a1384ce19f9 were meant to clear up noise. Unfortunately, noise persisted for those using higher sample rates and this was only solved by commit d2e8f641257d0d3af6e45d6ac2d6f9d56b8ea964 Since there are no more reports of noise, let's get rid of the implicit-fb quirks breaking UAC compliance. Signed-off-by: Geraldo Nascimento Link: https://lore.kernel.org/r/YVYSnoQ7nxLXT0Dq@geday Signed-off-by: Takashi Iwai --- sound/usb/implicit.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/usb/implicit.c b/sound/usb/implicit.c index 23767a14d126..70319c822c10 100644 --- a/sound/usb/implicit.c +++ b/sound/usb/implicit.c @@ -54,8 +54,6 @@ static const struct snd_usb_implicit_fb_match playback_implicit_fb_quirks[] = { /* Fixed EP */ /* FIXME: check the availability of generic matching */ - IMPLICIT_FB_FIXED_DEV(0x1397, 0x0001, 0x81, 1), /* Behringer UFX1604 */ - IMPLICIT_FB_FIXED_DEV(0x1397, 0x0002, 0x81, 1), /* Behringer UFX1204 */ IMPLICIT_FB_FIXED_DEV(0x2466, 0x8010, 0x81, 2), /* Fractal Audio Axe-Fx III */ IMPLICIT_FB_FIXED_DEV(0x31e9, 0x0001, 0x81, 2), /* Solid State Logic SSL2 */ IMPLICIT_FB_FIXED_DEV(0x31e9, 0x0002, 0x81, 2), /* Solid State Logic SSL2+ */ -- cgit v1.2.3 From 23939115be181bc5dbc33aa8471adcdbffa28910 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 1 Oct 2021 12:54:25 +0200 Subject: ALSA: usb-audio: Fix packet size calculation regression The commit d215f63d49da ("ALSA: usb-audio: Check available frames for the next packet size") introduced the available frame size check, but the conversion forgot to initialize the temporary variable properly, and it resulted in a bogus calculation. This patch fixes it. Fixes: d215f63d49da ("ALSA: usb-audio: Check available frames for the next packet size") Reported-by: Colin Ian King Link: https://lore.kernel.org/r/20211001104417.14291-1-colin.king@canonical.com Link: https://lore.kernel.org/r/20211001105425.16191-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/endpoint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 42c0d2db8ba8..743b8287cfcd 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -182,7 +182,7 @@ static int next_packet_size(struct snd_usb_endpoint *ep, unsigned int avail) if (ep->fill_max) return ep->maxframesize; - sample_accum += ep->sample_rem; + sample_accum = ep->sample_accum + ep->sample_rem; if (sample_accum >= ep->pps) { sample_accum -= ep->pps; ret = ep->packsize[1]; -- cgit v1.2.3 From 36df2427ac3ea04510368561c8cee22388a7434a Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 6 Oct 2021 16:22:14 +0200 Subject: ALSA: pcm: Add more disconnection checks at file ops In the case of hot-disconnection of a PCM device, all file operations except for close should be rejected. This patch adds more sanity checks in the file operation code paths. Link: https://lore.kernel.org/r/20211006142214.3089-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index d233cb3b41d8..46c643db18eb 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -3218,6 +3218,9 @@ static int snd_pcm_common_ioctl(struct file *file, if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; + if (substream->runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) + return -EBADFD; + res = snd_power_wait(substream->pcm->card); if (res < 0) return res; @@ -3344,6 +3347,9 @@ int snd_pcm_kernel_ioctl(struct snd_pcm_substream *substream, snd_pcm_uframes_t *frames = arg; snd_pcm_sframes_t result; + if (substream->runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) + return -EBADFD; + switch (cmd) { case SNDRV_PCM_IOCTL_FORWARD: { @@ -3386,7 +3392,8 @@ static ssize_t snd_pcm_read(struct file *file, char __user *buf, size_t count, if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; - if (runtime->status->state == SNDRV_PCM_STATE_OPEN) + if (runtime->status->state == SNDRV_PCM_STATE_OPEN || + runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) return -EBADFD; if (!frame_aligned(runtime, count)) return -EINVAL; @@ -3410,7 +3417,8 @@ static ssize_t snd_pcm_write(struct file *file, const char __user *buf, if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; - if (runtime->status->state == SNDRV_PCM_STATE_OPEN) + if (runtime->status->state == SNDRV_PCM_STATE_OPEN || + runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) return -EBADFD; if (!frame_aligned(runtime, count)) return -EINVAL; @@ -3436,7 +3444,8 @@ static ssize_t snd_pcm_readv(struct kiocb *iocb, struct iov_iter *to) if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; - if (runtime->status->state == SNDRV_PCM_STATE_OPEN) + if (runtime->status->state == SNDRV_PCM_STATE_OPEN || + runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) return -EBADFD; if (!iter_is_iovec(to)) return -EINVAL; @@ -3472,7 +3481,8 @@ static ssize_t snd_pcm_writev(struct kiocb *iocb, struct iov_iter *from) if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; - if (runtime->status->state == SNDRV_PCM_STATE_OPEN) + if (runtime->status->state == SNDRV_PCM_STATE_OPEN || + runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) return -EBADFD; if (!iter_is_iovec(from)) return -EINVAL; @@ -3511,6 +3521,9 @@ static __poll_t snd_pcm_poll(struct file *file, poll_table *wait) return ok | EPOLLERR; runtime = substream->runtime; + if (runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) + return ok | EPOLLERR; + poll_wait(file, &runtime->sleep, wait); mask = 0; @@ -3820,6 +3833,8 @@ static int snd_pcm_mmap(struct file *file, struct vm_area_struct *area) substream = pcm_file->substream; if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; + if (substream->runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) + return -EBADFD; offset = area->vm_pgoff << PAGE_SHIFT; switch (offset) { @@ -3856,6 +3871,8 @@ static int snd_pcm_fasync(int fd, struct file * file, int on) if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; + if (runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) + return -EBADFD; return fasync_helper(fd, file, on, &runtime->fasync); } -- cgit v1.2.3 From 59d7f5f6ddbc23f6ca4a0523c85dc18f027c80d9 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 7 Oct 2021 10:35:28 +0200 Subject: ALSA: usb-audio: Pass JOINT_DUPLEX info flag for implicit fb streams When a stream is in the implicit feedback mode, it's more or less tied with a capture stream. Passing SNDRV_PCM_INFO_JOINT_DUPLEX may help for user-space to understand the situation. Link: https://lore.kernel.org/r/20211007083528.4184-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/pcm.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index f09c7380a923..3bb095a3c9b6 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1080,6 +1080,13 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre if (err < 0) return err; + list_for_each_entry(fp, &subs->fmt_list, list) { + if (fp->implicit_fb) { + runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX; + break; + } + } + return 0; } -- cgit v1.2.3 From 6d27788160362a7ee6c0d317636fe4b1ddbe59a7 Mon Sep 17 00:00:00 2001 From: William Overton Date: Sun, 10 Oct 2021 15:58:41 +0100 Subject: ALSA: usb-audio: Add support for the Pioneer DJM 750MK2 Mixer/Soundcard The kernel already has support for very similar Pioneer djm products and this work is based on that. Added device to quirks-table.h and added control info to mixer_quirks.c. Tested on my hardware and all working. Signed-off-by: William Overton Link: https://lore.kernel.org/r/20211010145841.11907-1-willovertonuk@gmail.com Signed-off-by: Takashi Iwai --- sound/usb/mixer_quirks.c | 34 ++++++++++++++++++++++++++++ sound/usb/quirks-table.h | 58 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index 46082dc57be0..d489c1de3bae 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -2795,6 +2795,7 @@ static int snd_bbfpro_controls_create(struct usb_mixer_interface *mixer) #define SND_DJM_750_IDX 0x1 #define SND_DJM_850_IDX 0x2 #define SND_DJM_900NXS2_IDX 0x3 +#define SND_DJM_750MK2_IDX 0x4 #define SND_DJM_CTL(_name, suffix, _default_value, _windex) { \ @@ -2984,10 +2985,40 @@ static const struct snd_djm_ctl snd_djm_ctls_900nxs2[] = { SND_DJM_CTL("Ch5 Input", 900nxs2_cap5, 3, SND_DJM_WINDEX_CAP) }; +// DJM-750MK2 +static const u16 snd_djm_opts_750mk2_cap1[] = { + 0x0100, 0x0102, 0x0103, 0x0106, 0x0107, 0x0108, 0x0109, 0x010a }; +static const u16 snd_djm_opts_750mk2_cap2[] = { + 0x0200, 0x0202, 0x0203, 0x0206, 0x0207, 0x0208, 0x0209, 0x020a }; +static const u16 snd_djm_opts_750mk2_cap3[] = { + 0x0300, 0x0302, 0x0303, 0x0306, 0x0307, 0x0308, 0x0309, 0x030a }; +static const u16 snd_djm_opts_750mk2_cap4[] = { + 0x0400, 0x0402, 0x0403, 0x0406, 0x0407, 0x0408, 0x0409, 0x040a }; +static const u16 snd_djm_opts_750mk2_cap5[] = { + 0x0507, 0x0508, 0x0509, 0x050a, 0x0511, 0x0512, 0x0513, 0x0514 }; + +static const u16 snd_djm_opts_750mk2_pb1[] = { 0x0100, 0x0101, 0x0104 }; +static const u16 snd_djm_opts_750mk2_pb2[] = { 0x0200, 0x0201, 0x0204 }; +static const u16 snd_djm_opts_750mk2_pb3[] = { 0x0300, 0x0301, 0x0304 }; + + +static const struct snd_djm_ctl snd_djm_ctls_750mk2[] = { + SND_DJM_CTL("Capture Level", cap_level, 0, SND_DJM_WINDEX_CAPLVL), + SND_DJM_CTL("Ch1 Input", 750mk2_cap1, 2, SND_DJM_WINDEX_CAP), + SND_DJM_CTL("Ch2 Input", 750mk2_cap2, 2, SND_DJM_WINDEX_CAP), + SND_DJM_CTL("Ch3 Input", 750mk2_cap3, 2, SND_DJM_WINDEX_CAP), + SND_DJM_CTL("Ch4 Input", 750mk2_cap4, 2, SND_DJM_WINDEX_CAP), + SND_DJM_CTL("Ch5 Input", 750mk2_cap5, 3, SND_DJM_WINDEX_CAP), + SND_DJM_CTL("Ch1 Output", 750mk2_pb1, 0, SND_DJM_WINDEX_PB), + SND_DJM_CTL("Ch2 Output", 750mk2_pb2, 1, SND_DJM_WINDEX_PB), + SND_DJM_CTL("Ch3 Output", 750mk2_pb3, 2, SND_DJM_WINDEX_PB) +}; + static const struct snd_djm_device snd_djm_devices[] = { SND_DJM_DEVICE(250mk2), SND_DJM_DEVICE(750), + SND_DJM_DEVICE(750mk2), SND_DJM_DEVICE(850), SND_DJM_DEVICE(900nxs2) }; @@ -3235,6 +3266,9 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer) case USB_ID(0x08e4, 0x017f): /* Pioneer DJ DJM-750 */ err = snd_djm_controls_create(mixer, SND_DJM_750_IDX); break; + case USB_ID(0x2b73, 0x001b): /* Pioneer DJ DJM-750MK2 */ + err = snd_djm_controls_create(mixer, SND_DJM_750MK2_IDX); + break; case USB_ID(0x08e4, 0x0163): /* Pioneer DJ DJM-850 */ err = snd_djm_controls_create(mixer, SND_DJM_850_IDX); break; diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index e03043f7dad3..bc0116273e94 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -3850,6 +3850,64 @@ YAMAHA_DEVICE(0x7010, "UB99"), } } }, +{ + /* + * Pioneer DJ DJM-750MK2 + * 10 channels playback & 12 channels capture @ 48kHz S24LE + */ + USB_DEVICE_VENDOR_SPEC(0x2b73, 0x001b), + .driver_info = (unsigned long) &(const struct snd_usb_audio_quirk) { + .ifnum = QUIRK_ANY_INTERFACE, + .type = QUIRK_COMPOSITE, + .data = (const struct snd_usb_audio_quirk[]) { + { + .ifnum = 0, + .type = QUIRK_AUDIO_FIXED_ENDPOINT, + .data = &(const struct audioformat) { + .formats = SNDRV_PCM_FMTBIT_S24_3LE, + .channels = 10, + .iface = 0, + .altsetting = 1, + .altset_idx = 1, + .endpoint = 0x01, + .ep_attr = USB_ENDPOINT_XFER_ISOC| + USB_ENDPOINT_SYNC_ASYNC, + .rates = SNDRV_PCM_RATE_48000, + .rate_min = 48000, + .rate_max = 48000, + .nr_rates = 1, + .rate_table = (unsigned int[]) { + 48000 + } + } + }, + { + .ifnum = 0, + .type = QUIRK_AUDIO_FIXED_ENDPOINT, + .data = &(const struct audioformat) { + .formats = SNDRV_PCM_FMTBIT_S24_3LE, + .channels = 12, + .iface = 0, + .altsetting = 1, + .altset_idx = 1, + .endpoint = 0x82, + .ep_idx = 1, + .ep_attr = USB_ENDPOINT_XFER_ISOC| + USB_ENDPOINT_SYNC_ASYNC| + USB_ENDPOINT_USAGE_IMPLICIT_FB, + .rates = SNDRV_PCM_RATE_48000, + .rate_min = 48000, + .rate_max = 48000, + .nr_rates = 1, + .rate_table = (unsigned int[]) { 48000 } + } + }, + { + .ifnum = -1 + } + } + } +}, { /* * Pioneer DJ DJM-850 -- cgit v1.2.3 From 53451b6da8271905941eb1eb369db152c4bd92f2 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Mon, 11 Oct 2021 12:36:50 +0200 Subject: ALSA: usb-audio: Less restriction for low-latency playback mode The recent support for the improved low-latency playback mode applied the SNDRV_PCM_INFO_EXPLICIT_SYNC flag for the target streams, but this was a slight overkill. The use of the flag above disables effectively both PCM status and control mmaps, while basically what we want to track is only about the appl_ptr update. For less restriction, use a more proper flag, SNDRV_PCM_INFO_SYNC_APPLPTR instead, which disables only the control mmap. Fixes: d5f871f89e21 ("ALSA: usb-audio: Improved lowlatency playback support") Link: https://lore.kernel.org/r/20211011103650.10182-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 3bb095a3c9b6..95ec8eec1bb0 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1102,7 +1102,7 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream) /* need an explicit sync to catch applptr update in low-latency mode */ if (direction == SNDRV_PCM_STREAM_PLAYBACK && as->chip->lowlatency) - runtime->hw.info |= SNDRV_PCM_INFO_EXPLICIT_SYNC; + runtime->hw.info |= SNDRV_PCM_INFO_SYNC_APPLPTR; runtime->private_data = subs; subs->pcm_substream = substream; /* runtime PM is also done there */ -- cgit v1.2.3 From ac9b019d07ee1666e87f6832b1bde7e71618c4b0 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 14 Oct 2021 15:06:34 +0200 Subject: ALSA: usb-audio: Downgrade error message in get_ctl_value_v2() The error message in get_ctl_value_v2() (for UAC2/3) is shown via KERN_ERR level but it was intended to be rather a debug message as seen in get_ctl_value_v1() (for UAC1). This patch downgrade the printk level. Tested-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20211014130636.17860-2-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/mixer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index a2ce535df14b..a20af9243155 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -373,7 +373,7 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, if (ret < 0) { error: - usb_audio_err(chip, + usb_audio_dbg(chip, "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n", request, validx, idx, cval->val_type); return ret; -- cgit v1.2.3 From 509975c7789f84b4d98e06fe2db51ee4ec02eef5 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 14 Oct 2021 15:06:35 +0200 Subject: ALSA: usb-audio: Drop superfluous error message after disconnection The error from snd_usb_lock_shutdown() indicates that the device is disconnected, hence it makes no sense to show any further control message error in get_ctl_value_v2(). Return the error directly instead. Tested-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20211014130636.17860-3-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/mixer.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index a20af9243155..60d394361a43 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -361,9 +361,8 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, memset(buf, 0, sizeof(buf)); - ret = snd_usb_lock_shutdown(chip) ? -EIO : 0; - if (ret) - goto error; + if (snd_usb_lock_shutdown(chip)) + return -EIO; idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8); ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest, @@ -372,7 +371,6 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, snd_usb_unlock_shutdown(chip); if (ret < 0) { -error: usb_audio_dbg(chip, "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n", request, validx, idx, cval->val_type); -- cgit v1.2.3 From b96681bd58276e1c7ca4ca37bbaab9f8f1738d61 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 14 Oct 2021 15:06:36 +0200 Subject: ALSA: usb-audio: Initialize every feature unit once at probe time So far we used to read the current value of the mixer element dynamically at the first access, and the error from a GET_CUR message is treated as a fatal error (unless QUIRK_IGNORE_CTL_ERROR is set). It's rather inconvenient, as most of GET_CUR errors are no fatal, and we can continue operation with assumption of some fixed value. This patch makes the USB-audio driver to change the behavior at probe time; now it tries to initialize the current value of each mixer element that is built from a feature unit (those for typically for mixer volumes and switches). When a read failure happens, it tries to set the known minimum value. After that point, a cached value is used always, hence we won't hit GET_CUR message error any longer. The error from GET_CUR message is still shown as a warning normally, but only once at the probe time, and it'll keep operating. If the message is confirmed to be harmless, it can be shut up by QUIRK_IGNORE_CTL_ERROR quirk flag, too. Tested-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20211014130636.17860-4-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/mixer.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 60d394361a43..4555de9830c5 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -1199,12 +1199,32 @@ static void volume_control_quirks(struct usb_mixer_elem_info *cval, } } +/* forcibly initialize the current mixer value; if GET_CUR fails, set to + * the minimum as default + */ +static void init_cur_mix_raw(struct usb_mixer_elem_info *cval, int ch, int idx) +{ + int val, err; + + err = snd_usb_get_cur_mix_value(cval, ch, idx, &val); + if (!err) + return; + if (!cval->head.mixer->ignore_ctl_error) + usb_audio_warn(cval->head.mixer->chip, + "%d:%d: failed to get current value for ch %d (%d)\n", + cval->head.id, mixer_ctrl_intf(cval->head.mixer), + ch, err); + snd_usb_set_cur_mix_value(cval, ch, idx, cval->min); +} + /* * retrieve the minimum and maximum values for the specified control */ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval, int default_min, struct snd_kcontrol *kctl) { + int i, idx; + /* for failsafe */ cval->min = default_min; cval->max = cval->min + 1; @@ -1217,7 +1237,6 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval, } else { int minchn = 0; if (cval->cmask) { - int i; for (i = 0; i < MAX_CHANNELS; i++) if (cval->cmask & (1 << i)) { minchn = i + 1; @@ -1318,6 +1337,19 @@ no_res_check: } } + /* initialize all elements */ + if (!cval->cmask) { + init_cur_mix_raw(cval, 0, 0); + } else { + idx = 0; + for (i = 0; i < MAX_CHANNELS; i++) { + if (cval->cmask & (1 << i)) { + init_cur_mix_raw(cval, i + 1, idx); + idx++; + } + } + } + return 0; } -- cgit v1.2.3 From c18c4966033e6473a472fb65fbd5a6441603fbf7 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 14 Oct 2021 16:53:23 +0200 Subject: ALSA: pcm: Unify snd_pcm_delay() and snd_pcm_hwsync() Both snd_pcm_delay() and snd_pcm_hwsync() do the almost same thing. The only difference is that the former calculate the delay, so unify them as a code cleanup, and treat NULL delay argument only for hwsync operation. Also, the patch does a slight code refactoring in snd_pcm_delay(). The initialization of the delay value is done in the caller side now. Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20211014145323.26506-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 46c643db18eb..627b201b6084 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2932,32 +2932,24 @@ static snd_pcm_sframes_t snd_pcm_forward(struct snd_pcm_substream *substream, return ret; } -static int snd_pcm_hwsync(struct snd_pcm_substream *substream) -{ - int err; - - snd_pcm_stream_lock_irq(substream); - err = do_pcm_hwsync(substream); - snd_pcm_stream_unlock_irq(substream); - return err; -} - static int snd_pcm_delay(struct snd_pcm_substream *substream, snd_pcm_sframes_t *delay) { int err; - snd_pcm_sframes_t n = 0; snd_pcm_stream_lock_irq(substream); err = do_pcm_hwsync(substream); - if (!err) - n = snd_pcm_calc_delay(substream); + if (delay && !err) + *delay = snd_pcm_calc_delay(substream); snd_pcm_stream_unlock_irq(substream); - if (!err) - *delay = n; return err; } +static inline int snd_pcm_hwsync(struct snd_pcm_substream *substream) +{ + return snd_pcm_delay(substream, NULL); +} + static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream, struct snd_pcm_sync_ptr __user *_sync_ptr) { @@ -3275,7 +3267,7 @@ static int snd_pcm_common_ioctl(struct file *file, return snd_pcm_hwsync(substream); case SNDRV_PCM_IOCTL_DELAY: { - snd_pcm_sframes_t delay; + snd_pcm_sframes_t delay = 0; snd_pcm_sframes_t __user *res = arg; int err; -- cgit v1.2.3 From bea36afa102e37d5e4d9ea519f14d1c92d512e45 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:16 +0900 Subject: ALSA: firewire-motu: add message parser to gather meter information in register DSP model Some of MOTU models allows software to configure their DSP parameters by accessing to their registers. The models multiplex messages for status of DSP into isochronous packet as well as PCM frames. The message includes information of hardware metering, MIDI message, current parameters of DSP. For my convenience, I call them as 'register DSP' model. This patch adds message parser for them to gather hardware meter information. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 35 +++++ sound/firewire/motu/Makefile | 2 +- sound/firewire/motu/amdtp-motu.c | 6 + sound/firewire/motu/motu-protocol-v2.c | 14 +- sound/firewire/motu/motu-protocol-v3.c | 4 +- .../motu/motu-register-dsp-message-parser.c | 145 +++++++++++++++++++++ sound/firewire/motu/motu-stream.c | 6 + sound/firewire/motu/motu.c | 6 + sound/firewire/motu/motu.h | 8 ++ 9 files changed, 219 insertions(+), 7 deletions(-) create mode 100644 sound/firewire/motu/motu-register-dsp-message-parser.c diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index ae12826ed641..347fd7a05596 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -108,4 +108,39 @@ struct snd_firewire_tascam_state { __be32 data[SNDRV_FIREWIRE_TASCAM_STATE_COUNT]; }; +// In below MOTU models, software is allowed to control their DSP by accessing to registers. +// - 828mk2 +// - 896hd +// - Traveler +// - 8 pre +// - Ultralite +// - 4 pre +// - Audio Express +// +// On the other hand, the status of DSP is split into specific messages included in the sequence of +// isochronous packet. ALSA firewire-motu driver gathers the messages and allow userspace applications +// to read it via ioctl. In 828mk2, 896hd, and Traveler, hardware meter for all of physical inputs +// are put into the message, while one pair of physical outputs is selected. The selection is done by +// LSB one byte in asynchronous write quadlet transaction to 0x'ffff'f000'0b2c. +// +// I note that V3HD/V4HD uses asynchronous transaction for the purpose. The destination address is +// registered to 0x'ffff'f000'0b38 and '0b3c by asynchronous write quadlet request. The size of +// message differs between 23 and 51 quadlets. For the case, the number of mixer bus can be extended +// up to 12. + +#define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_COUNT 40 + +/** + * struct snd_firewire_motu_register_dsp_meter - the container for meter information in DSP + * controlled by register access + * @data: Signal level meters. The mapping between position and input/output channel is + * model-dependent. + * + * The structure expresses the part of DSP status for hardware meter. The u8 storage includes linear + * value for audio signal level between 0x00 and 0x7f. + */ +struct snd_firewire_motu_register_dsp_meter { + __u8 data[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_COUNT]; +}; + #endif /* _UAPI_SOUND_FIREWIRE_H_INCLUDED */ diff --git a/sound/firewire/motu/Makefile b/sound/firewire/motu/Makefile index acdf66564fb0..edbdf40c7162 100644 --- a/sound/firewire/motu/Makefile +++ b/sound/firewire/motu/Makefile @@ -4,5 +4,5 @@ CFLAGS_amdtp-motu.o := -I$(src) snd-firewire-motu-objs := motu.o amdtp-motu.o motu-transaction.o motu-stream.o \ motu-proc.o motu-pcm.o motu-midi.o motu-hwdep.o \ motu-protocol-v2.o motu-protocol-v3.o \ - motu-protocol-v1.o + motu-protocol-v1.o motu-register-dsp-message-parser.o obj-$(CONFIG_SND_FIREWIRE_MOTU) += snd-firewire-motu.o diff --git a/sound/firewire/motu/amdtp-motu.c b/sound/firewire/motu/amdtp-motu.c index a18c2c033e83..605b831492ac 100644 --- a/sound/firewire/motu/amdtp-motu.c +++ b/sound/firewire/motu/amdtp-motu.c @@ -333,6 +333,7 @@ static unsigned int process_ir_ctx_payloads(struct amdtp_stream *s, unsigned int packets, struct snd_pcm_substream *pcm) { + struct snd_motu *motu = container_of(s, struct snd_motu, tx_stream); struct amdtp_motu *p = s->protocol; unsigned int pcm_frames = 0; int i; @@ -357,6 +358,11 @@ static unsigned int process_ir_ctx_payloads(struct amdtp_stream *s, read_midi_messages(s, buf, data_blocks); } + if (motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP) { + snd_motu_register_dsp_message_parser_parse(motu, descs, packets, + s->data_block_quadlets); + } + // For tracepoints. if (trace_data_block_sph_enabled() || trace_data_block_message_enabled()) diff --git a/sound/firewire/motu/motu-protocol-v2.c b/sound/firewire/motu/motu-protocol-v2.c index 2bd4485e4bc7..a5f70efa2e88 100644 --- a/sound/firewire/motu/motu-protocol-v2.c +++ b/sound/firewire/motu/motu-protocol-v2.c @@ -275,7 +275,8 @@ const struct snd_motu_spec snd_motu_spec_828mk2 = { .name = "828mk2", .protocol_version = SND_MOTU_PROTOCOL_V2, .flags = SND_MOTU_SPEC_RX_MIDI_2ND_Q | - SND_MOTU_SPEC_TX_MIDI_2ND_Q, + SND_MOTU_SPEC_TX_MIDI_2ND_Q | + SND_MOTU_SPEC_REGISTER_DSP, .tx_fixed_pcm_chunks = {14, 14, 0}, .rx_fixed_pcm_chunks = {14, 14, 0}, }; @@ -283,7 +284,7 @@ const struct snd_motu_spec snd_motu_spec_828mk2 = { const struct snd_motu_spec snd_motu_spec_896hd = { .name = "896HD", .protocol_version = SND_MOTU_PROTOCOL_V2, - // No support for MIDI. + .flags = SND_MOTU_SPEC_REGISTER_DSP, .tx_fixed_pcm_chunks = {14, 14, 8}, .rx_fixed_pcm_chunks = {14, 14, 8}, }; @@ -292,7 +293,8 @@ const struct snd_motu_spec snd_motu_spec_traveler = { .name = "Traveler", .protocol_version = SND_MOTU_PROTOCOL_V2, .flags = SND_MOTU_SPEC_RX_MIDI_2ND_Q | - SND_MOTU_SPEC_TX_MIDI_2ND_Q, + SND_MOTU_SPEC_TX_MIDI_2ND_Q | + SND_MOTU_SPEC_REGISTER_DSP, .tx_fixed_pcm_chunks = {14, 14, 8}, .rx_fixed_pcm_chunks = {14, 14, 8}, }; @@ -301,7 +303,8 @@ const struct snd_motu_spec snd_motu_spec_ultralite = { .name = "UltraLite", .protocol_version = SND_MOTU_PROTOCOL_V2, .flags = SND_MOTU_SPEC_RX_MIDI_2ND_Q | - SND_MOTU_SPEC_TX_MIDI_2ND_Q, + SND_MOTU_SPEC_TX_MIDI_2ND_Q | + SND_MOTU_SPEC_REGISTER_DSP, .tx_fixed_pcm_chunks = {14, 14, 0}, .rx_fixed_pcm_chunks = {14, 14, 0}, }; @@ -310,7 +313,8 @@ const struct snd_motu_spec snd_motu_spec_8pre = { .name = "8pre", .protocol_version = SND_MOTU_PROTOCOL_V2, .flags = SND_MOTU_SPEC_RX_MIDI_2ND_Q | - SND_MOTU_SPEC_TX_MIDI_2ND_Q, + SND_MOTU_SPEC_TX_MIDI_2ND_Q | + SND_MOTU_SPEC_REGISTER_DSP, // Two dummy chunks always in the end of data block. .tx_fixed_pcm_chunks = {10, 10, 0}, .rx_fixed_pcm_chunks = {6, 6, 0}, diff --git a/sound/firewire/motu/motu-protocol-v3.c b/sound/firewire/motu/motu-protocol-v3.c index 56e4504e7ec9..d0dd587460de 100644 --- a/sound/firewire/motu/motu-protocol-v3.c +++ b/sound/firewire/motu/motu-protocol-v3.c @@ -293,7 +293,8 @@ const struct snd_motu_spec snd_motu_spec_audio_express = { .name = "AudioExpress", .protocol_version = SND_MOTU_PROTOCOL_V3, .flags = SND_MOTU_SPEC_RX_MIDI_2ND_Q | - SND_MOTU_SPEC_TX_MIDI_3RD_Q, + SND_MOTU_SPEC_TX_MIDI_3RD_Q | + SND_MOTU_SPEC_REGISTER_DSP, .tx_fixed_pcm_chunks = {10, 10, 0}, .rx_fixed_pcm_chunks = {10, 10, 0}, }; @@ -301,6 +302,7 @@ const struct snd_motu_spec snd_motu_spec_audio_express = { const struct snd_motu_spec snd_motu_spec_4pre = { .name = "4pre", .protocol_version = SND_MOTU_PROTOCOL_V3, + .flags = SND_MOTU_SPEC_REGISTER_DSP, .tx_fixed_pcm_chunks = {10, 10, 0}, .rx_fixed_pcm_chunks = {10, 10, 0}, }; diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c new file mode 100644 index 000000000000..efb9708b5b5f --- /dev/null +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// motu-register-dsp-message-parser.c - a part of driver for MOTU FireWire series +// +// Copyright (c) 2021 Takashi Sakamoto + +// Below models allow software to configure their DSP functions by asynchronous transaction +// to access their internal registers. +// * 828 mk2 +// * 896hd +// * Traveler +// * 8 pre +// * Ultralite +// * 4 pre +// * Audio Express +// +// Additionally, isochronous packets from the above models include messages to notify state of +// DSP. The messages are two set of 3 byte data in 2nd and 3rd quadlet of data block. When user +// operates hardware components such as dial and switch, corresponding messages are transferred. +// The messages include Hardware metering and MIDI messages as well. + +#include "motu.h" + +#define MSG_FLAG_POS 4 +#define MSG_FLAG_TYPE_MASK 0xf8 +#define MSG_FLAG_MIDI_MASK 0x01 +#define MSG_FLAG_MODEL_SPECIFIC_MASK 0x06 +#define MSG_FLAG_8PRE 0x00 +#define MSG_FLAG_ULTRALITE 0x04 +#define MSG_FLAG_TRAVELER 0x04 +#define MSG_FLAG_828MK2 0x04 +#define MSG_FLAG_896HD 0x04 +#define MSG_FLAG_4PRE 0x05 // MIDI mask is in 8th byte. +#define MSG_FLAG_AUDIOEXPRESS 0x05 // MIDI mask is in 8th byte. +#define MSG_FLAG_TYPE_SHIFT 3 +#define MSG_VALUE_POS 5 +#define MSG_MIDI_BYTE_POS 6 +#define MSG_METER_IDX_POS 7 + +// In 4 pre and Audio express, meter index is in 6th byte. MIDI flag is in 8th byte and MIDI byte +// is in 7th byte. +#define MSG_METER_IDX_POS_4PRE_AE 6 +#define MSG_MIDI_BYTE_POS_4PRE_AE 7 +#define MSG_FLAG_MIDI_POS_4PRE_AE 8 + +enum register_dsp_msg_type { + // Used for messages with no information. + INVALID = 0x00, + MIXER_SELECT = 0x01, + MIXER_SRC_GAIN = 0x02, + MIXER_SRC_PAN = 0x03, + MIXER_SRC_FLAG = 0x04, + MIXER_OUTPUT_PAIRED_VOLUME = 0x05, + MIXER_OUTPUT_PAIRED_FLAG = 0x06, + MAIN_OUTPUT_PAIRED_VOLUME = 0x07, + HP_OUTPUT_PAIRED_VOLUME = 0x08, + HP_OUTPUT_ASSIGN = 0x09, + // Transferred by all models but the purpose is still unknown. + UNKNOWN_0 = 0x0a, + // Specific to 828mk2, 896hd, Traveler. + UNKNOWN_2 = 0x0c, + // Specific to 828mk2, Traveler, and 896hd (not functional). + LINE_INPUT_BOOST = 0x0d, + // Specific to 828mk2, Traveler, and 896hd (not functional). + LINE_INPUT_NOMINAL_LEVEL = 0x0e, + // Specific to Ultralite, 4 pre, Audio express, and 8 pre (not functional). + INPUT_GAIN_AND_INVERT = 0x15, + // Specific to 4 pre, and Audio express. + INPUT_FLAG = 0x16, + // Specific to 4 pre, and Audio express. + MIXER_SRC_PAIRED_BALANCE = 0x17, + // Specific to 4 pre, and Audio express. + MIXER_SRC_PAIRED_WIDTH = 0x18, + // Transferred by all models. This type of message interposes the series of the other + // messages. The message delivers signal level up to 96.0 kHz. In 828mk2, 896hd, and + // Traveler, one of physical outputs is selected for the message. The selection is done + // by LSB one byte in asynchronous write quadlet transaction to 0x'ffff'f000'0b2c. + METER = 0x1f, +}; + +struct msg_parser { + struct snd_firewire_motu_register_dsp_meter meter; + bool meter_pos_quirk; +}; + +int snd_motu_register_dsp_message_parser_new(struct snd_motu *motu) +{ + struct msg_parser *parser; + parser = devm_kzalloc(&motu->card->card_dev, sizeof(*parser), GFP_KERNEL); + if (!parser) + return -ENOMEM; + if (motu->spec == &snd_motu_spec_4pre || motu->spec == &snd_motu_spec_audio_express) + parser->meter_pos_quirk = true; + motu->message_parser = parser; + return 0; +} + +int snd_motu_register_dsp_message_parser_init(struct snd_motu *motu) +{ + return 0; +} + +void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const struct pkt_desc *descs, + unsigned int desc_count, unsigned int data_block_quadlets) +{ + struct msg_parser *parser = motu->message_parser; + bool meter_pos_quirk = parser->meter_pos_quirk; + int i; + + for (i = 0; i < desc_count; ++i) { + const struct pkt_desc *desc = descs + i; + __be32 *buffer = desc->ctx_payload; + unsigned int data_blocks = desc->data_blocks; + int j; + + for (j = 0; j < data_blocks; ++j) { + u8 *b = (u8 *)buffer; + u8 msg_type = (b[MSG_FLAG_POS] & MSG_FLAG_TYPE_MASK) >> MSG_FLAG_TYPE_SHIFT; + u8 val = b[MSG_VALUE_POS]; + + buffer += data_block_quadlets; + + switch (msg_type) { + case METER: + { + u8 pos; + + if (!meter_pos_quirk) + pos = b[MSG_METER_IDX_POS]; + else + pos = b[MSG_METER_IDX_POS_4PRE_AE]; + + if (pos < 0x80) + pos &= 0x1f; + else + pos = (pos & 0x1f) + 20; + parser->meter.data[pos] = val; + break; + } + default: + break; + } + } + } +} diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c index 9e6ca39ebd7f..654b313ba98d 100644 --- a/sound/firewire/motu/motu-stream.c +++ b/sound/firewire/motu/motu-stream.c @@ -255,6 +255,12 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu) if (err < 0) return err; + if (motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP) { + err = snd_motu_register_dsp_message_parser_init(motu); + if (err < 0) + return err; + } + err = begin_session(motu); if (err < 0) { dev_err(&motu->unit->device, diff --git a/sound/firewire/motu/motu.c b/sound/firewire/motu/motu.c index f65426238d4c..0edf8f594a55 100644 --- a/sound/firewire/motu/motu.c +++ b/sound/firewire/motu/motu.c @@ -112,6 +112,12 @@ static int motu_probe(struct fw_unit *unit, const struct ieee1394_device_id *ent if (err < 0) goto error; + if (motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP) { + err = snd_motu_register_dsp_message_parser_new(motu); + if (err < 0) + goto error; + } + err = snd_card_register(card); if (err < 0) goto error; diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h index f1a830b358d4..8d6850bb925e 100644 --- a/sound/firewire/motu/motu.h +++ b/sound/firewire/motu/motu.h @@ -78,6 +78,8 @@ struct snd_motu { struct amdtp_domain domain; struct amdtp_motu_cache cache; + + void *message_parser; }; enum snd_motu_spec_flags { @@ -85,6 +87,7 @@ enum snd_motu_spec_flags { SND_MOTU_SPEC_RX_MIDI_3RD_Q = 0x0002, SND_MOTU_SPEC_TX_MIDI_2ND_Q = 0x0004, SND_MOTU_SPEC_TX_MIDI_3RD_Q = 0x0008, + SND_MOTU_SPEC_REGISTER_DSP = 0x0010, }; #define SND_MOTU_CLOCK_RATE_COUNT 6 @@ -270,4 +273,9 @@ static inline int snd_motu_protocol_cache_packet_formats(struct snd_motu *motu) return -ENXIO; } +int snd_motu_register_dsp_message_parser_new(struct snd_motu *motu); +int snd_motu_register_dsp_message_parser_init(struct snd_motu *motu); +void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const struct pkt_desc *descs, + unsigned int desc_count, unsigned int data_block_quadlets); + #endif -- cgit v1.2.3 From 90b28f3bb85c39b11daf29d473ef21a935c70ec5 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:17 +0900 Subject: ALSA: firewire-motu: add message parser for meter information in command DSP model Some of MOTU models allows software to configure their DSP parameters by command included in asynchronous transaction. The models multiplex messages for hardware meters into isochronous packet as well as PCM frames. For convenience, I call them as 'command DSP' model. This patch adds message parser for them to gather hardware meter information. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 30 ++++ sound/firewire/motu/Makefile | 3 +- sound/firewire/motu/amdtp-motu.c | 3 + .../motu/motu-command-dsp-message-parser.c | 160 +++++++++++++++++++++ sound/firewire/motu/motu-protocol-v3.c | 10 +- sound/firewire/motu/motu-stream.c | 4 + sound/firewire/motu/motu.c | 4 + sound/firewire/motu/motu.h | 7 + 8 files changed, 216 insertions(+), 5 deletions(-) create mode 100644 sound/firewire/motu/motu-command-dsp-message-parser.c diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index 347fd7a05596..82d4765fbeee 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -143,4 +143,34 @@ struct snd_firewire_motu_register_dsp_meter { __u8 data[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_COUNT]; }; +// In below MOTU models, software is allowed to control their DSP by command in frame of +// asynchronous transaction to 0x'ffff'0001'0000: +// +// - 828 mk3 (FireWire only and Hybrid) +// - 896 mk3 (FireWire only and Hybrid) +// - Ultralite mk3 (FireWire only and Hybrid) +// - Traveler mk3 +// - Track 16 +// +// On the other hand, the states of hardware meter is split into specific messages included in the +// sequence of isochronous packet. ALSA firewire-motu driver gathers the message and allow userspace +// application to read it via ioctl. + +#define SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT 400 + +/** + * struct snd_firewire_motu_command_dsp_meter - the container for meter information in DSP + * controlled by command + * @data: Signal level meters. The mapping between position and signal channel is model-dependent. + * + * The structure expresses the part of DSP status for hardware meter. The 32 bit storage is + * estimated to include IEEE 764 32 bit single precision floating point (binary32) value. It is + * expected to be linear value (not logarithm) for audio signal level between 0.0 and +1.0. However, + * the last two quadlets (data[398] and data[399]) are filled with 0xffffffff since they are the + * marker of one period. + */ +struct snd_firewire_motu_command_dsp_meter { + __u32 data[SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT]; +}; + #endif /* _UAPI_SOUND_FIREWIRE_H_INCLUDED */ diff --git a/sound/firewire/motu/Makefile b/sound/firewire/motu/Makefile index edbdf40c7162..3bef2a0b1e2e 100644 --- a/sound/firewire/motu/Makefile +++ b/sound/firewire/motu/Makefile @@ -4,5 +4,6 @@ CFLAGS_amdtp-motu.o := -I$(src) snd-firewire-motu-objs := motu.o amdtp-motu.o motu-transaction.o motu-stream.o \ motu-proc.o motu-pcm.o motu-midi.o motu-hwdep.o \ motu-protocol-v2.o motu-protocol-v3.o \ - motu-protocol-v1.o motu-register-dsp-message-parser.o + motu-protocol-v1.o motu-register-dsp-message-parser.o \ + motu-command-dsp-message-parser.o obj-$(CONFIG_SND_FIREWIRE_MOTU) += snd-firewire-motu.o diff --git a/sound/firewire/motu/amdtp-motu.c b/sound/firewire/motu/amdtp-motu.c index 605b831492ac..3ea91e281147 100644 --- a/sound/firewire/motu/amdtp-motu.c +++ b/sound/firewire/motu/amdtp-motu.c @@ -361,6 +361,9 @@ static unsigned int process_ir_ctx_payloads(struct amdtp_stream *s, if (motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP) { snd_motu_register_dsp_message_parser_parse(motu, descs, packets, s->data_block_quadlets); + } else if (motu->spec->flags & SND_MOTU_SPEC_COMMAND_DSP) { + snd_motu_command_dsp_message_parser_parse(motu, descs, packets, + s->data_block_quadlets); } // For tracepoints. diff --git a/sound/firewire/motu/motu-command-dsp-message-parser.c b/sound/firewire/motu/motu-command-dsp-message-parser.c new file mode 100644 index 000000000000..6716074f8bc1 --- /dev/null +++ b/sound/firewire/motu/motu-command-dsp-message-parser.c @@ -0,0 +1,160 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// motu-command-dsp-message-parser.c - a part of driver for MOTU FireWire series +// +// Copyright (c) 2021 Takashi Sakamoto + +// Below models allow software to configure their DSP function by command transferred in +// asynchronous transaction: +// * 828 mk3 (FireWire only and Hybrid) +// * 896 mk3 (FireWire only and Hybrid) +// * Ultralite mk3 (FireWire only and Hybrid) +// * Traveler mk3 +// * Track 16 +// +// Isochronous packets from the above models includes messages to report state of hardware meter. + +#include "motu.h" + +enum msg_parser_state { + INITIALIZED, + FRAGMENT_DETECTED, + AVAILABLE, +}; + +struct msg_parser { + enum msg_parser_state state; + unsigned int interval; + unsigned int message_count; + unsigned int fragment_pos; + unsigned int value_index; + u64 value; + struct snd_firewire_motu_command_dsp_meter meter; +}; + +int snd_motu_command_dsp_message_parser_new(struct snd_motu *motu) +{ + struct msg_parser *parser; + + parser = devm_kzalloc(&motu->card->card_dev, sizeof(*parser), GFP_KERNEL); + if (!parser) + return -ENOMEM; + motu->message_parser = parser; + + return 0; +} + +int snd_motu_command_dsp_message_parser_init(struct snd_motu *motu, enum cip_sfc sfc) +{ + struct msg_parser *parser = motu->message_parser; + + parser->state = INITIALIZED; + + // All of data blocks don't have messages with meaningful information. + switch (sfc) { + case CIP_SFC_176400: + case CIP_SFC_192000: + parser->interval = 4; + break; + case CIP_SFC_88200: + case CIP_SFC_96000: + parser->interval = 2; + break; + case CIP_SFC_32000: + case CIP_SFC_44100: + case CIP_SFC_48000: + default: + parser->interval = 1; + break; + } + + return 0; +} + +#define FRAGMENT_POS 6 +#define MIDI_BYTE_POS 7 +#define MIDI_FLAG_POS 8 +// One value of hardware meter consists of 4 messages. +#define FRAGMENTS_PER_VALUE 4 +#define VALUES_AT_IMAGE_END 0xffffffffffffffff + +void snd_motu_command_dsp_message_parser_parse(struct snd_motu *motu, const struct pkt_desc *descs, + unsigned int desc_count, unsigned int data_block_quadlets) +{ + struct msg_parser *parser = motu->message_parser; + unsigned int interval = parser->interval; + int i; + + for (i = 0; i < desc_count; ++i) { + const struct pkt_desc *desc = descs + i; + __be32 *buffer = desc->ctx_payload; + unsigned int data_blocks = desc->data_blocks; + int j; + + for (j = 0; j < data_blocks; ++j) { + u8 *b = (u8 *)buffer; + buffer += data_block_quadlets; + + switch (parser->state) { + case INITIALIZED: + { + u8 fragment = b[FRAGMENT_POS]; + + if (fragment > 0) { + parser->value = fragment; + parser->message_count = 1; + parser->state = FRAGMENT_DETECTED; + } + break; + } + case FRAGMENT_DETECTED: + { + if (parser->message_count % interval == 0) { + u8 fragment = b[FRAGMENT_POS]; + + parser->value >>= 8; + parser->value |= (u64)fragment << 56; + + if (parser->value == VALUES_AT_IMAGE_END) { + parser->state = AVAILABLE; + parser->fragment_pos = 0; + parser->value_index = 0; + parser->message_count = 0; + } + } + ++parser->message_count; + break; + } + case AVAILABLE: + default: + { + if (parser->message_count % interval == 0) { + u8 fragment = b[FRAGMENT_POS]; + + parser->value >>= 8; + parser->value |= (u64)fragment << 56; + ++parser->fragment_pos; + + if (parser->fragment_pos == 4) { + if (parser->value_index < + SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT) { + u32 val = (u32)(parser->value >> 32); + parser->meter.data[parser->value_index] = val; + ++parser->value_index; + } + parser->fragment_pos = 0; + } + + if (parser->value == VALUES_AT_IMAGE_END) { + parser->value_index = 0; + parser->fragment_pos = 0; + parser->message_count = 0; + } + } + ++parser->message_count; + break; + } + } + } + } +} diff --git a/sound/firewire/motu/motu-protocol-v3.c b/sound/firewire/motu/motu-protocol-v3.c index d0dd587460de..05608e8ca0bc 100644 --- a/sound/firewire/motu/motu-protocol-v3.c +++ b/sound/firewire/motu/motu-protocol-v3.c @@ -261,12 +261,12 @@ int snd_motu_protocol_v3_cache_packet_formats(struct snd_motu *motu) return 0; } - const struct snd_motu_spec snd_motu_spec_828mk3_fw = { .name = "828mk3", .protocol_version = SND_MOTU_PROTOCOL_V3, .flags = SND_MOTU_SPEC_RX_MIDI_3RD_Q | - SND_MOTU_SPEC_TX_MIDI_3RD_Q, + SND_MOTU_SPEC_TX_MIDI_3RD_Q | + SND_MOTU_SPEC_COMMAND_DSP, .tx_fixed_pcm_chunks = {18, 18, 14}, .rx_fixed_pcm_chunks = {14, 14, 10}, }; @@ -275,7 +275,8 @@ const struct snd_motu_spec snd_motu_spec_828mk3_hybrid = { .name = "828mk3", .protocol_version = SND_MOTU_PROTOCOL_V3, .flags = SND_MOTU_SPEC_RX_MIDI_3RD_Q | - SND_MOTU_SPEC_TX_MIDI_3RD_Q, + SND_MOTU_SPEC_TX_MIDI_3RD_Q | + SND_MOTU_SPEC_COMMAND_DSP, .tx_fixed_pcm_chunks = {18, 18, 14}, .rx_fixed_pcm_chunks = {14, 14, 14}, // Additional 4 dummy chunks at higher rate. }; @@ -284,7 +285,8 @@ const struct snd_motu_spec snd_motu_spec_ultralite_mk3 = { .name = "UltraLiteMk3", .protocol_version = SND_MOTU_PROTOCOL_V3, .flags = SND_MOTU_SPEC_RX_MIDI_3RD_Q | - SND_MOTU_SPEC_TX_MIDI_3RD_Q, + SND_MOTU_SPEC_TX_MIDI_3RD_Q | + SND_MOTU_SPEC_COMMAND_DSP, .tx_fixed_pcm_chunks = {18, 14, 10}, .rx_fixed_pcm_chunks = {14, 14, 14}, }; diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c index 654b313ba98d..64aec9c3eefd 100644 --- a/sound/firewire/motu/motu-stream.c +++ b/sound/firewire/motu/motu-stream.c @@ -259,6 +259,10 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu) err = snd_motu_register_dsp_message_parser_init(motu); if (err < 0) return err; + } else if (motu->spec->flags & SND_MOTU_SPEC_COMMAND_DSP) { + err = snd_motu_command_dsp_message_parser_init(motu, motu->tx_stream.sfc); + if (err < 0) + return err; } err = begin_session(motu); diff --git a/sound/firewire/motu/motu.c b/sound/firewire/motu/motu.c index 0edf8f594a55..5fc7ae475537 100644 --- a/sound/firewire/motu/motu.c +++ b/sound/firewire/motu/motu.c @@ -116,6 +116,10 @@ static int motu_probe(struct fw_unit *unit, const struct ieee1394_device_id *ent err = snd_motu_register_dsp_message_parser_new(motu); if (err < 0) goto error; + } else if (motu->spec->flags & SND_MOTU_SPEC_COMMAND_DSP) { + err = snd_motu_command_dsp_message_parser_new(motu); + if (err < 0) + goto error; } err = snd_card_register(card); diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h index 8d6850bb925e..d818ce4901c9 100644 --- a/sound/firewire/motu/motu.h +++ b/sound/firewire/motu/motu.h @@ -88,6 +88,7 @@ enum snd_motu_spec_flags { SND_MOTU_SPEC_TX_MIDI_2ND_Q = 0x0004, SND_MOTU_SPEC_TX_MIDI_3RD_Q = 0x0008, SND_MOTU_SPEC_REGISTER_DSP = 0x0010, + SND_MOTU_SPEC_COMMAND_DSP = 0x0020, }; #define SND_MOTU_CLOCK_RATE_COUNT 6 @@ -278,4 +279,10 @@ int snd_motu_register_dsp_message_parser_init(struct snd_motu *motu); void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const struct pkt_desc *descs, unsigned int desc_count, unsigned int data_block_quadlets); + +int snd_motu_command_dsp_message_parser_new(struct snd_motu *motu); +int snd_motu_command_dsp_message_parser_init(struct snd_motu *motu, enum cip_sfc sfc); +void snd_motu_command_dsp_message_parser_parse(struct snd_motu *motu, const struct pkt_desc *descs, + unsigned int desc_count, unsigned int data_block_quadlets); + #endif -- cgit v1.2.3 From 58b62ab7025912ce1be36e3ba19d49620a0161b6 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:18 +0900 Subject: ALSA: firewire-motu: add ioctl command to read cached hardware meter This patch adds new ioctl commands for userspace applications to read cached image about hardware meters in register DSP and command DSP models. The content of image differs depending on models. Model-specific parser should be implemented in userspace. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-4-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 2 + .../motu/motu-command-dsp-message-parser.c | 18 +++++++++ sound/firewire/motu/motu-hwdep.c | 44 ++++++++++++++++++++++ .../motu/motu-register-dsp-message-parser.c | 18 +++++++++ sound/firewire/motu/motu.h | 4 ++ 5 files changed, 86 insertions(+) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index 82d4765fbeee..a8df8fb03b52 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -80,6 +80,8 @@ union snd_firewire_event { #define SNDRV_FIREWIRE_IOCTL_LOCK _IO('H', 0xf9) #define SNDRV_FIREWIRE_IOCTL_UNLOCK _IO('H', 0xfa) #define SNDRV_FIREWIRE_IOCTL_TASCAM_STATE _IOR('H', 0xfb, struct snd_firewire_tascam_state) +#define SNDRV_FIREWIRE_IOCTL_MOTU_REGISTER_DSP_METER _IOR('H', 0xfc, struct snd_firewire_motu_register_dsp_meter) +#define SNDRV_FIREWIRE_IOCTL_MOTU_COMMAND_DSP_METER _IOR('H', 0xfd, struct snd_firewire_motu_command_dsp_meter) #define SNDRV_FIREWIRE_TYPE_DICE 1 #define SNDRV_FIREWIRE_TYPE_FIREWORKS 2 diff --git a/sound/firewire/motu/motu-command-dsp-message-parser.c b/sound/firewire/motu/motu-command-dsp-message-parser.c index 6716074f8bc1..18689fcfb288 100644 --- a/sound/firewire/motu/motu-command-dsp-message-parser.c +++ b/sound/firewire/motu/motu-command-dsp-message-parser.c @@ -23,6 +23,7 @@ enum msg_parser_state { }; struct msg_parser { + spinlock_t lock; enum msg_parser_state state; unsigned int interval; unsigned int message_count; @@ -39,6 +40,7 @@ int snd_motu_command_dsp_message_parser_new(struct snd_motu *motu) parser = devm_kzalloc(&motu->card->card_dev, sizeof(*parser), GFP_KERNEL); if (!parser) return -ENOMEM; + spin_lock_init(&parser->lock); motu->message_parser = parser; return 0; @@ -83,8 +85,11 @@ void snd_motu_command_dsp_message_parser_parse(struct snd_motu *motu, const stru { struct msg_parser *parser = motu->message_parser; unsigned int interval = parser->interval; + unsigned long flags; int i; + spin_lock_irqsave(&parser->lock, flags); + for (i = 0; i < desc_count; ++i) { const struct pkt_desc *desc = descs + i; __be32 *buffer = desc->ctx_payload; @@ -157,4 +162,17 @@ void snd_motu_command_dsp_message_parser_parse(struct snd_motu *motu, const stru } } } + + spin_unlock_irqrestore(&parser->lock, flags); +} + +void snd_motu_command_dsp_message_parser_copy_meter(struct snd_motu *motu, + struct snd_firewire_motu_command_dsp_meter *meter) +{ + struct msg_parser *parser = motu->message_parser; + unsigned long flags; + + spin_lock_irqsave(&parser->lock, flags); + memcpy(meter, &parser->meter, sizeof(*meter)); + spin_unlock_irqrestore(&parser->lock, flags); } diff --git a/sound/firewire/motu/motu-hwdep.c b/sound/firewire/motu/motu-hwdep.c index b5ced5d27758..7be576fe4516 100644 --- a/sound/firewire/motu/motu-hwdep.c +++ b/sound/firewire/motu/motu-hwdep.c @@ -155,6 +155,50 @@ static int hwdep_ioctl(struct snd_hwdep *hwdep, struct file *file, return hwdep_lock(motu); case SNDRV_FIREWIRE_IOCTL_UNLOCK: return hwdep_unlock(motu); + case SNDRV_FIREWIRE_IOCTL_MOTU_REGISTER_DSP_METER: + { + struct snd_firewire_motu_register_dsp_meter *meter; + int err; + + if (!(motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP)) + return -ENXIO; + + meter = kzalloc(sizeof(*meter), GFP_KERNEL); + if (!meter) + return -ENOMEM; + + snd_motu_register_dsp_message_parser_copy_meter(motu, meter); + + err = copy_to_user((void __user *)arg, meter, sizeof(*meter)); + kfree(meter); + + if (err) + return -EFAULT; + + return 0; + } + case SNDRV_FIREWIRE_IOCTL_MOTU_COMMAND_DSP_METER: + { + struct snd_firewire_motu_command_dsp_meter *meter; + int err; + + if (!(motu->spec->flags & SND_MOTU_SPEC_COMMAND_DSP)) + return -ENXIO; + + meter = kzalloc(sizeof(*meter), GFP_KERNEL); + if (!meter) + return -ENOMEM; + + snd_motu_command_dsp_message_parser_copy_meter(motu, meter); + + err = copy_to_user((void __user *)arg, meter, sizeof(*meter)); + kfree(meter); + + if (err) + return -EFAULT; + + return 0; + } default: return -ENOIOCTLCMD; } diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c index efb9708b5b5f..fe804615294c 100644 --- a/sound/firewire/motu/motu-register-dsp-message-parser.c +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -79,6 +79,7 @@ enum register_dsp_msg_type { }; struct msg_parser { + spinlock_t lock; struct snd_firewire_motu_register_dsp_meter meter; bool meter_pos_quirk; }; @@ -89,6 +90,7 @@ int snd_motu_register_dsp_message_parser_new(struct snd_motu *motu) parser = devm_kzalloc(&motu->card->card_dev, sizeof(*parser), GFP_KERNEL); if (!parser) return -ENOMEM; + spin_lock_init(&parser->lock); if (motu->spec == &snd_motu_spec_4pre || motu->spec == &snd_motu_spec_audio_express) parser->meter_pos_quirk = true; motu->message_parser = parser; @@ -105,8 +107,11 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str { struct msg_parser *parser = motu->message_parser; bool meter_pos_quirk = parser->meter_pos_quirk; + unsigned long flags; int i; + spin_lock_irqsave(&parser->lock, flags); + for (i = 0; i < desc_count; ++i) { const struct pkt_desc *desc = descs + i; __be32 *buffer = desc->ctx_payload; @@ -142,4 +147,17 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str } } } + + spin_unlock_irqrestore(&parser->lock, flags); +} + +void snd_motu_register_dsp_message_parser_copy_meter(struct snd_motu *motu, + struct snd_firewire_motu_register_dsp_meter *meter) +{ + struct msg_parser *parser = motu->message_parser; + unsigned long flags; + + spin_lock_irqsave(&parser->lock, flags); + memcpy(meter, &parser->meter, sizeof(*meter)); + spin_unlock_irqrestore(&parser->lock, flags); } diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h index d818ce4901c9..4f70036dea25 100644 --- a/sound/firewire/motu/motu.h +++ b/sound/firewire/motu/motu.h @@ -278,11 +278,15 @@ int snd_motu_register_dsp_message_parser_new(struct snd_motu *motu); int snd_motu_register_dsp_message_parser_init(struct snd_motu *motu); void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const struct pkt_desc *descs, unsigned int desc_count, unsigned int data_block_quadlets); +void snd_motu_register_dsp_message_parser_copy_meter(struct snd_motu *motu, + struct snd_firewire_motu_register_dsp_meter *meter); int snd_motu_command_dsp_message_parser_new(struct snd_motu *motu); int snd_motu_command_dsp_message_parser_init(struct snd_motu *motu, enum cip_sfc sfc); void snd_motu_command_dsp_message_parser_parse(struct snd_motu *motu, const struct pkt_desc *descs, unsigned int desc_count, unsigned int data_block_quadlets); +void snd_motu_command_dsp_message_parser_copy_meter(struct snd_motu *motu, + struct snd_firewire_motu_command_dsp_meter *meter); #endif -- cgit v1.2.3 From dc36a9755a572781903d79f8437d109b72662da5 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:19 +0900 Subject: ALSA: firewire-motu: parse messages for mixer source parameters in register-DSP model In register DSP models, current parameters of DSP are always reported by messages in isochronous packet. When user operates hardware component such as rotary knob, corresponding message is changed. This commit parses the message and cache current parameters of mixer source function, commonly available for all of register DSP models. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-5-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 28 ++++++++++ .../motu/motu-register-dsp-message-parser.c | 64 ++++++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index a8df8fb03b52..bb5ecff73896 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -145,6 +145,34 @@ struct snd_firewire_motu_register_dsp_meter { __u8 data[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_COUNT]; }; +#define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT 4 +#define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_SRC_COUNT 20 + +/** + * snd_firewire_motu_register_dsp_parameter - the container for parameters of DSP controlled + * by register access. + * @mixer.source.gain: The gain of source to mixer. + * @mixer.source.pan: The L/R balance of source to mixer. + * @mixer.source.flag: The flag of source to mixer, including mute, solo. + * @mixer.source.paired_balance: The L/R balance of paired source to mixer, only for 4 pre and + * Audio Express. + * @mixer.source.paired_width: The width of paired source to mixer, only for 4 pre and + * Audio Express. + * + * The structure expresses the set of parameters for DSP controlled by register access. + */ +struct snd_firewire_motu_register_dsp_parameter { + struct { + struct { + __u8 gain[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_SRC_COUNT]; + __u8 pan[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_SRC_COUNT]; + __u8 flag[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_SRC_COUNT]; + __u8 paired_balance[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_SRC_COUNT]; + __u8 paired_width[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_SRC_COUNT]; + } source[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT]; + } mixer; +}; + // In below MOTU models, software is allowed to control their DSP by command in frame of // asynchronous transaction to 0x'ffff'0001'0000: // diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c index fe804615294c..6df40e5ee9db 100644 --- a/sound/firewire/motu/motu-register-dsp-message-parser.c +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -82,6 +82,11 @@ struct msg_parser { spinlock_t lock; struct snd_firewire_motu_register_dsp_meter meter; bool meter_pos_quirk; + + struct snd_firewire_motu_register_dsp_parameter param; + u8 prev_mixer_src_type; + u8 mixer_ch; + u8 mixer_src_ch; }; int snd_motu_register_dsp_message_parser_new(struct snd_motu *motu) @@ -99,6 +104,12 @@ int snd_motu_register_dsp_message_parser_new(struct snd_motu *motu) int snd_motu_register_dsp_message_parser_init(struct snd_motu *motu) { + struct msg_parser *parser = motu->message_parser; + + parser->prev_mixer_src_type = INVALID; + parser->mixer_ch = 0xff; + parser->mixer_src_ch = 0xff; + return 0; } @@ -126,6 +137,59 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str buffer += data_block_quadlets; switch (msg_type) { + case MIXER_SELECT: + { + u8 mixer_ch = val / 0x20; + if (mixer_ch < SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT) { + parser->mixer_src_ch = 0; + parser->mixer_ch = mixer_ch; + } + break; + } + case MIXER_SRC_GAIN: + case MIXER_SRC_PAN: + case MIXER_SRC_FLAG: + case MIXER_SRC_PAIRED_BALANCE: + case MIXER_SRC_PAIRED_WIDTH: + { + struct snd_firewire_motu_register_dsp_parameter *param = &parser->param; + u8 mixer_ch = parser->mixer_ch; + u8 mixer_src_ch = parser->mixer_src_ch; + + if (msg_type != parser->prev_mixer_src_type) + mixer_src_ch = 0; + else + ++mixer_src_ch; + parser->prev_mixer_src_type = msg_type; + + if (mixer_ch < SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT && + mixer_src_ch < SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_SRC_COUNT) { + u8 mixer_ch = parser->mixer_ch; + + switch (msg_type) { + case MIXER_SRC_GAIN: + param->mixer.source[mixer_ch].gain[mixer_src_ch] = val; + break; + case MIXER_SRC_PAN: + param->mixer.source[mixer_ch].pan[mixer_src_ch] = val; + break; + case MIXER_SRC_FLAG: + param->mixer.source[mixer_ch].flag[mixer_src_ch] = val; + break; + case MIXER_SRC_PAIRED_BALANCE: + param->mixer.source[mixer_ch].paired_balance[mixer_src_ch] = val; + break; + case MIXER_SRC_PAIRED_WIDTH: + param->mixer.source[mixer_ch].paired_width[mixer_src_ch] = val; + break; + default: + break; + } + + parser->mixer_src_ch = mixer_src_ch; + } + break; + } case METER: { u8 pos; -- cgit v1.2.3 From ce69bed5557b05dd1918556d4e90c293382155ae Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:20 +0900 Subject: ALSA: firewire-motu: parse messages for mixer output parameters in register DSP model This commit parses message and cache current parameters of mixer output function, commonly available for all of register DSP model Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-6-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 6 ++++++ .../firewire/motu/motu-register-dsp-message-parser.c | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index bb5ecff73896..f663a26c5205 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -158,6 +158,8 @@ struct snd_firewire_motu_register_dsp_meter { * Audio Express. * @mixer.source.paired_width: The width of paired source to mixer, only for 4 pre and * Audio Express. + * @mixer.output.paired_volume: The volume of paired output from mixer. + * @mixer.output.paired_flag: The flag of paired output from mixer. * * The structure expresses the set of parameters for DSP controlled by register access. */ @@ -170,6 +172,10 @@ struct snd_firewire_motu_register_dsp_parameter { __u8 paired_balance[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_SRC_COUNT]; __u8 paired_width[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_SRC_COUNT]; } source[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT]; + struct { + __u8 paired_volume[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT]; + __u8 paired_flag[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT]; + } output; } mixer; }; diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c index 6df40e5ee9db..867cb09a3521 100644 --- a/sound/firewire/motu/motu-register-dsp-message-parser.c +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -190,6 +190,26 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str } break; } + case MIXER_OUTPUT_PAIRED_VOLUME: + case MIXER_OUTPUT_PAIRED_FLAG: + { + struct snd_firewire_motu_register_dsp_parameter *param = &parser->param; + u8 mixer_ch = parser->mixer_ch; + + if (mixer_ch < SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT) { + switch (msg_type) { + case MIXER_OUTPUT_PAIRED_VOLUME: + param->mixer.output.paired_volume[mixer_ch] = val; + break; + case MIXER_OUTPUT_PAIRED_FLAG: + param->mixer.output.paired_flag[mixer_ch] = val; + break; + default: + break; + } + } + break; + } case METER: { u8 pos; -- cgit v1.2.3 From 6ca81d2b6305a884da441fd0281ff01afd5f8c7e Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:21 +0900 Subject: ALSA: firewire-motu: parse messages for output parameters in register DSP model This commit parses message and cache current parameters of output function, commonly available for all of register DSP model. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-7-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 10 ++++++++++ sound/firewire/motu/motu-register-dsp-message-parser.c | 11 ++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index f663a26c5205..16ca7b43568b 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -160,6 +160,10 @@ struct snd_firewire_motu_register_dsp_meter { * Audio Express. * @mixer.output.paired_volume: The volume of paired output from mixer. * @mixer.output.paired_flag: The flag of paired output from mixer. + * @output.main_paired_volume: The volume of paired main output. + * @output.hp_paired_volume: The volume of paired hp output. + * @output.hp_paired_assignment: The source assigned to paired hp output. + * @output.reserved: Padding for 32 bit alignment for future extension. * * The structure expresses the set of parameters for DSP controlled by register access. */ @@ -177,6 +181,12 @@ struct snd_firewire_motu_register_dsp_parameter { __u8 paired_flag[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT]; } output; } mixer; + struct { + __u8 main_paired_volume; + __u8 hp_paired_volume; + __u8 hp_paired_assignment; + __u8 reserved[5]; + } output; }; // In below MOTU models, software is allowed to control their DSP by command in frame of diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c index 867cb09a3521..244f7ada851f 100644 --- a/sound/firewire/motu/motu-register-dsp-message-parser.c +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -54,7 +54,7 @@ enum register_dsp_msg_type { MIXER_OUTPUT_PAIRED_FLAG = 0x06, MAIN_OUTPUT_PAIRED_VOLUME = 0x07, HP_OUTPUT_PAIRED_VOLUME = 0x08, - HP_OUTPUT_ASSIGN = 0x09, + HP_OUTPUT_PAIRED_ASSIGNMENT = 0x09, // Transferred by all models but the purpose is still unknown. UNKNOWN_0 = 0x0a, // Specific to 828mk2, 896hd, Traveler. @@ -210,6 +210,15 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str } break; } + case MAIN_OUTPUT_PAIRED_VOLUME: + parser->param.output.main_paired_volume = val; + break; + case HP_OUTPUT_PAIRED_VOLUME: + parser->param.output.hp_paired_volume = val; + break; + case HP_OUTPUT_PAIRED_ASSIGNMENT: + parser->param.output.hp_paired_assignment = val; + break; case METER: { u8 pos; -- cgit v1.2.3 From 41cc23389f5fc64bdac78b73935a44bd5abc990d Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:22 +0900 Subject: ALSA: firewire-motu: parse messages for line input parameters in register DSP model This commit parses message and cache current parameters of line input function, available for MOTU 828 mk2 and Traveler. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-8-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 9 +++++++++ sound/firewire/motu/motu-register-dsp-message-parser.c | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index 16ca7b43568b..049934e2a53c 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -164,6 +164,10 @@ struct snd_firewire_motu_register_dsp_meter { * @output.hp_paired_volume: The volume of paired hp output. * @output.hp_paired_assignment: The source assigned to paired hp output. * @output.reserved: Padding for 32 bit alignment for future extension. + * @line_input.boost_flag: The flags of boost for line inputs, only for 828mk2 and Traveler. + * @line_input.nominal_level_flag: The flags of nominal level for line inputs, only for 828mk2 and + * Traveler. + * @line_input.reserved: Padding for 32 bit alignment for future extension. * * The structure expresses the set of parameters for DSP controlled by register access. */ @@ -187,6 +191,11 @@ struct snd_firewire_motu_register_dsp_parameter { __u8 hp_paired_assignment; __u8 reserved[5]; } output; + struct { + __u8 boost_flag; + __u8 nominal_level_flag; + __u8 reserved[6]; + } line_input; }; // In below MOTU models, software is allowed to control their DSP by command in frame of diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c index 244f7ada851f..85faf7a4e8a3 100644 --- a/sound/firewire/motu/motu-register-dsp-message-parser.c +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -219,6 +219,12 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str case HP_OUTPUT_PAIRED_ASSIGNMENT: parser->param.output.hp_paired_assignment = val; break; + case LINE_INPUT_BOOST: + parser->param.line_input.boost_flag = val; + break; + case LINE_INPUT_NOMINAL_LEVEL: + parser->param.line_input.nominal_level_flag = val; + break; case METER: { u8 pos; -- cgit v1.2.3 From 7d843c494a9b69d07bc0588124599e3f665a1496 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:23 +0900 Subject: ALSA: firewire-motu: parse messages for input parameters in register DSP model This commit parses message and cache current parameters of input function, available for MOTU Ultralite, 4 pre, and Audio Express. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-9-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 12 ++++++ .../motu/motu-register-dsp-message-parser.c | 43 +++++++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index 049934e2a53c..6366127e923e 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -147,6 +147,8 @@ struct snd_firewire_motu_register_dsp_meter { #define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT 4 #define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_SRC_COUNT 20 +#define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_INPUT_COUNT 10 +#define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_ALIGNED_INPUT_COUNT (SNDRV_FIREWIRE_MOTU_REGISTER_DSP_INPUT_COUNT + 2) /** * snd_firewire_motu_register_dsp_parameter - the container for parameters of DSP controlled @@ -168,6 +170,11 @@ struct snd_firewire_motu_register_dsp_meter { * @line_input.nominal_level_flag: The flags of nominal level for line inputs, only for 828mk2 and * Traveler. * @line_input.reserved: Padding for 32 bit alignment for future extension. + * @input.gain_and_invert: The value including gain and invert for input, only for Ultralite, 4 pre + * and Audio Express. + * @input.flag: The flag of input; e.g. jack detection, phantom power, and pad, only for Ultralite, + * 4 pre and Audio express. + * @reserved: Padding so that the size of structure is kept to 512 byte, but for future extension. * * The structure expresses the set of parameters for DSP controlled by register access. */ @@ -196,6 +203,11 @@ struct snd_firewire_motu_register_dsp_parameter { __u8 nominal_level_flag; __u8 reserved[6]; } line_input; + struct { + __u8 gain_and_invert[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_ALIGNED_INPUT_COUNT]; + __u8 flag[SNDRV_FIREWIRE_MOTU_REGISTER_DSP_ALIGNED_INPUT_COUNT]; + } input; + __u8 reserved[64]; }; // In below MOTU models, software is allowed to control their DSP by command in frame of diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c index 85faf7a4e8a3..d94ca4875714 100644 --- a/sound/firewire/motu/motu-register-dsp-message-parser.c +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -87,6 +87,9 @@ struct msg_parser { u8 prev_mixer_src_type; u8 mixer_ch; u8 mixer_src_ch; + + u8 input_ch; + u8 prev_msg_type; }; int snd_motu_register_dsp_message_parser_new(struct snd_motu *motu) @@ -109,6 +112,7 @@ int snd_motu_register_dsp_message_parser_init(struct snd_motu *motu) parser->prev_mixer_src_type = INVALID; parser->mixer_ch = 0xff; parser->mixer_src_ch = 0xff; + parser->prev_msg_type = INVALID; return 0; } @@ -225,6 +229,35 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str case LINE_INPUT_NOMINAL_LEVEL: parser->param.line_input.nominal_level_flag = val; break; + case INPUT_GAIN_AND_INVERT: + case INPUT_FLAG: + { + struct snd_firewire_motu_register_dsp_parameter *param = &parser->param; + u8 input_ch = parser->input_ch; + + if (parser->prev_msg_type != msg_type) + input_ch = 0; + else + ++input_ch; + + if (input_ch < SNDRV_FIREWIRE_MOTU_REGISTER_DSP_INPUT_COUNT) { + switch (msg_type) { + case INPUT_GAIN_AND_INVERT: + param->input.gain_and_invert[input_ch] = val; + break; + case INPUT_FLAG: + param->input.flag[input_ch] = val; + break; + default: + break; + } + parser->input_ch = input_ch; + } + break; + } + case UNKNOWN_0: + case UNKNOWN_2: + break; case METER: { u8 pos; @@ -239,11 +272,17 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str else pos = (pos & 0x1f) + 20; parser->meter.data[pos] = val; - break; + // The message for meter is interruptible to the series of other + // types of messages. Don't cache it. + fallthrough; } + case INVALID: default: - break; + // Don't cache it. + continue; } + + parser->prev_msg_type = msg_type; } } -- cgit v1.2.3 From ca15a09ccc5bd2731c5fcb667e6ea3bbbf8f5772 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:24 +0900 Subject: ALSA: firewire-motu: add ioctl command to read cached parameters in register DSP model This patch adds new ioctl command for userspace applications to read cached parameters of register DSP. The structured data includes model-dependent parameters. Userspace application should be carefully programmed so that what parameter is common and specific. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-10-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 1 + sound/firewire/motu/motu-hwdep.c | 21 +++++++++++++++++++++ .../motu/motu-register-dsp-message-parser.c | 11 +++++++++++ sound/firewire/motu/motu.h | 3 ++- 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index 6366127e923e..d52691655d79 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -82,6 +82,7 @@ union snd_firewire_event { #define SNDRV_FIREWIRE_IOCTL_TASCAM_STATE _IOR('H', 0xfb, struct snd_firewire_tascam_state) #define SNDRV_FIREWIRE_IOCTL_MOTU_REGISTER_DSP_METER _IOR('H', 0xfc, struct snd_firewire_motu_register_dsp_meter) #define SNDRV_FIREWIRE_IOCTL_MOTU_COMMAND_DSP_METER _IOR('H', 0xfd, struct snd_firewire_motu_command_dsp_meter) +#define SNDRV_FIREWIRE_IOCTL_MOTU_REGISTER_DSP_PARAMETER _IOR('H', 0xfe, struct snd_firewire_motu_register_dsp_parameter) #define SNDRV_FIREWIRE_TYPE_DICE 1 #define SNDRV_FIREWIRE_TYPE_FIREWORKS 2 diff --git a/sound/firewire/motu/motu-hwdep.c b/sound/firewire/motu/motu-hwdep.c index 7be576fe4516..699136b911c7 100644 --- a/sound/firewire/motu/motu-hwdep.c +++ b/sound/firewire/motu/motu-hwdep.c @@ -199,6 +199,27 @@ static int hwdep_ioctl(struct snd_hwdep *hwdep, struct file *file, return 0; } + case SNDRV_FIREWIRE_IOCTL_MOTU_REGISTER_DSP_PARAMETER: + { + struct snd_firewire_motu_register_dsp_parameter *param; + int err; + + if (!(motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP)) + return -ENXIO; + + param = kzalloc(sizeof(*param), GFP_KERNEL); + if (!param) + return -ENOMEM; + + snd_motu_register_dsp_message_parser_copy_parameter(motu, param); + + err = copy_to_user((void __user *)arg, param, sizeof(*param)); + kfree(param); + if (err) + return -EFAULT; + + return 0; + } default: return -ENOIOCTLCMD; } diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c index d94ca4875714..ed9fd0cef200 100644 --- a/sound/firewire/motu/motu-register-dsp-message-parser.c +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -299,3 +299,14 @@ void snd_motu_register_dsp_message_parser_copy_meter(struct snd_motu *motu, memcpy(meter, &parser->meter, sizeof(*meter)); spin_unlock_irqrestore(&parser->lock, flags); } + +void snd_motu_register_dsp_message_parser_copy_parameter(struct snd_motu *motu, + struct snd_firewire_motu_register_dsp_parameter *param) +{ + struct msg_parser *parser = motu->message_parser; + unsigned long flags; + + spin_lock_irqsave(&parser->lock, flags); + memcpy(param, &parser->param, sizeof(*param)); + spin_unlock_irqrestore(&parser->lock, flags); +} diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h index 4f70036dea25..fa0b3ab7b78d 100644 --- a/sound/firewire/motu/motu.h +++ b/sound/firewire/motu/motu.h @@ -280,7 +280,8 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str unsigned int desc_count, unsigned int data_block_quadlets); void snd_motu_register_dsp_message_parser_copy_meter(struct snd_motu *motu, struct snd_firewire_motu_register_dsp_meter *meter); - +void snd_motu_register_dsp_message_parser_copy_parameter(struct snd_motu *motu, + struct snd_firewire_motu_register_dsp_parameter *params); int snd_motu_command_dsp_message_parser_new(struct snd_motu *motu); int snd_motu_command_dsp_message_parser_init(struct snd_motu *motu, enum cip_sfc sfc); -- cgit v1.2.3 From 4c9eda8f37f9523f1d2ccbb442ce641e8c981c9f Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:25 +0900 Subject: ALSA: firewire-motu: queue event for parameter change in register DSP model This commit is a preparation to notify parameter change of register DSP to userspace application. A simple queue is added to store encoded data for the change as long as ALSA hwdep character device is opened by application. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-11-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- sound/firewire/motu/motu-hwdep.c | 2 + .../motu/motu-register-dsp-message-parser.c | 93 ++++++++++++++++++---- sound/firewire/motu/motu.h | 1 + 3 files changed, 82 insertions(+), 14 deletions(-) diff --git a/sound/firewire/motu/motu-hwdep.c b/sound/firewire/motu/motu-hwdep.c index 699136b911c7..389e59ff768b 100644 --- a/sound/firewire/motu/motu-hwdep.c +++ b/sound/firewire/motu/motu-hwdep.c @@ -258,5 +258,7 @@ int snd_motu_create_hwdep_device(struct snd_motu *motu) hwdep->private_data = motu; hwdep->exclusive = true; + motu->hwdep = hwdep; + return 0; } diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c index ed9fd0cef200..cda8e6d987cc 100644 --- a/sound/firewire/motu/motu-register-dsp-message-parser.c +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -78,6 +78,8 @@ enum register_dsp_msg_type { METER = 0x1f, }; +#define EVENT_QUEUE_SIZE 16 + struct msg_parser { spinlock_t lock; struct snd_firewire_motu_register_dsp_meter meter; @@ -90,6 +92,9 @@ struct msg_parser { u8 input_ch; u8 prev_msg_type; + + u32 event_queue[EVENT_QUEUE_SIZE]; + unsigned int push_pos; }; int snd_motu_register_dsp_message_parser_new(struct snd_motu *motu) @@ -117,6 +122,24 @@ int snd_motu_register_dsp_message_parser_init(struct snd_motu *motu) return 0; } +static void queue_event(struct snd_motu *motu, u8 msg_type, u8 identifier0, u8 identifier1, u8 val) +{ + struct msg_parser *parser = motu->message_parser; + unsigned int pos = parser->push_pos; + u32 entry; + + if (!motu->hwdep || motu->hwdep->used == 0) + return; + + entry = (msg_type << 24) | (identifier0 << 16) | (identifier1 << 8) | val; + parser->event_queue[pos] = entry; + + ++pos; + if (pos >= EVENT_QUEUE_SIZE) + pos = 0; + parser->push_pos = pos; +} + void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const struct pkt_desc *descs, unsigned int desc_count, unsigned int data_block_quadlets) { @@ -172,19 +195,34 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str switch (msg_type) { case MIXER_SRC_GAIN: - param->mixer.source[mixer_ch].gain[mixer_src_ch] = val; + if (param->mixer.source[mixer_ch].gain[mixer_src_ch] != val) { + queue_event(motu, msg_type, mixer_ch, mixer_src_ch, val); + param->mixer.source[mixer_ch].gain[mixer_src_ch] = val; + } break; case MIXER_SRC_PAN: - param->mixer.source[mixer_ch].pan[mixer_src_ch] = val; + if (param->mixer.source[mixer_ch].pan[mixer_src_ch] != val) { + queue_event(motu, msg_type, mixer_ch, mixer_src_ch, val); + param->mixer.source[mixer_ch].pan[mixer_src_ch] = val; + } break; case MIXER_SRC_FLAG: - param->mixer.source[mixer_ch].flag[mixer_src_ch] = val; + if (param->mixer.source[mixer_ch].flag[mixer_src_ch] != val) { + queue_event(motu, msg_type, mixer_ch, mixer_src_ch, val); + param->mixer.source[mixer_ch].flag[mixer_src_ch] = val; + } break; case MIXER_SRC_PAIRED_BALANCE: - param->mixer.source[mixer_ch].paired_balance[mixer_src_ch] = val; + if (param->mixer.source[mixer_ch].paired_balance[mixer_src_ch] != val) { + queue_event(motu, msg_type, mixer_ch, mixer_src_ch, val); + param->mixer.source[mixer_ch].paired_balance[mixer_src_ch] = val; + } break; case MIXER_SRC_PAIRED_WIDTH: - param->mixer.source[mixer_ch].paired_width[mixer_src_ch] = val; + if (param->mixer.source[mixer_ch].paired_width[mixer_src_ch] != val) { + queue_event(motu, msg_type, mixer_ch, mixer_src_ch, val); + param->mixer.source[mixer_ch].paired_width[mixer_src_ch] = val; + } break; default: break; @@ -203,10 +241,16 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str if (mixer_ch < SNDRV_FIREWIRE_MOTU_REGISTER_DSP_MIXER_COUNT) { switch (msg_type) { case MIXER_OUTPUT_PAIRED_VOLUME: - param->mixer.output.paired_volume[mixer_ch] = val; + if (param->mixer.output.paired_volume[mixer_ch] != val) { + queue_event(motu, msg_type, mixer_ch, 0, val); + param->mixer.output.paired_volume[mixer_ch] = val; + } break; case MIXER_OUTPUT_PAIRED_FLAG: - param->mixer.output.paired_flag[mixer_ch] = val; + if (param->mixer.output.paired_flag[mixer_ch] != val) { + queue_event(motu, msg_type, mixer_ch, 0, val); + param->mixer.output.paired_flag[mixer_ch] = val; + } break; default: break; @@ -215,19 +259,34 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str break; } case MAIN_OUTPUT_PAIRED_VOLUME: - parser->param.output.main_paired_volume = val; + if (parser->param.output.main_paired_volume != val) { + queue_event(motu, msg_type, 0, 0, val); + parser->param.output.main_paired_volume = val; + } break; case HP_OUTPUT_PAIRED_VOLUME: - parser->param.output.hp_paired_volume = val; + if (parser->param.output.hp_paired_volume != val) { + queue_event(motu, msg_type, 0, 0, val); + parser->param.output.hp_paired_volume = val; + } break; case HP_OUTPUT_PAIRED_ASSIGNMENT: - parser->param.output.hp_paired_assignment = val; + if (parser->param.output.hp_paired_assignment != val) { + queue_event(motu, msg_type, 0, 0, val); + parser->param.output.hp_paired_assignment = val; + } break; case LINE_INPUT_BOOST: - parser->param.line_input.boost_flag = val; + if (parser->param.line_input.boost_flag != val) { + queue_event(motu, msg_type, 0, 0, val); + parser->param.line_input.boost_flag = val; + } break; case LINE_INPUT_NOMINAL_LEVEL: - parser->param.line_input.nominal_level_flag = val; + if (parser->param.line_input.nominal_level_flag != val) { + queue_event(motu, msg_type, 0, 0, val); + parser->param.line_input.nominal_level_flag = val; + } break; case INPUT_GAIN_AND_INVERT: case INPUT_FLAG: @@ -243,10 +302,16 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str if (input_ch < SNDRV_FIREWIRE_MOTU_REGISTER_DSP_INPUT_COUNT) { switch (msg_type) { case INPUT_GAIN_AND_INVERT: - param->input.gain_and_invert[input_ch] = val; + if (param->input.gain_and_invert[input_ch] != val) { + queue_event(motu, msg_type, input_ch, 0, val); + param->input.gain_and_invert[input_ch] = val; + } break; case INPUT_FLAG: - param->input.flag[input_ch] = val; + if (param->input.flag[input_ch] != val) { + queue_event(motu, msg_type, input_ch, 0, val); + param->input.flag[input_ch] = val; + } break; default: break; diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h index fa0b3ab7b78d..9703d3af59ec 100644 --- a/sound/firewire/motu/motu.h +++ b/sound/firewire/motu/motu.h @@ -74,6 +74,7 @@ struct snd_motu { int dev_lock_count; bool dev_lock_changed; wait_queue_head_t hwdep_wait; + struct snd_hwdep *hwdep; struct amdtp_domain domain; -- cgit v1.2.3 From 634ec0b2906efd46f6f57977e172aa3470aca432 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 15 Oct 2021 17:08:26 +0900 Subject: ALSA: firewire-motu: notify event for parameter change in register DSP model This commit copies queued event for change of register DSP into userspace when application operates ALSA hwdep character device. The notification occurs only when packet streaming is running. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211015080826.34847-12-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 8 ++++ sound/firewire/motu/motu-hwdep.c | 46 +++++++++++++++++----- .../motu/motu-register-dsp-message-parser.c | 39 ++++++++++++++++++ sound/firewire/motu/motu.h | 2 + 4 files changed, 86 insertions(+), 9 deletions(-) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index d52691655d79..76190a0cb069 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -13,6 +13,7 @@ #define SNDRV_FIREWIRE_EVENT_DIGI00X_MESSAGE 0x746e736c #define SNDRV_FIREWIRE_EVENT_MOTU_NOTIFICATION 0x64776479 #define SNDRV_FIREWIRE_EVENT_TASCAM_CONTROL 0x7473636d +#define SNDRV_FIREWIRE_EVENT_MOTU_REGISTER_DSP_CHANGE 0x4d545244 struct snd_firewire_event_common { unsigned int type; /* SNDRV_FIREWIRE_EVENT_xxx */ @@ -65,6 +66,12 @@ struct snd_firewire_event_tascam_control { struct snd_firewire_tascam_change changes[0]; }; +struct snd_firewire_event_motu_register_dsp_change { + unsigned int type; + __u32 count; // The number of changes. + __u32 changes[]; // Encoded event for change of register DSP. +}; + union snd_firewire_event { struct snd_firewire_event_common common; struct snd_firewire_event_lock_status lock_status; @@ -73,6 +80,7 @@ union snd_firewire_event { struct snd_firewire_event_digi00x_message digi00x_message; struct snd_firewire_event_tascam_control tascam_control; struct snd_firewire_event_motu_notification motu_notification; + struct snd_firewire_event_motu_register_dsp_change motu_register_dsp_change; }; diff --git a/sound/firewire/motu/motu-hwdep.c b/sound/firewire/motu/motu-hwdep.c index 389e59ff768b..9c2e457ce692 100644 --- a/sound/firewire/motu/motu-hwdep.c +++ b/sound/firewire/motu/motu-hwdep.c @@ -25,7 +25,8 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, spin_lock_irq(&motu->lock); - while (!motu->dev_lock_changed && motu->msg == 0) { + while (!motu->dev_lock_changed && motu->msg == 0 && + snd_motu_register_dsp_message_parser_count_event(motu) == 0) { prepare_to_wait(&motu->hwdep_wait, &wait, TASK_INTERRUPTIBLE); spin_unlock_irq(&motu->lock); schedule(); @@ -40,20 +41,46 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS; event.lock_status.status = (motu->dev_lock_count > 0); motu->dev_lock_changed = false; + spin_unlock_irq(&motu->lock); - count = min_t(long, count, sizeof(event.lock_status)); - } else { + count = min_t(long, count, sizeof(event)); + if (copy_to_user(buf, &event, count)) + return -EFAULT; + } else if (motu->msg > 0) { event.motu_notification.type = SNDRV_FIREWIRE_EVENT_MOTU_NOTIFICATION; event.motu_notification.message = motu->msg; motu->msg = 0; + spin_unlock_irq(&motu->lock); - count = min_t(long, count, sizeof(event.motu_notification)); - } + count = min_t(long, count, sizeof(event)); + if (copy_to_user(buf, &event, count)) + return -EFAULT; + } else if (snd_motu_register_dsp_message_parser_count_event(motu) > 0) { + size_t consumed = 0; + u32 __user *ptr; + u32 ev; - spin_unlock_irq(&motu->lock); + spin_unlock_irq(&motu->lock); - if (copy_to_user(buf, &event, count)) - return -EFAULT; + // Header is filled later. + consumed += sizeof(event.motu_register_dsp_change); + + while (consumed < count && + snd_motu_register_dsp_message_parser_copy_event(motu, &ev)) { + ptr = (u32 __user *)(buf + consumed); + if (put_user(ev, ptr)) + return -EFAULT; + consumed += sizeof(ev); + } + + event.motu_register_dsp_change.type = SNDRV_FIREWIRE_EVENT_MOTU_REGISTER_DSP_CHANGE; + event.motu_register_dsp_change.count = + (consumed - sizeof(event.motu_register_dsp_change)) / 4; + if (copy_to_user(buf, &event, sizeof(event.motu_register_dsp_change))) + return -EFAULT; + + count = consumed; + } return count; } @@ -67,7 +94,8 @@ static __poll_t hwdep_poll(struct snd_hwdep *hwdep, struct file *file, poll_wait(file, &motu->hwdep_wait, wait); spin_lock_irq(&motu->lock); - if (motu->dev_lock_changed || motu->msg) + if (motu->dev_lock_changed || motu->msg || + snd_motu_register_dsp_message_parser_count_event(motu) > 0) events = EPOLLIN | EPOLLRDNORM; else events = 0; diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c index cda8e6d987cc..cbc06b3b70f6 100644 --- a/sound/firewire/motu/motu-register-dsp-message-parser.c +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -95,6 +95,7 @@ struct msg_parser { u32 event_queue[EVENT_QUEUE_SIZE]; unsigned int push_pos; + unsigned int pull_pos; }; int snd_motu_register_dsp_message_parser_new(struct snd_motu *motu) @@ -122,6 +123,7 @@ int snd_motu_register_dsp_message_parser_init(struct snd_motu *motu) return 0; } +// Rough implementaion of queue without overrun check. static void queue_event(struct snd_motu *motu, u8 msg_type, u8 identifier0, u8 identifier1, u8 val) { struct msg_parser *parser = motu->message_parser; @@ -145,6 +147,7 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str { struct msg_parser *parser = motu->message_parser; bool meter_pos_quirk = parser->meter_pos_quirk; + unsigned int pos = parser->push_pos; unsigned long flags; int i; @@ -351,6 +354,9 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str } } + if (pos != parser->push_pos) + wake_up(&motu->hwdep_wait); + spin_unlock_irqrestore(&parser->lock, flags); } @@ -375,3 +381,36 @@ void snd_motu_register_dsp_message_parser_copy_parameter(struct snd_motu *motu, memcpy(param, &parser->param, sizeof(*param)); spin_unlock_irqrestore(&parser->lock, flags); } + +unsigned int snd_motu_register_dsp_message_parser_count_event(struct snd_motu *motu) +{ + struct msg_parser *parser = motu->message_parser; + + if (parser->pull_pos > parser->push_pos) + return EVENT_QUEUE_SIZE - parser->pull_pos + parser->push_pos; + else + return parser->push_pos - parser->pull_pos; +} + +bool snd_motu_register_dsp_message_parser_copy_event(struct snd_motu *motu, u32 *event) +{ + struct msg_parser *parser = motu->message_parser; + unsigned int pos = parser->pull_pos; + unsigned long flags; + + if (pos == parser->push_pos) + return false; + + spin_lock_irqsave(&parser->lock, flags); + + *event = parser->event_queue[pos]; + + ++pos; + if (pos >= EVENT_QUEUE_SIZE) + pos = 0; + parser->pull_pos = pos; + + spin_unlock_irqrestore(&parser->lock, flags); + + return true; +} diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h index 9703d3af59ec..79704ae6a73e 100644 --- a/sound/firewire/motu/motu.h +++ b/sound/firewire/motu/motu.h @@ -283,6 +283,8 @@ void snd_motu_register_dsp_message_parser_copy_meter(struct snd_motu *motu, struct snd_firewire_motu_register_dsp_meter *meter); void snd_motu_register_dsp_message_parser_copy_parameter(struct snd_motu *motu, struct snd_firewire_motu_register_dsp_parameter *params); +unsigned int snd_motu_register_dsp_message_parser_count_event(struct snd_motu *motu); +bool snd_motu_register_dsp_message_parser_copy_event(struct snd_motu *motu, u32 *event); int snd_motu_command_dsp_message_parser_new(struct snd_motu *motu); int snd_motu_command_dsp_message_parser_init(struct snd_motu *motu, enum cip_sfc sfc); -- cgit v1.2.3 From 3c05f1477e62ea5a0a8797ba6a545b1dc751fb31 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Fri, 15 Oct 2021 23:26:02 -0700 Subject: ALSA: ISA: not for M68K On m68k, compiling drivers under SND_ISA causes build errors: ../sound/core/isadma.c: In function 'snd_dma_program': ../sound/core/isadma.c:33:17: error: implicit declaration of function 'claim_dma_lock' [-Werror=implicit-function-declaration] 33 | flags = claim_dma_lock(); | ^~~~~~~~~~~~~~ ../sound/core/isadma.c:41:9: error: implicit declaration of function 'release_dma_lock' [-Werror=implicit-function-declaration] 41 | release_dma_lock(flags); | ^~~~~~~~~~~~~~~~ ../sound/isa/sb/sb16_main.c: In function 'snd_sb16_playback_prepare': ../sound/isa/sb/sb16_main.c:253:72: error: 'DMA_AUTOINIT' undeclared (first use in this function) 253 | snd_dma_program(dma, runtime->dma_addr, size, DMA_MODE_WRITE | DMA_AUTOINIT); | ^~~~~~~~~~~~ ../sound/isa/sb/sb16_main.c:253:72: note: each undeclared identifier is reported only once for each function it appears in ../sound/isa/sb/sb16_main.c: In function 'snd_sb16_capture_prepare': ../sound/isa/sb/sb16_main.c:322:71: error: 'DMA_AUTOINIT' undeclared (first use in this function) 322 | snd_dma_program(dma, runtime->dma_addr, size, DMA_MODE_READ | DMA_AUTOINIT); | ^~~~~~~~~~~~ and more... Signed-off-by: Randy Dunlap Cc: Jaroslav Kysela Cc: Takashi Iwai Cc: alsa-devel@alsa-project.org Cc: linux-m68k@lists.linux-m68k.org Cc: Geert Uytterhoeven Link: https://lore.kernel.org/r/20211016062602.3588-1-rdunlap@infradead.org Signed-off-by: Takashi Iwai --- sound/core/Makefile | 2 ++ sound/isa/Kconfig | 2 +- sound/pci/Kconfig | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/sound/core/Makefile b/sound/core/Makefile index d774792850f3..79e1407cd0de 100644 --- a/sound/core/Makefile +++ b/sound/core/Makefile @@ -9,7 +9,9 @@ ifneq ($(CONFIG_SND_PROC_FS),) snd-y += info.o snd-$(CONFIG_SND_OSSEMUL) += info_oss.o endif +ifneq ($(CONFIG_M68K),y) snd-$(CONFIG_ISA_DMA_API) += isadma.o +endif snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o snd-$(CONFIG_SND_VMASTER) += vmaster.o snd-$(CONFIG_SND_JACK) += ctljack.o jack.o diff --git a/sound/isa/Kconfig b/sound/isa/Kconfig index 6ffa48dd5983..570b88e0b201 100644 --- a/sound/isa/Kconfig +++ b/sound/isa/Kconfig @@ -22,7 +22,7 @@ config SND_SB16_DSP menuconfig SND_ISA bool "ISA sound devices" depends on ISA || COMPILE_TEST - depends on ISA_DMA_API + depends on ISA_DMA_API && !M68K default y help Support for sound devices connected via the ISA bus. diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig index 93bc9bef7641..41ce12597177 100644 --- a/sound/pci/Kconfig +++ b/sound/pci/Kconfig @@ -279,6 +279,7 @@ config SND_CS46XX_NEW_DSP config SND_CS5530 tristate "CS5530 Audio" depends on ISA_DMA_API && (X86_32 || COMPILE_TEST) + depends on !M68K select SND_SB16_DSP help Say Y here to include support for audio on Cyrix/NatSemi CS5530 chips. -- cgit v1.2.3 From a25684a956468ee8bbbee44649e41e5d447e5adc Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Sun, 17 Oct 2021 09:48:57 +0200 Subject: ALSA: memalloc: Support for non-contiguous page allocation This patch adds the support for allocation of non-contiguous DMA pages in the common memalloc helper. It's another SG-buffer type, but unlike the existing one, this is directional and requires the explicit sync / invalidation of dirty pages on non-coherent architectures. For this enhancement, the following points are changed: - snd_dma_device stores the DMA direction. - snd_dma_device stores need_sync flag indicating whether the explicit sync is required or not. - A new variant of helper functions, snd_dma_alloc_dir_pages() and *_all() are introduced; the old snd_dma_alloc_pages() and *_all() kept as just wrappers with DMA_BIDIRECTIONAL. - A new helper snd_dma_buffer_sync() is introduced; this gets called in the appropriate places. - A new allocation type, SNDRV_DMA_TYPE_NONCONTIG, is introduced. When the driver allocates pages with this new type, and it may require the SNDRV_PCM_INFO_EXPLICIT_SYNC flag set to the PCM hardware.info for taking the full control of PCM applptr and hwptr changes (that implies disabling the mmap of control/status data). When the buffer allocation is managed by snd_pcm_set_managed_buffer(), this flag is automatically set depending on the result of dma_need_sync() internally. Otherwise, if the buffer is managed manually, the driver has to set the flag explicitly, too. The explicit sync between CPU and device for non-coherent memory is performed at the points before and after read/write transfer as well as the applptr/hwptr syncptr ioctl. In the case of mmap mode, user-space is supposed to call the syncptr ioctl with the hwptr flag to update and fetch the status at first; this corresponds to CPU-sync. Then user-space advances the applptr via syncptr ioctl again with applptr flag, and this corresponds to the device sync with flushing. Other than the DMA direction and the explicit sync, the usage of this new buffer type is almost equivalent with the existing SNDRV_DMA_TYPE_DEV_SG; you can get the page and the address via snd_sgbuf_get_page() and snd_sgbuf_get_addr(), also calculate the continuous pages via snd_sgbuf_get_chunk_size(). For those SG-page handling, the non-contig type shares the same ops with the vmalloc handler. As we do always vmap the SG pages at first, the actual address can be deduced from the vmapped address easily without iterating the SG-list. Link: https://lore.kernel.org/r/20211017074859.24112-2-tiwai@suse.de Signed-off-by: Takashi Iwai --- include/sound/memalloc.h | 46 +++++++++++++++++-- sound/core/memalloc.c | 109 ++++++++++++++++++++++++++++++++++++++++---- sound/core/memalloc_local.h | 1 + sound/core/pcm_compat.c | 4 ++ sound/core/pcm_lib.c | 5 ++ sound/core/pcm_local.h | 7 +++ sound/core/pcm_memory.c | 13 ++++-- sound/core/pcm_native.c | 17 +++++++ 8 files changed, 187 insertions(+), 15 deletions(-) diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h index b197e3f431c1..1457eba1ac53 100644 --- a/include/sound/memalloc.h +++ b/include/sound/memalloc.h @@ -9,16 +9,20 @@ #ifndef __SOUND_MEMALLOC_H #define __SOUND_MEMALLOC_H +#include #include struct device; struct vm_area_struct; +struct sg_table; /* * buffer device info */ struct snd_dma_device { int type; /* SNDRV_DMA_TYPE_XXX */ + enum dma_data_direction dir; /* DMA direction */ + bool need_sync; /* explicit sync needed? */ struct device *dev; /* generic device */ }; @@ -45,6 +49,7 @@ struct snd_dma_device { #define SNDRV_DMA_TYPE_DEV_IRAM SNDRV_DMA_TYPE_DEV #endif #define SNDRV_DMA_TYPE_VMALLOC 7 /* vmalloc'ed buffer */ +#define SNDRV_DMA_TYPE_NONCONTIG 8 /* non-coherent SG buffer */ /* * info for buffer allocation @@ -66,22 +71,55 @@ static inline unsigned int snd_sgbuf_aligned_pages(size_t size) } /* allocate/release a buffer */ -int snd_dma_alloc_pages(int type, struct device *dev, size_t size, - struct snd_dma_buffer *dmab); +int snd_dma_alloc_dir_pages(int type, struct device *dev, + enum dma_data_direction dir, size_t size, + struct snd_dma_buffer *dmab); + +static inline int snd_dma_alloc_pages(int type, struct device *dev, + size_t size, struct snd_dma_buffer *dmab) +{ + return snd_dma_alloc_dir_pages(type, dev, DMA_BIDIRECTIONAL, size, dmab); +} + int snd_dma_alloc_pages_fallback(int type, struct device *dev, size_t size, struct snd_dma_buffer *dmab); void snd_dma_free_pages(struct snd_dma_buffer *dmab); int snd_dma_buffer_mmap(struct snd_dma_buffer *dmab, struct vm_area_struct *area); +enum snd_dma_sync_mode { SNDRV_DMA_SYNC_CPU, SNDRV_DMA_SYNC_DEVICE }; +#ifdef CONFIG_HAS_DMA +void snd_dma_buffer_sync(struct snd_dma_buffer *dmab, + enum snd_dma_sync_mode mode); +#else +static inline void snd_dma_buffer_sync(struct snd_dma_buffer *dmab, + enum snd_dma_sync_mode mode) {} +#endif + +void snd_dma_buffer_sync(struct snd_dma_buffer *dmab, + enum snd_dma_sync_mode mode); + dma_addr_t snd_sgbuf_get_addr(struct snd_dma_buffer *dmab, size_t offset); struct page *snd_sgbuf_get_page(struct snd_dma_buffer *dmab, size_t offset); unsigned int snd_sgbuf_get_chunk_size(struct snd_dma_buffer *dmab, unsigned int ofs, unsigned int size); /* device-managed memory allocator */ -struct snd_dma_buffer *snd_devm_alloc_pages(struct device *dev, int type, - size_t size); +struct snd_dma_buffer *snd_devm_alloc_dir_pages(struct device *dev, int type, + enum dma_data_direction dir, + size_t size); + +static inline struct snd_dma_buffer * +snd_devm_alloc_pages(struct device *dev, int type, size_t size) +{ + return snd_devm_alloc_dir_pages(dev, type, DMA_BIDIRECTIONAL, size); +} + +static inline struct sg_table * +snd_dma_noncontig_sg_table(struct snd_dma_buffer *dmab) +{ + return dmab->private_data; +} #endif /* __SOUND_MEMALLOC_H */ diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index c7c943c661e6..11f9a68bf94c 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #ifdef CONFIG_X86 #include @@ -39,9 +40,11 @@ static void *__snd_dma_alloc_pages(struct snd_dma_buffer *dmab, size_t size) } /** - * snd_dma_alloc_pages - allocate the buffer area according to the given type + * snd_dma_alloc_dir_pages - allocate the buffer area according to the given + * type and direction * @type: the DMA buffer type * @device: the device pointer + * @dir: DMA direction * @size: the buffer size to allocate * @dmab: buffer allocation record to store the allocated data * @@ -51,8 +54,9 @@ static void *__snd_dma_alloc_pages(struct snd_dma_buffer *dmab, size_t size) * Return: Zero if the buffer with the given size is allocated successfully, * otherwise a negative value on error. */ -int snd_dma_alloc_pages(int type, struct device *device, size_t size, - struct snd_dma_buffer *dmab) +int snd_dma_alloc_dir_pages(int type, struct device *device, + enum dma_data_direction dir, size_t size, + struct snd_dma_buffer *dmab) { if (WARN_ON(!size)) return -ENXIO; @@ -62,6 +66,7 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size, size = PAGE_ALIGN(size); dmab->dev.type = type; dmab->dev.dev = device; + dmab->dev.dir = dir; dmab->bytes = 0; dmab->addr = 0; dmab->private_data = NULL; @@ -71,7 +76,7 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size, dmab->bytes = size; return 0; } -EXPORT_SYMBOL(snd_dma_alloc_pages); +EXPORT_SYMBOL(snd_dma_alloc_dir_pages); /** * snd_dma_alloc_pages_fallback - allocate the buffer area according to the given type with fallback @@ -129,9 +134,10 @@ static void __snd_release_pages(struct device *dev, void *res) } /** - * snd_devm_alloc_pages - allocate the buffer and manage with devres + * snd_devm_alloc_dir_pages - allocate the buffer and manage with devres * @dev: the device pointer * @type: the DMA buffer type + * @dir: DMA direction * @size: the buffer size to allocate * * Allocate buffer pages depending on the given type and manage using devres. @@ -144,7 +150,8 @@ static void __snd_release_pages(struct device *dev, void *res) * The function returns the snd_dma_buffer object at success, or NULL if failed. */ struct snd_dma_buffer * -snd_devm_alloc_pages(struct device *dev, int type, size_t size) +snd_devm_alloc_dir_pages(struct device *dev, int type, + enum dma_data_direction dir, size_t size) { struct snd_dma_buffer *dmab; int err; @@ -157,7 +164,7 @@ snd_devm_alloc_pages(struct device *dev, int type, size_t size) if (!dmab) return NULL; - err = snd_dma_alloc_pages(type, dev, size, dmab); + err = snd_dma_alloc_dir_pages(type, dev, dir, size, dmab); if (err < 0) { devres_free(dmab); return NULL; @@ -166,7 +173,7 @@ snd_devm_alloc_pages(struct device *dev, int type, size_t size) devres_add(dev, dmab); return dmab; } -EXPORT_SYMBOL_GPL(snd_devm_alloc_pages); +EXPORT_SYMBOL_GPL(snd_devm_alloc_dir_pages); /** * snd_dma_buffer_mmap - perform mmap of the given DMA buffer @@ -185,6 +192,26 @@ int snd_dma_buffer_mmap(struct snd_dma_buffer *dmab, } EXPORT_SYMBOL(snd_dma_buffer_mmap); +#ifdef CONFIG_HAS_DMA +/** + * snd_dma_buffer_sync - sync DMA buffer between CPU and device + * @dmab: buffer allocation information + * @mod: sync mode + */ +void snd_dma_buffer_sync(struct snd_dma_buffer *dmab, + enum snd_dma_sync_mode mode) +{ + const struct snd_malloc_ops *ops; + + if (!dmab || !dmab->dev.need_sync) + return; + ops = snd_dma_get_ops(dmab); + if (ops && ops->sync) + ops->sync(dmab, mode); +} +EXPORT_SYMBOL_GPL(snd_dma_buffer_sync); +#endif /* CONFIG_HAS_DMA */ + /** * snd_sgbuf_get_addr - return the physical address at the corresponding offset * @dmab: buffer allocation information @@ -468,6 +495,71 @@ static const struct snd_malloc_ops snd_dma_wc_ops = { .mmap = snd_dma_wc_mmap, }; #endif /* CONFIG_X86 */ + +/* + * Non-contiguous pages allocator + */ +static void *snd_dma_noncontig_alloc(struct snd_dma_buffer *dmab, size_t size) +{ + struct sg_table *sgt; + void *p; + + sgt = dma_alloc_noncontiguous(dmab->dev.dev, size, dmab->dev.dir, + DEFAULT_GFP, 0); + if (!sgt) + return NULL; + dmab->dev.need_sync = dma_need_sync(dmab->dev.dev, dmab->dev.dir); + p = dma_vmap_noncontiguous(dmab->dev.dev, size, sgt); + if (p) + dmab->private_data = sgt; + else + dma_free_noncontiguous(dmab->dev.dev, size, sgt, dmab->dev.dir); + return p; +} + +static void snd_dma_noncontig_free(struct snd_dma_buffer *dmab) +{ + dma_vunmap_noncontiguous(dmab->dev.dev, dmab->area); + dma_free_noncontiguous(dmab->dev.dev, dmab->bytes, dmab->private_data, + dmab->dev.dir); +} + +static int snd_dma_noncontig_mmap(struct snd_dma_buffer *dmab, + struct vm_area_struct *area) +{ + return dma_mmap_noncontiguous(dmab->dev.dev, area, + dmab->bytes, dmab->private_data); +} + +static void snd_dma_noncontig_sync(struct snd_dma_buffer *dmab, + enum snd_dma_sync_mode mode) +{ + if (mode == SNDRV_DMA_SYNC_CPU) { + if (dmab->dev.dir == DMA_TO_DEVICE) + return; + dma_sync_sgtable_for_cpu(dmab->dev.dev, dmab->private_data, + dmab->dev.dir); + invalidate_kernel_vmap_range(dmab->area, dmab->bytes); + } else { + if (dmab->dev.dir == DMA_FROM_DEVICE) + return; + flush_kernel_vmap_range(dmab->area, dmab->bytes); + dma_sync_sgtable_for_device(dmab->dev.dev, dmab->private_data, + dmab->dev.dir); + } +} + +static const struct snd_malloc_ops snd_dma_noncontig_ops = { + .alloc = snd_dma_noncontig_alloc, + .free = snd_dma_noncontig_free, + .mmap = snd_dma_noncontig_mmap, + .sync = snd_dma_noncontig_sync, + /* re-use vmalloc helpers for get_* ops */ + .get_addr = snd_dma_vmalloc_get_addr, + .get_page = snd_dma_vmalloc_get_page, + .get_chunk_size = snd_dma_vmalloc_get_chunk_size, +}; + #endif /* CONFIG_HAS_DMA */ /* @@ -479,6 +571,7 @@ static const struct snd_malloc_ops *dma_ops[] = { #ifdef CONFIG_HAS_DMA [SNDRV_DMA_TYPE_DEV] = &snd_dma_dev_ops, [SNDRV_DMA_TYPE_DEV_WC] = &snd_dma_wc_ops, + [SNDRV_DMA_TYPE_NONCONTIG] = &snd_dma_noncontig_ops, #ifdef CONFIG_GENERIC_ALLOCATOR [SNDRV_DMA_TYPE_DEV_IRAM] = &snd_dma_iram_ops, #endif /* CONFIG_GENERIC_ALLOCATOR */ diff --git a/sound/core/memalloc_local.h b/sound/core/memalloc_local.h index 9f2e0a608b49..a6f3a87194da 100644 --- a/sound/core/memalloc_local.h +++ b/sound/core/memalloc_local.h @@ -10,6 +10,7 @@ struct snd_malloc_ops { unsigned int (*get_chunk_size)(struct snd_dma_buffer *dmab, unsigned int ofs, unsigned int size); int (*mmap)(struct snd_dma_buffer *dmab, struct vm_area_struct *area); + void (*sync)(struct snd_dma_buffer *dmab, enum snd_dma_sync_mode mode); }; #ifdef CONFIG_SND_DMA_SGBUF diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c index dfe5a64e19d2..e4e176854ce7 100644 --- a/sound/core/pcm_compat.c +++ b/sound/core/pcm_compat.c @@ -453,6 +453,8 @@ static int snd_pcm_ioctl_sync_ptr_x32(struct snd_pcm_substream *substream, sstatus.suspended_state = status->suspended_state; sstatus.audio_tstamp = status->audio_tstamp; snd_pcm_stream_unlock_irq(substream); + if (!(sflags & SNDRV_PCM_SYNC_PTR_APPL)) + snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE); if (put_user(sstatus.state, &src->s.status.state) || put_user(sstatus.hw_ptr, &src->s.status.hw_ptr) || put_user(sstatus.tstamp.tv_sec, &src->s.status.tstamp_sec) || @@ -533,6 +535,8 @@ static int snd_pcm_ioctl_sync_ptr_buggy(struct snd_pcm_substream *substream, sync_ptr.s.status.suspended_state = status->suspended_state; sync_ptr.s.status.audio_tstamp = status->audio_tstamp; snd_pcm_stream_unlock_irq(substream); + if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) + snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE); if (copy_to_user(_sync_ptr, &sync_ptr, sizeof(sync_ptr))) return -EFAULT; return 0; diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index a144a3f68e9e..4f4b4739f987 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -106,6 +106,7 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram frames -= transfer; ofs = 0; } + snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE); } #ifdef CONFIG_SND_DEBUG @@ -2256,8 +2257,12 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, goto _end_unlock; } snd_pcm_stream_unlock_irq(substream); + if (!is_playback) + snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_CPU); err = writer(substream, appl_ofs, data, offset, frames, transfer); + if (is_playback) + snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE); snd_pcm_stream_lock_irq(substream); if (err < 0) goto _end_unlock; diff --git a/sound/core/pcm_local.h b/sound/core/pcm_local.h index fe9689b8a6a6..ecb21697ae3a 100644 --- a/sound/core/pcm_local.h +++ b/sound/core/pcm_local.h @@ -73,4 +73,11 @@ void snd_pcm_sync_stop(struct snd_pcm_substream *substream, bool sync_irq); for ((subs) = (pcm)->streams[str].substream; (subs); \ (subs) = (subs)->next) +static inline void snd_pcm_dma_buffer_sync(struct snd_pcm_substream *substream, + enum snd_dma_sync_mode mode) +{ + if (substream->runtime->info & SNDRV_PCM_INFO_EXPLICIT_SYNC) + snd_dma_buffer_sync(snd_pcm_get_dma_buf(substream), mode); +} + #endif /* __SOUND_CORE_PCM_LOCAL_H */ diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c index 7fbd1ccbb5b0..b70ce3b69ab4 100644 --- a/sound/core/pcm_memory.c +++ b/sound/core/pcm_memory.c @@ -32,15 +32,20 @@ module_param(max_alloc_per_card, ulong, 0644); MODULE_PARM_DESC(max_alloc_per_card, "Max total allocation bytes per card."); static int do_alloc_pages(struct snd_card *card, int type, struct device *dev, - size_t size, struct snd_dma_buffer *dmab) + int str, size_t size, struct snd_dma_buffer *dmab) { + enum dma_data_direction dir; int err; if (max_alloc_per_card && card->total_pcm_alloc_bytes + size > max_alloc_per_card) return -ENOMEM; - err = snd_dma_alloc_pages(type, dev, size, dmab); + if (str == SNDRV_PCM_STREAM_PLAYBACK) + dir = DMA_TO_DEVICE; + else + dir = DMA_FROM_DEVICE; + err = snd_dma_alloc_dir_pages(type, dev, dir, size, dmab); if (!err) { mutex_lock(&card->memory_mutex); card->total_pcm_alloc_bytes += dmab->bytes; @@ -77,7 +82,7 @@ static int preallocate_pcm_pages(struct snd_pcm_substream *substream, do { err = do_alloc_pages(card, dmab->dev.type, dmab->dev.dev, - size, dmab); + substream->stream, size, dmab); if (err != -ENOMEM) return err; if (no_fallback) @@ -177,6 +182,7 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry, if (do_alloc_pages(card, substream->dma_buffer.dev.type, substream->dma_buffer.dev.dev, + substream->stream, size, &new_dmab) < 0) { buffer->error = -ENOMEM; pr_debug("ALSA pcmC%dD%d%c,%d:%s: cannot preallocate for size %zu\n", @@ -418,6 +424,7 @@ int snd_pcm_lib_malloc_pages(struct snd_pcm_substream *substream, size_t size) if (do_alloc_pages(card, substream->dma_buffer.dev.type, substream->dma_buffer.dev.dev, + substream->stream, size, dmab) < 0) { kfree(dmab); pr_debug("ALSA pcmC%dD%d%c,%d:%s: cannot preallocate for size %zu\n", diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 627b201b6084..621883e71194 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2685,6 +2685,13 @@ int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, goto error; } + /* automatically set EXPLICIT_SYNC flag in the managed mode whenever + * the DMA buffer requires it + */ + if (substream->managed_buffer_alloc && + substream->dma_buffer.dev.need_sync) + substream->runtime->hw.info |= SNDRV_PCM_INFO_EXPLICIT_SYNC; + *rsubstream = substream; return 0; @@ -2912,6 +2919,8 @@ static snd_pcm_sframes_t snd_pcm_rewind(struct snd_pcm_substream *substream, ret = rewind_appl_ptr(substream, frames, snd_pcm_hw_avail(substream)); snd_pcm_stream_unlock_irq(substream); + if (ret >= 0) + snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE); return ret; } @@ -2929,6 +2938,8 @@ static snd_pcm_sframes_t snd_pcm_forward(struct snd_pcm_substream *substream, ret = forward_appl_ptr(substream, frames, snd_pcm_avail(substream)); snd_pcm_stream_unlock_irq(substream); + if (ret >= 0) + snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE); return ret; } @@ -2942,6 +2953,8 @@ static int snd_pcm_delay(struct snd_pcm_substream *substream, if (delay && !err) *delay = snd_pcm_calc_delay(substream); snd_pcm_stream_unlock_irq(substream); + snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_CPU); + return err; } @@ -2992,6 +3005,8 @@ static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream, sync_ptr.s.status.suspended_state = status->suspended_state; sync_ptr.s.status.audio_tstamp = status->audio_tstamp; snd_pcm_stream_unlock_irq(substream); + if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) + snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE); if (copy_to_user(_sync_ptr, &sync_ptr, sizeof(sync_ptr))) return -EFAULT; return 0; @@ -3088,6 +3103,8 @@ static int snd_pcm_ioctl_sync_ptr_compat(struct snd_pcm_substream *substream, sstatus.suspended_state = status->suspended_state; sstatus.audio_tstamp = status->audio_tstamp; snd_pcm_stream_unlock_irq(substream); + if (!(sflags & SNDRV_PCM_SYNC_PTR_APPL)) + snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE); if (put_user(sstatus.state, &src->s.status.state) || put_user(sstatus.hw_ptr, &src->s.status.hw_ptr) || put_user(sstatus.tstamp.tv_sec, &src->s.status.tstamp_sec) || -- cgit v1.2.3 From 73325f60e2ed28f04032d43c2828b73776cfefd0 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Sun, 17 Oct 2021 09:48:58 +0200 Subject: ALSA: memalloc: Support for non-coherent page allocation Following to the addition of non-contiguous pages, this patch adds the new contiguous non-coherent page allocation to the standard memalloc helper. Like the previous non-contig type, this non-coherent type is also directional and requires the explicit sync, too. Hence the driver using this type of buffer may need to set SNDRV_PCM_INFO_EXPLICIT_SYNC flag to the PCM hardware.info as well, unless it's set up in the managed mode. Link: https://lore.kernel.org/r/20211017074859.24112-3-tiwai@suse.de Signed-off-by: Takashi Iwai --- include/sound/memalloc.h | 1 + sound/core/memalloc.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h index 1457eba1ac53..0bdaa6753282 100644 --- a/include/sound/memalloc.h +++ b/include/sound/memalloc.h @@ -50,6 +50,7 @@ struct snd_dma_device { #endif #define SNDRV_DMA_TYPE_VMALLOC 7 /* vmalloc'ed buffer */ #define SNDRV_DMA_TYPE_NONCONTIG 8 /* non-coherent SG buffer */ +#define SNDRV_DMA_TYPE_NONCOHERENT 9 /* non-coherent buffer */ /* * info for buffer allocation diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index 11f9a68bf94c..99681e651223 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -560,6 +560,52 @@ static const struct snd_malloc_ops snd_dma_noncontig_ops = { .get_chunk_size = snd_dma_vmalloc_get_chunk_size, }; +/* + * Non-coherent pages allocator + */ +static void *snd_dma_noncoherent_alloc(struct snd_dma_buffer *dmab, size_t size) +{ + dmab->dev.need_sync = dma_need_sync(dmab->dev.dev, dmab->dev.dir); + return dma_alloc_noncoherent(dmab->dev.dev, size, &dmab->addr, + dmab->dev.dir, DEFAULT_GFP); +} + +static void snd_dma_noncoherent_free(struct snd_dma_buffer *dmab) +{ + dma_free_noncoherent(dmab->dev.dev, dmab->bytes, dmab->area, + dmab->addr, dmab->dev.dir); +} + +static int snd_dma_noncoherent_mmap(struct snd_dma_buffer *dmab, + struct vm_area_struct *area) +{ + area->vm_page_prot = vm_get_page_prot(area->vm_flags); + return dma_mmap_pages(dmab->dev.dev, area, + area->vm_end - area->vm_start, + virt_to_page(dmab->area)); +} + +static void snd_dma_noncoherent_sync(struct snd_dma_buffer *dmab, + enum snd_dma_sync_mode mode) +{ + if (mode == SNDRV_DMA_SYNC_CPU) { + if (dmab->dev.dir != DMA_TO_DEVICE) + dma_sync_single_for_cpu(dmab->dev.dev, dmab->addr, + dmab->bytes, dmab->dev.dir); + } else { + if (dmab->dev.dir != DMA_FROM_DEVICE) + dma_sync_single_for_device(dmab->dev.dev, dmab->addr, + dmab->bytes, dmab->dev.dir); + } +} + +static const struct snd_malloc_ops snd_dma_noncoherent_ops = { + .alloc = snd_dma_noncoherent_alloc, + .free = snd_dma_noncoherent_free, + .mmap = snd_dma_noncoherent_mmap, + .sync = snd_dma_noncoherent_sync, +}; + #endif /* CONFIG_HAS_DMA */ /* @@ -572,6 +618,7 @@ static const struct snd_malloc_ops *dma_ops[] = { [SNDRV_DMA_TYPE_DEV] = &snd_dma_dev_ops, [SNDRV_DMA_TYPE_DEV_WC] = &snd_dma_wc_ops, [SNDRV_DMA_TYPE_NONCONTIG] = &snd_dma_noncontig_ops, + [SNDRV_DMA_TYPE_NONCOHERENT] = &snd_dma_noncoherent_ops, #ifdef CONFIG_GENERIC_ALLOCATOR [SNDRV_DMA_TYPE_DEV_IRAM] = &snd_dma_iram_ops, #endif /* CONFIG_GENERIC_ALLOCATOR */ -- cgit v1.2.3 From 2d9ea39917a4e4293bc2caea902c7059a330b611 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Sun, 17 Oct 2021 09:48:59 +0200 Subject: ALSA: memalloc: Convert x86 SG-buffer handling with non-contiguous type We've had an x86-specific SG-buffer handling code, but now it can be merged gracefully with the standard non-contiguous DMA pages. After the migration, SNDRV_DMA_TYPE_DMA_SG becomes identical with SNDRV_DMA_TYPE_NONCONTIG on x86, while others still fall back to SNDRV_DMA_TYPE_DEV. The remaining problem is about the SG-buffer with WC pages: the DMA core stuff on x86 doesn't treat it well, so we still need some special handling to manipulate the page attribute manually. The mmap handler for SNDRV_DMA_TYPE_DEV_SG_WC still returns -ENOENT intentionally for the fallback to the default handler. Link: https://lore.kernel.org/r/20211017074859.24112-4-tiwai@suse.de Signed-off-by: Takashi Iwai --- include/sound/memalloc.h | 14 ++-- sound/core/Makefile | 1 - sound/core/memalloc.c | 51 +++++++++++- sound/core/sgbuf.c | 201 ----------------------------------------------- 4 files changed, 54 insertions(+), 213 deletions(-) delete mode 100644 sound/core/sgbuf.c diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h index 0bdaa6753282..df615052dad4 100644 --- a/include/sound/memalloc.h +++ b/include/sound/memalloc.h @@ -36,13 +36,6 @@ struct snd_dma_device { #define SNDRV_DMA_TYPE_CONTINUOUS 1 /* continuous no-DMA memory */ #define SNDRV_DMA_TYPE_DEV 2 /* generic device continuous */ #define SNDRV_DMA_TYPE_DEV_WC 5 /* continuous write-combined */ -#ifdef CONFIG_SND_DMA_SGBUF -#define SNDRV_DMA_TYPE_DEV_SG 3 /* generic device SG-buffer */ -#define SNDRV_DMA_TYPE_DEV_WC_SG 6 /* SG write-combined */ -#else -#define SNDRV_DMA_TYPE_DEV_SG SNDRV_DMA_TYPE_DEV /* no SG-buf support */ -#define SNDRV_DMA_TYPE_DEV_WC_SG SNDRV_DMA_TYPE_DEV_WC -#endif #ifdef CONFIG_GENERIC_ALLOCATOR #define SNDRV_DMA_TYPE_DEV_IRAM 4 /* generic device iram-buffer */ #else @@ -51,6 +44,13 @@ struct snd_dma_device { #define SNDRV_DMA_TYPE_VMALLOC 7 /* vmalloc'ed buffer */ #define SNDRV_DMA_TYPE_NONCONTIG 8 /* non-coherent SG buffer */ #define SNDRV_DMA_TYPE_NONCOHERENT 9 /* non-coherent buffer */ +#ifdef CONFIG_SND_DMA_SGBUF +#define SNDRV_DMA_TYPE_DEV_SG SNDRV_DMA_TYPE_NONCONTIG +#define SNDRV_DMA_TYPE_DEV_WC_SG 6 /* SG write-combined */ +#else +#define SNDRV_DMA_TYPE_DEV_SG SNDRV_DMA_TYPE_DEV /* no SG-buf support */ +#define SNDRV_DMA_TYPE_DEV_WC_SG SNDRV_DMA_TYPE_DEV_WC +#endif /* * info for buffer allocation diff --git a/sound/core/Makefile b/sound/core/Makefile index 79e1407cd0de..350d704ced98 100644 --- a/sound/core/Makefile +++ b/sound/core/Makefile @@ -19,7 +19,6 @@ snd-$(CONFIG_SND_JACK) += ctljack.o jack.o snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_misc.o \ pcm_memory.o memalloc.o snd-pcm-$(CONFIG_SND_PCM_TIMER) += pcm_timer.o -snd-pcm-$(CONFIG_SND_DMA_SGBUF) += sgbuf.o snd-pcm-$(CONFIG_SND_PCM_ELD) += pcm_drm_eld.o snd-pcm-$(CONFIG_SND_PCM_IEC958) += pcm_iec958.o diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index 99681e651223..acdebecf1a2e 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -560,6 +560,50 @@ static const struct snd_malloc_ops snd_dma_noncontig_ops = { .get_chunk_size = snd_dma_vmalloc_get_chunk_size, }; +/* x86-specific SG-buffer with WC pages */ +#ifdef CONFIG_SND_DMA_SGBUF +#define vmalloc_to_virt(v) (unsigned long)page_to_virt(vmalloc_to_page(v)) + +static void *snd_dma_sg_wc_alloc(struct snd_dma_buffer *dmab, size_t size) +{ + void *p = snd_dma_noncontig_alloc(dmab, size); + size_t ofs; + + if (!p) + return NULL; + for (ofs = 0; ofs < size; ofs += PAGE_SIZE) + set_memory_uc(vmalloc_to_virt(p + ofs), 1); + return p; +} + +static void snd_dma_sg_wc_free(struct snd_dma_buffer *dmab) +{ + size_t ofs; + + for (ofs = 0; ofs < dmab->bytes; ofs += PAGE_SIZE) + set_memory_wb(vmalloc_to_virt(dmab->area + ofs), 1); + snd_dma_noncontig_free(dmab); +} + +static int snd_dma_sg_wc_mmap(struct snd_dma_buffer *dmab, + struct vm_area_struct *area) +{ + area->vm_page_prot = pgprot_writecombine(area->vm_page_prot); + /* FIXME: dma_mmap_noncontiguous() works? */ + return -ENOENT; /* continue with the default mmap handler */ +} + +const struct snd_malloc_ops snd_dma_sg_wc_ops = { + .alloc = snd_dma_sg_wc_alloc, + .free = snd_dma_sg_wc_free, + .mmap = snd_dma_sg_wc_mmap, + .sync = snd_dma_noncontig_sync, + .get_addr = snd_dma_vmalloc_get_addr, + .get_page = snd_dma_vmalloc_get_page, + .get_chunk_size = snd_dma_vmalloc_get_chunk_size, +}; +#endif /* CONFIG_SND_DMA_SGBUF */ + /* * Non-coherent pages allocator */ @@ -619,14 +663,13 @@ static const struct snd_malloc_ops *dma_ops[] = { [SNDRV_DMA_TYPE_DEV_WC] = &snd_dma_wc_ops, [SNDRV_DMA_TYPE_NONCONTIG] = &snd_dma_noncontig_ops, [SNDRV_DMA_TYPE_NONCOHERENT] = &snd_dma_noncoherent_ops, +#ifdef CONFIG_SND_DMA_SGBUF + [SNDRV_DMA_TYPE_DEV_WC_SG] = &snd_dma_sg_wc_ops, +#endif #ifdef CONFIG_GENERIC_ALLOCATOR [SNDRV_DMA_TYPE_DEV_IRAM] = &snd_dma_iram_ops, #endif /* CONFIG_GENERIC_ALLOCATOR */ #endif /* CONFIG_HAS_DMA */ -#ifdef CONFIG_SND_DMA_SGBUF - [SNDRV_DMA_TYPE_DEV_SG] = &snd_dma_sg_ops, - [SNDRV_DMA_TYPE_DEV_WC_SG] = &snd_dma_sg_ops, -#endif }; static const struct snd_malloc_ops *snd_dma_get_ops(struct snd_dma_buffer *dmab) diff --git a/sound/core/sgbuf.c b/sound/core/sgbuf.c deleted file mode 100644 index 8352a5cdb19f..000000000000 --- a/sound/core/sgbuf.c +++ /dev/null @@ -1,201 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -/* - * Scatter-Gather buffer - * - * Copyright (c) by Takashi Iwai - */ - -#include -#include -#include -#include -#include -#include "memalloc_local.h" - -struct snd_sg_page { - void *buf; - dma_addr_t addr; -}; - -struct snd_sg_buf { - int size; /* allocated byte size */ - int pages; /* allocated pages */ - int tblsize; /* allocated table size */ - struct snd_sg_page *table; /* address table */ - struct page **page_table; /* page table (for vmap/vunmap) */ - struct device *dev; -}; - -/* table entries are align to 32 */ -#define SGBUF_TBL_ALIGN 32 -#define sgbuf_align_table(tbl) ALIGN((tbl), SGBUF_TBL_ALIGN) - -static void snd_dma_sg_free(struct snd_dma_buffer *dmab) -{ - struct snd_sg_buf *sgbuf = dmab->private_data; - struct snd_dma_buffer tmpb; - int i; - - if (!sgbuf) - return; - - vunmap(dmab->area); - dmab->area = NULL; - - tmpb.dev.type = SNDRV_DMA_TYPE_DEV; - if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG) - tmpb.dev.type = SNDRV_DMA_TYPE_DEV_WC; - tmpb.dev.dev = sgbuf->dev; - for (i = 0; i < sgbuf->pages; i++) { - if (!(sgbuf->table[i].addr & ~PAGE_MASK)) - continue; /* continuous pages */ - tmpb.area = sgbuf->table[i].buf; - tmpb.addr = sgbuf->table[i].addr & PAGE_MASK; - tmpb.bytes = (sgbuf->table[i].addr & ~PAGE_MASK) << PAGE_SHIFT; - snd_dma_free_pages(&tmpb); - } - - kfree(sgbuf->table); - kfree(sgbuf->page_table); - kfree(sgbuf); - dmab->private_data = NULL; -} - -#define MAX_ALLOC_PAGES 32 - -static void *snd_dma_sg_alloc(struct snd_dma_buffer *dmab, size_t size) -{ - struct snd_sg_buf *sgbuf; - unsigned int i, pages, chunk, maxpages; - struct snd_dma_buffer tmpb; - struct snd_sg_page *table; - struct page **pgtable; - int type = SNDRV_DMA_TYPE_DEV; - pgprot_t prot = PAGE_KERNEL; - void *area; - - dmab->private_data = sgbuf = kzalloc(sizeof(*sgbuf), GFP_KERNEL); - if (!sgbuf) - return NULL; - if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG) { - type = SNDRV_DMA_TYPE_DEV_WC; -#ifdef pgprot_noncached - prot = pgprot_noncached(PAGE_KERNEL); -#endif - } - sgbuf->dev = dmab->dev.dev; - pages = snd_sgbuf_aligned_pages(size); - sgbuf->tblsize = sgbuf_align_table(pages); - table = kcalloc(sgbuf->tblsize, sizeof(*table), GFP_KERNEL); - if (!table) - goto _failed; - sgbuf->table = table; - pgtable = kcalloc(sgbuf->tblsize, sizeof(*pgtable), GFP_KERNEL); - if (!pgtable) - goto _failed; - sgbuf->page_table = pgtable; - - /* allocate pages */ - maxpages = MAX_ALLOC_PAGES; - while (pages > 0) { - chunk = pages; - /* don't be too eager to take a huge chunk */ - if (chunk > maxpages) - chunk = maxpages; - chunk <<= PAGE_SHIFT; - if (snd_dma_alloc_pages_fallback(type, dmab->dev.dev, - chunk, &tmpb) < 0) { - if (!sgbuf->pages) - goto _failed; - size = sgbuf->pages * PAGE_SIZE; - break; - } - chunk = tmpb.bytes >> PAGE_SHIFT; - for (i = 0; i < chunk; i++) { - table->buf = tmpb.area; - table->addr = tmpb.addr; - if (!i) - table->addr |= chunk; /* mark head */ - table++; - *pgtable++ = virt_to_page(tmpb.area); - tmpb.area += PAGE_SIZE; - tmpb.addr += PAGE_SIZE; - } - sgbuf->pages += chunk; - pages -= chunk; - if (chunk < maxpages) - maxpages = chunk; - } - - sgbuf->size = size; - area = vmap(sgbuf->page_table, sgbuf->pages, VM_MAP, prot); - if (!area) - goto _failed; - return area; - - _failed: - snd_dma_sg_free(dmab); /* free the table */ - return NULL; -} - -static dma_addr_t snd_dma_sg_get_addr(struct snd_dma_buffer *dmab, - size_t offset) -{ - struct snd_sg_buf *sgbuf = dmab->private_data; - dma_addr_t addr; - - addr = sgbuf->table[offset >> PAGE_SHIFT].addr; - addr &= ~((dma_addr_t)PAGE_SIZE - 1); - return addr + offset % PAGE_SIZE; -} - -static struct page *snd_dma_sg_get_page(struct snd_dma_buffer *dmab, - size_t offset) -{ - struct snd_sg_buf *sgbuf = dmab->private_data; - unsigned int idx = offset >> PAGE_SHIFT; - - if (idx >= (unsigned int)sgbuf->pages) - return NULL; - return sgbuf->page_table[idx]; -} - -static unsigned int snd_dma_sg_get_chunk_size(struct snd_dma_buffer *dmab, - unsigned int ofs, - unsigned int size) -{ - struct snd_sg_buf *sg = dmab->private_data; - unsigned int start, end, pg; - - start = ofs >> PAGE_SHIFT; - end = (ofs + size - 1) >> PAGE_SHIFT; - /* check page continuity */ - pg = sg->table[start].addr >> PAGE_SHIFT; - for (;;) { - start++; - if (start > end) - break; - pg++; - if ((sg->table[start].addr >> PAGE_SHIFT) != pg) - return (start << PAGE_SHIFT) - ofs; - } - /* ok, all on continuous pages */ - return size; -} - -static int snd_dma_sg_mmap(struct snd_dma_buffer *dmab, - struct vm_area_struct *area) -{ - if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG) - area->vm_page_prot = pgprot_writecombine(area->vm_page_prot); - return -ENOENT; /* continue with the default mmap handler */ -} - -const struct snd_malloc_ops snd_dma_sg_ops = { - .alloc = snd_dma_sg_alloc, - .free = snd_dma_sg_free, - .get_addr = snd_dma_sg_get_addr, - .get_page = snd_dma_sg_get_page, - .get_chunk_size = snd_dma_sg_get_chunk_size, - .mmap = snd_dma_sg_mmap, -}; -- cgit v1.2.3 From b15706471abe916a16a38bee4434612998d869d2 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Mon, 18 Oct 2021 08:37:00 +0200 Subject: ALSA: firewire: Fix C++ style comments in uapi header UAPI headers are built with -std=c90 and C++ style comments are explicitly prohibited. The recent commit overlooked the rule and caused the error at header installation. This patch corrects those. Fixes: bea36afa102e ("ALSA: firewire-motu: add message parser to gather meter information in register DSP model") Fixes: 90b28f3bb85c ("ALSA: firewire-motu: add message parser for meter information in command DSP model") Fixes: 634ec0b2906e ("ALSA: firewire-motu: notify event for parameter change in register DSP model") Reported-by: Stephen Rothwell Acked-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211018113812.0a16efb0@canb.auug.org.au Link: https://lore.kernel.org/r/20211018063700.30834-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 70 +++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index 76190a0cb069..e52a97b3ceaa 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -68,8 +68,8 @@ struct snd_firewire_event_tascam_control { struct snd_firewire_event_motu_register_dsp_change { unsigned int type; - __u32 count; // The number of changes. - __u32 changes[]; // Encoded event for change of register DSP. + __u32 count; /* The number of changes. */ + __u32 changes[]; /* Encoded event for change of register DSP. */ }; union snd_firewire_event { @@ -119,25 +119,27 @@ struct snd_firewire_tascam_state { __be32 data[SNDRV_FIREWIRE_TASCAM_STATE_COUNT]; }; -// In below MOTU models, software is allowed to control their DSP by accessing to registers. -// - 828mk2 -// - 896hd -// - Traveler -// - 8 pre -// - Ultralite -// - 4 pre -// - Audio Express -// -// On the other hand, the status of DSP is split into specific messages included in the sequence of -// isochronous packet. ALSA firewire-motu driver gathers the messages and allow userspace applications -// to read it via ioctl. In 828mk2, 896hd, and Traveler, hardware meter for all of physical inputs -// are put into the message, while one pair of physical outputs is selected. The selection is done by -// LSB one byte in asynchronous write quadlet transaction to 0x'ffff'f000'0b2c. -// -// I note that V3HD/V4HD uses asynchronous transaction for the purpose. The destination address is -// registered to 0x'ffff'f000'0b38 and '0b3c by asynchronous write quadlet request. The size of -// message differs between 23 and 51 quadlets. For the case, the number of mixer bus can be extended -// up to 12. +/* + * In below MOTU models, software is allowed to control their DSP by accessing to registers. + * - 828mk2 + * - 896hd + * - Traveler + * - 8 pre + * - Ultralite + * - 4 pre + * - Audio Express + * + * On the other hand, the status of DSP is split into specific messages included in the sequence of + * isochronous packet. ALSA firewire-motu driver gathers the messages and allow userspace applications + * to read it via ioctl. In 828mk2, 896hd, and Traveler, hardware meter for all of physical inputs + * are put into the message, while one pair of physical outputs is selected. The selection is done by + * LSB one byte in asynchronous write quadlet transaction to 0x'ffff'f000'0b2c. + * + * I note that V3HD/V4HD uses asynchronous transaction for the purpose. The destination address is + * registered to 0x'ffff'f000'0b38 and '0b3c by asynchronous write quadlet request. The size of + * message differs between 23 and 51 quadlets. For the case, the number of mixer bus can be extended + * up to 12. + */ #define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_COUNT 40 @@ -219,18 +221,20 @@ struct snd_firewire_motu_register_dsp_parameter { __u8 reserved[64]; }; -// In below MOTU models, software is allowed to control their DSP by command in frame of -// asynchronous transaction to 0x'ffff'0001'0000: -// -// - 828 mk3 (FireWire only and Hybrid) -// - 896 mk3 (FireWire only and Hybrid) -// - Ultralite mk3 (FireWire only and Hybrid) -// - Traveler mk3 -// - Track 16 -// -// On the other hand, the states of hardware meter is split into specific messages included in the -// sequence of isochronous packet. ALSA firewire-motu driver gathers the message and allow userspace -// application to read it via ioctl. +/* + * In below MOTU models, software is allowed to control their DSP by command in frame of + * asynchronous transaction to 0x'ffff'0001'0000: + * + * - 828 mk3 (FireWire only and Hybrid) + * - 896 mk3 (FireWire only and Hybrid) + * - Ultralite mk3 (FireWire only and Hybrid) + * - Traveler mk3 + * - Track 16 + * + * On the other hand, the states of hardware meter is split into specific messages included in the + * sequence of isochronous packet. ALSA firewire-motu driver gathers the message and allow userspace + * application to read it via ioctl. + */ #define SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT 400 -- cgit v1.2.3 From 5aec579e08e4f2be7103ae264ac8f34883eb9273 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Mon, 18 Oct 2021 13:40:35 +0200 Subject: ALSA: uapi: Fix a C++ style comment in asound.h UAPI header should have no C++ style comment but only in the traditional C style comment, but there is still one place we used it mistakenly. This patch corrects it. Fixes: 542283566679 ("ALSA: ctl: remove unused macro for timestamping of elem_value") Reviewed-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211018114035.18433-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- include/uapi/sound/asound.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index 5859ca0a1439..5fbb79e30819 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -1002,7 +1002,7 @@ typedef int __bitwise snd_ctl_elem_iface_t; #define SNDRV_CTL_ELEM_ACCESS_WRITE (1<<1) #define SNDRV_CTL_ELEM_ACCESS_READWRITE (SNDRV_CTL_ELEM_ACCESS_READ|SNDRV_CTL_ELEM_ACCESS_WRITE) #define SNDRV_CTL_ELEM_ACCESS_VOLATILE (1<<2) /* control value may be changed without a notification */ -// (1 << 3) is unused. +/* (1 << 3) is unused. */ #define SNDRV_CTL_ELEM_ACCESS_TLV_READ (1<<4) /* TLV read is possible */ #define SNDRV_CTL_ELEM_ACCESS_TLV_WRITE (1<<5) /* TLV write is possible */ #define SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE (SNDRV_CTL_ELEM_ACCESS_TLV_READ|SNDRV_CTL_ELEM_ACCESS_TLV_WRITE) -- cgit v1.2.3 From 7d2a0df24227337cf42b024c91708f542ca4ff90 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 19 Oct 2021 08:05:35 +0200 Subject: ALSA: memalloc: Drop superfluous snd_dma_buffer_sync() declaration snd_dma_buffer_sync() is declared twice, and the one outside the ifdef CONFIG_HAS_DMA could lead to a build error when CONFIG_HAS_DMA=n. As it's an overlooked leftover after rebase, drop this line. Fixes: a25684a95646 ("ALSA: memalloc: Support for non-contiguous page allocation") Reported-by: Stephen Rothwell Link: https://lore.kernel.org/r/20211019165402.4fa82c38@canb.auug.org.au Link: https://lore.kernel.org/r/20211019060536.26089-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- include/sound/memalloc.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h index df615052dad4..653dfffb3ac8 100644 --- a/include/sound/memalloc.h +++ b/include/sound/memalloc.h @@ -97,9 +97,6 @@ static inline void snd_dma_buffer_sync(struct snd_dma_buffer *dmab, enum snd_dma_sync_mode mode) {} #endif -void snd_dma_buffer_sync(struct snd_dma_buffer *dmab, - enum snd_dma_sync_mode mode); - dma_addr_t snd_sgbuf_get_addr(struct snd_dma_buffer *dmab, size_t offset); struct page *snd_sgbuf_get_page(struct snd_dma_buffer *dmab, size_t offset); unsigned int snd_sgbuf_get_chunk_size(struct snd_dma_buffer *dmab, -- cgit v1.2.3 From f917c04fac45b70ff38633675f86ab4b51782205 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 19 Oct 2021 08:05:36 +0200 Subject: ALSA: memalloc: Fix a typo in snd_dma_buffer_sync() description It caused a warning for kernel-doc build. Fixes: a25684a95646 ("ALSA: memalloc: Support for non-contiguous page allocation") Reported-by: Stephen Rothwell Link: https://lore.kernel.org/r/20211019165402.4fa82c38@canb.auug.org.au Link: https://lore.kernel.org/r/20211019060536.26089-2-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/core/memalloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index acdebecf1a2e..99cd0f67daa1 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -196,7 +196,7 @@ EXPORT_SYMBOL(snd_dma_buffer_mmap); /** * snd_dma_buffer_sync - sync DMA buffer between CPU and device * @dmab: buffer allocation information - * @mod: sync mode + * @mode: sync mode */ void snd_dma_buffer_sync(struct snd_dma_buffer *dmab, enum snd_dma_sync_mode mode) -- cgit v1.2.3 From a0d21bb3279476c777434c40d969ea88ca64f9aa Mon Sep 17 00:00:00 2001 From: Chengfeng Ye Date: Sun, 24 Oct 2021 03:46:11 -0700 Subject: ALSA: gus: fix null pointer dereference on pointer block The pointer block return from snd_gf1_dma_next_block could be null, so there is a potential null pointer dereference issue. Fix this by adding a null check before dereference. Signed-off-by: Chengfeng Ye Link: https://lore.kernel.org/r/20211024104611.9919-1-cyeaa@connect.ust.hk Signed-off-by: Takashi Iwai --- sound/isa/gus/gus_dma.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/isa/gus/gus_dma.c b/sound/isa/gus/gus_dma.c index a1c770d826dd..6d664dd8dde0 100644 --- a/sound/isa/gus/gus_dma.c +++ b/sound/isa/gus/gus_dma.c @@ -126,6 +126,8 @@ static void snd_gf1_dma_interrupt(struct snd_gus_card * gus) } block = snd_gf1_dma_next_block(gus); spin_unlock(&gus->dma_lock); + if (!block) + return; snd_gf1_dma_program(gus, block->addr, block->buf_addr, block->count, (unsigned short) block->cmd); kfree(block); #if 0 -- cgit v1.2.3 From b97053df0f04747c3c1e021ecbe99db675342954 Mon Sep 17 00:00:00 2001 From: Chengfeng Ye Date: Sun, 24 Oct 2021 04:17:36 -0700 Subject: ALSA: usb-audio: fix null pointer dereference on pointer cs_desc The pointer cs_desc return from snd_usb_find_clock_source could be null, so there is a potential null pointer dereference issue. Fix this by adding a null check before dereference. Signed-off-by: Chengfeng Ye Link: https://lore.kernel.org/r/20211024111736.11342-1-cyeaa@connect.ust.hk Signed-off-by: Takashi Iwai --- sound/usb/clock.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 7dd71d342443..4dfe76416794 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -496,6 +496,10 @@ int snd_usb_set_sample_rate_v2v3(struct snd_usb_audio *chip, union uac23_clock_source_desc *cs_desc; cs_desc = snd_usb_find_clock_source(chip, clock, fmt->protocol); + + if (!cs_desc) + return 0; + if (fmt->protocol == UAC_VERSION_3) bmControls = le32_to_cpu(cs_desc->v3.bmControls); else -- cgit v1.2.3 From 9b371c6cc37f954360989eec41c2ddc5a6b83917 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 25 Oct 2021 14:11:41 +0200 Subject: ALSA: 6fire: fix control and bulk message timeouts USB control and bulk message timeouts are specified in milliseconds and should specifically not vary with CONFIG_HZ. Fixes: c6d43ba816d1 ("ALSA: usb/6fire - Driver for TerraTec DMX 6Fire USB") Cc: stable@vger.kernel.org # 2.6.39 Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20211025121142.6531-2-johan@kernel.org Signed-off-by: Takashi Iwai --- sound/usb/6fire/comm.c | 2 +- sound/usb/6fire/firmware.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c index 43a2a62d66f7..49629d4bb327 100644 --- a/sound/usb/6fire/comm.c +++ b/sound/usb/6fire/comm.c @@ -95,7 +95,7 @@ static int usb6fire_comm_send_buffer(u8 *buffer, struct usb_device *dev) int actual_len; ret = usb_interrupt_msg(dev, usb_sndintpipe(dev, COMM_EP), - buffer, buffer[1] + 2, &actual_len, HZ); + buffer, buffer[1] + 2, &actual_len, 1000); if (ret < 0) return ret; else if (actual_len != buffer[1] + 2) diff --git a/sound/usb/6fire/firmware.c b/sound/usb/6fire/firmware.c index 8981e61f2da4..c51abc54d2f8 100644 --- a/sound/usb/6fire/firmware.c +++ b/sound/usb/6fire/firmware.c @@ -160,7 +160,7 @@ static int usb6fire_fw_ezusb_write(struct usb_device *device, { return usb_control_msg_send(device, 0, type, USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, - value, 0, data, len, HZ, GFP_KERNEL); + value, 0, data, len, 1000, GFP_KERNEL); } static int usb6fire_fw_ezusb_read(struct usb_device *device, @@ -168,7 +168,7 @@ static int usb6fire_fw_ezusb_read(struct usb_device *device, { return usb_control_msg_recv(device, 0, type, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, - value, 0, data, len, HZ, GFP_KERNEL); + value, 0, data, len, 1000, GFP_KERNEL); } static int usb6fire_fw_fpga_write(struct usb_device *device, @@ -178,7 +178,7 @@ static int usb6fire_fw_fpga_write(struct usb_device *device, int ret; ret = usb_bulk_msg(device, usb_sndbulkpipe(device, FPGA_EP), data, len, - &actual_len, HZ); + &actual_len, 1000); if (ret < 0) return ret; else if (actual_len != len) -- cgit v1.2.3 From f4000b58b64344871d7b27c05e73932f137cfef6 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 25 Oct 2021 14:11:42 +0200 Subject: ALSA: line6: fix control and interrupt message timeouts USB control and interrupt message timeouts are specified in milliseconds and should specifically not vary with CONFIG_HZ. Fixes: 705ececd1c60 ("Staging: add line6 usb driver") Cc: stable@vger.kernel.org # 2.6.30 Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20211025121142.6531-3-johan@kernel.org Signed-off-by: Takashi Iwai --- sound/usb/line6/driver.c | 14 +++++++------- sound/usb/line6/driver.h | 2 +- sound/usb/line6/podhd.c | 6 +++--- sound/usb/line6/toneport.c | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index 9602929b7de9..59faa5a9a714 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -113,12 +113,12 @@ int line6_send_raw_message(struct usb_line6 *line6, const char *buffer, retval = usb_interrupt_msg(line6->usbdev, usb_sndintpipe(line6->usbdev, properties->ep_ctrl_w), (char *)frag_buf, frag_size, - &partial, LINE6_TIMEOUT * HZ); + &partial, LINE6_TIMEOUT); } else { retval = usb_bulk_msg(line6->usbdev, usb_sndbulkpipe(line6->usbdev, properties->ep_ctrl_w), (char *)frag_buf, frag_size, - &partial, LINE6_TIMEOUT * HZ); + &partial, LINE6_TIMEOUT); } if (retval) { @@ -347,7 +347,7 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data, ret = usb_control_msg_send(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, (datalen << 8) | 0x21, address, NULL, 0, - LINE6_TIMEOUT * HZ, GFP_KERNEL); + LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(line6->ifcdev, "read request failed (error %d)\n", ret); goto exit; @@ -360,7 +360,7 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data, ret = usb_control_msg_recv(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, 0x0012, 0x0000, &len, 1, - LINE6_TIMEOUT * HZ, GFP_KERNEL); + LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(line6->ifcdev, "receive length failed (error %d)\n", ret); @@ -387,7 +387,7 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data, /* receive the result: */ ret = usb_control_msg_recv(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, - 0x0013, 0x0000, data, datalen, LINE6_TIMEOUT * HZ, + 0x0013, 0x0000, data, datalen, LINE6_TIMEOUT, GFP_KERNEL); if (ret) dev_err(line6->ifcdev, "read failed (error %d)\n", ret); @@ -417,7 +417,7 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data, ret = usb_control_msg_send(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, - 0x0022, address, data, datalen, LINE6_TIMEOUT * HZ, + 0x0022, address, data, datalen, LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(line6->ifcdev, @@ -430,7 +430,7 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data, ret = usb_control_msg_recv(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, - 0x0012, 0x0000, status, 1, LINE6_TIMEOUT * HZ, + 0x0012, 0x0000, status, 1, LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(line6->ifcdev, diff --git a/sound/usb/line6/driver.h b/sound/usb/line6/driver.h index 71d3da1db8c8..ecf3a2b39c7e 100644 --- a/sound/usb/line6/driver.h +++ b/sound/usb/line6/driver.h @@ -27,7 +27,7 @@ #define LINE6_FALLBACK_INTERVAL 10 #define LINE6_FALLBACK_MAXPACKETSIZE 16 -#define LINE6_TIMEOUT 1 +#define LINE6_TIMEOUT 1000 #define LINE6_BUFSIZE_LISTEN 64 #define LINE6_MIDI_MESSAGE_MAXLEN 256 diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c index 28794a35949d..b24bc82f89e3 100644 --- a/sound/usb/line6/podhd.c +++ b/sound/usb/line6/podhd.c @@ -190,7 +190,7 @@ static int podhd_dev_start(struct usb_line6_podhd *pod) ret = usb_control_msg_send(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, 0x11, 0, - NULL, 0, LINE6_TIMEOUT * HZ, GFP_KERNEL); + NULL, 0, LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret); goto exit; @@ -200,7 +200,7 @@ static int podhd_dev_start(struct usb_line6_podhd *pod) ret = usb_control_msg_recv(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, 0x11, 0x0, - init_bytes, 3, LINE6_TIMEOUT * HZ, GFP_KERNEL); + init_bytes, 3, LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(pod->line6.ifcdev, "receive length failed (error %d)\n", ret); @@ -220,7 +220,7 @@ static int podhd_dev_start(struct usb_line6_podhd *pod) USB_REQ_SET_FEATURE, USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT, 1, 0, - NULL, 0, LINE6_TIMEOUT * HZ, GFP_KERNEL); + NULL, 0, LINE6_TIMEOUT, GFP_KERNEL); exit: return ret; } diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c index 4e5693c97aa4..e33df58740a9 100644 --- a/sound/usb/line6/toneport.c +++ b/sound/usb/line6/toneport.c @@ -128,7 +128,7 @@ static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2) ret = usb_control_msg_send(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, - cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ, + cmd1, cmd2, NULL, 0, LINE6_TIMEOUT, GFP_KERNEL); if (ret) { -- cgit v1.2.3 From 55f261b73a7e1cb254577c3536cef8f415de220a Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 26 Oct 2021 11:54:01 +0200 Subject: ALSA: ua101: fix division by zero at probe Add the missing endpoint max-packet sanity check to probe() to avoid division by zero in alloc_stream_buffers() in case a malicious device has broken descriptors (or when doing descriptor fuzz testing). Note that USB core will reject URBs submitted for endpoints with zero wMaxPacketSize but that drivers doing packet-size calculations still need to handle this (cf. commit 2548288b4fb0 ("USB: Fix: Don't skip endpoint descriptors with maxpacket=0")). Fixes: 63978ab3e3e9 ("sound: add Edirol UA-101 support") Cc: stable@vger.kernel.org # 2.6.34 Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20211026095401.26522-1-johan@kernel.org Signed-off-by: Takashi Iwai --- sound/usb/misc/ua101.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/usb/misc/ua101.c b/sound/usb/misc/ua101.c index 5834d1dc317e..4f6b20ed29dd 100644 --- a/sound/usb/misc/ua101.c +++ b/sound/usb/misc/ua101.c @@ -1000,7 +1000,7 @@ static int detect_usb_format(struct ua101 *ua) fmt_playback->bSubframeSize * ua->playback.channels; epd = &ua->intf[INTF_CAPTURE]->altsetting[1].endpoint[0].desc; - if (!usb_endpoint_is_isoc_in(epd)) { + if (!usb_endpoint_is_isoc_in(epd) || usb_endpoint_maxp(epd) == 0) { dev_err(&ua->dev->dev, "invalid capture endpoint\n"); return -ENXIO; } @@ -1008,7 +1008,7 @@ static int detect_usb_format(struct ua101 *ua) ua->capture.max_packet_bytes = usb_endpoint_maxp(epd); epd = &ua->intf[INTF_PLAYBACK]->altsetting[1].endpoint[0].desc; - if (!usb_endpoint_is_isoc_out(epd)) { + if (!usb_endpoint_is_isoc_out(epd) || usb_endpoint_maxp(epd) == 0) { dev_err(&ua->dev->dev, "invalid playback endpoint\n"); return -ENXIO; } -- cgit v1.2.3 From 375f8426ed994addd2be4d76febc946a6fdd8280 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 28 Oct 2021 09:09:11 +0200 Subject: ALSA: hda/realtek: Add a quirk for HP OMEN 15 mute LED HP OMEN 15 laptop requires the quirk to fiddle with COEF 0x0b bit 2 for toggling the mute LED. It's already implemented for other HP laptops, and we just need to add a proper fixup entry. BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=214735 Cc: Link: https://lore.kernel.org/r/20211028070911.18891-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/pci/hda/patch_realtek.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 1bbe22769ca9..55c7f6fa70d6 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -8574,6 +8574,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { ALC285_FIXUP_HP_GPIO_AMP_INIT), SND_PCI_QUIRK(0x103c, 0x8783, "HP ZBook Fury 15 G7 Mobile Workstation", ALC285_FIXUP_HP_GPIO_AMP_INIT), + SND_PCI_QUIRK(0x103c, 0x8788, "HP OMEN 15", ALC285_FIXUP_HP_MUTE_LED), SND_PCI_QUIRK(0x103c, 0x87c8, "HP", ALC287_FIXUP_HP_GPIO_LED), SND_PCI_QUIRK(0x103c, 0x87e5, "HP ProBook 440 G8 Notebook PC", ALC236_FIXUP_HP_GPIO_LED), SND_PCI_QUIRK(0x103c, 0x87e7, "HP ProBook 450 G8 Notebook PC", ALC236_FIXUP_HP_GPIO_LED), -- cgit v1.2.3 From d593f78e3b53891692162d7b4e68f341c5869295 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 27 Oct 2021 21:55:27 +0900 Subject: ALSA: firewire-motu: fix null pointer dereference when polling hwdep character device ALSA firewire-motu driver recently got support for event notification via ALSA HwDep interface for register DSP models. However, when polling ALSA HwDep cdev, the driver can cause null pointer dereference for the other models due to accessing to unallocated memory or uninitialized memory. This commit fixes the bug by check the type of model before accessing to the memory. Reported-by: kernel test robot Suggested-by: Takashi Iwai Fixes: 634ec0b2906e ("ALSA: firewire-motu: notify event for parameter change in register DSP model") Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211027125529.54295-2-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- sound/firewire/motu/motu-hwdep.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/sound/firewire/motu/motu-hwdep.c b/sound/firewire/motu/motu-hwdep.c index 9c2e457ce692..a900fc0e7644 100644 --- a/sound/firewire/motu/motu-hwdep.c +++ b/sound/firewire/motu/motu-hwdep.c @@ -16,6 +16,14 @@ #include "motu.h" +static bool has_dsp_event(struct snd_motu *motu) +{ + if (motu->spec->flags & SND_MOTU_SPEC_REGISTER_DSP) + return (snd_motu_register_dsp_message_parser_count_event(motu) > 0); + else + return false; +} + static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, loff_t *offset) { @@ -25,8 +33,7 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, spin_lock_irq(&motu->lock); - while (!motu->dev_lock_changed && motu->msg == 0 && - snd_motu_register_dsp_message_parser_count_event(motu) == 0) { + while (!motu->dev_lock_changed && motu->msg == 0 && !has_dsp_event(motu)) { prepare_to_wait(&motu->hwdep_wait, &wait, TASK_INTERRUPTIBLE); spin_unlock_irq(&motu->lock); schedule(); @@ -55,7 +62,7 @@ static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, count = min_t(long, count, sizeof(event)); if (copy_to_user(buf, &event, count)) return -EFAULT; - } else if (snd_motu_register_dsp_message_parser_count_event(motu) > 0) { + } else if (has_dsp_event(motu)) { size_t consumed = 0; u32 __user *ptr; u32 ev; @@ -94,8 +101,7 @@ static __poll_t hwdep_poll(struct snd_hwdep *hwdep, struct file *file, poll_wait(file, &motu->hwdep_wait, wait); spin_lock_irq(&motu->lock); - if (motu->dev_lock_changed || motu->msg || - snd_motu_register_dsp_message_parser_count_event(motu) > 0) + if (motu->dev_lock_changed || motu->msg || has_dsp_event(motu)) events = EPOLLIN | EPOLLRDNORM; else events = 0; -- cgit v1.2.3 From 0f166e1257a1e24571381b857632b6c7c6ac3a0b Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 27 Oct 2021 21:55:28 +0900 Subject: ALSA: firewire-motu: refine parser for meter information in register DSP models After further investigation, I realize that the total number of elements in array is not enough to store all of related messages from device. This commit refines meter array and message parser. In terms of channel identifier, register DSP models are classified to two categories: 1. the target of output is selectable 828mk2, 896hd, and Traveler are in the category. They transfer messages with channel identifier between 0x00 and 0x13 for input meters, therefore 20 elements are needed to store. On the other hand, they transfer messages with channel identifier for one pair of output meters. The selection is done by asynchronous write transaction to offset 0x'ffff'f000'0b2c. The table for relationship between written value and available identifiers is below: ============= =============== written value identifier pair ============= =============== 0x00000b00 0x80/0x81 0x00000b01 0x82/0x83 ... ... 0x00000b0b 0x96/0x97 ... ... 0x00000b10 0xa0/0xa1 ... ... 0x00000b3f 0xfe/0xff ... ... greater 0xfe/0xff ============= =============== Actually in the above three models, 0x96/0x97 pair is the maximum. Thus the number of available output meter is 24. 2. all of output is available 8 pre, Ultralite, Audio Express, and 4 pre are in the category. They transfer messages for output meters without any selection. The table for available identifier for each direction is below: ============== ========= ========== model input output ============== ========= ========== 8 pre 0x00-0x0f 0x82-0x8d Ultralite 0x00-0x09 0x82-0x8f Audio Express 0x00-0x09 0x80-0x8d 4 pre 0x00-0x09 0x80-0x8d ============== ========= ========== Some of available identifiers might not be used for actual output meters. Anyway, 24 plus 24 elements accommodate the input/output meters. I note that isochronous packet from V3HD/V4HD deliver no message. Notification by asynchronous transaction to registered address seems to be used for the purpose as well as for change of mixer parameter. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211027125529.54295-3-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 5 ++++- sound/firewire/motu/motu-register-dsp-message-parser.c | 14 +++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index e52a97b3ceaa..68eb0e43c052 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -141,7 +141,10 @@ struct snd_firewire_tascam_state { * up to 12. */ -#define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_COUNT 40 +#define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_INPUT_COUNT 24 +#define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_OUTPUT_COUNT 24 +#define SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_COUNT \ + (SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_INPUT_COUNT + SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_OUTPUT_COUNT) /** * struct snd_firewire_motu_register_dsp_meter - the container for meter information in DSP diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c index cbc06b3b70f6..0c587567540f 100644 --- a/sound/firewire/motu/motu-register-dsp-message-parser.c +++ b/sound/firewire/motu/motu-register-dsp-message-parser.c @@ -335,11 +335,15 @@ void snd_motu_register_dsp_message_parser_parse(struct snd_motu *motu, const str else pos = b[MSG_METER_IDX_POS_4PRE_AE]; - if (pos < 0x80) - pos &= 0x1f; - else - pos = (pos & 0x1f) + 20; - parser->meter.data[pos] = val; + if (pos < SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_INPUT_COUNT) { + parser->meter.data[pos] = val; + } else if (pos >= 0x80) { + pos -= (0x80 - SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_INPUT_COUNT); + + if (pos < SNDRV_FIREWIRE_MOTU_REGISTER_DSP_METER_COUNT) + parser->meter.data[pos] = val; + } + // The message for meter is interruptible to the series of other // types of messages. Don't cache it. fallthrough; -- cgit v1.2.3 From 407359d44ed33974137b9158da356d53f999dcf2 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 27 Oct 2021 21:55:29 +0900 Subject: ALSA: firewire-motu: export meter information to userspace as float value In command DSP models, one meter information consists of 4 bytes for IEEE 764 floating point (binary32). In previous patch, it is exported to userspace as 32 bit storage since the storage is also handled in ALSA firewire-motu driver as well in kernel space in which floating point arithmetic is not preferable. On the other hand, ALSA firewire-motu driver doesn't perform floating point calculation. The driver just gather meter information from isochronous packets and fill structure fields for userspace. In 'header' target of Kbuild, UAPI headers are processed before installed. In this timing, #ifdef macro with __KERNEL__ is removed. This mechanism is useful in the case so that the 32 bit storage can be accessible as u32 type in kernel space and float type in user space. We can see the same usage in ''struct acct_v3' in 'include/uapi/linux/acct.h'. This commit is for the above idea. Additionally, due to message protocol, meter information is filled with 0xffffffff in the end of period but 0xffffffff is invalid as binary32. To avoid confusion in userspace application, the last two elements are left without any assignment. Suggested-by: Takashi Iwai Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211027125529.54295-4-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- include/uapi/sound/firewire.h | 8 +++++--- sound/firewire/motu/motu-command-dsp-message-parser.c | 7 +++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h index 68eb0e43c052..39cf6eb75940 100644 --- a/include/uapi/sound/firewire.h +++ b/include/uapi/sound/firewire.h @@ -248,12 +248,14 @@ struct snd_firewire_motu_register_dsp_parameter { * * The structure expresses the part of DSP status for hardware meter. The 32 bit storage is * estimated to include IEEE 764 32 bit single precision floating point (binary32) value. It is - * expected to be linear value (not logarithm) for audio signal level between 0.0 and +1.0. However, - * the last two quadlets (data[398] and data[399]) are filled with 0xffffffff since they are the - * marker of one period. + * expected to be linear value (not logarithm) for audio signal level between 0.0 and +1.0. */ struct snd_firewire_motu_command_dsp_meter { +#ifdef __KERNEL__ __u32 data[SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT]; +#else + float data[SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT]; +#endif }; #endif /* _UAPI_SOUND_FIREWIRE_H_INCLUDED */ diff --git a/sound/firewire/motu/motu-command-dsp-message-parser.c b/sound/firewire/motu/motu-command-dsp-message-parser.c index 18689fcfb288..9efe4d364baf 100644 --- a/sound/firewire/motu/motu-command-dsp-message-parser.c +++ b/sound/firewire/motu/motu-command-dsp-message-parser.c @@ -141,12 +141,15 @@ void snd_motu_command_dsp_message_parser_parse(struct snd_motu *motu, const stru ++parser->fragment_pos; if (parser->fragment_pos == 4) { + // Skip the last two quadlets since they could be + // invalid value (0xffffffff) as floating point + // number. if (parser->value_index < - SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT) { + SNDRV_FIREWIRE_MOTU_COMMAND_DSP_METER_COUNT - 2) { u32 val = (u32)(parser->value >> 32); parser->meter.data[parser->value_index] = val; - ++parser->value_index; } + ++parser->value_index; parser->fragment_pos = 0; } -- cgit v1.2.3 From cddcd5472abb7b8a9c37ccbcf0908b79740a01b5 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 28 Oct 2021 22:03:25 +0900 Subject: ALSA: oxfw: fix functional regression for Mackie Onyx 1640i in v5.14 or later A user reports functional regression for Mackie Onyx 1640i that the device generates slow sound with ALSA oxfw driver which supports media clock recovery. Although the device is based on OXFW971 ASIC, it does not transfer isochronous packet with own event frequency as expected. The device seems to adjust event frequency according to events in received isochronous packets in the beginning of packet streaming. This is unknown quirk. This commit fixes the regression to turn the recovery off in driver side. As a result, nominal frequency is used in duplex packet streaming between device and driver. For stability of sampling rate in events of transferred isochronous packet, 4,000 isochronous packets are skipped in the beginning of packet streaming. Reference: https://github.com/takaswie/snd-firewire-improve/issues/38 Fixes: 029ffc429440 ("ALSA: oxfw: perform sequence replay for media clock recovery") Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211028130325.45772-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- sound/firewire/oxfw/oxfw-stream.c | 7 ++++++- sound/firewire/oxfw/oxfw.c | 8 ++++++++ sound/firewire/oxfw/oxfw.h | 5 +++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/sound/firewire/oxfw/oxfw-stream.c b/sound/firewire/oxfw/oxfw-stream.c index fff18b5d4e05..f4a702def397 100644 --- a/sound/firewire/oxfw/oxfw-stream.c +++ b/sound/firewire/oxfw/oxfw-stream.c @@ -9,7 +9,7 @@ #include #define AVC_GENERIC_FRAME_MAXIMUM_BYTES 512 -#define READY_TIMEOUT_MS 200 +#define READY_TIMEOUT_MS 600 /* * According to datasheet of Oxford Semiconductor: @@ -367,6 +367,11 @@ int snd_oxfw_stream_start_duplex(struct snd_oxfw *oxfw) // Just after changing sampling transfer frequency, many cycles are // skipped for packet transmission. tx_init_skip_cycles = 400; + } else if (oxfw->quirks & SND_OXFW_QUIRK_VOLUNTARY_RECOVERY) { + // It takes a bit time for target device to adjust event frequency + // according to nominal event frequency in isochronous packets from + // ALSA oxfw driver. + tx_init_skip_cycles = 4000; } else { replay_seq = true; } diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c index daf731364695..b496f87841ae 100644 --- a/sound/firewire/oxfw/oxfw.c +++ b/sound/firewire/oxfw/oxfw.c @@ -25,6 +25,7 @@ #define MODEL_SATELLITE 0x00200f #define MODEL_SCS1M 0x001000 #define MODEL_DUET_FW 0x01dddd +#define MODEL_ONYX_1640I 0x001640 #define SPECIFIER_1394TA 0x00a02d #define VERSION_AVC 0x010001 @@ -192,6 +193,13 @@ static int detect_quirks(struct snd_oxfw *oxfw, const struct ieee1394_device_id // OXFW971-based models may transfer events by blocking method. if (!(oxfw->quirks & SND_OXFW_QUIRK_JUMBO_PAYLOAD)) oxfw->quirks |= SND_OXFW_QUIRK_BLOCKING_TRANSMISSION; + + if (model == MODEL_ONYX_1640I) { + //Unless receiving packets without NOINFO packet, the device transfers + //mostly half of events in packets than expected. + oxfw->quirks |= SND_OXFW_QUIRK_IGNORE_NO_INFO_PACKET | + SND_OXFW_QUIRK_VOLUNTARY_RECOVERY; + } } return 0; diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h index c13034f6c2ca..d728e451a25c 100644 --- a/sound/firewire/oxfw/oxfw.h +++ b/sound/firewire/oxfw/oxfw.h @@ -47,6 +47,11 @@ enum snd_oxfw_quirk { // the device to process audio data even if the value is invalid in a point of // IEC 61883-1/6. SND_OXFW_QUIRK_IGNORE_NO_INFO_PACKET = 0x10, + // Loud Technologies Mackie Onyx 1640i seems to configure OXFW971 ASIC so that it decides + // event frequency according to events in received isochronous packets. The device looks to + // performs media clock recovery voluntarily. In the recovery, the packets with NO_INFO + // are ignored, thus driver should transfer packets with timestamp. + SND_OXFW_QUIRK_VOLUNTARY_RECOVERY = 0x20, }; /* This is an arbitrary number for convinience. */ -- cgit v1.2.3 From 2672e1970ab051f0333fdbc61a55b7616f4f5778 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 29 Oct 2021 10:28:47 +0900 Subject: ALSA: firewire-motu: remove TODO for interaction with userspace about control message Now UAPI of ALSA firewire stack got enough functions to interact with userspace for protocol of MOTU FireWire series. Let's remove the relevant TODO. Signed-off-by: Takashi Sakamoto Link: https://lore.kernel.org/r/20211029012847.11839-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Iwai --- sound/firewire/motu/amdtp-motu.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/firewire/motu/amdtp-motu.c b/sound/firewire/motu/amdtp-motu.c index 3ea91e281147..2fb52f481d12 100644 --- a/sound/firewire/motu/amdtp-motu.c +++ b/sound/firewire/motu/amdtp-motu.c @@ -424,8 +424,6 @@ static unsigned int process_it_ctx_payloads(struct amdtp_stream *s, if (p->midi_ports) write_midi_messages(s, buf, data_blocks); - // TODO: how to interact control messages between userspace? - write_sph(p->cache, buf, data_blocks, s->data_block_quadlets); } -- cgit v1.2.3