From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 89F30C4CECC for ; Sun, 15 Sep 2019 15:51:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 65CBB21479 for ; Sun, 15 Sep 2019 15:51:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731820AbfIOPv3 (ORCPT ); Sun, 15 Sep 2019 11:51:29 -0400 Received: from mx2.suse.de ([195.135.220.15]:53328 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1731635AbfIOPv3 (ORCPT ); Sun, 15 Sep 2019 11:51:29 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id CE738AF10; Sun, 15 Sep 2019 15:51:26 +0000 (UTC) Date: Sun, 15 Sep 2019 17:51:26 +0200 Message-ID: From: Takashi Iwai To: Stefan Wahren Cc: Eric Anholt , Greg Kroah-Hartman , linux-rpi-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging: bcm2835-audio: Fix draining behavior regression In-Reply-To: <3b41702b-1cc2-233d-94a6-ece8f5a19957@gmx.net> References: <20190914152405.7416-1-tiwai@suse.de> <3b41702b-1cc2-233d-94a6-ece8f5a19957@gmx.net> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 15 Sep 2019 15:54:16 +0200, Stefan Wahren wrote: > > Hi Takashi, > > Am 14.09.19 um 17:24 schrieb Takashi Iwai: > > The PCM draining behavior got broken since the recent refactoring, and > > this turned out to be the incorrect expectation of the firmware > > behavior regarding "draining". While I expected the "drain" flag at > > the stop operation would do processing the queued samples, it seems > > rather dropping the samples. > > > > As a quick fix, just drop the SNDRV_PCM_INFO_DRAIN_TRIGGER flag, so > > that the driver uses the normal PCM draining procedure. Also, put > > some caution comment to the function for future readers not to fall > > into the same pitfall. > > > > Fixes: d7ca3a71545b ("staging: bcm2835-audio: Operate non-atomic PCM ops") > > BugLink: https://github.com/raspberrypi/linux/issues/2983 > > thanks for taking care of this. Wouldn't it be better to add the link to > the new comment to provide more context of the unexpected behavior? Yeah, a bit more explanation would be better. Unfortunately I've been traveling in the last week (and will be traveling again in the next weeks), so had little time for caring and testing with any actual hardware... > > Nevertheless: > > Acked-by: Stefan Wahren Thanks! Takashi > > Cc: stable@vger.kernel.org > > Signed-off-by: Takashi Iwai > > --- > > drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c | 4 ++-- > > drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 1 + > > 2 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c > > index bc1eaa3a0773..826016c3431a 100644 > > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c > > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c > > @@ -12,7 +12,7 @@ > > static const struct snd_pcm_hardware snd_bcm2835_playback_hw = { > > .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER | > > SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | > > - SNDRV_PCM_INFO_DRAIN_TRIGGER | SNDRV_PCM_INFO_SYNC_APPLPTR), > > + SNDRV_PCM_INFO_SYNC_APPLPTR), > > .formats = SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S16_LE, > > .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_8000_48000, > > .rate_min = 8000, > > @@ -29,7 +29,7 @@ static const struct snd_pcm_hardware snd_bcm2835_playback_hw = { > > static const struct snd_pcm_hardware snd_bcm2835_playback_spdif_hw = { > > .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER | > > SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | > > - SNDRV_PCM_INFO_DRAIN_TRIGGER | SNDRV_PCM_INFO_SYNC_APPLPTR), > > + SNDRV_PCM_INFO_SYNC_APPLPTR), > > .formats = SNDRV_PCM_FMTBIT_S16_LE, > > .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_44100 | > > SNDRV_PCM_RATE_48000, > > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c > > index 23fba01107b9..c6f9cf1913d2 100644 > > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c > > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c > > @@ -289,6 +289,7 @@ int bcm2835_audio_stop(struct bcm2835_alsa_stream *alsa_stream) > > VC_AUDIO_MSG_TYPE_STOP, false); > > } > > > > +/* FIXME: this doesn't seem working as expected for "draining" */ > > int bcm2835_audio_drain(struct bcm2835_alsa_stream *alsa_stream) > > { > > struct vc_audio_msg m = { >