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 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 0C16EC4360F for ; Fri, 5 Apr 2019 15:21:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DAD242186A for ; Fri, 5 Apr 2019 15:21:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731498AbfDEPVT (ORCPT ); Fri, 5 Apr 2019 11:21:19 -0400 Received: from mx2.suse.de ([195.135.220.15]:36080 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726291AbfDEPVT (ORCPT ); Fri, 5 Apr 2019 11:21:19 -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 3C228AD9C; Fri, 5 Apr 2019 15:21:17 +0000 (UTC) Date: Fri, 05 Apr 2019 17:21:15 +0200 Message-ID: From: Takashi Iwai To: Timo Wischer Cc: , , , Subject: Re: [PATCH 09/10] ALSA: pcm: Add snd_pcm_ops for snd_pcm_link() In-Reply-To: <00d97a34-c055-c7ef-dbfd-e17c0f6cfff3@de.adit-jv.com> References: <1553529644-5654-1-git-send-email-twischer@de.adit-jv.com> <1553586574-18608-1-git-send-email-twischer@de.adit-jv.com> <1553586574-18608-5-git-send-email-twischer@de.adit-jv.com> <3ad5c313-ba2c-b541-8930-3f2515dfca72@de.adit-jv.com> <68cfbf7a-5f1e-50d2-2b97-4d3c5d360b36@de.adit-jv.com> <00f4c8db-44cf-b555-f3ca-42916b1e7ceb@de.adit-jv.com> <00d97a34-c055-c7ef-dbfd-e17c0f6cfff3@de.adit-jv.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 List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 04 Apr 2019 12:18:33 +0200, Timo Wischer wrote: > > >>> In principle, the PCM ops is supposed to be safe for operating a > >>> certain stuff. If a state change may happen during the operation, > >>> this should be called inside PCM stream lock. So the design of the > >>> new callback itself is fragile. > >>> > >>> then, it comes to a point to re-setup the timer at the link change. > >>> The idea is interesting, but it's a wrong usage of PCM link feature, > >>> to be honest. > >>> > >>> That said, I find the changes up to patch 8 are acceptable (with some > >>> fixes expected), but the link feature isn't. > > Hi Takashi, > > could you provide your feedback to the other patches? The patch 4 must be superfluous, at least. The trigger callback is called within the stream lock, so you really don't need to save flags. The patch 8 needs more consideration. I'm not fond of the current way to select the timer source. For example, the ALSA timer source isn't necessarily bound with cards. It can be a hrtimer, or system timer backend ALSA global timer, too. This can't be chosen in the suggested way. Also, the use of tasklet is superfluous. The timer instance can be called via tasklet by itself if you pass the flag properly. > I would like to apply these 8 patches first and then I would provide > an addition patch to get an ALSA control to select the sound timer. > I fear this would be the best idea (replacing the snd_pcm_link() > feature) because the ALSA control is accessible by any audio user and > we could use ALSA hooks to select the right sound timer when opening > an ALSA device. It depends on the implementation. Usually you "lock" such a control that is bound with a PCM stream at opening, so that other process won't be able to change. Such a trick is used for the dynamic routing via control element for emu10k1, for example. But, whether it's really usable, I don't know. Let's see. thanks, Takashi From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 09/10] ALSA: pcm: Add snd_pcm_ops for snd_pcm_link() Date: Fri, 05 Apr 2019 17:21:15 +0200 Message-ID: References: <1553529644-5654-1-git-send-email-twischer@de.adit-jv.com> <1553586574-18608-1-git-send-email-twischer@de.adit-jv.com> <1553586574-18608-5-git-send-email-twischer@de.adit-jv.com> <3ad5c313-ba2c-b541-8930-3f2515dfca72@de.adit-jv.com> <68cfbf7a-5f1e-50d2-2b97-4d3c5d360b36@de.adit-jv.com> <00f4c8db-44cf-b555-f3ca-42916b1e7ceb@de.adit-jv.com> <00d97a34-c055-c7ef-dbfd-e17c0f6cfff3@de.adit-jv.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Return-path: In-Reply-To: <00d97a34-c055-c7ef-dbfd-e17c0f6cfff3@de.adit-jv.com> Sender: linux-kernel-owner@vger.kernel.org To: Timo Wischer Cc: broonie@kernel.org, perex@perex.cz, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org List-Id: alsa-devel@alsa-project.org On Thu, 04 Apr 2019 12:18:33 +0200, Timo Wischer wrote: > > >>> In principle, the PCM ops is supposed to be safe for operating a > >>> certain stuff. If a state change may happen during the operation, > >>> this should be called inside PCM stream lock. So the design of the > >>> new callback itself is fragile. > >>> > >>> then, it comes to a point to re-setup the timer at the link change. > >>> The idea is interesting, but it's a wrong usage of PCM link feature, > >>> to be honest. > >>> > >>> That said, I find the changes up to patch 8 are acceptable (with some > >>> fixes expected), but the link feature isn't. > > Hi Takashi, > > could you provide your feedback to the other patches? The patch 4 must be superfluous, at least. The trigger callback is called within the stream lock, so you really don't need to save flags. The patch 8 needs more consideration. I'm not fond of the current way to select the timer source. For example, the ALSA timer source isn't necessarily bound with cards. It can be a hrtimer, or system timer backend ALSA global timer, too. This can't be chosen in the suggested way. Also, the use of tasklet is superfluous. The timer instance can be called via tasklet by itself if you pass the flag properly. > I would like to apply these 8 patches first and then I would provide > an addition patch to get an ALSA control to select the sound timer. > I fear this would be the best idea (replacing the snd_pcm_link() > feature) because the ALSA control is accessible by any audio user and > we could use ALSA hooks to select the right sound timer when opening > an ALSA device. It depends on the implementation. Usually you "lock" such a control that is bound with a PCM stream at opening, so that other process won't be able to change. Such a trick is used for the dynamic routing via control element for emu10k1, for example. But, whether it's really usable, I don't know. Let's see. thanks, Takashi