From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH] ALSA: sb: remove needless evaluation in implementation for copy callback Date: Wed, 24 May 2017 18:28:50 +0200 Message-ID: References: <20170523134042.1056-1-o-takashi@sakamocchi.jp> <12415b17-21dc-1b23-85ee-ead930607df4@sakamocchi.jp> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 2097926680F for ; Wed, 24 May 2017 18:28:51 +0200 (CEST) In-Reply-To: <12415b17-21dc-1b23-85ee-ead930607df4@sakamocchi.jp> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Sakamoto Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org On Wed, 24 May 2017 17:01:07 +0200, Takashi Sakamoto wrote: > > Hi, > > On May 23 2017 22:49, Takashi Iwai wrote: > > On Tue, 23 May 2017 15:40:42 +0200, > > Takashi Sakamoto wrote: > >> > >> In design of ALSA pcm core, 'struct snd_pcm_ops.copy' is expected to > >> copy PCM frames, according to frame alignment on intermediate buffer for > >> userspace and dedicated buffer for data transmission. In this callback, > >> value of 'channel' argument depends on the frame alignment, which drivers > >> registers to runtime of PCM substream. When target devices can handle > >> non-interleaved buffer, this value has positive value, otherwise negative. > >> > >> ALSA driver for PCM component of EMU8000 chip is programmed with local > >> macro to switch the frame alignment. The 'copy' operation in > >> non-interleaved side has evaluation of the 'channel' argument (actually > >> it's 'voice' argument). This is useless. > >> > >> This commit remove the evaluation. > >> > >> Signed-off-by: Takashi Sakamoto > > > > Passing -1 to the channel was valid even for non-interleaved access. > > It was meant to apply to all channels. > > > > The call with channel = -1 itself might have been dropped meanwhile, > > so removing it may be OK now. But the description is confusing as if > > it were incorrectly implemented. > > https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/tree/Documentation/sound/kernel-api/writing-an-alsa-driver.rst#n3598 > > > You need to check the channel argument, and if it's -1, copy the > > whole channels. Otherwise, you have to copy only the specified > > channel. > > Hm. We need to correct this API documentation, because there's no > codes to pass negative value to the argument for vector buffer > operation. I'll post my proposal later. Just leave them as is. We're working on these codes in anyway, and touching them just confuse the development. If it were a fix, I'd take it quickly. But it's nothing but a very minor optimization, so it's really not worth to spend your time. thanks, Takashi