Date: Tue, 23 Nov 2004 17:22:07 -0500 From: Anish Mistry <mistry.7@osu.edu> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-current@freebsd.org Subject: Re: ATI TV Wonder support Message-ID: <200411231722.25392.mistry.7@osu.edu> In-Reply-To: <200411231648.17128.jhb@FreeBSD.org> References: <B23E4FA7-3C32-11D9-BA1A-000A95841F44@po.cwru.edu> <200411231636.29953.jhb@FreeBSD.org> <200411231648.17128.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart1394750.p0qH7Ocs9P Content-Type: multipart/mixed; boundary="Boundary-01=_Pg7oBudYOV4Jn6z" Content-Transfer-Encoding: 7bit Content-Disposition: inline --Boundary-01=_Pg7oBudYOV4Jn6z Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On Tuesday 23 November 2004 04:48 pm, John Baldwin wrote: > On Tuesday 23 November 2004 04:36 pm, John Baldwin wrote: > > On Tuesday 23 November 2004 03:07 pm, Anish Mistry wrote: > > > On Tuesday 23 November 2004 02:51 pm, John Baldwin wrote: > > > > On Tuesday 23 November 2004 02:43 pm, Anish Mistry wrote: > > > > > On Tuesday 23 November 2004 11:34 am, you wrote: > > > > > > On Monday 22 November 2004 11:00 pm, Justin Hibbits wrote: > > > > > > > On Nov 22, 2004, at 22:56, Anish Mistry wrote: > > > > > > > > On Monday 22 November 2004 09:29 pm, you wrote: > > > > > > > >> On Nov 22, 2004, at 21:22, Anish Mistry wrote: > > > > > > > >>> On Sunday 21 November 2004 10:00 pm, Justin Hibbits=20 wrote: > > > > > > > >>>> This patch gives more or less full ATI TV Wonder > > > > > > > >>>> support to the bktr driver. > > > > > > > >>> > > > > > > > >>> Awesome. > > > > > > > >>> > > > > > > > >>>> The sound doesn't mute at close, but that might be > > > > > > > >>>> xawtv's fault, but > > > > > > > >>>> I > > > > > > > >>>> don't know which to accuse or look at. But, other than > > > > > > > >>>> that, it seems pretty good. > > > > > > > >>> > > > > > > > >>> I've having some trouble using it though, the sound > > > > > > > >>> doesn't seem to work > > > > > > > >>> just as before. Is there something extra I need to add > > > > > > > >>> to my kernel besides bktr? > > > > > > > >>> On boot: > > > > > > > >>> bktr0: <BrookTree 878> mem 0xe3101000-0xe3101fff irq 11 > > > > > > > >>> at device 8.0 on > > > > > > > >>> pci0 > > > > > > > >>> bktr0: [GIANT-LOCKED] > > > > > > > >>> bktr0: Detected a MSP3445G-B8 at 0x80 > > > > > > > >>> bktr0: ATI TV Wonder, Philips NTSC tuner, msp3400c > > > > > > > >>> stereo. pci0: <multimedia> at device 8.1 (no driver > > > > > > > >>> attached) And when I start it: > > > > > > > >>> bktr0: Detected a MSP3445G-B8 at 0x80 > > > > > > > >>> > > > > > > > >>> You should also send-pr this if you haven't already. > > > > > > > >>> > > > > > > > >>> Thanks, > > > > > > > >>> > > > > > > > >>> -- > > > > > > > >>> Anish Mistry > > > > > > > >> > > > > > > > >> Oh, you need to build it with the msp3400c driver, so add > > > > > > > >> the line: > > > > > > > >> > > > > > > > >> options BKTR_NEW_MSP34XX_DRIVER > > > > > > > >> > > > > > > > >> to your config file. The card doesn't have any mux, only > > > > > > > >> that chip, so > > > > > > > >> you need the driver. I've found a hackish solution to > > > > > > > >> the sound problem, by resetting the card on exit. It's > > > > > > > >> horrible, so I won't include it in the patch, but it > > > > > > > >> works for me. If you want to use this too, add: > > > > > > > >> > > > > > > > >> if (bktr->card.msp3400c ) > > > > > > > >> msp_dpl_reset( bktr, bktr->msp_addr ); > > > > > > > >> > > > > > > > >> to bktr_core.c, at line 1171. > > > > > > > >> > > > > > > > >> > > > > > > > >> I'll resend my previous message as a send-pr later, when > > > > > > > >> I have a little more time to test it. > > > > > > > > > > > > > > > > Thanks, but I'm getting a weird panic with 5-STABLE, which > > > > > > > > I'm assuming you > > > > > > > > aren't seeing. Any ideas? > > > > > > > > > > > > > > > > panic: sleeping without a mutex > > > > > > > > KDB: enter: panic > > > > > > > > [thread 100012] > > > > > > > > Stopped at kbd_enter+0x2c: leave > > > > > > > > db> tr > > > > > > > > kbd_enter(c0635ca1,100,c1b6c54c,c1b41320,0) at > > > > > > > > kbd_enter+0x2c panic(c06363e5,0,0,0,c1b37c00) at > > > > > > > > panic+0x10a > > > > > > > > msleep(c1b6c54c,0,4c,c064007d,0) at msleep+0x2bf > > > > > > > > msp3410d_thread(c1bd8000,d0181d48,c1db8000,c845e1d4,0) at > > > > > > > > msp3410d_thread+0x5b > > > > > > > > fork_exit(c045e1d4,c1db8000,d0181d48) at fork_exit+0x7e > > > > > > > > fork_trampoline() at fork_trampoline+0x8 > > > > > > > > --- trap 0x1, eip=3D0, esp=3D0xd0181d7c, ebd=3D0 --- > > > > > > > > > > > > > > > > dmesg: > > > > > > > > http://am-productions.biz/docs/bigguy.txt.gz > > > > > > > > kernel config: > > > > > > > > http://am-productions.biz/docs/BIGGUY.gz > > > > > > > > -- > > > > > > > > Anish Mistry > > > > > > > > > > > > > > Oh yes, you need to compile it without witness support, or > > > > > > > invariants (DDB is fine, though). It, of course, cuts out a > > > > > > > ton of useful debugging info for other parts, but it's the > > > > > > > only way, unfortunately. > > > > > > > > > > > > That's a bug you'll need to fix or the driver can go to sleep > > > > > > forever and never wake up. > > > > > > > > > > I figured something like. Do you have any pointers of where to > > > > > get started and examples of how things like this have been fixed > > > > > in other drivers? > > > > > > > > Well, probably the code is not Giant-safe yet, so you'll need to > > > > figure out why Giant isn't being held in the driver. > > > > > > Would it be better to first make the code Giant-safe, or just go > > > ahead and add the locking to make it MP-safe? or are these 2 > > > seperate things? > > > > Getting the code to be MP-safe and thus not need Giant is a larger > > task than just adding some assertions and figuring out why Giant isn't > > held when it should be. It maybe that it simply needs D_NEEDSGIANT > > added to the flags in its cdevsw. > > Actually, it looks like the patch changes the code to kick off a new > kthread and that new kthread is going to need to acquire Giant at the > start of its main() function and drop it before calling kthread_exit() > and/or returning. How about the attached patch? http://am-productions.biz/docs/msp34xx-giant-locking.c.diff =2D-=20 Anish Mistry --Boundary-01=_Pg7oBudYOV4Jn6z Content-Type: text/x-diff; charset="iso-8859-1"; name="msp34xx-giant-locking.c.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="msp34xx-giant-locking.c.diff" =2D-- msp34xx.c.orig Mon Nov 22 22:57:42 2004 +++ msp34xx.c Tue Nov 23 17:10:16 2004 @@ -702,6 +702,7 @@ =09 dprintk("msp3400: thread started\n"); =09 + mtx_lock(&Giant); for (;;) { if (msp->rmmod) goto done; @@ -892,6 +893,7 @@ =20 msp->kthread =3D NULL; wakeup(&msp->kthread); + mtx_unlock(&Giant); =20 kthread_exit(0); } @@ -936,6 +938,7 @@ =20 dprintk("msp3410: thread started\n"); =09 + mtx_lock(&Giant); for (;;) { if (msp->rmmod) goto done; @@ -1114,9 +1117,10 @@ done: dprintk("msp3410: thread: exit\n"); msp->active =3D 0; =2D +=09 msp->kthread =3D NULL; wakeup(&msp->kthread); + mtx_unlock(&Giant); =20 kthread_exit(0); } @@ -1213,12 +1217,14 @@ if (msp->kthread)=20 { /* XXX mutex lock required */ + mtx_lock(&Giant); msp->rmmod =3D 1; msp->watch_stereo =3D 0; wakeup(msp->kthread); =20 while (msp->kthread) tsleep(&msp->kthread, PRIBIO, "wait for kthread", hz/10); + mtx_unlock(&Giant); } =20 if (client->msp3400c_info !=3D NULL) { --Boundary-01=_Pg7oBudYOV4Jn6z-- --nextPart1394750.p0qH7Ocs9P Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (FreeBSD) iD8DBQBBo7ghxqA5ziudZT0RAoXWAKCJYO6Gem4fEQoUmNdpsjGg/rJ6KwCfZjDs 5IwChxXe8oeIxLxRDg3KGoQ= =k+2L -----END PGP SIGNATURE----- --nextPart1394750.p0qH7Ocs9P--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200411231722.25392.mistry.7>