From patchwork Wed Sep 12 15:04:50 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Sakamoto X-Patchwork-Id: 10597693 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 0B80414E5 for ; Wed, 12 Sep 2018 15:05:32 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EE1A62A32A for ; Wed, 12 Sep 2018 15:05:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E264A2A344; Wed, 12 Sep 2018 15:05:31 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1F87D2A32D for ; Wed, 12 Sep 2018 15:05:31 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id 608C026782E; Wed, 12 Sep 2018 17:05:14 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 6E92B26779A; Wed, 12 Sep 2018 17:05:11 +0200 (CEST) Received: from mail-pl1-f196.google.com (mail-pl1-f196.google.com [209.85.214.196]) by alsa0.perex.cz (Postfix) with ESMTP id CE0932673F4 for ; Wed, 12 Sep 2018 17:05:08 +0200 (CEST) Received: by mail-pl1-f196.google.com with SMTP id j8-v6so1101543pll.12 for ; Wed, 12 Sep 2018 08:05:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=YYUPRMY8fx2KrfwhW9s0uHm7sDq/QPauB9P7wGKlris=; b=P5jjbaM6B9j/j7F8Ft0ziRyGY+8NVuFVocfzOYwkhyPgK54GcV5C+O3k8dkbbS3Y9I t3wHC+8D/azzP8+pFP8PXpwlcF6mgAyxUv5TGR5cj0qaMncUjvllcrSnD3dv614HskDK DGbYMOUy4ZSI7rInbOV2vhenbuKU7GfRoE434cjkuFAKbH379QxLGLOwUETODVwmxidA Ai67FarFE+BsCX92lU+pdlSI7sQW2wOYqU6VYQOv0xXDEvwi5gnUYkPNi590eEb8XPbB 6y6A3nyGdyfpyZ8O2VaCowHoZTicIGZnru6k6qCXq37nVvzlTnTTp1X0IBbXROTZz/RX rBXQ== X-Gm-Message-State: APzg51CGAbKtG5JxJYNizivmC/Iua3bJp8yfR/+9qw9hcPyL38ogasSn BtL9aLOUmHcEyVv2M+RZT0o= X-Google-Smtp-Source: ANB0VdajF42dGhRlEIS2DB6sQNuABpi1mGKw73z7xSHhWuoXSJBZ1ic1djWdM0N1FfrJ5psFUcDhIQ== X-Received: by 2002:a17:902:bf0b:: with SMTP id bi11-v6mr2836558plb.76.1536764707624; Wed, 12 Sep 2018 08:05:07 -0700 (PDT) Received: from localhost.localdomain ([2405:6580:9660:3200:cd2c:62df:f21:c526]) by smtp.gmail.com with ESMTPSA id l85-v6sm3643196pfk.34.2018.09.12.08.05.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 12 Sep 2018 08:05:06 -0700 (PDT) From: Takashi Sakamoto To: clemens@ladisch.de, tiwai@suse.de Date: Thu, 13 Sep 2018 00:04:50 +0900 Message-Id: <20180912150450.16334-3-o-takashi@sakamocchi.jp> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180912150450.16334-1-o-takashi@sakamocchi.jp> References: <20180912150450.16334-1-o-takashi@sakamocchi.jp> Cc: alsa-devel@alsa-project.org, ffado-devel@lists.sourceforge.net Subject: [alsa-devel] [PATCH 2/2][RFC] ALSA: firewire-lib: cancel processing packets in struct snd_pcm_ops.pointer() call X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP In current implementation of IEC 61883-1/6 engine in ALSA firewire stack, there's two chances to handle isochronous packets in process contexts. One is a callback of struct snd_pcm_ops.pointer() and another is the one of .ack(). When alsa-lib applications queue/dequeue PCM frames according to mmap scenario, the isochronous packets are handled in below call graph. while () ->poll() ->snd_pcm_hwsync() ->ioctl(HWSYNC) ->struct snd_pcm_ops.pointer() = amdtp_stream_pcm_pointer() ->fw_iso_context_flush_completions() ->snd_pcm_mmap_begin() (queue/dequeue PCM frames here.) ->snd_pcm_mmap_commit() ->ioctl(SYNC_PTR) ->struct snd_pcm_ops.ack() = amdtp_stream_pcm_ack() ->fw_iso_context_flush_completions() In an above iteration, two calls of fw_iso_context_flush_completions() are executed with quite small interval, thus few packets can be handled in a call of .ack(). This is inefficient. Although .ack() ensures to be called in process contexts, .pointer() is called in interrupt contexts as well as process context. Especially, this engine is designed to call snd_pcm_period_elapsed() in software IRQ context of tasklet, scheduled in both of process context and interrupt context of 1394 OHCI IR/IT context. To reduce closest two call of .pointer(), a condition to judge running context is used in .pointer. An idea to flush packets in a callback of .pointer() requires the above extra care. On the other hand, a callback of .ack() has no care like that, except for being called within acquired lock. Therefore it's reasonable to cancel .pointer() side. This commit cancels flushing packets in a callback of .pointer(). The callback of .pointer just returns the number of PCM frames in intermediate PCM buffer or SNDRV_PCM_POS_XRUN. As a result: while () ->poll() ->snd_pcm_hwsync() ->ioctl(HWSYNC) ->struct snd_pcm_ops.pointer() = amdtp_stream_pcm_pointer() ->snd_pcm_mmap_begin() (queue/dequeue PCM frames here.) ->snd_pcm_mmap_commit() ->ioctl(SYNC_PTR) ->struct snd_pcm_ops.ack() = amdtp_stream_pcm_ack() ->fw_iso_context_flush_completions() Signed-off-by: Takashi Sakamoto --- sound/firewire/amdtp-stream.c | 31 ------------------------------- sound/firewire/amdtp-stream.h | 12 +++++++++++- 2 files changed, 11 insertions(+), 32 deletions(-) diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 3520dc2bec61..48f17264fc35 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -922,37 +922,6 @@ int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed) } EXPORT_SYMBOL(amdtp_stream_start); -/** - * amdtp_stream_pcm_pointer - get the PCM buffer position - * @s: the AMDTP stream that transports the PCM data - * - * Returns the current buffer position, in frames. - */ -unsigned long amdtp_stream_pcm_pointer(struct amdtp_stream *s) -{ - /* - * This function is called in software IRQ context of period_tasklet or - * process context. - * - * When the software IRQ context was scheduled by software IRQ context - * of IR/IT contexts, queued packets were already handled. Therefore, - * no need to flush the queue in buffer anymore. - * - * When the process context reach here, some packets will be already - * queued in the buffer. These packets should be handled immediately - * to keep better granularity of PCM pointer. - * - * Later, the process context will sometimes schedules software IRQ - * context of the period_tasklet. Then, no need to flush the queue by - * the same reason as described for IR/IT contexts. - */ - if (!in_interrupt() && amdtp_stream_running(s)) - fw_iso_context_flush_completions(s->context); - - return READ_ONCE(s->pcm_buffer_pointer); -} -EXPORT_SYMBOL(amdtp_stream_pcm_pointer); - /** * amdtp_stream_pcm_ack - acknowledge queued PCM frames * @s: the AMDTP stream that transfers the PCM frames diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index e45de3eecfe3..8a96419c462b 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -168,7 +168,6 @@ int amdtp_stream_add_pcm_hw_constraints(struct amdtp_stream *s, struct snd_pcm_runtime *runtime); void amdtp_stream_pcm_prepare(struct amdtp_stream *s); -unsigned long amdtp_stream_pcm_pointer(struct amdtp_stream *s); int amdtp_stream_pcm_ack(struct amdtp_stream *s); void amdtp_stream_pcm_abort(struct amdtp_stream *s); @@ -229,6 +228,17 @@ static inline bool cip_sfc_is_base_44100(enum cip_sfc sfc) return sfc & 1; } +/** + * amdtp_stream_pcm_pointer - get the PCM buffer position + * @s: the AMDTP stream that transports the PCM data + * + * Returns the current buffer position, in frames. + */ +static inline unsigned long amdtp_stream_pcm_pointer(struct amdtp_stream *s) +{ + return READ_ONCE(s->pcm_buffer_pointer); +} + /** * amdtp_stream_wait_callback - sleep till callbacked or timeout * @s: the AMDTP stream