Re: pckbd volume keys (part 1), diff to test
On Fri, May 23, 2014 at 12:42 PM, Alexandre Ratchov a...@caoua.org wrote: On Wed, Apr 30, 2014 at 01:06:48AM +0200, Alexandre Ratchov wrote: This diff attempts to unify volume keys; it makes pckbd and ukbd volume keys behave like all other volume keys (acpithinkpad, acpiasus, macppc/abtn and similar drivers): simply adjust the hardware volume without passing keystroke events to upper layers (i.e. consume the keystroke). If your volume keys tend to mess the volume while in X (example mplayer), try this diff and see if it makes things better (or worse). No test reports so far. To test this: start X, then: - press the vol - button many times (don't hold is pressed), until volume goes to zero. - start a movie in mplayer, there's no sound as volume is zero. - press the vol + button and hold it down; now mplayer indicates the volume reached the maximum. Still you don't hear anything. Confusing, isn't it? Then rebuild the kernel with this diff and retry. With the diff volume keys are simple and deterministic: they simply adjust the volume and don't trigger hot-keys or whatever. Tested on my laptop: it follows the principle of least astonishment, thus I like it. Thanks, David
Re: pckbd volume keys (part 1), diff to test
From: David Coppa dco...@gmail.com Date: Mon, 26 May 2014 13:23:21 +0200 On Fri, May 23, 2014 at 12:42 PM, Alexandre Ratchov a...@caoua.org wrote: On Wed, Apr 30, 2014 at 01:06:48AM +0200, Alexandre Ratchov wrote: This diff attempts to unify volume keys; it makes pckbd and ukbd volume keys behave like all other volume keys (acpithinkpad, acpiasus, macppc/abtn and similar drivers): simply adjust the hardware volume without passing keystroke events to upper layers (i.e. consume the keystroke). If your volume keys tend to mess the volume while in X (example mplayer), try this diff and see if it makes things better (or worse). No test reports so far. To test this: start X, then: - press the vol - button many times (don't hold is pressed), until volume goes to zero. - start a movie in mplayer, there's no sound as volume is zero. - press the vol + button and hold it down; now mplayer indicates the volume reached the maximum. Still you don't hear anything. Confusing, isn't it? Then rebuild the kernel with this diff and retry. With the diff volume keys are simple and deterministic: they simply adjust the volume and don't trigger hot-keys or whatever. Tested on my laptop: it follows the principle of least astonishment, thus I like it. But as I said before, the problem is that this breaks the visual feedback feature in desktop environments and applications like Gnome. We really need a discussion about the desired behaviour of the volume keys that involves porters as well as a broader range of users. It is impossible to judge diffs like this one without having a clear picture of the desired end result. That said, I think that: 1. We need a kernel interface for toggling between the volume keys only directly manipulating the mixer and having them only generate events. This doesn't necessarily have to be a user knob; it could be something that applications that want to see events and manage the sound volume themselve would flip. 2. Mixer control needs to be integrated with sndio, such that applications that elect to receive events can act upon them by using a consistent API.
Re: pckbd volume keys (part 1), diff to test
On Mon, May 26, 2014 at 02:19:28PM +0200, Mark Kettenis wrote: But as I said before, the problem is that this breaks the visual feedback feature in desktop environments and applications like Gnome. The diff is to make pckbd keys behave like all other volume keys (thinkpad, asus, macppc). For gnome this means all buttons models work the same way. Anyway, to answer the gnome question (applies to any X program): (1) either the program is reading the mixer control to produce the visual feedback, in which case the diff doesn't break it. (2) or the program uses volume keys as hot-keys, in which case visual feedback is not properly working. We really need a discussion about the desired behaviour of the volume keys that involves porters as well as a broader range of users. It is impossible to judge diffs like this one without having a clear picture of the desired end result. That said, I think that: 1. We need a kernel interface for toggling between the volume keys only directly manipulating the mixer and having them only generate events. This doesn't necessarily have to be a user knob; it could be something that applications that want to see events and manage the sound volume themselve would flip. I've included such a diff for completeness (see my part 2 e-mail), for those who want to experiment with the concept. 2. Mixer control needs to be integrated with sndio, such that applications that elect to receive events can act upon them by using a consistent API. 100% agree, we've a almost working diff for this. But once the mixer is handled by sndio, we'll still need the pckbd fix discussed here. Without the fix volume keys would trigger two possibly inconsistent actions. Example, the user presses press the toggle mute button, the kernel/pckbd toggles the speakers mute control, then gnome/kde/whatever sees the hot-key, toggles the mute control again (through sndio), resulting in speakers not being muted. -- Alexandre
Re: pckbd volume keys (part 1), diff to test
On Fri, May 23, 2014 at 12:42:31PM +0200, Alexandre Ratchov wrote: On Wed, Apr 30, 2014 at 01:06:48AM +0200, Alexandre Ratchov wrote: This diff attempts to unify volume keys; it makes pckbd and ukbd volume keys behave like all other volume keys (acpithinkpad, acpiasus, macppc/abtn and similar drivers): simply adjust the hardware volume without passing keystroke events to upper layers (i.e. consume the keystroke). If your volume keys tend to mess the volume while in X (example mplayer), try this diff and see if it makes things better (or worse). No test reports so far. To test this: start X, then: - press the vol - button many times (don't hold is pressed), until volume goes to zero. - start a movie in mplayer, there's no sound as volume is zero. - press the vol + button and hold it down; now mplayer indicates the volume reached the maximum. Still you don't hear anything. Confusing, isn't it? Then rebuild the kernel with this diff and retry. With the diff volume keys are simple and deterministic: they simply adjust the volume and don't trigger hot-keys or whatever. -- Alexandre I tried it on a Thinkpad X201 and on a Samsung 900X3F (dmesg from both below). Works as advertised. I like it. Remi OpenBSD 5.5-current (GENERIC.MP) #142: Fri May 23 02:21:32 MDT 2014 t...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 8357658624 (7970MB) avail mem = 8126418944 (7749MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.6 @ 0xe0010 (78 entries) bios0: vendor LENOVO version 6QET61WW (1.31 ) date 10/26/2010 bios0: LENOVO 3626GN8 acpi0 at bios0: rev 2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP SSDT ECDT APIC MCFG HPET ASF! BOOT SSDT TCPA DMAR SSDT SSDT SSDT acpi0: wakeup devices LID_(S3) SLPB(S3) IGBE(S4) EXP1(S4) EXP2(S4) EXP3(S4) EXP4(S4) EXP5(S4) EHC1(S3) EHC2(S3) HDEF(S4) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpiec0 at acpi0 acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz, 2926.48 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 132MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.1.0, IBE cpu1 at mainbus0: apid 1 (application processor) cpu1: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz, 2926.00 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 1, core 0, package 0 cpu2 at mainbus0: apid 4 (application processor) cpu2: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz, 2926.00 MHz cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC cpu2: 256KB 64b/line 8-way L2 cache cpu2: smt 0, core 2, package 0 cpu3 at mainbus0: apid 5 (application processor) cpu3: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz, 2926.00 MHz cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC cpu3: 256KB 64b/line 8-way L2 cache cpu3: smt 1, core 2, package 0 ioapic0 at mainbus0: apid 1 pa 0xfec0, version 20, 24 pins ioapic0: misconfigured as apic 2, remapped to apid 1 acpimcfg0 at acpi0 addr 0xe000, bus 0-255 acpihpet0 at acpi0: 14318179 Hz acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus -1 (PEG_) acpiprt2 at acpi0: bus 13 (EXP1) acpiprt3 at acpi0: bus -1 (EXP2) acpiprt4 at acpi0: bus -1 (EXP3) acpiprt5 at acpi0: bus 5 (EXP4) acpiprt6 at acpi0: bus 2 (EXP5) acpicpu0 at acpi0: C3, C1, PSS acpicpu1 at acpi0: C3, C1, PSS acpicpu2 at acpi0: C3, C1, PSS acpicpu3 at acpi0: C3, C1, PSS acpipwrres0 at acpi0: PUBS, resource for EHC1, EHC2 acpitz0 at acpi0: critical temperature is 100 degC acpibtn0 at acpi0: LID_ acpibtn1 at acpi0: SLPB acpibat0 at acpi0: BAT0 model 42T4694 serial 1523 type LION oem SANYO acpibat1 at acpi0: BAT1 not present acpiac0 at acpi0: AC unit online acpithinkpad0 at acpi0 acpidock0 at acpi0: GDCK not docked (0) cpu0: Enhanced SpeedStep 2926 MHz: speeds: 2667, 2666, 2533, 2399, 2266, 2133, 1999, 1866, 1733, 1599, 1466, 1333, 1199 MHz pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 Intel Core Host rev 0x02 vga1
Re: pckbd volume keys (part 1), diff to test
On Wed, Apr 30, 2014 at 01:06:48AM +0200, Alexandre Ratchov wrote: This diff attempts to unify volume keys; it makes pckbd and ukbd volume keys behave like all other volume keys (acpithinkpad, acpiasus, macppc/abtn and similar drivers): simply adjust the hardware volume without passing keystroke events to upper layers (i.e. consume the keystroke). If your volume keys tend to mess the volume while in X (example mplayer), try this diff and see if it makes things better (or worse). No test reports so far. To test this: start X, then: - press the vol - button many times (don't hold is pressed), until volume goes to zero. - start a movie in mplayer, there's no sound as volume is zero. - press the vol + button and hold it down; now mplayer indicates the volume reached the maximum. Still you don't hear anything. Confusing, isn't it? Then rebuild the kernel with this diff and retry. With the diff volume keys are simple and deterministic: they simply adjust the volume and don't trigger hot-keys or whatever. -- Alexandre
Re: pckbd volume keys (part 1), diff to test
On Wed, Apr 30, 2014 at 01:06:48AM +0200, Alexandre Ratchov wrote:
This diff attempts to unify volume keys; it makes pckbd and ukbd
volume keys behave like all other volume keys (acpithinkpad,
acpiasus, macppc/abtn and similar drivers): simply adjust the
hardware volume without passing keystroke events to upper layers
(i.e. consume the keystroke).
If your volume keys tend to mess the volume while in X (example
mplayer), try this diff and see if it makes things better (or
worse).
+#ifdef WSDISPLAY_COMPAT_RAWKBD
+ rc = pckbd_decode(sc-id, data, type, key, sc-rawkbd);
+#else
+ rc = pckbd_decode(sc-id, data, type, key, 0);
^^
a new diff with the typo fixed, thanks to Kent R. Spillner
Index: dev/pckbc/pckbd.c
===
RCS file: /cvs/src/sys/dev/pckbc/pckbd.c,v
retrieving revision 1.37
diff -u -p -r1.37 pckbd.c
--- dev/pckbc/pckbd.c 23 Mar 2014 11:48:23 - 1.37
+++ dev/pckbc/pckbd.c 19 May 2014 07:31:57 -
@@ -177,7 +177,7 @@ int pckbd_init(struct pckbd_internal *,
void pckbd_input(void *, int);
static int pckbd_decode(struct pckbd_internal *, int,
- u_int *, int *);
+ u_int *, int *, int);
static int pckbd_led_encode(int);
struct pckbd_internal pckbd_consdata;
@@ -788,7 +788,8 @@ pckbd_scancode_translate(struct pckbd_in
}
static int
-pckbd_decode(struct pckbd_internal *id, int datain, u_int *type, int *dataout)
+pckbd_decode(struct pckbd_internal *id, int datain, u_int *type,
+int *dataout, int raw)
{
int key;
int releasing;
@@ -832,8 +833,8 @@ pckbd_decode(struct pckbd_internal *id,
id-t_lastchar = 0;
*type = WSCONS_EVENT_KEY_UP;
} else {
- /* Always ignore typematic keys */
- if (key == id-t_lastchar)
+ /* Always ignore typematic keys, except in raw-mode */
+ if (key == id-t_lastchar !raw)
return 0;
id-t_lastchar = key;
*type = WSCONS_EVENT_KEY_DOWN;
@@ -894,16 +895,25 @@ void
pckbd_input(void *vsc, int data)
{
struct pckbd_softc *sc = vsc;
- int rc, type, key;
+ int rc, type, key, consumed;
data = pckbd_scancode_translate(sc-id, data);
if (data == 0)
return;
- rc = pckbd_decode(sc-id, data, type, key);
+#ifdef WSDISPLAY_COMPAT_RAWKBD
+ rc = pckbd_decode(sc-id, data, type, key, sc-rawkbd);
+#else
+ rc = pckbd_decode(sc-id, data, type, key, 0);
+#endif
+ consumed = (rc != 0) ? wskbd_volkey(sc-sc_wskbddev, type, key) : 0;
#ifdef WSDISPLAY_COMPAT_RAWKBD
if (sc-rawkbd) {
+ if (consumed) {
+ sc-sc_rawcnt = 0;
+ return;
+ }
sc-sc_rawbuf[sc-sc_rawcnt++] = (char)data;
if (rc != 0 || sc-sc_rawcnt == sizeof(sc-sc_rawbuf)) {
@@ -911,15 +921,10 @@ pckbd_input(void *vsc, int data)
sc-sc_rawcnt);
sc-sc_rawcnt = 0;
}
-
- /*
-* Pass audio keys to wskbd_input anyway.
-*/
- if (rc == 0 || (key != 160 key != 174 key != 176))
- return;
+ return;
}
#endif
- if (rc != 0)
+ if (rc != 0 !consumed)
wskbd_input(sc-sc_wskbddev, type, key);
}
@@ -1024,7 +1029,7 @@ pckbd_cngetc(void *v, u_int *type, int *
if (val == 0)
continue;
- if (pckbd_decode(t, val, type, data))
+ if (pckbd_decode(t, val, type, data, 0))
return;
}
}
Index: dev/usb/hidkbd.c
===
RCS file: /cvs/src/sys/dev/usb/hidkbd.c,v
retrieving revision 1.12
diff -u -p -r1.12 hidkbd.c
--- dev/usb/hidkbd.c12 May 2014 09:50:44 - 1.12
+++ dev/usb/hidkbd.c19 May 2014 07:31:58 -
@@ -412,6 +412,9 @@ hidkbd_decode(struct hidkbd *kbd, struct
for (i = j = 0; i nkeys; i++) {
key = ibuf[i];
c = hidkbd_trtab[key CODEMASK];
+ if (wskbd_volkey(kbd-sc_wskbddev, (key RELEASE) ?
+ WSCONS_EVENT_KEY_UP : WSCONS_EVENT_KEY_DOWN, c))
+ continue;
if (c == NN)
continue;
if (c 0x80)
@@ -426,24 +429,7 @@ hidkbd_decode(struct hidkbd *kbd, struct
}
s = spltty();
wskbd_rawinput(kbd-sc_wskbddev, cbuf, j);
-
- /*
-* Pass audio keys to wskbd_input anyway.
-*/
- for (i = 0; i nkeys; i++) {
-
pckbd volume keys (part 1), diff to test
This diff attempts to unify volume keys; it makes pckbd and ukbd
volume keys behave like all other volume keys (acpithinkpad,
acpiasus, macppc/abtn and similar drivers): simply adjust the
hardware volume without passing keystroke events to upper layers
(i.e. consume the keystroke).
If your volume keys tend to mess the volume while in X (example
mplayer), try this diff and see if it makes things better (or
worse).
--- sys/dev/pckbc/pckbd.c.orig Tue Apr 29 19:29:09 2014
+++ sys/dev/pckbc/pckbd.c Tue Apr 29 22:46:30 2014
@@ -177,7 +177,7 @@ int pckbd_init(struct pckbd_internal *, pckbc_tag_t, p
void pckbd_input(void *, int);
static int pckbd_decode(struct pckbd_internal *, int,
- u_int *, int *);
+ u_int *, int *, int);
static int pckbd_led_encode(int);
struct pckbd_internal pckbd_consdata;
@@ -788,7 +788,8 @@ pckbd_scancode_translate(struct pckbd_internal *id, in
}
static int
-pckbd_decode(struct pckbd_internal *id, int datain, u_int *type, int *dataout)
+pckbd_decode(struct pckbd_internal *id, int datain, u_int *type,
+int *dataout, int raw)
{
int key;
int releasing;
@@ -832,8 +833,8 @@ pckbd_decode(struct pckbd_internal *id, int datain, u_
id-t_lastchar = 0;
*type = WSCONS_EVENT_KEY_UP;
} else {
- /* Always ignore typematic keys */
- if (key == id-t_lastchar)
+ /* Always ignore typematic keys, except in raw-mode */
+ if (key == id-t_lastchar !raw)
return 0;
id-t_lastchar = key;
*type = WSCONS_EVENT_KEY_DOWN;
@@ -894,16 +895,25 @@ void
pckbd_input(void *vsc, int data)
{
struct pckbd_softc *sc = vsc;
- int rc, type, key;
+ int rc, type, key, consumed;
data = pckbd_scancode_translate(sc-id, data);
if (data == 0)
return;
- rc = pckbd_decode(sc-id, data, type, key);
+#ifdef WSDISPLAY_COMPAT_RAWKBD
+ rc = pckbd_decode(sc-id, data, type, key, sc-rawkbd);
+#else
+ rc = pckbd_decode(sc-id, data, type, key, 0);
+#endif
+ consumed = (rc != 0) ? wskbd_volkey(sc-sc_wskbddev, type, key) : 0;
#ifdef WSDISPLAY_COMPAT_RAWKBD
if (sc-rawkbd) {
+ if (consumed) {
+ sc-sc_rawcnt = 0;
+ return;
+ }
sc-sc_rawbuf[sc-sc_rawcnt++] = (char)data;
if (rc != 0 || sc-sc_rawcnt == sizeof(sc-sc_rawbuf)) {
@@ -911,15 +921,10 @@ pckbd_input(void *vsc, int data)
sc-sc_rawcnt);
sc-sc_rawcnt = 0;
}
-
- /*
-* Pass audio keys to wskbd_input anyway.
-*/
- if (rc == 0 || (key != 160 key != 174 key != 176))
- return;
+ return;
}
#endif
- if (rc != 0)
+ if (rc != 0 !consumed)
wskbd_input(sc-sc_wskbddev, type, key);
}
@@ -1024,7 +1029,7 @@ pckbd_cngetc(void *v, u_int *type, int *data)
if (val == 0)
continue;
- if (pckbd_decode(t, val, type, data))
+ if (pckbd_decode(t, val, type, data, 0))
return;
}
}
--- sys/dev/usb/hidkbd.c.orig Tue Apr 29 19:29:09 2014
+++ sys/dev/usb/hidkbd.cTue Apr 29 19:29:24 2014
@@ -411,6 +411,9 @@ hidkbd_decode(struct hidkbd *kbd, struct hidkbd_data *
for (i = j = 0; i nkeys; i++) {
key = ibuf[i];
c = hidkbd_trtab[key CODEMASK];
+ if (wskbd_volkey(kbd-sc_wskbddev, (key RELEASE) ?
+ WSCONS_EVENT_KEY_UP : WSCONS_EVENT_KEY_DOWN, c))
+ continue;
if (c == NN)
continue;
if (c 0x80)
@@ -425,24 +428,7 @@ hidkbd_decode(struct hidkbd *kbd, struct hidkbd_data *
}
s = spltty();
wskbd_rawinput(kbd-sc_wskbddev, cbuf, j);
-
- /*
-* Pass audio keys to wskbd_input anyway.
-*/
- for (i = 0; i nkeys; i++) {
- key = ibuf[i];
- switch (key CODEMASK) {
- case 127:
- case 128:
- case 129:
- wskbd_input(kbd-sc_wskbddev,
- key RELEASE ? WSCONS_EVENT_KEY_UP :
- WSCONS_EVENT_KEY_DOWN, key CODEMASK);
- break;
- }
- }
splx(s);
-
return;
}
#endif
@@ -450,6 +436,10 @@ hidkbd_decode(struct hidkbd *kbd, struct hidkbd_data *
s =
