Re: ukbd(4): support apple brightness keys

2020-10-29 Thread Klemens Nanni 
On Wed, Oct 28, 2020 at 12:08:25AM +0100, Tobias Heider wrote:
> > What about KS_Cmd_BrightnessUp and KS_Cmd_BrightnessDown?
> 
> Right, here's a new diff using those wskbd commands.
> I couldn't find any standardized UHID key codes for brightness keys
> so I chose 232 and 233 which are currently unused and in the RESERVED range.
> 
> I also included the regenerated ukbdmap.c in the diff below for convenience.
Looks good to me and works in combination with your ofw_machdep diff.

OK kn

Re: ukbd(4): support apple brightness keys

2020-10-27 Thread Tobias Heider 
> What about KS_Cmd_BrightnessUp and KS_Cmd_BrightnessDown?

Right, here's a new diff using those wskbd commands.
I couldn't find any standardized UHID key codes for brightness keys
so I chose 232 and 233 which are currently unused and in the RESERVED range.

I also included the regenerated ukbdmap.c in the diff below for convenience.

Feedback, ok?

Index: hid/hidkbd.c
===
RCS file: /cvs/src/sys/dev/hid/hidkbd.c,v
retrieving revision 1.5
diff -u -p -r1.5 hidkbd.c
--- hid/hidkbd.c30 May 2017 07:40:24 -  1.5
+++ hid/hidkbd.c27 Oct 2020 22:37:05 -
@@ -397,7 +407,7 @@ hidkbd_decode(struct hidkbd *kbd, struct
wskbd_rawinput(kbd->sc_wskbddev, cbuf, j);
 
/*
-* Pass audio keys to wskbd_input anyway.
+* Pass audio and brightness keys to wskbd_input anyway.
 */
for (i = 0; i < nkeys; i++) {
key = ibuf[i];
@@ -405,6 +415,8 @@ hidkbd_decode(struct hidkbd *kbd, struct
case 127:
case 128:
case 129:
+   case 232:
+   case 233:
wskbd_input(kbd->sc_wskbddev,
key & RELEASE ?  WSCONS_EVENT_KEY_UP :
  WSCONS_EVENT_KEY_DOWN, key & CODEMASK);
Index: usb/makemap.awk
===
RCS file: /cvs/src/sys/dev/usb/makemap.awk,v
retrieving revision 1.14
diff -u -p -r1.14 makemap.awk
--- usb/makemap.awk 20 Nov 2013 17:27:32 -  1.14
+++ usb/makemap.awk 27 Oct 2020 22:37:05 -
@@ -341,6 +341,8 @@ $1 == "#define" || $1 == "#undef" {
lines[124] = "KC(124),\tKS_Copy,"
lines[125] = "KC(125),\tKS_Paste,"
lines[126] = "KC(126),\tKS_Find,"
+   lines[232] = "KC(232),\tKS_Cmd_BrightnessUp,"
+   lines[233] = "KC(233),\tKS_Cmd_BrightnessDown,"
}
 
for (i = 0; i < 256; i++)
Index: usb/ukbd.c
===
RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
retrieving revision 1.79
diff -u -p -r1.79 ukbd.c
--- usb/ukbd.c  23 Aug 2020 11:08:02 -  1.79
+++ usb/ukbd.c  27 Oct 2020 22:37:05 -
@@ -559,6 +559,8 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
{ 66, 0 },  /* F9 -> audio next */
 #endif
 #ifdef __macppc__
+   { 58, 233 },/* F1 -> screen brightness down */
+   { 59, 232 },/* F2 -> screen brightness up */
{ 60, 127 },/* F3 -> audio mute */
{ 61, 129 },/* F4 -> audio lower */
{ 62, 128 },/* F5 -> audio raise */
Index: usb/ukbdmap.c
===
RCS file: /cvs/src/sys/dev/usb/ukbdmap.c,v
retrieving revision 1.44
diff -u -p -r1.44 ukbdmap.c
--- usb/ukbdmap.c   11 May 2019 14:20:17 -  1.44
+++ usb/ukbdmap.c   27 Oct 2020 22:37:05 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: ukbdmap.c,v 1.44 2019/05/11 14:20:17 abieber Exp $*/
+/* $OpenBSD$   */
 
 /*
  * THIS FILE IS AUTOMAGICALLY GENERATED.  DO NOT EDIT.
@@ -185,6 +185,8 @@ static const keysym_t ukbd_keydesc_us[] 
 KC(229),   KS_Shift_R,
 KC(230),   KS_Cmd2,KS_Alt_R,   KS_Multi_key,
 KC(231),   KS_Meta_R,
+KC(232),   KS_Cmd_BrightnessUp,
+KC(233),   KS_Cmd_BrightnessDown,
 };
 
 #if !defined(WSKBD_NO_INTL_LAYOUTS)
Index: wscons/wskbd.c
===
RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
retrieving revision 1.106
diff -u -p -r1.106 wskbd.c
--- wscons/wskbd.c  29 Jul 2020 05:53:52 -  1.106
+++ wscons/wskbd.c  27 Oct 2020 22:37:06 -
@@ -1492,6 +1492,20 @@ internal_command(struct wskbd_softc *sc,
 #endif
 #endif
 
+#if NWSDISPLAY > 0
+   switch(ksym) {
+   case KS_Cmd_BrightnessUp:
+   wsdisplay_brightness_step(sc->sc_displaydv, 1);
+   return (1);
+   case KS_Cmd_BrightnessDown:
+   wsdisplay_brightness_step(sc->sc_displaydv, -1);
+   return (1);
+   case KS_Cmd_BrightnessRotate:
+   wsdisplay_brightness_cycle(sc->sc_displaydv);
+   return (1);
+   }
+#endif
+
if (!MOD_ONESET(sc->id, MOD_COMMAND) &&
!MOD_ALLSET(sc->id, MOD_COMMAND1 | MOD_COMMAND2))
return (0);
@@ -1555,15 +1569,6 @@ internal_command(struct wskbd_softc *sc,
change_displayparam(sc, WSDISPLAYIO_PARAM_BACKLIGHT,
ksym == KS_Cmd_BacklightOff ? -1 : 1,
ksym == KS_Cmd_BacklightToggle ? 1 : 0);
-   return (1);
-   case KS_Cmd_BrightnessUp:
-

Re: ukbd(4): support apple brightness keys

2020-10-27 Thread Klemens Nanni 
On Tue, Oct 27, 2020 at 02:58:55PM +0100, Tobias Heider wrote:
> I think you're missing the previous diff, see
> https://marc.info/?l=openbsd-tech&m=160373302219962&w=2
Right, my bad.  -CURRENT with your diff works as expected in all regards.

Re: ukbd(4): support apple brightness keys

2020-10-27 Thread Mark Kettenis 
> Date: Tue, 27 Oct 2020 00:16:16 +0100
> From: Tobias Heider 
> 
> Hi,
> 
> the diff below makes the brightness keys work on apple powerbooks > 5,6
> where the keyboard attaches via ukbd(4).
> I'm wondering if it would be better to go through wskbd as is done with
> audio, but we don't have keycodes allocated for brightness up/down.

What about KS_Cmd_BrightnessUp and KS_Cmd_BrightnessDown?

> Thoughts? ok?
> 
> Index: ukbd.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 ukbd.c
> --- ukbd.c23 Aug 2020 11:08:02 -  1.79
> +++ ukbd.c26 Oct 2020 23:10:05 -
> @@ -71,6 +71,7 @@
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -86,6 +87,10 @@ intukbddebug = 0;
>  #define DPRINTFN(n,x)
>  #endif
>  
> +#define UKBD_PARAM   (1 << 15)
> +#define UKBD_BRIGHTNESS_UP   0x1
> +#define UKBD_BRIGHTNESS_DOWN 0x2
> +
>  const kbd_t ukbd_countrylayout[1 + HCC_MAX] = {
>   (kbd_t)-1,
>   (kbd_t)-1,  /* arabic */
> @@ -159,6 +164,7 @@ void  ukbd_db_enter(void *);
>  int  ukbd_enable(void *, int);
>  void ukbd_set_leds(void *, int);
>  int  ukbd_ioctl(void *, u_long, caddr_t, int, struct proc *);
> +void ukbd_set_display_param(int);
>  
>  const struct wskbd_accessops ukbd_accessops = {
>   ukbd_enable,
> @@ -180,7 +186,7 @@ const struct cfattach ukbd_ca = {
>  
>  struct ukbd_translation {
>   uint8_t original;
> - uint8_t translation;
> + uint16_t translation;
>  };
>  
>  #ifdef __loongson__
> @@ -192,7 +198,7 @@ void  ukbd_apple_iso_munge(void *, uint8_
>  void ukbd_apple_iso_mba_munge(void *, uint8_t *, u_int);
>  void ukbd_apple_translate(void *, uint8_t *, u_int,
>   const struct ukbd_translation *, u_int);
> -uint8_t  ukbd_translate(const struct ukbd_translation *, size_t, 
> uint8_t);
> +uint16_t ukbd_translate(const struct ukbd_translation *, size_t, 
> uint8_t);
>  
>  int
>  ukbd_match(struct device *parent, void *match, void *aux)
> @@ -511,7 +517,7 @@ ukbd_cnattach(void)
>   return (0);
>  }
>  
> -uint8_t
> +uint16_t
>  ukbd_translate(const struct ukbd_translation *table, size_t tsize,
>  uint8_t keycode)
>  {
> @@ -527,13 +533,19 @@ ukbd_apple_translate(void *vsc, uint8_t 
>  {
>   struct ukbd_softc *sc = vsc;
>   struct hidkbd *kbd = &sc->sc_kbd;
> - uint8_t *pos, *spos, *epos, xlat;
> + uint8_t *pos, *spos, *epos;
> + uint16_t xlat;
>  
>   spos = ibuf + kbd->sc_keycodeloc.pos / 8;
>   epos = spos + kbd->sc_nkeycode;
>  
>   for (pos = spos; pos != epos; pos++) {
>   xlat = ukbd_translate(trans, tlen, *pos);
> + if (xlat & UKBD_PARAM) {
> + ukbd_set_display_param(xlat & ~UKBD_PARAM);
> + *pos = 0;
> + return;
> + }
>   if (xlat != 0)
>   *pos = xlat;
>   }
> @@ -548,8 +560,6 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
>   { 40, 73 }, /* return -> insert */
>   { 42, 76 }, /* backspace -> delete */
>  #ifdef notyet
> - { 58, 0 },  /* F1 -> screen brightness down */
> - { 59, 0 },  /* F2 -> screen brightness up */
>   { 60, 0 },  /* F3 */
>   { 61, 0 },  /* F4 */
>   { 62, 0 },  /* F5 -> keyboard backlight down */
> @@ -559,6 +569,8 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
>   { 66, 0 },  /* F9 -> audio next */
>  #endif
>  #ifdef __macppc__
> + { 58, UKBD_PARAM | UKBD_BRIGHTNESS_DOWN },  /* F1 -> screen 
> brightness down */
> + { 59, UKBD_PARAM | UKBD_BRIGHTNESS_UP  },   /* F2 -> screen 
> brightness up */
>   { 60, 127 },/* F3 -> audio mute */
>   { 61, 129 },/* F4 -> audio lower */
>   { 62, 128 },/* F5 -> audio raise */
> @@ -641,6 +653,21 @@ ukbd_apple_iso_mba_munge(void *vsc, uint
>   ukbd_apple_translate(vsc, ibuf, ilen, apple_iso_trans,
>   nitems(apple_iso_trans));
>   ukbd_apple_mba_munge(vsc, ibuf, ilen);
> +}
> +
> +void
> +ukbd_set_display_param(int num)
> +{
> + switch(num) {
> + case UKBD_BRIGHTNESS_UP:
> + wsdisplay_brightness_step(NULL, 1);
> + break;
> + case UKBD_BRIGHTNESS_DOWN:
> + wsdisplay_brightness_step(NULL, -1);
> + break;
> + default:
> + break;
> + }
>  }
>  
>  #ifdef __loongson__
> 
>

Re: ukbd(4): support apple brightness keys

2020-10-27 Thread Tobias Heider 
On Tue, Oct 27, 2020 at 02:22:24PM +0100, Klemens Nanni wrote:
> On Tue, Oct 27, 2020 at 12:16:16AM +0100, Tobias Heider wrote:
> > the diff below makes the brightness keys work on apple powerbooks > 5,6
> > where the keyboard attaches via ukbd(4).
> > I'm wondering if it would be better to go through wskbd as is done with
> > audio, but we don't have keycodes allocated for brightness up/down.
> Running the latest snapshot on my PowerBook5,8 the keys do not work,
> neither does `wsconsctl display.brightness=50' or so.
> 
> Testing this before your diff, I noticed that the sensor values change
> each time I print them (random stack garbage?), same thing when setting
> values, i.e. setting brightness to 50 as above shows something like this
> 
>   display.brightness -> 0.23948792%
> 
> Applying your diff changes nothing for me.
> 

I think you're missing the previous diff, see
https://marc.info/?l=openbsd-tech&m=160373302219962&w=2

Re: ukbd(4): support apple brightness keys

2020-10-27 Thread Klemens Nanni 
On Tue, Oct 27, 2020 at 12:16:16AM +0100, Tobias Heider wrote:
> the diff below makes the brightness keys work on apple powerbooks > 5,6
> where the keyboard attaches via ukbd(4).
> I'm wondering if it would be better to go through wskbd as is done with
> audio, but we don't have keycodes allocated for brightness up/down.
Running the latest snapshot on my PowerBook5,8 the keys do not work,
neither does `wsconsctl display.brightness=50' or so.

Testing this before your diff, I noticed that the sensor values change
each time I print them (random stack garbage?), same thing when setting
values, i.e. setting brightness to 50 as above shows something like this

display.brightness -> 0.23948792%

Applying your diff changes nothing for me.

ukbd(4): support apple brightness keys

2020-10-26 Thread Tobias Heider 
Hi,

the diff below makes the brightness keys work on apple powerbooks > 5,6
where the keyboard attaches via ukbd(4).
I'm wondering if it would be better to go through wskbd as is done with
audio, but we don't have keycodes allocated for brightness up/down.

Thoughts? ok?

Index: ukbd.c
===
RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
retrieving revision 1.79
diff -u -p -r1.79 ukbd.c
--- ukbd.c  23 Aug 2020 11:08:02 -  1.79
+++ ukbd.c  26 Oct 2020 23:10:05 -
@@ -71,6 +71,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -86,6 +87,10 @@ int  ukbddebug = 0;
 #define DPRINTFN(n,x)
 #endif
 
+#define UKBD_PARAM (1 << 15)
+#define UKBD_BRIGHTNESS_UP 0x1
+#define UKBD_BRIGHTNESS_DOWN   0x2
+
 const kbd_t ukbd_countrylayout[1 + HCC_MAX] = {
(kbd_t)-1,
(kbd_t)-1,  /* arabic */
@@ -159,6 +164,7 @@ voidukbd_db_enter(void *);
 intukbd_enable(void *, int);
 void   ukbd_set_leds(void *, int);
 intukbd_ioctl(void *, u_long, caddr_t, int, struct proc *);
+void   ukbd_set_display_param(int);
 
 const struct wskbd_accessops ukbd_accessops = {
ukbd_enable,
@@ -180,7 +186,7 @@ const struct cfattach ukbd_ca = {
 
 struct ukbd_translation {
uint8_t original;
-   uint8_t translation;
+   uint16_t translation;
 };
 
 #ifdef __loongson__
@@ -192,7 +198,7 @@ voidukbd_apple_iso_munge(void *, uint8_
 void   ukbd_apple_iso_mba_munge(void *, uint8_t *, u_int);
 void   ukbd_apple_translate(void *, uint8_t *, u_int,
const struct ukbd_translation *, u_int);
-uint8_tukbd_translate(const struct ukbd_translation *, size_t, 
uint8_t);
+uint16_t   ukbd_translate(const struct ukbd_translation *, size_t, 
uint8_t);
 
 int
 ukbd_match(struct device *parent, void *match, void *aux)
@@ -511,7 +517,7 @@ ukbd_cnattach(void)
return (0);
 }
 
-uint8_t
+uint16_t
 ukbd_translate(const struct ukbd_translation *table, size_t tsize,
 uint8_t keycode)
 {
@@ -527,13 +533,19 @@ ukbd_apple_translate(void *vsc, uint8_t 
 {
struct ukbd_softc *sc = vsc;
struct hidkbd *kbd = &sc->sc_kbd;
-   uint8_t *pos, *spos, *epos, xlat;
+   uint8_t *pos, *spos, *epos;
+   uint16_t xlat;
 
spos = ibuf + kbd->sc_keycodeloc.pos / 8;
epos = spos + kbd->sc_nkeycode;
 
for (pos = spos; pos != epos; pos++) {
xlat = ukbd_translate(trans, tlen, *pos);
+   if (xlat & UKBD_PARAM) {
+   ukbd_set_display_param(xlat & ~UKBD_PARAM);
+   *pos = 0;
+   return;
+   }
if (xlat != 0)
*pos = xlat;
}
@@ -548,8 +560,6 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
{ 40, 73 }, /* return -> insert */
{ 42, 76 }, /* backspace -> delete */
 #ifdef notyet
-   { 58, 0 },  /* F1 -> screen brightness down */
-   { 59, 0 },  /* F2 -> screen brightness up */
{ 60, 0 },  /* F3 */
{ 61, 0 },  /* F4 */
{ 62, 0 },  /* F5 -> keyboard backlight down */
@@ -559,6 +569,8 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
{ 66, 0 },  /* F9 -> audio next */
 #endif
 #ifdef __macppc__
+   { 58, UKBD_PARAM | UKBD_BRIGHTNESS_DOWN },  /* F1 -> screen 
brightness down */
+   { 59, UKBD_PARAM | UKBD_BRIGHTNESS_UP  },   /* F2 -> screen 
brightness up */
{ 60, 127 },/* F3 -> audio mute */
{ 61, 129 },/* F4 -> audio lower */
{ 62, 128 },/* F5 -> audio raise */
@@ -641,6 +653,21 @@ ukbd_apple_iso_mba_munge(void *vsc, uint
ukbd_apple_translate(vsc, ibuf, ilen, apple_iso_trans,
nitems(apple_iso_trans));
ukbd_apple_mba_munge(vsc, ibuf, ilen);
+}
+
+void
+ukbd_set_display_param(int num)
+{
+   switch(num) {
+   case UKBD_BRIGHTNESS_UP:
+   wsdisplay_brightness_step(NULL, 1);
+   break;
+   case UKBD_BRIGHTNESS_DOWN:
+   wsdisplay_brightness_step(NULL, -1);
+   break;
+   default:
+   break;
+   }
 }
 
 #ifdef __loongson__