On Wed, 19 Jul 2023 02:03:26 +0200
Tobias Heider <tobias.hei...@stusta.de> wrote:

> > ok anyone?
> 
> No one interested in working keyboard backlight shortcuts?
> Don't get scared by the powerbook part, a lot of this is reusable for other 
> laptop models.

The arch/macppc/* part is ok gkoehler@, except for 2 minor style
issues at the top of adb.c.  I have an issue with the wscons part.
See my comments below.

On Fri, 14 Jul 2023 17:53:41 +0000 (UTC)
jon@elytron.openbsd.amsterdam wrote:

> Index: arch/macppc/dev/adb.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/macppc/dev/adb.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 adb.c
> --- arch/macppc/dev/adb.c     11 Apr 2023 00:45:07 -0000      1.50
> +++ arch/macppc/dev/adb.c     13 Jul 2023 21:17:17 -0000
> @@ -102,6 +102,8 @@
>  #include <macppc/dev/pm_direct.h>
>  #include <macppc/dev/viareg.h>
>  
> +#include <dev/wscons/wsconsio.h>
> +
>  #include "apm.h"
>  
>  #define printf_intr printf

1st minor style issue: Please move this next to one of the other
#include <dev/...> lines.

> @@ -242,6 +244,12 @@ void     setsoftadb(void);
>  int  adb_intr(void *arg);
>  void adb_cuda_autopoll(void);
>  void         adb_cuda_fileserver_mode(void);
> +uint8_t       pmu_backlight; /* keyboard backlight value */
> +int  pmu_get_backlight(struct wskbd_backlight *);
> +int  pmu_set_backlight(struct wskbd_backlight *);
> +extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
> +extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
> +
>  
>  #ifndef SMALL_KERNEL
>  void adb_shutdown(void *);

2nd minor style issue: Please delete the extra space before
"pmu_backlight".

> @@ -1730,8 +1738,11 @@ adbattach(struct device *parent, struct 
>  
>       if (adbHardware == ADB_HW_CUDA)
>               adb_cuda_fileserver_mode();
> -     if (adbHardware == ADB_HW_PMU)
> +     if (adbHardware == ADB_HW_PMU) {
> +             wskbd_get_backlight = pmu_get_backlight;
> +             wskbd_set_backlight = pmu_set_backlight;
>               pmu_fileserver_mode(1);
> +     }
>  
>       /*
>        * XXX If the machine doesn't have an ADB bus (PowerBook5,6+)

This code enables pmu_{get,set}_backlight for almost all macppc
models, including those without a keyboard backlight.  I am ok with
this, because I don't know how to detect exactly which models do or
don't have a backlight.

I booted this diff on an iMac G4 Flat Panel (PowerMac6,1), which
has no internal keyboard (so no backlight).  It boots with
"wsconsctl keyboard.backlight" at 0%.  Commands like "wsconsctl
keyboard.backlight=100" change the number but have no other effect.

I also booted it on an older PowerBook G4 (PowerBook5,4).  This has
a different keyboard backlight; this diff doesn't support it, so
"wsconsctl keyboard.backlight=100" does nothing.

> @@ -1757,4 +1768,20 @@ adbattach(struct device *parent, struct 
>       if (adbHardware == ADB_HW_CUDA)
>               adb_cuda_autopoll();
>       adb_polling = 0;
> +}
> +
> +int
> +pmu_get_backlight(struct wskbd_backlight *kbl)
> +{
> +     kbl->min = 0;
> +     kbl->max = 0xff;
> +     kbl->curval = pmu_backlight;
> +     return 0;
> +}
> +
> +int
> +pmu_set_backlight(struct wskbd_backlight *kbl)
> +{
> +     pmu_backlight = kbl->curval;
> +     return pmu_set_kbl(pmu_backlight);
>  }
> Index: arch/macppc/dev/pm_direct.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/macppc/dev/pm_direct.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 pm_direct.c
> --- arch/macppc/dev/pm_direct.c       28 Dec 2022 07:40:23 -0000      1.34
> +++ arch/macppc/dev/pm_direct.c       13 Jul 2023 21:17:24 -0000
> @@ -853,3 +853,22 @@ pmu_fileserver_mode(int on)
>       }
>       pmgrop(&p);
>  }
> +
> +int
> +pmu_set_kbl(unsigned int level)
> +{
> +     if (level > 0xff)
> +             return (EINVAL);
> +
> +     PMData p;
> +
> +     p.command = 0x4F;
> +     p.num_data = 3;
> +     p.s_buf = p.r_buf = p.data;
> +     p.data[0] = 0;
> +     p.data[1] = 0;
> +     p.data[2] = level;
> +     pmgrop(&p);
> +     return (0);
> +}
> +

I would leave this command number 0x4F as is.  It would have a PMU_*
name in pm_direct.h, but I don't know what name to use.  My notes use
0x4f = PMU_SET_CONTROL, but that might not be a good name.

> Index: arch/macppc/dev/pm_direct.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/macppc/dev/pm_direct.h,v
> retrieving revision 1.15
> diff -u -p -r1.15 pm_direct.h
> --- arch/macppc/dev/pm_direct.h       21 Oct 2022 22:42:36 -0000      1.15
> +++ arch/macppc/dev/pm_direct.h       13 Jul 2023 21:17:24 -0000
> @@ -67,6 +67,7 @@ struct pmu_battery_info
>  };
>  
>  int pm_battery_info(int, struct pmu_battery_info *);
> +int pmu_set_kbl(unsigned int);
>  
>  void pm_eject_pcmcia(int);
>  void pmu_fileserver_mode(int);
> Index: dev/hid/hidkbd.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/hid/hidkbd.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 hidkbd.c
> --- dev/hid/hidkbd.c  9 Jul 2023 08:02:13 -0000       1.9
> +++ dev/hid/hidkbd.c  13 Jul 2023 21:17:42 -0000
> @@ -143,6 +143,10 @@ static const struct hidkbd_translation a
>       { 60, 127 },    /* F3 -> audio mute */
>       { 61, 129 },    /* F4 -> audio lower */
>       { 62, 128 },    /* F5 -> audio raise */
> +     { 63, 83 },     /* F6 -> num lock */
> +     { 65, 234 },    /* F8 -> backlight toggle*/
> +     { 66, 236 },    /* F9 -> backlight lower */
> +     { 67, 235 },    /* F10 -> backlight raise*/
>  #else
>       { 63, 102 },    /* F6 -> sleep */
>       { 67, 127 },    /* F10 -> audio mute */
> @@ -569,6 +573,9 @@ hidkbd_decode(struct hidkbd *kbd, struct
>                       case 129:
>                       case 232:
>                       case 233:
> +                     case 234:
> +                     case 235:
> +                     case 236:
>                               wskbd_input(kbd->sc_wskbddev,
>                                   key & RELEASE ?  WSCONS_EVENT_KEY_UP :
>                                     WSCONS_EVENT_KEY_DOWN, key & CODEMASK);
> Index: dev/usb/makemap.awk
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/makemap.awk,v
> retrieving revision 1.16
> diff -u -p -r1.16 makemap.awk
> --- dev/usb/makemap.awk       9 Jul 2023 08:02:13 -0000       1.16
> +++ dev/usb/makemap.awk       13 Jul 2023 21:18:24 -0000
> @@ -343,6 +343,9 @@ $1 == "#define" || $1 == "#undef" {
>                       lines[126] = "    KC(126),\tKS_Find,"
>                       lines[232] = "    KC(232),\tKS_Cmd_BrightnessUp,"
>                       lines[233] = "    KC(233),\tKS_Cmd_BrightnessDown,"
> +                     lines[234] = "    KC(234),\tKS_Cmd_KbdBacklightToggle,"
> +                     lines[235] = "    KC(235),\tKS_Cmd_KbdBacklightDown,"
> +                     lines[236] = "    KC(236),\tKS_Cmd_KbdBacklightUp,"
>               }
>  
>               for (i = 0; i < 256; i++)
> Index: dev/usb/ukbdmap.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/ukbdmap.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 ukbdmap.c
> --- dev/usb/ukbdmap.c 9 Jul 2023 08:04:09 -0000       1.48
> +++ dev/usb/ukbdmap.c 13 Jul 2023 21:18:38 -0000
> @@ -187,6 +187,9 @@ static const keysym_t ukbd_keydesc_us[] 
>      KC(231), KS_Meta_R,
>      KC(232), KS_Cmd_BrightnessUp,
>      KC(233), KS_Cmd_BrightnessDown,
> +             KC(234),        KS_Cmd_KbdBacklightToggle,
> +             KC(235),        KS_Cmd_KbdBacklightUp,
> +             KC(236),        KS_Cmd_KbdBacklightDown,
>  };
>  
>  #if !defined(WSKBD_NO_INTL_LAYOUTS)

These lines have the wrong indentation.  This file is automagically
generated, so I don't know how this happened.

> Index: dev/wscons/wskbd.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
> retrieving revision 1.115
> diff -u -p -r1.115 wskbd.c
> --- dev/wscons/wskbd.c        9 Jul 2023 08:02:14 -0000       1.115
> +++ dev/wscons/wskbd.c        13 Jul 2023 21:19:54 -0000
> @@ -1544,6 +1544,22 @@ internal_command(struct wskbd_softc *sc,
>  #endif
>  #endif
>  
> +     switch (ksym) {
> +     case KS_Cmd_KbdBacklightToggle:
> +     case KS_Cmd_KbdBacklightDown:
> +     case KS_Cmd_KbdBacklightUp:{
> +             struct wskbd_backlight data;
> +             (*wskbd_get_backlight)(&data);
> +             int step = (data.max - data.min + 1) / 8;
> +             int val = (ksym == KS_Cmd_KbdBacklightUp) ? data.curval + step 
> +                     : (ksym == KS_Cmd_KbdBacklightDown) ? data.curval - step
> +                     : (data.curval) ? 0 : (data.max - data.min + 1) / 2;
> +             data.curval = (val > 0xff) ? 0xff : (val < 0) ? 0 : val;
> +             (*wskbd_set_backlight)(&data);
> +             return (1);
> +     }
> +     }
> +
>  #if NWSDISPLAY > 0
>       switch(ksym) {
>       case KS_Cmd_BrightnessUp:

We should check if (wskbd_get_backlight != NULL), like we do for
WSKBDIO_GETBACKLIGHT.  Same for wskbd_set_backlight.

> Index: dev/wscons/wsksymdef.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/wscons/wsksymdef.h,v
> retrieving revision 1.41
> diff -u -p -r1.41 wsksymdef.h
> --- dev/wscons/wsksymdef.h    9 Jul 2023 08:02:14 -0000       1.41
> +++ dev/wscons/wsksymdef.h    13 Jul 2023 21:19:59 -0000
> @@ -668,6 +676,9 @@
>  #define KS_Cmd_ScrollFwd     0xf42d
>  #define KS_Cmd_KbdReset              0xf42e
>  #define KS_Cmd_Sleep         0xf42f
> +#define KS_Cmd_KbdBacklightToggle            0xf430
> +#define KS_Cmd_KbdBacklightUp                0xf431
> +#define KS_Cmd_KbdBacklightDown              0xf432
>  
>  /*
>   * Group 5 (internal)
> 

Reply via email to