Date: Wed, 10 Aug 2005 18:10:04 GMT From: Ariff Abdullah <skywizard@MyBSD.org.my> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/84728: [sound] [patch] ac97 broken mixing capabilities checking Message-ID: <200508101810.j7AIA45i053223@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/84728; it has been noted by GNATS.
From: Ariff Abdullah <skywizard@MyBSD.org.my>
To: michaels@sdf.lonestar.org
Cc: FreeBSD-gnats-submit@FreeBSD.org, michaels.maillist@gmail.com
Subject: Re: kern/84728: [sound] [patch] ac97 broken mixing capabilities
checking
Date: Thu, 11 Aug 2005 02:02:05 +0800
On Wed, 10 Aug 2005 03:27:15 +0000
michaels@sdf.lonestar.org wrote:
>
> >Number: 84728
> >Category: kern
> >Synopsis: [sound] [patch] ac97 broken mixing capabilities checking
> >Confidential: no
> >Severity: non-critical
> >Priority: medium
> >Responsible: freebsd-bugs
> >State: open
> >Quarter:
> >Keywords:
> >Date-Required:
> >Class: change-request
> >Submitter-Id: current-users
> >Arrival-Date: Wed Aug 10 03:30:11 GMT 2005
> >Closed-Date:
> >Last-Modified:
> >Originator: Michael Seyfert
> >Release: FreeBSD 7.0-CURRENT i386
> >Organization:
> >Environment:
> System: FreeBSD icemach.my.domain 7.0-CURRENT i386
>
> >Description:
>
> new sys/dev/sound/pcm/ac97.c introduces some problems with mixer
> capabilities checking.
> It does not properly set the "width of control field" for cards with
> a non-default width. As such the volume control is misaligned ( I can
> only change the volume from 50% to 100% on my audigy card.)
>
> Also, there's an if statement and for loops that logically don't do
> anything at all, which has me wondering about the quality of this code.
>
> I think the author (Ariff Abdullah <skywizard@MyBSD.org.my>) should redo it.
>
> >How-To-Repeat:
> Audigy 2 card...
>
> >Fix:
>
> Here's a patch to revert this code to the previous version.
>
> --- sys/dev/sound/pcm/ac97.c Sun Jul 31 08:28:31 2005
> +++ /home/michael/work/ac97.c Tue Aug 9 19:12:01 2005
> @@ -611,39 +611,15 @@
> for (i = 0; i < 32; i++) {
> k = codec->noext? codec->mix[i].enable : 1;
> if (k && (codec->mix[i].reg > 0)) {
> - j = old = ac97_rdcd(codec, codec->mix[i].reg);
> - if (!(j & 0x8000)) {
> - ac97_wrcd(codec, codec->mix[i].reg, j | 0x8000);
> - j = ac97_rdcd(codec, codec->mix[i].reg);
> - }
> - if ((j & 0x8000)) {
> - j = ((1 << 6) - 1) << codec->mix[i].ofs;
> - if (codec->mix[i].mute)
> - j |= 0x8000;
> - ac97_wrcd(codec, codec->mix[i].reg, j);
> - j = ac97_rdcd(codec, codec->mix[i].reg) & j;
> - j >>= codec->mix[i].ofs;
> - if (codec->mix[i].reg == AC97_MIX_TONE &&
> - ((j & 0x0001) == 0x0000))
> - j >>= 1;
> - for (k = 0; j != 0; k++)
> - j >>= 1;
> - for (j = 0; k != 0; j++)
> - k >>= 1;
> - if (j != 0) {
> - codec->mix[i].enable = 1;
> -#if 0
> - codec->mix[i].bits = j;
> -#endif
> - } else
> - codec->mix[i].enable = 0;
> - } else
> - codec->mix[i].enable = 0;
> + old = ac97_rdcd(codec, codec->mix[i].reg);
> + ac97_wrcd(codec, codec->mix[i].reg, 0x3f);
> + j = ac97_rdcd(codec, codec->mix[i].reg);
> ac97_wrcd(codec, codec->mix[i].reg, old);
> + codec->mix[i].enable = (j != 0 && j != old)? 1 : 0;
> + for (k = 1; j & (1 << k); k++);
> + codec->mix[i].bits = j? k - codec->mix[i].ofs : 0;
> }
> -#if 0
> - printf("mixch %d, en=%d, b=%d\n", i, codec->mix[i].enable, codec->mix[i].bits);
> -#endif
> + /* printf("mixch %d, en=%d, b=%d\n", i, codec->mix[i].enable, codec->mix[i].bits); */
> }
>
> device_printf(codec->dev, "<%s>\n",
> >Release-Note:
> >Audit-Trail:
> >Unformatted:
> _______________________________________________
> freebsd-bugs@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
> To unsubscribe, send any mail to "freebsd-bugs-unsubscribe@freebsd.org"
Please try this patch:
--- sys/dev/sound/pcm/ac97.c.orig Thu Aug 11 01:50:30 2005
+++ sys/dev/sound/pcm/ac97.c Thu Aug 11 01:52:35 2005
@@ -608,33 +608,40 @@
if (codec_patch)
codec_patch(codec);
+ /*
+ * YES. Do ac97_rdcd() T W I C E.
+ */
for (i = 0; i < 32; i++) {
k = codec->noext? codec->mix[i].enable : 1;
if (k && (codec->mix[i].reg > 0)) {
+ (void)ac97_rdcd(codec, codec->mix[i].reg);
j = old = ac97_rdcd(codec, codec->mix[i].reg);
if (!(j & 0x8000)) {
ac97_wrcd(codec, codec->mix[i].reg, j | 0x8000);
+ (void)ac97_rdcd(codec, codec->mix[i].reg);
j = ac97_rdcd(codec, codec->mix[i].reg);
}
if ((j & 0x8000)) {
- j = ((1 << 6) - 1) << codec->mix[i].ofs;
+ j = ((1 << codec->mix[i].bits) - 1) << codec->mix[i].ofs;
if (codec->mix[i].mute)
j |= 0x8000;
ac97_wrcd(codec, codec->mix[i].reg, j);
- j = ac97_rdcd(codec, codec->mix[i].reg) & j;
- j >>= codec->mix[i].ofs;
+ (void)ac97_rdcd(codec, codec->mix[i].reg);
+ k = ac97_rdcd(codec, codec->mix[i].reg) & (j & ~0x8000);
+ k >>= codec->mix[i].ofs;
if (codec->mix[i].reg == AC97_MIX_TONE &&
- ((j & 0x0001) == 0x0000))
- j >>= 1;
- for (k = 0; j != 0; k++)
- j >>= 1;
+ ((k & 0x0001) == 0x0000))
+ k >>= 1;
for (j = 0; k != 0; j++)
k >>= 1;
if (j != 0) {
- codec->mix[i].enable = 1;
#if 0
- codec->mix[i].bits = j;
+ device_printf(codec->dev,
+ "%d: %d -> %d\n",
+ i, codec->mix[i].bits, j);
#endif
+ codec->mix[i].enable = 1;
+ codec->mix[i].bits = j;
} else
codec->mix[i].enable = 0;
} else
--
Ariff Abdullah
MyBSD
http://www.MyBSD.org.my (IPv6/IPv4)
http://staff.MyBSD.org.my (IPv6/IPv4)
http://tomoyo.MyBSD.org.my (IPv6/IPv4)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200508101810.j7AIA45i053223>