svn commit: r308424 - head/sys/arm/broadcom/bcm2835

[Hans Petter Selasky]

On 11/07/16 20:32, Hans Petter Selasky wrote:
> On 11/07/16 20:23, Oleksandr Tymoshenko wrote:
>>
>>> On Nov 7, 2016, at 10:27 AM, Hans Petter Selasky <hps at selasky.org>
>>> wrote:
>>>
>>> On 11/07/16 18:38, Oleksandr Tymoshenko wrote:
>>>> +        bcm2835_audio_unlock(sc);
>>>> +        cv_signal(&sc->worker_cv);
>>>
>>>
>>> Shouldn't cv_signal() be done locked, so that you don't loose any
>>> transactions? CV's only wakeup the treads that are sleeping right
>>> there and then.
>>
>> Hi Hans,
>>
>> In this case it doesn’t matter. bcm2835_audio_xxx lock functions are
>> used to keep channel state consistent. The actual audio hw
>> reprogramming happens in worker thread which only picks up latest
>> state of the virtual channel, there is no need to run every
>> transaction in sequence.
>>
>
> Hi,
>
> It is not about running in sequence, but that if the worker thread is
> not sleeping, but on the way to sleep, it will never get woken up unless
> you use proper locks here!
>
> --HPS

Hi,

Also the teardown sequence for the worker thread looks a bit broken, 
that it doesn't wait for the thread to exit.

I've made a patch, attached which I think is the right way to do it.

Try opening and closing /dev/dsp in a loop with some DSP ioctls and see 
what happens.

--HPS
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bcm2835_audio.diff
Type: text/x-patch
Size: 2431 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20161107/f64aa18a/attachment.bin>