- Alsa-devel - mailman.alsa-project.org

Re: [PATCH 08/12] ASoC: mediatek: mt8188: add platform driver
by Trevor Wu (吳文良) 06 Oct '22

06 Oct '22

Hi maintainers, As I mentioned in the comment, this patch depends on a reviewing series. Should I remove the related code from this patch in case of the build error? Thanks, Trevor On Sat, 2022-10-01 at 17:17 +0800, kernel test robot wrote: > Hi Trevor, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on broonie-sound/for-next] > [also build test ERROR on broonie-spi/for-next tiwai-sound/for-next > linus/master v6.0-rc7 next-20220930] > [If your patch is applied to the wrong git tree, kindly drop us a > note. > And when submitting patch, we suggest to use '--base' as documented > in > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch*_base… > ] > > url: > https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/… > > base: > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git… > for-next > config: s390-allyesconfig > compiler: s390-linux-gcc (GCC) 12.1.0 > reproduce (this is a W=1 build): > wget > https://urldefense.com/v3/__https://raw.githubusercontent.com/intel/lkp-tes… > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # > https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commit/6… > > git remote add linux-review > https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux__;!!CTRN… > > git fetch --no-tags linux-review Trevor-Wu/ASoC-mediatek-Add- > support-for-MT8188-SoC/20220930-230009 > git checkout 65c941d291852582ccbe6a8196ff3b60fb4d1243 > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 > make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash > > If you fix the issue, kindly add following tag where applicable > > Reported-by: kernel test robot <lkp(a)intel.com> > > All errors (new ones prefixed by >>): > > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c: In function > 'bus_protect_enable': > > > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2649:16: error: > > > 'MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP1' undeclared (first use in > > > this function); did you mean > > > 'MT8186_TOP_AXI_PROT_EN_1_DIS_STEP1'? > > 2649 | mask = MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP1; > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | MT8186_TOP_AXI_PROT_EN_1_DIS_STEP1 > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2649:16: note: each > undeclared identifier is reported only once for each function it > appears in > > > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2650:30: error: > > > 'MT8188_TOP_AXI_PROT_EN_2_SET' undeclared (first use in this > > > function); did you mean 'MT8186_TOP_AXI_PROT_EN_2_SET'? > > 2650 | regmap_write(regmap, MT8188_TOP_AXI_PROT_EN_2_SET, > mask); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | MT8186_TOP_AXI_PROT_EN_2_SET > In file included from include/linux/regmap.h:20, > from sound/soc/mediatek/mt8188/mt8188-afe- > common.h:15, > from sound/soc/mediatek/mt8188/mt8188-afe- > pcm.c:24: > > > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2652:48: error: > > > 'MT8188_TOP_AXI_PROT_EN_2_STA' undeclared (first use in this > > > function); did you mean 'MT8186_TOP_AXI_PROT_EN_2_STA'? > > 2652 | ret = regmap_read_poll_timeout(regmap, > MT8188_TOP_AXI_PROT_EN_2_STA, > | ^~~~~~~~~~~ > ~~~~~~~~~~~~~~~~~ > include/linux/iopoll.h:46:28: note: in definition of macro > 'read_poll_timeout' > 46 | (val) = op(args); \ > | ^~~~ > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2652:15: note: in > expansion of macro 'regmap_read_poll_timeout' > 2652 | ret = regmap_read_poll_timeout(regmap, > MT8188_TOP_AXI_PROT_EN_2_STA, > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2659:16: error: > > > 'MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP2' undeclared (first use in > > > this function); did you mean > > > 'MT8186_TOP_AXI_PROT_EN_1_DIS_STEP1'? > > 2659 | mask = MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP2; > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | MT8186_TOP_AXI_PROT_EN_1_DIS_STEP1 > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c: In function > 'bus_protect_disable': > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2678:16: error: > 'MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP2' undeclared (first use in this > function); did you mean 'MT8186_TOP_AXI_PROT_EN_1_DIS_STEP1'? > 2678 | mask = MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP2; > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | MT8186_TOP_AXI_PROT_EN_1_DIS_STEP1 > > > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2679:30: error: > > > 'MT8188_TOP_AXI_PROT_EN_2_CLR' undeclared (first use in this > > > function); did you mean 'MT8186_TOP_AXI_PROT_EN_2_CLR'? > > 2679 | regmap_write(regmap, MT8188_TOP_AXI_PROT_EN_2_CLR, > mask); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | MT8186_TOP_AXI_PROT_EN_2_CLR > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2681:48: error: > 'MT8188_TOP_AXI_PROT_EN_2_STA' undeclared (first use in this > function); did you mean 'MT8186_TOP_AXI_PROT_EN_2_STA'? > 2681 | ret = regmap_read_poll_timeout(regmap, > MT8188_TOP_AXI_PROT_EN_2_STA, > | ^~~~~~~~~~~ > ~~~~~~~~~~~~~~~~~ > include/linux/iopoll.h:46:28: note: in definition of macro > 'read_poll_timeout' > 46 | (val) = op(args); \ > | ^~~~ > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2681:15: note: in > expansion of macro 'regmap_read_poll_timeout' > 2681 | ret = regmap_read_poll_timeout(regmap, > MT8188_TOP_AXI_PROT_EN_2_STA, > | ^~~~~~~~~~~~~~~~~~~~~~~~ > sound/soc/mediatek/mt8188/mt8188-afe-pcm.c:2688:16: error: > 'MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP1' undeclared (first use in this > function); did you mean 'MT8186_TOP_AXI_PROT_EN_1_DIS_STEP1'? > 2688 | mask = MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP1; > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | MT8186_TOP_AXI_PROT_EN_1_DIS_STEP1 > > > vim +2649 sound/soc/mediatek/mt8188/mt8188-afe-pcm.c > > 2641 > 2642 static int bus_protect_enable(struct regmap *regmap) > 2643 { > 2644 int ret; > 2645 u32 val; > 2646 u32 mask; > 2647 > 2648 val = 0; > > 2649 mask = MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP1; > > 2650 regmap_write(regmap, > > MT8188_TOP_AXI_PROT_EN_2_SET, mask); > > 2651 > > 2652 ret = regmap_read_poll_timeout(regmap, > > MT8188_TOP_AXI_PROT_EN_2_STA, > > 2653 val, (val & > mask) == mask, > 2654 MT8188_DELAY_US, > MT8188_TIMEOUT_US); > 2655 if (ret) > 2656 return ret; > 2657 > 2658 val = 0; > > 2659 mask = MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP2; > > 2660 regmap_write(regmap, > MT8188_TOP_AXI_PROT_EN_2_SET, mask); > 2661 > 2662 ret = regmap_read_poll_timeout(regmap, > MT8188_TOP_AXI_PROT_EN_2_STA, > 2663 val, (val & > mask) == mask, > 2664 MT8188_DELAY_US, > MT8188_TIMEOUT_US); > 2665 if (ret) > 2666 return ret; > 2667 > 2668 return 0; > 2669 } > 2670 > 2671 static int bus_protect_disable(struct regmap *regmap) > 2672 { > 2673 int ret; > 2674 u32 val; > 2675 u32 mask; > 2676 > 2677 val = 0; > 2678 mask = MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP2; > > 2679 regmap_write(regmap, > > MT8188_TOP_AXI_PROT_EN_2_CLR, mask); > > 2680 > 2681 ret = regmap_read_poll_timeout(regmap, > MT8188_TOP_AXI_PROT_EN_2_STA, > 2682 val, !(val & > mask), > 2683 MT8188_DELAY_US, > MT8188_TIMEOUT_US); > 2684 if (ret) > 2685 return ret; > 2686 > 2687 val = 0; > 2688 mask = MT8188_TOP_AXI_PROT_EN_2_AUDIO_STEP1; > 2689 regmap_write(regmap, > MT8188_TOP_AXI_PROT_EN_2_CLR, mask); > 2690 > 2691 ret = regmap_read_poll_timeout(regmap, > MT8188_TOP_AXI_PROT_EN_2_STA, > 2692 val, !(val & > mask), > 2693 MT8188_DELAY_US, > MT8188_TIMEOUT_US); > 2694 if (ret) > 2695 return ret; > 2696 > 2697 return 0; > 2698 } > 2699 >

2 2

[PATCH 1/2] ASoC: dt-bindings: add pin-switches to audio-graph-card2.yaml
by Claudio Cabral 05 Oct '22

05 Oct '22

Document support for pin-switches in audio-graph-card2 this devicetree property behaves the same way as its homonym in simple-card Reviewed-by: Alvin Šipraga <alsi(a)bang-olufsen.dk> Signed-off-by: Alvin Šipraga <alsi(a)bang-olufsen.dk> Signed-off-by: Claudio Cabral <clca(a)bang-olufsen.dk> --- Documentation/devicetree/bindings/sound/audio-graph-card2.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card2.yaml b/Documentation/devicetree/bindings/sound/audio-graph-card2.yaml index 7416067c945e..ab2cc83d6b4b 100644 --- a/Documentation/devicetree/bindings/sound/audio-graph-card2.yaml +++ b/Documentation/devicetree/bindings/sound/audio-graph-card2.yaml @@ -32,6 +32,9 @@ properties: codec2codec: type: object description: Codec to Codec node + pin-switches: + description: the widget names for which pin switches must be created. + $ref: /schemas/types.yaml#/definitions/string-array required: - compatible -- 2.37.3

2 2

wm8962 0-001a: ASoC: error at soc_component_read_no_lock on wm8962.0-001a: -16
by Wang, Xiaolei 05 Oct '22

05 Oct '22

Hi Jaroslav When I tested the audio, I found that some registers in the codec of wm8962.c will fail to read in the mainline kernel. I found that wm8962_i2c_probe will enter runtime suspend after startup, but in the following program: snd_soc_bind_card snd_soc_dapm_new_widgets soc_dapm_read snd_soc_component_read(dapm->component, reg); There is no runtime resume, which cannot perform normal reading and writing for volatile reg, because it has been switched to regcache only in the process of runtime suspend. I can't find the right place to resume, can you give me any advice? thanks xiaolei

3 4

ALSA multi plugin error when combining two soundcards
by GitHub issues - opened 05 Oct '22

05 Oct '22

alsa-project/alsa-lib issue #273 was opened from anthonio9: My goal is to mix the output of two sound cards (RAVENNA from [bondagit](https://github.com/bondagit/aes67-linux-daemon.git) and hifiberry), then pass it to a DSP pcm plugin. As it's not possible with `dmix` (dmix only supports output to hw devices) the reasonable next step was `multi` plugin, but my tries always end up in a failure. Below is a quite standard `multi` configuration inspired by alsa docs that I'm using: ```sh pcm.quad { type multi slaves.a.pcm "hw:RAVENNA" slaves.a.channels 2 slaves.b.pcm "hw:sndrpihifiberry" slaves.b.channels 2 bindings.0.slave a bindings.0.channel 0 bindings.1.slave a bindings.1.channel 1 bindings.2.slave b bindings.2.channel 0 bindings.3.slave b bindings.3.channel 1 master 1 } pcm.quad2 { type plug slave.pcm "quad" } ``` This is run with `alsaloop` to be input into an extplug plugin that easily accepts 4 channels of audio - `alsadsp`: ```sh $ alsaloop -C quad -P alsadsp -c 4 Rate 48000Hz not available for capture quad: Invalid argument Unable to set parameters for capture quad stream: Invalid argument Loopback start failure. ``` This results in a failure, moreover running `quad2` also results in the same type of error. Next step was to try plug pcm: ```sh pcm.quad_plug { type multi slaves.a.pcm "plughw:RAVENNA" slaves.a.channels 2 slaves.b.pcm "plughw:sndrpihifiberry" slaves.b.channels 2 bindings.0.slave a bindings.0.channel 0 bindings.1.slave a bindings.1.channel 1 bindings.2.slave a bindings.2.channel 0 bindings.3.slave a bindings.3.channel 1 } pcm.quad_plug2 { type plug slave.pcm "quad_plug" } ``` With `quad_plug` the same error occurs, with `quad_plug2` the error is as below: ```sh $ alsaloop -C quad_plug2 -P alsadsp -c 4 Broken configuration for capture quad_plug2 PCM: no configurations available: Invalid argument Unable to set parameters for capture quad_plug2 stream: Invalid argument Loopback start failure. ``` Then the last thing I tried was to force the 48kHz sampling rate directly on the hardware devices with the help of the following config: ```sh pcm.quad_48k { type multi slaves.a { pcm { type plug slave.pcm "hw:RAVENNA" slave.rate 48000 } channels 2 } slaves.b { pcm { type plug slave.pcm "hw:sndrpihifiberry" slave.rate 48000 } channels 2 } bindings.0.slave a bindings.0.channel 0 bindings.1.slave a bindings.1.channel 1 bindings.2.slave b bindings.2.channel 0 bindings.3.slave b bindings.3.channel 1 } ``` and again it ends up with a failure: ```sh $ alsaloop -C quad_48k -P alsadsp -c 4 Sample format not available for capture quad_48k: Invalid argument Unable to set parameters for capture quad_48k stream: Invalid argument Loopback start failure. ``` I know that both these cards are capable of running 48kHz, I'm doing that all the time. So what could be the cause of this error? `alsadsp` wants them to be 48kHz and if run separately it all works well, both cards set themselves according to `alsadsp` requirements. It works even if there's only one card in the multi plugin that occupies all 4 output ports - my conclusion is that it's not a problem of `alsadsp`. Are there any other steps that could help solving the issue? Issue URL : https://github.com/alsa-project/alsa-lib/issues/273 Repository URL: https://github.com/alsa-project/alsa-lib

1 0

Add support for getting list of supported codecs
by GitHub pull_request - edited 05 Oct '22

05 Oct '22

alsa-project/tinycompress pull request #16 was edited from SanchayanMaity: Right now, there is no way to expose a list of codecs from `tinycompress`. While one could theoretically call `is_codec_supported` multiple times in an application to check against a list of codecs, an API would make it easy to enumerate supported codecs. The IOCTL `SNDRV_COMPRESS_GET_CAPS` after all already exists. Our use case is Pipewire where we would like the compressed sink node to advertise only codecs supported by the underlying hardware. So far we were just advertising the complete list of codecs and let node linking fail at runtime based on result from `is_codec_supported`. Request URL : https://github.com/alsa-project/tinycompress/pull/16 Patch URL : https://github.com/alsa-project/tinycompress/pull/16.patch Repository URL: https://github.com/alsa-project/tinycompress

1 0

[PATCH 1/1] ALSA: remove ALC289_FIXUP_DUAL_SPK for Dell 5530
by Callum Osmotherly 05 Oct '22

05 Oct '22

From: Callum Osmotherly <callum.osmotherly(a)gmail.com> After some feedback from users with Dell Precision 5530 machines, this patch reverts the previous change to add ALC289_FIXUP_DUAL_SPK. While it improved the speaker output quality, it caused the headphone jack to have an audible "pop" sound when power saving was toggled. Signed-off-by: Callum Osmotherly <callum.osmotherly(a)gmail.com> --- sound/pci/hda/patch_realtek.c | 1 - 1 file changed, 1 deletion(-) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index d35934f..ab6d736 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -8836,7 +8836,6 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x1028, 0x0871, "Dell Precision 3630", ALC255_FIXUP_DELL_HEADSET_MIC), SND_PCI_QUIRK(0x1028, 0x0872, "Dell Precision 3630", ALC255_FIXUP_DELL_HEADSET_MIC), SND_PCI_QUIRK(0x1028, 0x0873, "Dell Precision 3930", ALC255_FIXUP_DUMMY_LINEOUT_VERB), - SND_PCI_QUIRK(0x1028, 0x087d, "Dell Precision 5530", ALC289_FIXUP_DUAL_SPK), SND_PCI_QUIRK(0x1028, 0x08ad, "Dell WYSE AIO", ALC225_FIXUP_DELL_WYSE_AIO_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x08ae, "Dell WYSE NB", ALC225_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x0935, "Dell", ALC274_FIXUP_DELL_AIO_LINEOUT_VERB), -- 2.37.3

2 1

[PATCH] ALSA: hda/hdmi: run eld notify in delay work
by Brent Lu 05 Oct '22

05 Oct '22

During resolution change, display driver would disable HDMI audio then enable it in a short time. There is possibility that eld notify for HDMI audio enable is called when previous runtime suspend is still running. In this case, the elf nofity just returns and not updating the status of corresponding HDMI pin/port. Here we move the eld nofity to a delay work so we don't lose it. Signed-off-by: Brent Lu <brent.lu(a)intel.com> --- sound/pci/hda/patch_hdmi.c | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 6c209cd26c0c..a4c305ee8ff9 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2907,13 +2907,21 @@ static int intel_port2pin(struct hda_codec *codec, int port) return spec->port_map[port]; } -static void intel_pin_eld_notify(void *audio_ptr, int port, int pipe) +struct pin_eld_notify { + void *audio_ptr; + int port; + int pipe; + struct delayed_work notify_work; +}; + +static void pin_eld_notify_work(struct work_struct *work) { - struct hda_codec *codec = audio_ptr; + struct pin_eld_notify *notify = container_of(work, struct pin_eld_notify, notify_work.work); + struct hda_codec *codec = notify->audio_ptr; int pin_nid; - int dev_id = pipe; + int dev_id = notify->pipe; - pin_nid = intel_port2pin(codec, port); + pin_nid = intel_port2pin(codec, notify->port); if (!pin_nid) return; /* skip notification during system suspend (but not in runtime PM); @@ -2922,13 +2930,33 @@ static void intel_pin_eld_notify(void *audio_ptr, int port, int pipe) if (codec->core.dev.power.power_state.event == PM_EVENT_SUSPEND) return; /* ditto during suspend/resume process itself */ - if (snd_hdac_is_in_pm(&codec->core)) + if (snd_hdac_is_in_pm(&codec->core)) { + schedule_delayed_work(&notify->notify_work, msecs_to_jiffies(10)); return; + } snd_hdac_i915_set_bclk(&codec->bus->core); check_presence_and_report(codec, pin_nid, dev_id); } +static void intel_pin_eld_notify(void *audio_ptr, int port, int pipe) +{ + struct hda_codec *codec = audio_ptr; + struct device *dev = hda_codec_dev(codec); + struct pin_eld_notify *notify; + + notify = devm_kzalloc(dev, sizeof(struct pin_eld_notify), GFP_KERNEL); + if (!notify) + return; + + notify->audio_ptr = audio_ptr; + notify->port = port; + notify->pipe = pipe; + INIT_DELAYED_WORK(&notify->notify_work, pin_eld_notify_work); + + schedule_delayed_work(&notify->notify_work, 0); +} + static const struct drm_audio_component_audio_ops intel_audio_ops = { .pin2port = intel_pin2port, .pin_eld_notify = intel_pin_eld_notify, -- 2.25.1

3 7

[PATCH] ALSA: usb-audio: Split endpoint setups for hw_params and prepare (take#2)
by Takashi Iwai 05 Oct '22

05 Oct '22

This is a second attempt to fix the bug appearing on Android with the recent kernel; the first try was ff878b408a03 and reverted at commit 79764ec772bc. The details taken from the v1 patch: One of the former changes for the endpoint management was the more consistent setup of endpoints at hw_params. snd_usb_endpoint_configure() is a single function that does the full setup, and it's called from both PCM hw_params and prepare callbacks. Although the EP setup at the prepare phase is usually skipped (by checking need_setup flag), it may be still effective in some cases like suspend/resume that requires the interface setup again. As it's a full and single setup, the invocation of snd_usb_endpoint_configure() includes not only the USB interface setup but also the buffer release and allocation. OTOH, doing the buffer release and re-allocation at PCM prepare phase is rather superfluous, and better to be done only in the hw_params phase. For those optimizations, this patch splits the endpoint setup to two phases: snd_usb_endpoint_set_params() and snd_usb_endpoint_prepare(), to be called from hw_params and from prepare, respectively. Note that this patch changes the driver operation slightly, effectively moving the USB interface setup again to PCM prepare stage instead of hw_params stage, while the buffer allocation and such initializations are still done at hw_params stage. And, the change of the USB interface setup timing (moving to prepare) gave an interesting "fix", too: it was reported that the recent kernels caused silent output at the beginning on playbacks on some devices on Android, and this change casually fixed the regression. It seems that those devices are picky about the sample rate change (or the interface change?), and don't follow the too immediate rate changes. Meanwhile, Android operates the PCM in the following order: - open, then hw_params with the possibly highest sample rate - close without prepare - re-open, hw_params with the normal sample rate - prepare, and start streaming This procedure ended up the hw_params twice with different rates, and because the recent kernel did set up the sample rate twice one and after, it screwed up the device. OTOH, the earlier kernels didn't set up the USB interface at hw_params, hence this problem didn't appear. Now, with this patch, the USB interface setup is again back to the prepare phase, and it works around the problem automagically. Although we should address the sample rate problem in a more solid way in future, let's keep things working as before for now. *** What's new in the take#2 patch: - The regression caused by the v1 patch (bko#216500) was due to the missing check of need_setup flag at hw_params. Now the check is added, and the snd_usb_endpoint_set_params() call is skipped when the running EP is re-opened. - There was another bug in v1 where the clock reference rate wasn't updated at hw_params phase, which may lead to a lack of the proper hw constraints when an application doesn't issue the prepare but only the hw_params call. This patch fixes it as well by tracking the clock rate change in the prepare callback with a new flag "need_update" for the clock reference object, just like others. - The configure_endpoints() are simplified and folded back into snd_usb_pcm_prepare(). Fixes: bf6313a0ff76 ("ALSA: usb-audio: Refactor endpoint management") Fixes: ff878b408a03 ("ALSA: usb-audio: Split endpoint setups for hw_params and prepare") Reported-by: chihhao chen <chihhao.chen(a)mediatek.com> Link: https://lore.kernel.org/r/87e6d6ae69d68dc588ac9acc8c0f24d6188375c3.camel@me… Link: https://lore.kernel.org/r/20220901124136.4984-1-tiwai@suse.de Link: https://bugzilla.kernel.org/show_bug.cgi?id=216500 Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/usb/endpoint.c | 76 +++++++++++++++++++++++++++----------------- sound/usb/endpoint.h | 6 ++-- sound/usb/pcm.c | 51 ++++++++++++----------------- 3 files changed, 70 insertions(+), 63 deletions(-) diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index eb71df9da831..0c94ebc98e90 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -40,6 +40,7 @@ struct snd_usb_clock_ref { unsigned char clock; atomic_t locked; int rate; + bool need_setup; struct list_head list; }; @@ -758,7 +759,8 @@ bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip, * The endpoint needs to be closed via snd_usb_endpoint_close() later. * * Note that this function doesn't configure the endpoint. The substream - * needs to set it up later via snd_usb_endpoint_configure(). + * needs to set it up later via snd_usb_endpoint_set_params() and + * snd_usb_endpoint_prepare(). */ struct snd_usb_endpoint * snd_usb_endpoint_open(struct snd_usb_audio *chip, @@ -1289,15 +1291,39 @@ static int sync_ep_set_params(struct snd_usb_endpoint *ep) return -ENOMEM; } +/* update the rate of the referred clock; return the actual rate */ +static int update_clock_ref_rate(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep) +{ + struct snd_usb_clock_ref *clock = ep->clock_ref; + int rate = ep->cur_rate; + + if (!clock || clock->rate == rate) + return rate; + if (clock->rate) { + if (atomic_read(&clock->locked)) + return clock->rate; + if (clock->rate != rate) { + usb_audio_err(chip, "Mismatched sample rate %d vs %d for EP 0x%x\n", + clock->rate, rate, ep->ep_num); + return clock->rate; + } + } + clock->rate = rate; + clock->need_setup = true; + return rate; +} + /* * snd_usb_endpoint_set_params: configure an snd_usb_endpoint * + * It's called either from hw_params callback. * Determine the number of URBs to be used on this endpoint. * An endpoint must be configured before it can be started. * An endpoint that is already running can not be reconfigured. */ -static int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, - struct snd_usb_endpoint *ep) +int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep) { const struct audioformat *fmt = ep->cur_audiofmt; int err; @@ -1349,49 +1375,46 @@ static int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, ep->maxframesize = ep->maxpacksize / ep->cur_frame_bytes; ep->curframesize = ep->curpacksize / ep->cur_frame_bytes; - return 0; + return update_clock_ref_rate(chip, ep); } static int init_sample_rate(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep) { struct snd_usb_clock_ref *clock = ep->clock_ref; - int err; + int rate, err; - if (clock) { - if (atomic_read(&clock->locked)) - return 0; - if (clock->rate == ep->cur_rate) - return 0; - if (clock->rate && clock->rate != ep->cur_rate) { - usb_audio_dbg(chip, "Mismatched sample rate %d vs %d for EP 0x%x\n", - clock->rate, ep->cur_rate, ep->ep_num); - return -EINVAL; - } - } + rate = update_clock_ref_rate(chip, ep); + if (rate < 0) + return rate; + if (clock && !clock->need_setup) + return 0; - err = snd_usb_init_sample_rate(chip, ep->cur_audiofmt, ep->cur_rate); - if (err < 0) + err = snd_usb_init_sample_rate(chip, ep->cur_audiofmt, rate); + if (err < 0) { + if (clock) + clock->rate = 0; /* reset rate */ return err; + } if (clock) - clock->rate = ep->cur_rate; + clock->need_setup = false; return 0; } /* - * snd_usb_endpoint_configure: Configure the endpoint + * snd_usb_endpoint_prepare: Prepare the endpoint * * This function sets up the EP to be fully usable state. - * It's called either from hw_params or prepare callback. + * It's called either from prepare callback. * The function checks need_setup flag, and performs nothing unless needed, * so it's safe to call this multiple times. * * This returns zero if unchanged, 1 if the configuration has changed, * or a negative error code. */ -int snd_usb_endpoint_configure(struct snd_usb_audio *chip, - struct snd_usb_endpoint *ep) +int snd_usb_endpoint_prepare(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep) { bool iface_first; int err = 0; @@ -1412,9 +1435,6 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip, if (err < 0) goto unlock; } - err = snd_usb_endpoint_set_params(chip, ep); - if (err < 0) - goto unlock; goto done; } @@ -1442,10 +1462,6 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip, if (err < 0) goto unlock; - err = snd_usb_endpoint_set_params(chip, ep); - if (err < 0) - goto unlock; - err = snd_usb_select_mode_quirk(chip, ep->cur_audiofmt); if (err < 0) goto unlock; diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 6a9af04cf175..e67ea28faa54 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -17,8 +17,10 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip, bool is_sync_ep); 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_set_params(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep); +int snd_usb_endpoint_prepare(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, diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index d45d1d7e6664..e721fc12acde 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -433,35 +433,6 @@ static void close_endpoints(struct snd_usb_audio *chip, } } -static int configure_endpoints(struct snd_usb_audio *chip, - struct snd_usb_substream *subs) -{ - int err; - - if (subs->data_endpoint->need_setup) { - /* stop any running stream beforehand */ - if (stop_endpoints(subs, false)) - sync_pending_stops(subs); - if (subs->sync_endpoint) { - err = snd_usb_endpoint_configure(chip, subs->sync_endpoint); - if (err < 0) - return err; - } - err = snd_usb_endpoint_configure(chip, subs->data_endpoint); - if (err < 0) - return err; - snd_usb_set_format_quirk(subs, subs->cur_audiofmt); - } else { - if (subs->sync_endpoint) { - err = snd_usb_endpoint_configure(chip, subs->sync_endpoint); - if (err < 0) - return err; - } - } - - return 0; -} - /* * hw_params callback * @@ -551,7 +522,16 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, subs->cur_audiofmt = fmt; mutex_unlock(&chip->mutex); - ret = configure_endpoints(chip, subs); + if (!subs->data_endpoint->need_setup) + goto unlock; + + if (subs->sync_endpoint) { + ret = snd_usb_endpoint_set_params(chip, subs->sync_endpoint); + if (ret < 0) + goto unlock; + } + + ret = snd_usb_endpoint_set_params(chip, subs->data_endpoint); unlock: if (ret < 0) @@ -634,9 +614,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) goto unlock; } - ret = configure_endpoints(chip, subs); + if (subs->sync_endpoint) { + ret = snd_usb_endpoint_prepare(chip, subs->sync_endpoint); + if (ret < 0) + goto unlock; + } + + ret = snd_usb_endpoint_prepare(chip, subs->data_endpoint); if (ret < 0) goto unlock; + else if (ret > 0) + snd_usb_set_format_quirk(subs, subs->cur_audiofmt); + ret = 0; /* reset the pointer */ subs->buffer_bytes = frames_to_bytes(runtime, runtime->buffer_size); -- 2.35.3

2 3

Change to RemainAfterExit=yes in alsa-restore.service.in
by GitHub pull_request - opened 04 Oct '22

04 Oct '22

alsa-project/alsa-utils pull request #171 was opened from stele95: Fresh Manjaro installation. Default volume value was 75%, no matter to what value I change it, it resets to 75% on reboot/shutdown. After changing to `RemainAfterExit=yes` from `RemainAfterExit=true` in `alsa-restore.service`, the volume value is properly saved and restored on reboot/shutdown. I did check the following [link](https://www.freedesktop.org/software/systemd/man/systemd.service.html) and it says: > RemainAfterExit= > Takes a boolean value that specifies whether the service shall be considered active even when all its processes exited. Defaults to no. But if you look at `Example 4. Stoppable oneshot service`, it clearly sets `RemainAfterExit=yes` with `Type=oneshot`. Request URL : https://github.com/alsa-project/alsa-utils/pull/171 Patch URL : https://github.com/alsa-project/alsa-utils/pull/171.patch Repository URL: https://github.com/alsa-project/alsa-utils

1 0

[PATCH] Revert "ASoC: soc-component: using pm_runtime_resume_and_get instead of pm_runtime_get_sync"
by Peter Ujfalusi 04 Oct '22

04 Oct '22

From: Peter Ujfalusi <peter.ujfalusi(a)gmail.com> This reverts commit 08fc2a7448afc1660ec2f1b5c437fcd14155a7ee. The reverted commit causes the following warnigs: Runtime PM usage count underflow! This is due to the fact that the pm_runtime_resume_and_get() is calling pm_runtime_put_noidle() in case of < 0 return value of pm_runtime_get_sync() which includes the -EACCES. The change is wrong as -EACCES is returned in case of 'nested' get_sync() and it is a valid use of PM runtime. Fixes: 08fc2a7448af ("ASoC: soc-component: using pm_runtime_resume_and_get instead of pm_runtime_get_sync") Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)gmail.com> --- sound/soc/soc-component.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index 659b9ade4158..e12f8244242b 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -1213,9 +1213,11 @@ int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd, int i; for_each_rtd_components(rtd, i, component) { - int ret = pm_runtime_resume_and_get(component->dev); - if (ret < 0 && ret != -EACCES) + int ret = pm_runtime_get_sync(component->dev); + if (ret < 0 && ret != -EACCES) { + pm_runtime_put_noidle(component->dev); return soc_component_ret(component, ret); + } /* mark stream if succeeded */ soc_component_mark_push(component, stream, pm); } -- 2.37.3

2 1