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: >=20 > On 11/07/16 20:32, Hans Petter Selasky wrote: >> On 11/07/16 20:23, Oleksandr Tymoshenko wrote: >>>=20 >>>> On Nov 7, 2016, at 10:27 AM, Hans Petter Selasky <hps@selasky.org> >>>> wrote: >>>>=20 >>>> On 11/07/16 18:38, Oleksandr Tymoshenko wrote: >>>>> + bcm2835_audio_unlock(sc); >>>>> + cv_signal(&sc->worker_cv); >>>>=20 >>>>=20 >>>> 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. >>>=20 >>> Hi Hans, >>>=20 >>> In this case it doesn=E2=80=99t 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. >>>=20 >>=20 >> Hi, >>=20 >> 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! >>=20 >> --HPS >=20 > Hi, >=20 > Also the teardown sequence for the worker thread looks a bit broken, = that it doesn't wait for the thread to exit. >=20 > I've made a patch, attached which I think is the right way to do it. >=20 > 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=E2=80=99ll test and commit = it.=20=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D127D87C-EC4F-4211-B8C0-6BD431756DF0>