Re: [PATCH] ALSA: sb: remove needless evaluation in implementation for copy callback

[Takashi Iwai <tiwai@suse.de>]

On Wed, 24 May 2017 18:28:50 +0200,
Takashi Iwai wrote:
> 
> 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 <o-takashi@sakamocchi.jp>
> > >
> > > 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.

Despite the situation, I decided to take these two patches.  Also, the
API correction isn't needed -- the behavior I described was about the
pre-historic one.


thanks,

Takashi