Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

[Hans Verkuil <hverkuil@xs4all.nl>]

On 10/16/2014 04:48 PM, Takashi Iwai wrote:
> At Thu, 16 Oct 2014 08:39:14 -0600,
> Shuah Khan wrote:
>>
>> On 10/16/2014 08:16 AM, Takashi Iwai wrote:
>>> At Thu, 16 Oct 2014 08:10:52 -0600,
>>> Shuah Khan wrote:
>>>>
>>>> On 10/16/2014 08:01 AM, Takashi Iwai wrote:
>>>>> At Thu, 16 Oct 2014 07:10:37 -0600,
>>>>> Shuah Khan wrote:
>>>>>>
>>>>>> On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
>>>>>>> On 10/14/2014 04:58 PM, Shuah Khan wrote:
>>>>>>> [...]
>>>>>>>>        switch (cmd) {
>>>>>>>>        case SNDRV_PCM_TRIGGER_START:
>>>>>>>> +        err = media_get_audio_tkn(&subs->dev->dev);
>>>>>>>> +        if (err == -EBUSY) {
>>>>>>>> +            dev_info(&subs->dev->dev, "%s device is busy\n",
>>>>>>>> +                    __func__);
>>>>>>>
>>>>>>> In my opinion this should not dev_info() as this is out of band error
>>>>>>> signaling and also as the potential to spam the log. The userspace
>>>>>>> application is already properly notified by the return code.
>>>>>>>
>>>>>>
>>>>>> Yes it has the potential to flood the dmesg especially with alsa,
>>>>>> I will remove the dev_info().
>>>>>
>>>>> Yes.  And, I think doing this in the trigger isn't the best.
>>>>> Why not in open & close?
>>>>
>>>> My first cut of this change was in open and close. I saw pulseaudio
>>>> application go into this loop trying to open the device. To avoid
>>>> such problems, I went with trigger stat and stop. That made all the
>>>> pulseaudio continues attempts to open problems go away.
>>>
>>> But now starting the stream gives the error, and PA would loop it
>>> again, no?
>>>
>>>
>>>>> Also, is this token restriction needed only for PCM?  No mixer or
>>>>> other controls?
>>>>
>>>> snd_pcm_ops are the only ones media drivers implement and use. So
>>>> I don't think mixer is needed. If it is needed, it is not to hard
>>>> to add for that case.
>>>
>>> Well, then I wonder what resource does actually conflict with
>>> usb-audio and media drivers at all...?
>>>
>>
>> audio for dvb/v4l apps gets disrupted when audio app starts. For
>> example, dvb or v4l app tuned to a channel, and when an audio app
>> starts. audio path needs protected to avoid conflicts between
>> digital and analog applications as well.
>
> OK, then concentrating on only PCM is fine.
>
> But, I'm still not convinced about doing the token management in the
> trigger.  The reason -EBUSY doesn't work is that it's the very same
> error code when a PCM device is blocked by other processes.  And
> -EAGAIN is interpreted by PCM core to -EBUSY for historical reasons.
>
> How applications (e.g. PA) should behave if the token get fails?
> Shouldn't it retry or totally give up?

Quite often media apps open the alsa device at the start and then switch
between TV, radio or DVB mode. If the alsa device would claim the tuner
just by being opened (as opposed to actually using the tuner, which happens
when you start streaming), then that would make it impossible for the
application to switch tuner mode. In general you want to avoid that open()
will start configuring hardware since that can quite often be slow. Tuner
configuration in particular can be slow since several common tuners need
to load firmware over i2c. You only want to do that when it is really needed,
and not when some application (udev!) opens the device just to examine what
sort of device it is.

So claiming the tuner in the trigger seems to be the right place. If
returning EBUSY is a poor error code for alsa, then we can use something else
for that. EACCES perhaps?

Regards,

	Hans