Bug 2243

Is this still a problem using a current version of xorg?
(Flagging as enhancement)

Yes, this is still a missing feature in xorg 7.0

There's something definatly wrong with the numlock handling though, sometimes
the num lock light goes out, but num lock is still active. When pressing num
lock, the light stays out (num lock is then actually disabled),press it again,
the light comes on and num lock is active again.

FYI: XGL seems to have a similair bug. It's got this feature (copys num lock
status from the terminal it's starting from), but doesn't reset the light on the
keyboard (light is on, num lock is off and visa versa). I realize this has got
little to do with the xorg project, but it might be using the same / similair
code (if I'm not mistaking XGL uses some of the modules from 7.0)

For the switch off of led, this is most probably https://bugs.freedesktop.org/show_bug.cgi?id=313

And yes, reading the leds state from the kernel when starting the server would be a real benefit

Bugzilla Upgrade Mass Bug Change

NEEDSINFO state was removed in Bugzilla 3.x, reopening any bugs previously listed as NEEDSINFO.

  - benjsc
    fd.o Wrangler

BTW, it should be just a matter of adding an OS-specific function that returns the current LED state, and to call it at initialization of the keyboard driver. On Linux, it's simply the ioctl(KDGETLED) call.

Exactly - I fixed this in the xf86-input-keyboard driver for Solaris in
commit 7148a0c3e04668b7608295ebdf94579f6acfa544, which simply queried 
the LED state at startup and posted fake lock key events to initialize
the capslock & numlock states.

(In reply to comment #5)
> BTW, it should be just a matter of adding an OS-specific function that returns
> the current LED state, and to call it at initialization of the keyboard driver.
> On Linux, it's simply the ioctl(KDGETLED) call.

if you can supply a patch, I'll push it.

Created attachment 13702 [details] [review]
Automatically inherit the vt's numlock and capslock status

This patch just works the se same as sun's.
Note however that it will inherit the target VT's status, not the one from which the user would run the server for instance. If that VT doesn't exist yet, then Linux kernel policies go into action and that's not an Xorg story.

(In reply to comment #8)
> Created an attachment (id=13702) [details]
> Automatically inherit the vt's numlock and capslock status
> 
> This patch just works the se same as sun's.
> Note however that it will inherit the target VT's status, not the one from
> which the user would run the server for instance. If that VT doesn't exist yet,
> then Linux kernel policies go into action and that's not an Xorg story.
> 

Pushed, thanks. 

commit 0f3716db01681876cc385727beeb842af5b950d3

  Hi,

  I had users reporting inverted num lock leds for some time.

  The problem is that mandriva has some a package named "numlock"
that is run when the X Server starts, and uses xkb calls to try
to make the X Server num lock led match the console led.

  I made a new patch that works with the mandriva "numlock" scripts
active or disabled, but it may be useful to others, so I am posting
it here.

Created attachment 14223 [details] [review]
0001-Alternate-patch-to-preserve-NumLock-CapsLock-led.patch

Your patch is definitely better.  My patch just reproduced the bug of the Sun case :)

>  Also, it is assumed that the os specific GetLeds will properly use bit 0 for caps lock and bit 1 for num lock.

Well, it is already the case :)

  Thanks. Adding myself to CC :-)

  Actually I wrote this new version first because of the
inverted num lock state (that I found out was caused by the
mandriva "numlock" package), but it also apparently fixed
another problem, where sometimes, when num lock was in the
correct state, it would flash when pressing caps lock.

  Also, the patch should work for all operating systems.

I've reverted my original Solaris patch & Samuel's Linux patch and committed
Paulo's in their place.

Reopening this bug because:

The keyboard now may send key events before it is enabled. The states for an input devices are 
DEVICE_INIT: device is present and being initialised
DEVICE_ON: device is switched on (enabled) and may now send events. 

The stages are separate, and a device that is only present should not generate events. With this patch here, the device sends release events during DEVICE_INIT. Admittedly this is only cosmetic in current master, but in MPX it brings the server down, since a device doesn't have sprites etc. associated until it is enabled.

Paulo: can you move the event generation to the DEVICE_ON stage? I think it is just be a few lines change but not sure how it would affect the results.

(In reply to comment #15)
> Reopening this bug because:
> 
> The keyboard now may send key events before it is enabled. The states for an
> input devices are 
> DEVICE_INIT: device is present and being initialised
> DEVICE_ON: device is switched on (enabled) and may now send events. 
> 
> The stages are separate, and a device that is only present should not generate
> events. With this patch here, the device sends release events during
> DEVICE_INIT. Admittedly this is only cosmetic in current master, but in MPX it
> brings the server down, since a device doesn't have sprites etc. associated
> until it is enabled.
> 
> Paulo: can you move the event generation to the DEVICE_ON stage? I think it is
> just be a few lines change but not sure how it would affect the results.

  The only thing I am a bit unsure is about hotplug; if it will reset the
associated xkb state when a device is disabled. Otherwise, sending the
events in a DEVICE_ON not following a DEVICE_INIT may confuse some
internal state.
  But doing something weird like unsetting leds at DEVICE_OFF may not be
the right thing to do, as it would also change the virtual console state,
so I am assuming that it is required (or a noop) to send again the events
in DEVICE_OFF followed by DEVICE_ON.

Created attachment 15765 [details] [review]
0001-Don-t-send-events-in-DEVICE_INIT-only-after-DEVICE_.patch

(In reply to comment #17)
> Created an attachment (id=15765) [details]
> 0001-Don-t-send-events-in-DEVICE_INIT-only-after-DEVICE_.patch


Looks good.

(In reply to comment #16)
>   The only thing I am a bit unsure is about hotplug; if it will reset the
> associated xkb state when a device is disabled. Otherwise, sending the
> events in a DEVICE_ON not following a DEVICE_INIT may confuse some
> internal state.

look at dix/devices.c:DisableDevice(). It doesn't to much other than move the device into off_devices and calling DEVICE_OFF.

Created attachment 15788 [details] [review]
0001-Don-t-send-events-in-DEVICE_INIT-only-after-DEVICE_.patch

(In reply to comment #18)
> >   The only thing I am a bit unsure is about hotplug; if it will reset the
> > associated xkb state when a device is disabled. Otherwise, sending the
> > events in a DEVICE_ON not following a DEVICE_INIT may confuse some
> > internal state.
> 
> look at dix/devices.c:DisableDevice(). It doesn't to much other than move the
> device into off_devices and calling DEVICE_OFF.

  In this new patch version, I added:
/* Used to know when the first DEVICE_ON after a DEVICE_INIT is called */
#define INITFLAG	(1 << 31)
as it is required to know in what state it is, so now it will only send
events if it is the first DEVICE_ON after a DEVICE_INIT, otherwise, it
will check if leds state changed while it was in DEVICE_OFF state.

  The only problem I can think of now is if different X input drivers are
used to access the same "kernel device" and change leds state, otherwise,
this patch should work properly if leds state are changed, during
DEVICE_OFF without the X server internals knowing about it; like doing a
vt switch a changing state from console or possibly another X Server.

(In reply to comment #19)
> Created an attachment (id=15788) [details]
> 0001-Don-t-send-events-in-DEVICE_INIT-only-after-DEVICE_.patch

please put parenthesis around the trinary operator and bitwise ANDs. I can't remember which one has precedence and its making it really hard to read.

>   The only problem I can think of now is if different X input drivers are
> used to access the same "kernel device" and change leds state

i'd say dont worry about this case unless you can come up with a common use-case that doesnt involve evdev :)

On Wed, Apr 09, 2008 at 07:37:18PM -0700, bugzilla-daemon@freedesktop.org wrote:
> --- Comment #20 from Peter Hutterer <peter@cs.unisa.edu.au>  2008-04-09 19:37:17 PST ---
> (In reply to comment #19)
> >   The only problem I can think of now is if different X input drivers are
> > used to access the same "kernel device" and change leds state
> 
> i'd say dont worry about this case unless you can come up with a common
> use-case that doesnt involve evdev :)

Er, even then, evdev does a grab, so as mentioned innumerable times
before, this can never happen and you shouldn't worry about it.

Pushed as a1866e2e73f0b401cd8e92fc9ee8db1791585936. Thanks.