Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 05 Apr 2019 17:21:15 +0200
Message-ID: <s5h8swoxytw.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Timo Wischer <twischer@de.adit-jv.com>
Cc:     <broonie@kernel.org>, <perex@perex.cz>,
        <alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>
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>
        <s5ho95ym3s7.wl-tiwai@suse.de>
        <3ad5c313-ba2c-b541-8930-3f2515dfca72@de.adit-jv.com>
        <s5hr2at67ew.wl-tiwai@suse.de>
        <68cfbf7a-5f1e-50d2-2b97-4d3c5d360b36@de.adit-jv.com>
        <s5ha7hh62yg.wl-tiwai@suse.de>
        <c99f074c-99e2-7213-6738-67d08d44d030@de.adit-jv.com>
        <s5hef6slm00.wl-tiwai@suse.de>
        <00f4c8db-44cf-b555-f3ca-42916b1e7ceb@de.adit-jv.com>
        <s5h7ecklkr2.wl-tiwai@suse.de>
        <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

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