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=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED 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 32890FD21E1 for ; Mon, 30 Jul 2018 15:32:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E991A20894 for ; Mon, 30 Jul 2018 15:32:27 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E991A20894 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727667AbeG3RHz (ORCPT ); Mon, 30 Jul 2018 13:07:55 -0400 Received: from mx2.suse.de ([195.135.220.15]:49070 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726623AbeG3RHz (ORCPT ); Mon, 30 Jul 2018 13:07:55 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 12808ADE7; Mon, 30 Jul 2018 15:32:23 +0000 (UTC) Date: Mon, 30 Jul 2018 17:32:21 +0200 Message-ID: From: Takashi Iwai To: "Pierre-Louis Bossart" Cc: "Akshu Agrawal" , "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." , , , "Liam Girdwood" , "Mark Brown" , "open list" Subject: Re: [alsa-devel] [PATCH] ASoC: soc-pcm: Use delay set in pointer function In-Reply-To: References: <1532686422-1790-1-git-send-email-akshu.agrawal@amd.com> <66c8b8c4-bdd0-0129-5e5b-850890cfdb8d@linux.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/26 (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 Mon, 30 Jul 2018 17:15:44 +0200, Pierre-Louis Bossart wrote: > > On 7/27/18 11:28 PM, Agrawal, Akshu wrote: > > > > > > On 7/27/2018 8:39 PM, Pierre-Louis Bossart wrote: > >> On 7/27/18 5:13 AM, Akshu Agrawal wrote: > >>> There are cases where a pointer function populates > >>> runtime->delay, such as: > >>> ./sound/pci/hda/hda_controller.c > >>> ./sound/soc/intel/atom/sst-mfld-platform-pcm.c > >>> > >>> Also, in some cases cpu dai used is generic and the pcm > >>> driver needs to set delay. > >>> > >>> This delay was getting lost and was overwritten by delays > >>> from codec or cpu dai delay function if exposed. > >> > >> Humm, yes the runtime->delay set in the .pointer function would be lost > >> without this change, but the delay would still be provided in the > >> followup call to .delay. > >> With your change, the same delay will be accounted for twice? > >> > > > > It will not be accounted twice because no driver which is setting > > runtime->delay is defining .delay op for cpu_dai. Vice versa is also > > true, the drivers which define .delay for cpu_dai don't set > > runtime->delay. And I think this is expected from drivers else it would > > be a bug from their side. > > what do you mean my 'no driver'? Can you clarify if this is based on > analysis of the code or by-design. I don't recall having seen any > guidelines on this topic, and it's quite likely that different people > have different interpretation on how delay is supposed to be reported. Currently the problem seems to be the ambiguity of delay callback. Through a quick glance, Akshu's patch looks correct to me. The delay value that was calculated in some drivers aren't taken properly because the current soc_pcm_pointer() presumes that the delay calculation is provided *only* by delay callback. The two drivers suggested in the patch set runtime->delay in its pointer callback, and these values are gone. That said, if delay callback of CPU dai provides the additional delay, the patch does correct thing. OTOH, if CPU dai provides the base delay instead, we need to clarify that it's rather a must; the delay calculation in pointer callback becomes bogus in this scenario. thanks, Takashi > > > > > .delay for codec_dai anyway is different and has to be accounted for. > > > > Thanks, > > Akshu > >>> > >>> Signed-off-by: Akshu Agrawal > >>> --- > >>> sound/soc/soc-pcm.c | 5 ++++- > >>> 1 file changed, 4 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c > >>> index 98be04b..b1a2bc2 100644 > >>> --- a/sound/soc/soc-pcm.c > >>> +++ b/sound/soc/soc-pcm.c > >>> @@ -1179,6 +1179,9 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream) > >>> snd_pcm_sframes_t codec_delay = 0; > >>> int i; > >>> + /* clearing the previous delay */ > >>> + runtime->delay = 0; > >>> + > >>> for_each_rtdcom(rtd, rtdcom) { > >>> component = rtdcom->component; > >>> @@ -1203,7 +1206,7 @@ static snd_pcm_uframes_t > >>> soc_pcm_pointer(struct snd_pcm_substream *substream) > >>> } > >>> delay += codec_delay; > >>> - runtime->delay = delay; > >>> + runtime->delay += delay; > >>> return offset; > >>> } > >>> > >> > > _______________________________________________ > > Alsa-devel mailing list > > Alsa-devel@alsa-project.org > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > > > >