Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934196AbcK3Kp6 (ORCPT ); Wed, 30 Nov 2016 05:45:58 -0500 Received: from smtp-proxy001.phy.lolipop.jp ([157.7.104.42]:38539 "EHLO smtp-proxy001.phy.lolipop.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932789AbcK3Kpu (ORCPT ); Wed, 30 Nov 2016 05:45:50 -0500 Subject: Re: [PATCH 2/3 v2] ALSA: usb-audio: avoid setting of sample rate multiple times on bus To: Jiada Wang , perex@perex.cz, tiwai@suse.com References: <20161130075923.15205-1-jiada_wang@mentor.com> <20161130075923.15205-3-jiada_wang@mentor.com> Cc: alsa-devel@alsa-project.org, apape@de.adit-jv.com, linux-kernel@vger.kernel.org, Mark_Craske@mentor.com From: Takashi Sakamoto Message-ID: Date: Wed, 30 Nov 2016 19:45:45 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161130075923.15205-3-jiada_wang@mentor.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2717 Lines: 77 Hi Jiada, I don't oppose this patch. Nevertheless, your description is not necessarily correct. On Nov 30 2016 16:59, Jiada Wang wrote: > From: Daniel Girnus > > ALSA usually calls the prepare function twice before starting the playback: > 1. On hw_params call from userland and > 2. internally when starting the stream. ALSA PCM core in kernel land doesn't perform like this. In alsa-lib, 'snd_pcm_hw_params()' calls 'snd_pcm_hw_params_internal()' and 'snd_pcm_prepare()' sequentially. http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm.c;h=cd87bc759ded95953e332b7e8d56b0f2d5b4185d;hb=HEAD#l853 In system call level (e.g. see by strace(1)), this looks like two ioctl(2)s with 'SNDRV_PCM_IOCTL_HW_PARAMS' and 'SNDRV_PCM_IOCTL_PREPARE'. Well, when applications are written to execute 'snd_pcm_hw_params()' and 'snd_pcm_hw_prepare()' sequentially, additional ioctl(2) with 'SNDRV_PCM_IOCTL_PREPARE' appears. PulseAudio is this kind of application. I indicated the useless in 2014, but it still remains: https://lists.freedesktop.org/archives/pulseaudio-discuss/2014-January/019773.html You have the misunderstanding due to a nature of alsa-lib and tendency of major applications, from my point of view. > Some device are not able to manage this and they will stop playback > if the sample rate will be configured several times over USB protocol. > > Signed-off-by: Jens Lorenz > Signed-off-by: Jiada Wang > --- > sound/usb/pcm.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c > index 44d178e..a522c9a 100644 > --- a/sound/usb/pcm.c > +++ b/sound/usb/pcm.c > @@ -806,17 +806,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) > if (ret < 0) > goto unlock; > > - iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface); > - alts = &iface->altsetting[subs->cur_audiofmt->altset_idx]; > - ret = snd_usb_init_sample_rate(subs->stream->chip, > - subs->cur_audiofmt->iface, > - alts, > - subs->cur_audiofmt, > - subs->cur_rate); > - if (ret < 0) > - goto unlock; > - > if (subs->need_setup_ep) { > + > + iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface); > + alts = &iface->altsetting[subs->cur_audiofmt->altset_idx]; > + ret = snd_usb_init_sample_rate(subs->stream->chip, > + subs->cur_audiofmt->iface, > + alts, > + subs->cur_audiofmt, > + subs->cur_rate); > + if (ret < 0) > + goto unlock; > + > ret = configure_endpoint(subs); > if (ret < 0) > goto unlock; Regards Takashi Sakamoto