Re: macppc PowerBook5,4: add missing volume control

2020-06-04 Thread Theo de Raadt
Mark Kettenis  wrote:

> Apple just made too big a mess of their OFW device tree :(.

this isn't even the worst...


I'm also ok with George's hack.



Re: macppc PowerBook5,4: add missing volume control

2020-06-04 Thread Mark Kettenis
> Date: Thu, 4 Jun 2020 17:34:44 -0400
> From: George Koehler 
> 
> This diff is for macppc "model PowerBook5,4".  It adds the missing
> audio volume control by changing the driver from aoa(4) to snapper(4).
> 
> Before the diff, I needed to put my ear near the speaker to check if
> audio was playing.  After the diff, the speaker was so loud (about
> output.level=0.75) that I used sndioctl to turn it down.  The diff
> uses strcmp(hw_prod, "PowerBook5,4") to attach the other driver.
> 
> In OpenBSD,
>   aoa(4) is i2s with no volume control.
>   snapper(4) is i2s with TAS3004 volume control.
> In Mac OS X, system profiler shows TAS3004 audio on my PowerBook5,4.
> 
> NetBSD doesn't check the "PowerBook5,4" model string.  NetBSD's
> sys/arch/macppc/dev/snapper.c snapper_defer() looks for a "deq" in
> the device tree.  No "deq" means no hardware volume control.
> 
> OpenBSD defines a DEQaddr and doesn't check "deq".  We do look for
> the kiic(4) on macobio(4) -- this device has "deq" -- but this
> happens late in snapper_defer(), after kiic(4) attaches, which is
> after aoa(4) or snapper(4) attaches.  By checking "PowerBook5,4", I
> can pick aoa(4) or snapper(4) without knowing kiic(4).  See part of
> my dmesg and eeprom -p, below the diff.
> 
> OK to commit?

While it isn't "right", this is probably the least invasive way to
handle this.  Apple just made too big a mess of their OFW device tree :(.

ok kettenis@

> Index: share/man/man4/man4.macppc/aoa.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.macppc/aoa.4,v
> retrieving revision 1.9
> diff -u -p -r1.9 aoa.4
> --- share/man/man4/man4.macppc/aoa.4  2 Jul 2016 16:28:50 -   1.9
> +++ share/man/man4/man4.macppc/aoa.4  4 Jun 2020 20:39:33 -
> @@ -46,8 +46,6 @@ driver include:
>  .Pp
>  .Bl -dash -offset indent -compact
>  .It
> -PowerBook5,4
> -.It
>  PowerMac7,3
>  .It
>  PowerMac9,1
> Index: share/man/man4/man4.macppc/snapper.4
> ===
> RCS file: /cvs/src/share/man/man4/man4.macppc/snapper.4,v
> retrieving revision 1.16
> diff -u -p -r1.16 snapper.4
> --- share/man/man4/man4.macppc/snapper.4  15 Jan 2015 20:37:36 -  
> 1.16
> +++ share/man/man4/man4.macppc/snapper.4  4 Jun 2020 20:39:33 -
> @@ -57,6 +57,8 @@ PowerBook5,2
>  .It
>  PowerBook5,3
>  .It
> +PowerBook5,4
> +.It
>  PowerBook5,5
>  .It
>  PowerBook5,6
> Index: sys/arch/macppc/dev/aoa.c
> ===
> RCS file: /cvs/src/sys/arch/macppc/dev/aoa.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 aoa.c
> --- sys/arch/macppc/dev/aoa.c 19 Sep 2016 06:46:43 -  1.9
> +++ sys/arch/macppc/dev/aoa.c 4 Jun 2020 20:39:33 -
> @@ -57,6 +57,8 @@ void aoa_attach(struct device *, struct 
>  void aoa_defer(struct device *);
>  void aoa_set_volume(struct aoa_softc *, int, int);
>  
> +extern char *hw_prod;
> +
>  struct cfattach aoa_ca = {
>   sizeof(struct aoa_softc), aoa_match, aoa_attach
>  };
> @@ -107,7 +109,8 @@ aoa_match(struct device *parent, void *m
>   bzero(compat, sizeof compat);
>   OF_getprop(soundchip, "compatible", compat, sizeof compat);
>  
> - if (strcmp(compat, "AOAKeylargo") == 0)
> + if (strcmp(compat, "AOAKeylargo") == 0 &&
> + strcmp(hw_prod, "PowerBook5,4") != 0)
>   return (1);
>   if (strcmp(compat, "AOAK2") == 0)
>   return (1);
> Index: sys/arch/macppc/dev/snapper.c
> ===
> RCS file: /cvs/src/sys/arch/macppc/dev/snapper.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 snapper.c
> --- sys/arch/macppc/dev/snapper.c 19 Sep 2016 06:46:43 -  1.37
> +++ sys/arch/macppc/dev/snapper.c 4 Jun 2020 20:39:34 -
> @@ -70,6 +70,8 @@ void snapper_set_input(struct snapper_so
>  int tas3004_write(struct snapper_softc *, u_int, const void *);
>  int tas3004_init(struct snapper_softc *);
>  
> +extern char *hw_prod;
> +
>  struct cfattach snapper_ca = {
>   sizeof(struct snapper_softc), snapper_match, snapper_attach
>  };
> @@ -442,6 +444,9 @@ snapper_match(struct device *parent, voi
>   bzero(compat, sizeof compat);
>   OF_getprop(soundchip, "compatible", compat, sizeof compat);
>  
> + if (strcmp(compat, "AOAKeylargo") == 0 &&
> + strcmp(hw_prod, "PowerBook5,4") == 0)
> + return (1);
>   if (strcmp(compat, "snapper") == 0)
>   return (1);
>  
> 
> Part of dmesg:
> [ using 1155980 bytes of bsd ELF symbol table ]
> console out [ATY,Jasper_A] console in [keyboard]USB and ADB found, using ADB
> using parent ATY,JasperParent:: memaddr b800, size 800 : consaddr 
> b8008000 : ioaddr b002, size 2: width 1280 linebytes 1280 height 854 
> depth 8
> Copyright (c) 1982, 1986, 1989, 1991, 1993
>   The Regents of the University of California.  All rights reserved.
> Copyright (c) 

macppc PowerBook5,4: add missing volume control

2020-06-04 Thread George Koehler
This diff is for macppc "model PowerBook5,4".  It adds the missing
audio volume control by changing the driver from aoa(4) to snapper(4).

Before the diff, I needed to put my ear near the speaker to check if
audio was playing.  After the diff, the speaker was so loud (about
output.level=0.75) that I used sndioctl to turn it down.  The diff
uses strcmp(hw_prod, "PowerBook5,4") to attach the other driver.

In OpenBSD,
  aoa(4) is i2s with no volume control.
  snapper(4) is i2s with TAS3004 volume control.
In Mac OS X, system profiler shows TAS3004 audio on my PowerBook5,4.

NetBSD doesn't check the "PowerBook5,4" model string.  NetBSD's
sys/arch/macppc/dev/snapper.c snapper_defer() looks for a "deq" in
the device tree.  No "deq" means no hardware volume control.

OpenBSD defines a DEQaddr and doesn't check "deq".  We do look for
the kiic(4) on macobio(4) -- this device has "deq" -- but this
happens late in snapper_defer(), after kiic(4) attaches, which is
after aoa(4) or snapper(4) attaches.  By checking "PowerBook5,4", I
can pick aoa(4) or snapper(4) without knowing kiic(4).  See part of
my dmesg and eeprom -p, below the diff.

OK to commit?

Index: share/man/man4/man4.macppc/aoa.4
===
RCS file: /cvs/src/share/man/man4/man4.macppc/aoa.4,v
retrieving revision 1.9
diff -u -p -r1.9 aoa.4
--- share/man/man4/man4.macppc/aoa.42 Jul 2016 16:28:50 -   1.9
+++ share/man/man4/man4.macppc/aoa.44 Jun 2020 20:39:33 -
@@ -46,8 +46,6 @@ driver include:
 .Pp
 .Bl -dash -offset indent -compact
 .It
-PowerBook5,4
-.It
 PowerMac7,3
 .It
 PowerMac9,1
Index: share/man/man4/man4.macppc/snapper.4
===
RCS file: /cvs/src/share/man/man4/man4.macppc/snapper.4,v
retrieving revision 1.16
diff -u -p -r1.16 snapper.4
--- share/man/man4/man4.macppc/snapper.415 Jan 2015 20:37:36 -  
1.16
+++ share/man/man4/man4.macppc/snapper.44 Jun 2020 20:39:33 -
@@ -57,6 +57,8 @@ PowerBook5,2
 .It
 PowerBook5,3
 .It
+PowerBook5,4
+.It
 PowerBook5,5
 .It
 PowerBook5,6
Index: sys/arch/macppc/dev/aoa.c
===
RCS file: /cvs/src/sys/arch/macppc/dev/aoa.c,v
retrieving revision 1.9
diff -u -p -r1.9 aoa.c
--- sys/arch/macppc/dev/aoa.c   19 Sep 2016 06:46:43 -  1.9
+++ sys/arch/macppc/dev/aoa.c   4 Jun 2020 20:39:33 -
@@ -57,6 +57,8 @@ void aoa_attach(struct device *, struct 
 void aoa_defer(struct device *);
 void aoa_set_volume(struct aoa_softc *, int, int);
 
+extern char *hw_prod;
+
 struct cfattach aoa_ca = {
sizeof(struct aoa_softc), aoa_match, aoa_attach
 };
@@ -107,7 +109,8 @@ aoa_match(struct device *parent, void *m
bzero(compat, sizeof compat);
OF_getprop(soundchip, "compatible", compat, sizeof compat);
 
-   if (strcmp(compat, "AOAKeylargo") == 0)
+   if (strcmp(compat, "AOAKeylargo") == 0 &&
+   strcmp(hw_prod, "PowerBook5,4") != 0)
return (1);
if (strcmp(compat, "AOAK2") == 0)
return (1);
Index: sys/arch/macppc/dev/snapper.c
===
RCS file: /cvs/src/sys/arch/macppc/dev/snapper.c,v
retrieving revision 1.37
diff -u -p -r1.37 snapper.c
--- sys/arch/macppc/dev/snapper.c   19 Sep 2016 06:46:43 -  1.37
+++ sys/arch/macppc/dev/snapper.c   4 Jun 2020 20:39:34 -
@@ -70,6 +70,8 @@ void snapper_set_input(struct snapper_so
 int tas3004_write(struct snapper_softc *, u_int, const void *);
 int tas3004_init(struct snapper_softc *);
 
+extern char *hw_prod;
+
 struct cfattach snapper_ca = {
sizeof(struct snapper_softc), snapper_match, snapper_attach
 };
@@ -442,6 +444,9 @@ snapper_match(struct device *parent, voi
bzero(compat, sizeof compat);
OF_getprop(soundchip, "compatible", compat, sizeof compat);
 
+   if (strcmp(compat, "AOAKeylargo") == 0 &&
+   strcmp(hw_prod, "PowerBook5,4") == 0)
+   return (1);
if (strcmp(compat, "snapper") == 0)
return (1);
 

Part of dmesg:
[ using 1155980 bytes of bsd ELF symbol table ]
console out [ATY,Jasper_A] console in [keyboard]USB and ADB found, using ADB
using parent ATY,JasperParent:: memaddr b800, size 800 : consaddr 
b8008000 : ioaddr b002, size 2: width 1280 linebytes 1280 height 854 
depth 8
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2020 OpenBSD. All rights reserved.  https://www.OpenBSD.org

OpenBSD 6.7-current (GENERIC) #1: Thu Jun  4 16:12:14 EDT 2020
kern...@wisconsin.my.domain:/usr/src/sys57/arch/macppc/compile/GENERIC
real mem = 1073741824 (1024MB)
avail mem = 1026486272 (978MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root: model PowerBook5,4
cpu0 at mainbus0: 7447A (Revision