Date: Mon, 7 Nov 2016 12:30:35 -0800 From: Oleksandr Tymoshenko <gonzo@freebsd.org> To: Hans Petter Selasky <hps@selasky.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r308424 - head/sys/arm/broadcom/bcm2835 Message-ID: <D127D87C-EC4F-4211-B8C0-6BD431756DF0@freebsd.org> In-Reply-To: <2b03afc6-9bfa-1124-7b70-2532751bfed9@selasky.org> References: <201611071738.uA7HceYu045944@repo.freebsd.org> <f8169b11-56d8-4566-f9e9-e387dd9b939e@selasky.org> <680D84F2-65BF-48DD-8D11-311B1F65A634@freebsd.org> <c7cd871d-9007-6de4-7063-2680e259713f@selasky.org> <2b03afc6-9bfa-1124-7b70-2532751bfed9@selasky.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Nov 7, 2016, at 11:51 AM, Hans Petter Selasky <hps@selasky.org> wrote: > > 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@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. Thanks for patch Hans. Looks good to me. I’ll test and commit it.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D127D87C-EC4F-4211-B8C0-6BD431756DF0>