Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 15 May 2020 09:21:10 +0200
Message-ID: <s5htv0hoe8p.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Brent Lu <brent.lu@intel.com>
Cc:     <alsa-devel@alsa-project.org>, Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>,
        Baolin Wang <baolin.wang@linaro.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Richard Fontana <rfontana@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        paulhsia <paulhsia@chromium.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ALSA: pcm: fix incorrect hw_base increase
In-Reply-To: <1589515779-20987-1-git-send-email-brent.lu@intel.com>
References: <1589515779-20987-1-git-send-email-brent.lu@intel.com>
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

On Fri, 15 May 2020 06:09:39 +0200,
Brent Lu wrote:
> 
> The hw_base will be increased by runtime->buffer_size frames
> unconditionally if the runtime->status->hw_ptr is not updated for over
> half of buffer time. As the hw_base increases, so does the
> runtime->status->hw_ptr which could lead to invalid return value when
> user space program calls snd_pcm_avail() function.
> 
> By updating runtime->hw_ptr_jiffies each time the HWSYNC is called,
> the hw_base will keep the same when buffer stall happens, so does the
> hw_ptr.

Updating hw_ptr_jiffies at that code path looks correct, but it still
leaves the question why this condition happens.  It means that the
actual hwptr isn't changed and yet only jiffies increase
significantly; it means that the hardware can't report proper pointer,
and it should have set SNDRV_PCM_INFO_BATCH flag, then the jiffies
check is skipped.

With which hardware and under which situation did it happen (and the
patch fixed)?


thanks,

Takashi

> 
> Signed-off-by: Brent Lu <brent.lu@intel.com>
> ---
>  sound/core/pcm_lib.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
> index 872a852..d531e1b 100644
> --- a/sound/core/pcm_lib.c
> +++ b/sound/core/pcm_lib.c
> @@ -433,6 +433,7 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
>  
>   no_delta_check:
>  	if (runtime->status->hw_ptr == new_hw_ptr) {
> +		runtime->hw_ptr_jiffies = curr_jiffies;
>  		update_audio_tstamp(substream, &curr_tstamp, &audio_tstamp);
>  		return 0;
>  	}
> -- 
> 2.7.4
>