Re:[PATCH v9] tty: Fix the keyboard led light display problem

Re:[PATCH v9] tty: Fix the keyboard led light display problem

[常廉志]

> Switching from the desktop environment to the tty environment,
> the state of the keyboard led lights and the state of the keyboard
> lock are inconsistent. This is because the attribute kb->kbdmode
> of the tty bound in the desktop environment (Xorg) is set to
> VC_OFF, which causes the ledstate and kb->ledflagstate
> values of the bound tty to always be 0, which causes the switch
> from the desktop When to the tty environment, the LED light
> status is inconsistent with the keyboard lock status.
> 
> Signed-off-by: lianzhi chang <changlianzhi@uniontech.com>
> ---
>  v7-->v8:
>  Optimize the implementation of kbd_update_ledstate function
 > 
>  Why not adopt the opinions of Greg KH and Andy Shevchenko:
>  (1) In the structure struct input_dev, the definition of led is
>  like this: unsigned long led[BITS_TO_LONGS(LED_CNT)]; If you
>  define it like this: unsigned long newstate = *dev->led; I
>  always feel that there is still big end and Little endian problem.
>  (2) The test_bit function is used to avoid the problem of large
>  and small ends, and the current algorithm (v8) also exists
>  elsewhere in the kernel: the atkbd_set_leds function (drivers/
>  input/keyboard/atkbd.c).
>  (3) In the current keyboard.c code, the code is already very good,
>  and it is already relatively independent. If you modify the type
>  of ledstate to u64 or modify the macro definitions such as
>  VC_NUMLOCK, it feels that it is not very meaningful, and this It
>  will also cause other related modifications. Of course, this is
>  only my current opinion. If everyone still feels that it is
>  necessary to modify, I will do it this way. Of course, this
>  process may be a bit longer, and I think it is necessary to
>  conduct more tests.
>  
>  v9: Change description information: xorg-->Xorg
> ……

Hi, friends, I would like to ask whether this version of patch is possible, if not,
I will try my best to find a way to complete the next version!
Thanks.
--
lianzhi chang

Re: [PATCH v9] tty: Fix the keyboard led light display problem

[Greg KH]

On Mon, Nov 01, 2021 at 08:35:47PM +0800, 常廉志 wrote:
> > Switching from the desktop environment to the tty environment,
> > the state of the keyboard led lights and the state of the keyboard
> > lock are inconsistent. This is because the attribute kb->kbdmode
> > of the tty bound in the desktop environment (Xorg) is set to
> > VC_OFF, which causes the ledstate and kb->ledflagstate
> > values of the bound tty to always be 0, which causes the switch
> > from the desktop When to the tty environment, the LED light
> > status is inconsistent with the keyboard lock status.
> > 
> > Signed-off-by: lianzhi chang <changlianzhi@uniontech.com>
> > ---
> >  v7-->v8:
> >  Optimize the implementation of kbd_update_ledstate function
>  > 
> >  Why not adopt the opinions of Greg KH and Andy Shevchenko:
> >  (1) In the structure struct input_dev, the definition of led is
> >  like this: unsigned long led[BITS_TO_LONGS(LED_CNT)]; If you
> >  define it like this: unsigned long newstate = *dev->led; I
> >  always feel that there is still big end and Little endian problem.
> >  (2) The test_bit function is used to avoid the problem of large
> >  and small ends, and the current algorithm (v8) also exists
> >  elsewhere in the kernel: the atkbd_set_leds function (drivers/
> >  input/keyboard/atkbd.c).
> >  (3) In the current keyboard.c code, the code is already very good,
> >  and it is already relatively independent. If you modify the type
> >  of ledstate to u64 or modify the macro definitions such as
> >  VC_NUMLOCK, it feels that it is not very meaningful, and this It
> >  will also cause other related modifications. Of course, this is
> >  only my current opinion. If everyone still feels that it is
> >  necessary to modify, I will do it this way. Of course, this
> >  process may be a bit longer, and I think it is necessary to
> >  conduct more tests.
> >  
> >  v9: Change description information: xorg-->Xorg
> > ……
> 
> Hi, friends, I would like to ask whether this version of patch is possible, if not,
> I will try my best to find a way to complete the next version!

It's the merge window right now, we can't do anything with this until
after 5.16-rc1 is out.  So give us until then at the least please, then
I will review it again.

thanks,

greg k-h

Re: [PATCH v9] tty: Fix the keyboard led light display problem

[dmitry.torokhov]

On Mon, Nov 01, 2021 at 01:48:25PM +0100, Greg KH wrote:
> On Mon, Nov 01, 2021 at 08:35:47PM +0800, 常廉志 wrote:
> > > Switching from the desktop environment to the tty environment,
> > > the state of the keyboard led lights and the state of the keyboard
> > > lock are inconsistent. This is because the attribute kb->kbdmode
> > > of the tty bound in the desktop environment (Xorg) is set to
> > > VC_OFF, which causes the ledstate and kb->ledflagstate
> > > values of the bound tty to always be 0, which causes the switch
> > > from the desktop When to the tty environment, the LED light
> > > status is inconsistent with the keyboard lock status.
> > > 
> > > Signed-off-by: lianzhi chang <changlianzhi@uniontech.com>
> > > ---
> > >  v7-->v8:
> > >  Optimize the implementation of kbd_update_ledstate function
> >  > 
> > >  Why not adopt the opinions of Greg KH and Andy Shevchenko:
> > >  (1) In the structure struct input_dev, the definition of led is
> > >  like this: unsigned long led[BITS_TO_LONGS(LED_CNT)]; If you
> > >  define it like this: unsigned long newstate = *dev->led; I
> > >  always feel that there is still big end and Little endian problem.
> > >  (2) The test_bit function is used to avoid the problem of large
> > >  and small ends, and the current algorithm (v8) also exists
> > >  elsewhere in the kernel: the atkbd_set_leds function (drivers/
> > >  input/keyboard/atkbd.c).
> > >  (3) In the current keyboard.c code, the code is already very good,
> > >  and it is already relatively independent. If you modify the type
> > >  of ledstate to u64 or modify the macro definitions such as
> > >  VC_NUMLOCK, it feels that it is not very meaningful, and this It
> > >  will also cause other related modifications. Of course, this is
> > >  only my current opinion. If everyone still feels that it is
> > >  necessary to modify, I will do it this way. Of course, this
> > >  process may be a bit longer, and I think it is necessary to
> > >  conduct more tests.
> > >  
> > >  v9: Change description information: xorg-->Xorg
> > > ……
> > 
> > Hi, friends, I would like to ask whether this version of patch is possible, if not,
> > I will try my best to find a way to complete the next version!
> 
> It's the merge window right now, we can't do anything with this until
> after 5.16-rc1 is out.  So give us until then at the least please, then
> I will review it again.

As I mentioned, the state of physical LED on a keyboard does not
necessarily reflect state of logical LED state in tty/vt. One can assign
LED on their keyboard to be an indicator of being connected to mains by
assigning "AC-trigger" to it. So the way this patch tries to fix the
issue (by reading internal state of an individual input device) is
wrong.

We keep separate led and lock states for each VT instance, and they
should be applied when switching to/from a VT. Are you saying that in
certain scenarios this switch is not happening? Can see if that can be
fixed?

Thanks.

-- 
Dmitry

Re: [PATCH v9] tty: Fix the keyboard led light display problem

[常廉志]

On Mon, Nov 01, 2021 at 01:48:25PM +0100, Greg KH wrote:
> > On Mon, Nov 01, 2021 at 08:35:47PM +0800, 常廉志 wrote:
> > > > Switching from the desktop environment to the tty environment,
> > > > the state of the keyboard led lights and the state of the keyboard
> > > > lock are inconsistent. This is because the attribute kb->kbdmode
> > > > of the tty bound in the desktop environment (Xorg) is set to
> > > > VC_OFF, which causes the ledstate and kb->ledflagstate
> > > > values of the bound tty to always be 0, which causes the switch
> > > > from the desktop When to the tty environment, the LED light
> > > > status is inconsistent with the keyboard lock status.
> > > >
> > > > Signed-off-by: lianzhi chang <changlianzhi@uniontech.com>
> > > > ---
> > > >  v7-->v8:
> > > >  Optimize the implementation of kbd_update_ledstate function
> > >  >
> > > >  Why not adopt the opinions of Greg KH and Andy Shevchenko:
> > > >  (1) In the structure struct input_dev, the definition of led is
> > > >  like this: unsigned long led[BITS_TO_LONGS(LED_CNT)]; If you
> > > >  define it like this: unsigned long newstate = *dev->led; I
> > > >  always feel that there is still big end and Little endian problem.
> > > >  (2) The test_bit function is used to avoid the problem of large
> > > >  and small ends, and the current algorithm (v8) also exists
> > > >  elsewhere in the kernel: the atkbd_set_leds function (drivers/
> > > >  input/keyboard/atkbd.c).
> > > >  (3) In the current keyboard.c code, the code is already very good,
> > > >  and it is already relatively independent. If you modify the type
> > > >  of ledstate to u64 or modify the macro definitions such as
> > > >  VC_NUMLOCK, it feels that it is not very meaningful, and this It
> > > >  will also cause other related modifications. Of course, this is
> > > >  only my current opinion. If everyone still feels that it is
> > > >  necessary to modify, I will do it this way. Of course, this
> > > >  process may be a bit longer, and I think it is necessary to
> > > >  conduct more tests.
> > > > 
> > > >  v9: Change description information: xorg-->Xorg
> > > > ……
> > >
> > > Hi, friends, I would like to ask whether this version of patch is possible, if not,
> > > I will try my best to find a way to complete the next version!
> >
> > It's the merge window right now, we can't do anything with this until
> > after 5.16-rc1 is out.  So give us until then at the least please, then
> > I will review it again.
> 
> As I mentioned, the state of physical LED on a keyboard does not
> necessarily reflect state of logical LED state in tty/vt. One can assign
> LED on their keyboard to be an indicator of being connected to mains by
> assigning "AC-trigger" to it. So the way this patch tries to fix the
> issue (by reading internal state of an individual input device) is
> wrong.
> 
> We keep separate led and lock states for each VT instance, and they
> should be applied when switching to/from a VT. Are you saying that in
> certain scenarios this switch is not happening? Can see if that can be
> fixed?
> 

Hi Dmitry, I don’t know if I fully understand what you mean, but I will 
try to fully explain the intent of the current patch.
(1) What is the current bug phenomenon? I will describe with the Num 
Lock indicator as the object here.

Phenomenon 1: Suppose that Xorg is bound to tty1 in the desktop environment. 
At this time, the Num light of the keyboard is on, and the keypad can input numbers 
normally; assume that the state of the keyboard light saved by tty2 itself is the 
opposite (the Num light is off, The keypad cannot enter numbers); at this time, 
if we use the key combination "ctrl+alt+F2" to switch the system to tty2, we will find 
that the Num light is still on, but the keypad cannot enter numbers.

Phenomenon 2: Assuming that you are currently in the tty2 environment, the Num 
light of the keyboard is on, and the keypad can input numbers normally; assume that 
the Num state saved by Xorg is the same as that of tty2 (the Num light is on, and the 
keypad can input numbers normally); At this point, if we use the key combination 
"ctrl+alt+F1" to switch the system to tty1 (that is, to switch to the desktop environment)
, we will find that the Num light will not light up, but the small keyboard can input numbers .

(2) Why do these two phenomena occur?
The variable static unsigned int ledstate is defined in keyboard.c. ledstate should be used to 
tell VT the current state of the keyboard light, because after each VT sets the state of the 
keyboard light, it will synchronize the latest keyboard light state to ledstate( (Implemented 
in the kbd_bh() function).

Then the problem comes. The scope of ledstate is only in VT, and it cannot include all the 
scenes where the keyboard light is set. And, in the desktop environment, "kb->kbdmode == 
VC_OFF" of tty1, at this time, through the NumLock button, only Xorg's own state can be 
changed, and the led state stored by tty1 cannot be changed (implemented in the kbd_keycode() 
function), This results in that the kb->ledflagstate stored by tty1 itself and the ledstate in the tty 
environment are always 0 at this time.

When we switch tty, the VT code compares the current tty's kb->ledflagstate and ledstate values.
If they are inconsistent, change the state of the keyboard light (implemented in the kbd_bh() function).

In phenomenon 1, in the desktop environment, although the actual state saved by xorg is 1, the state 
of ledstate of tty is always 0. In the environment of tty2, the state of kb->ledflagstate of tty2 is also 0. 
At this point, in the kbd_bh() function, comparing these two values ​​is equal, there is no need to set the 
led light state to the keyboard. So after switching to tty2, the Num light is still on, but the small 
keyboard cannot input numbers.

In phenomenon 2, in the tty2 environment, the state of ledstate is set to 1, but the kb->ledflagstate of
tty1 is 0. At this time, the two values ​​are not equal in the kbd_bh() function, so set the led The light
status to the keyboard. Xorg did not redistribute the configuration during this process is also one of
the reasons. And even if Xorg re-issues the configuration at this time, it will cause confusion and only
one can be set.

(3) How to solve it?
To solve the problem of phenomenon 1, we must first enable ledstate to correctly reflect the current 
state of the keyboard light. Therefore, the solution to all versions of patch is to synchronize the 
latest led state of the input device to ledstate.
At the same time, to solve the problem of phenomenon 2, when switching to tty1, kbd_bh() also 
determines the current tty's "kb->kbdmode == VC_OFF". If it is true, don't set the keyboard light 
state. At the same time, Xorg should also re-issue the status of the keyboard light to ensure that it 
is correct (I also submitted a patch for this, refer to 
https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/764).

(4) At this time, each VT instance still maintains a separate led and lock state, which are applied 
when switching to a VT. This is unaffected.

Thanks.
--
lianzhi chang

Re: [PATCH v9] tty: Fix the keyboard led light display problem

[dmitry.torokhov]

Hi lianzhi,

On Tue, Nov 02, 2021 at 11:16:56AM +0800, 常廉志 wrote:
> 
> Hi Dmitry, I don’t know if I fully understand what you mean, but I will 
> try to fully explain the intent of the current patch.
> (1) What is the current bug phenomenon? I will describe with the Num 
> Lock indicator as the object here.
> 
> Phenomenon 1: Suppose that Xorg is bound to tty1 in the desktop environment. 
> At this time, the Num light of the keyboard is on, and the keypad can input numbers 
> normally; assume that the state of the keyboard light saved by tty2 itself is the 
> opposite (the Num light is off, The keypad cannot enter numbers); at this time, 
> if we use the key combination "ctrl+alt+F2" to switch the system to tty2, we will find 
> that the Num light is still on, but the keypad cannot enter numbers.
> 
> Phenomenon 2: Assuming that you are currently in the tty2 environment, the Num 
> light of the keyboard is on, and the keypad can input numbers normally; assume that 
> the Num state saved by Xorg is the same as that of tty2 (the Num light is on, and the 
> keypad can input numbers normally); At this point, if we use the key combination 
> "ctrl+alt+F1" to switch the system to tty1 (that is, to switch to the desktop environment)
> , we will find that the Num light will not light up, but the small keyboard can input numbers .
> 
> (2) Why do these two phenomena occur?
> The variable static unsigned int ledstate is defined in keyboard.c. ledstate should be used to 
> tell VT the current state of the keyboard light, because after each VT sets the state of the 
> keyboard light, it will synchronize the latest keyboard light state to ledstate( (Implemented 
> in the kbd_bh() function).
> 
> Then the problem comes. The scope of ledstate is only in VT, and it cannot include all the 
> scenes where the keyboard light is set. And, in the desktop environment, "kb->kbdmode == 
> VC_OFF" of tty1, at this time, through the NumLock button, only Xorg's own state can be 
> changed, and the led state stored by tty1 cannot be changed (implemented in the kbd_keycode() 
> function), This results in that the kb->ledflagstate stored by tty1 itself and the ledstate in the tty 
> environment are always 0 at this time.
> 
> When we switch tty, the VT code compares the current tty's kb->ledflagstate and ledstate values.
> If they are inconsistent, change the state of the keyboard light (implemented in the kbd_bh() function).
> 
> In phenomenon 1, in the desktop environment, although the actual state saved by xorg is 1, the state 
> of ledstate of tty is always 0. In the environment of tty2, the state of kb->ledflagstate of tty2 is also 0. 
> At this point, in the kbd_bh() function, comparing these two values ​​is equal, there is no need to set the 
> led light state to the keyboard. So after switching to tty2, the Num light is still on, but the small 
> keyboard cannot input numbers.
> 
> In phenomenon 2, in the tty2 environment, the state of ledstate is set to 1, but the kb->ledflagstate of
> tty1 is 0. At this time, the two values ​​are not equal in the kbd_bh() function, so set the led The light
> status to the keyboard. Xorg did not redistribute the configuration during this process is also one of
> the reasons. And even if Xorg re-issues the configuration at this time, it will cause confusion and only
> one can be set.
> 
> (3) How to solve it?
> To solve the problem of phenomenon 1, we must first enable ledstate to correctly reflect the current 
> state of the keyboard light. Therefore, the solution to all versions of patch is to synchronize the 
> latest led state of the input device to ledstate.

You assume that input's device NumLock LED reflects the state of
terminal. That does not have to be the case.

Now how to solve this... On VT switch redraw_screen() calls
vt_set_leds_compute_shiftstate(). Can we do something like:

	/*
	 * On VT switch pretend our led state is opposite of target
	 * state to ensure we refresh all leds.
	 */
	spin_lock_irqsave(&led_lock, flags);
	leds = getleds();
	leds |= (unsigned int)kbd->lockstate << 8;
	ledstate = ~leds;
	spin_unlock_irqrestore(&led_lock, flags);

	set_leds();

?

> At the same time, to solve the problem of phenomenon 2, when switching to tty1, kbd_bh() also 
> determines the current tty's "kb->kbdmode == VC_OFF". If it is true, don't set the keyboard light 
> state. At the same time, Xorg should also re-issue the status of the keyboard light to ensure that it 
> is correct (I also submitted a patch for this, refer to 
> https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/764).

That makes sense.

Thanks.

-- 
Dmitry

Re: [PATCH v9] tty: Fix the keyboard led light display problem

[常廉志]

> >
> > Hi Dmitry, I don’t know if I fully understand what you mean, but I will
> > try to fully explain the intent of the current patch.
> > (1) What is the current bug phenomenon? I will describe with the Num
> > Lock indicator as the object here.
> >
> > Phenomenon 1: Suppose that Xorg is bound to tty1 in the desktop environment.
> > At this time, the Num light of the keyboard is on, and the keypad can input numbers
> > normally; assume that the state of the keyboard light saved by tty2 itself is the
> > opposite (the Num light is off, The keypad cannot enter numbers); at this time,
> > if we use the key combination "ctrl+alt+F2" to switch the system to tty2, we will find
> > that the Num light is still on, but the keypad cannot enter numbers.
> >
> > Phenomenon 2: Assuming that you are currently in the tty2 environment, the Num
> > light of the keyboard is on, and the keypad can input numbers normally; assume that
> > the Num state saved by Xorg is the same as that of tty2 (the Num light is on, and the
> > keypad can input numbers normally); At this point, if we use the key combination
> > "ctrl+alt+F1" to switch the system to tty1 (that is, to switch to the desktop environment)
> > , we will find that the Num light will not light up, but the small keyboard can input numbers .
> >
> > (2) Why do these two phenomena occur?
> > The variable static unsigned int ledstate is defined in keyboard.c. ledstate should be used to
> > tell VT the current state of the keyboard light, because after each VT sets the state of the
> > keyboard light, it will synchronize the latest keyboard light state to ledstate( (Implemented
> > in the kbd_bh() function).
> >
> > Then the problem comes. The scope of ledstate is only in VT, and it cannot include all the
> > scenes where the keyboard light is set. And, in the desktop environment, "kb->kbdmode ==
> > VC_OFF" of tty1, at this time, through the NumLock button, only Xorg's own state can be
> > changed, and the led state stored by tty1 cannot be changed (implemented in the kbd_keycode()
> > function), This results in that the kb->ledflagstate stored by tty1 itself and the ledstate in the tty
> > environment are always 0 at this time.
> >
> > When we switch tty, the VT code compares the current tty's kb->ledflagstate and ledstate values.
> > If they are inconsistent, change the state of the keyboard light (implemented in the kbd_bh() function).
> >
> > In phenomenon 1, in the desktop environment, although the actual state saved by xorg is 1, the state
> > of ledstate of tty is always 0. In the environment of tty2, the state of kb->ledflagstate of tty2 is also 0.
> > At this point, in the kbd_bh() function, comparing these two values ​​is equal, there is no need to set the
> > led light state to the keyboard. So after switching to tty2, the Num light is still on, but the small
> > keyboard cannot input numbers.
> >
> > In phenomenon 2, in the tty2 environment, the state of ledstate is set to 1, but the kb->ledflagstate of
> > tty1 is 0. At this time, the two values ​​are not equal in the kbd_bh() function, so set the led The light
> > status to the keyboard. Xorg did not redistribute the configuration during this process is also one of
> > the reasons. And even if Xorg re-issues the configuration at this time, it will cause confusion and only
> > one can be set.
> >
> > (3) How to solve it?
> > To solve the problem of phenomenon 1, we must first enable ledstate to correctly reflect the current
> > state of the keyboard light. Therefore, the solution to all versions of patch is to synchronize the
> > latest led state of the input device to ledstate.

> You assume that input's device NumLock LED reflects the state of
> terminal. That does not have to be the case.

> Now how to solve this... On VT switch redraw_screen() calls
> vt_set_leds_compute_shiftstate(). Can we do something like:

> /*
> * On VT switch pretend our led state is opposite of target
> * state to ensure we refresh all leds.
> */
> spin_lock_irqsave(&led_lock, flags);
> leds = getleds();
> leds |= (unsigned int)kbd->lockstate << 8;
> ledstate = ~leds;
> spin_unlock_irqrestore(&led_lock, flags);
> 
> set_leds();
> 
> ?
Hi Dmitry:
/*
* The following piece of code exists in the kbd_bh() function
*/
spin_lock_irqsave(&led_lock, flags);
leds = getleds();
leds |= (unsigned int)kbd->lockstate << 8;
ledstate = ~leds;
spin_unlock_irqrestore(&led_lock, flags);

Moreover, the process of calling the set_leds() function is 
the process of calling the kbd_bh function:
static void set_leds(void)
{
tasklet_schedule(&keyboard_tasklet);
}
static DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh);

I don't really understand what you mean here, but one thing 
can be confirmed, my patch just synchronizes the current input 
device's led state to ledstate. Moreover, after VT's 
kb->ledflagstate is set to the input device, it will also 
be synchronized to ledstate (the original logic of the kbd_bh() 
function), which does not destroy the original internal logic of 
VT. In addition, I have tested it, whether it is switching 
between the desktop environment (tty1) and tty2~6, or switching 
between tty2~6, the indicator status of the keyboard light is 
correct, and it is normal in the multi-keyboard state. . 
Of course, I need to add the Xorg repair patch I mentioned 
earlier.

Thanks.
--
lianzhi chang

Re: [PATCH v9] tty: Fix the keyboard led light display problem

[dmitry.torokhov]

On Tue, Nov 02, 2021 at 03:09:20PM +0800, 常廉志 wrote:
> > >
> > > Hi Dmitry, I don’t know if I fully understand what you mean, but I will
> > > try to fully explain the intent of the current patch.
> > > (1) What is the current bug phenomenon? I will describe with the Num
> > > Lock indicator as the object here.
> > >
> > > Phenomenon 1: Suppose that Xorg is bound to tty1 in the desktop environment.
> > > At this time, the Num light of the keyboard is on, and the keypad can input numbers
> > > normally; assume that the state of the keyboard light saved by tty2 itself is the
> > > opposite (the Num light is off, The keypad cannot enter numbers); at this time,
> > > if we use the key combination "ctrl+alt+F2" to switch the system to tty2, we will find
> > > that the Num light is still on, but the keypad cannot enter numbers.
> > >
> > > Phenomenon 2: Assuming that you are currently in the tty2 environment, the Num
> > > light of the keyboard is on, and the keypad can input numbers normally; assume that
> > > the Num state saved by Xorg is the same as that of tty2 (the Num light is on, and the
> > > keypad can input numbers normally); At this point, if we use the key combination
> > > "ctrl+alt+F1" to switch the system to tty1 (that is, to switch to the desktop environment)
> > > , we will find that the Num light will not light up, but the small keyboard can input numbers .
> > >
> > > (2) Why do these two phenomena occur?
> > > The variable static unsigned int ledstate is defined in keyboard.c. ledstate should be used to
> > > tell VT the current state of the keyboard light, because after each VT sets the state of the
> > > keyboard light, it will synchronize the latest keyboard light state to ledstate( (Implemented
> > > in the kbd_bh() function).
> > >
> > > Then the problem comes. The scope of ledstate is only in VT, and it cannot include all the
> > > scenes where the keyboard light is set. And, in the desktop environment, "kb->kbdmode ==
> > > VC_OFF" of tty1, at this time, through the NumLock button, only Xorg's own state can be
> > > changed, and the led state stored by tty1 cannot be changed (implemented in the kbd_keycode()
> > > function), This results in that the kb->ledflagstate stored by tty1 itself and the ledstate in the tty
> > > environment are always 0 at this time.
> > >
> > > When we switch tty, the VT code compares the current tty's kb->ledflagstate and ledstate values.
> > > If they are inconsistent, change the state of the keyboard light (implemented in the kbd_bh() function).
> > >
> > > In phenomenon 1, in the desktop environment, although the actual state saved by xorg is 1, the state
> > > of ledstate of tty is always 0. In the environment of tty2, the state of kb->ledflagstate of tty2 is also 0.
> > > At this point, in the kbd_bh() function, comparing these two values ​​is equal, there is no need to set the
> > > led light state to the keyboard. So after switching to tty2, the Num light is still on, but the small
> > > keyboard cannot input numbers.
> > >
> > > In phenomenon 2, in the tty2 environment, the state of ledstate is set to 1, but the kb->ledflagstate of
> > > tty1 is 0. At this time, the two values ​​are not equal in the kbd_bh() function, so set the led The light
> > > status to the keyboard. Xorg did not redistribute the configuration during this process is also one of
> > > the reasons. And even if Xorg re-issues the configuration at this time, it will cause confusion and only
> > > one can be set.
> > >
> > > (3) How to solve it?
> > > To solve the problem of phenomenon 1, we must first enable ledstate to correctly reflect the current
> > > state of the keyboard light. Therefore, the solution to all versions of patch is to synchronize the
> > > latest led state of the input device to ledstate.
> 
> > You assume that input's device NumLock LED reflects the state of
> > terminal. That does not have to be the case.
> 
> > Now how to solve this... On VT switch redraw_screen() calls
> > vt_set_leds_compute_shiftstate(). Can we do something like:
> 
> > /*
> > * On VT switch pretend our led state is opposite of target
> > * state to ensure we refresh all leds.
> > */
> > spin_lock_irqsave(&led_lock, flags);
> > leds = getleds();
> > leds |= (unsigned int)kbd->lockstate << 8;
> > ledstate = ~leds;
> > spin_unlock_irqrestore(&led_lock, flags);
> > 
> > set_leds();
> > 
> > ?
> Hi Dmitry:
> /*
> * The following piece of code exists in the kbd_bh() function
> */
> spin_lock_irqsave(&led_lock, flags);
> leds = getleds();
> leds |= (unsigned int)kbd->lockstate << 8;
> ledstate = ~leds;

  ^^^^^^^^^^^^^^^^^

This is not the exact code that exists in kbd_bh(). It lacks the line
above which should cause LEDs be synchronized once set_leds()/kbd_bh()
runs.

> spin_unlock_irqrestore(&led_lock, flags);
> 
> Moreover, the process of calling the set_leds() function is 
> the process of calling the kbd_bh function:
> static void set_leds(void)
> {
> tasklet_schedule(&keyboard_tasklet);
> }
> static DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh);
> 
> I don't really understand what you mean here, but one thing 
> can be confirmed, my patch just synchronizes the current input 
> device's led state to ledstate. Moreover, after VT's 
> kb->ledflagstate is set to the input device, it will also 
> be synchronized to ledstate (the original logic of the kbd_bh() 
> function), which does not destroy the original internal logic of 
> VT. In addition, I have tested it, whether it is switching 
> between the desktop environment (tty1) and tty2~6, or switching 
> between tty2~6, the indicator status of the keyboard light is 
> correct, and it is normal in the multi-keyboard state. . 

And I keep telling you that your approach to solving the problem is not
correct because state of a random input device is not necessarily
connected to the state of a VT.

Thanks.

-- 
Dmitry

[PATCH v9] tty: Fix the keyboard led light display problem

[lianzhi chang]

Switching from the desktop environment to the tty environment,
the state of the keyboard led lights and the state of the keyboard
lock are inconsistent. This is because the attribute kb->kbdmode
of the tty bound in the desktop environment (Xorg) is set to
VC_OFF, which causes the ledstate and kb->ledflagstate
values of the bound tty to always be 0, which causes the switch
from the desktop When to the tty environment, the LED light
status is inconsistent with the keyboard lock status.

Signed-off-by: lianzhi chang <changlianzhi@uniontech.com>
---
 v7-->v8:
 Optimize the implementation of kbd_update_ledstate function
 
 Why not adopt the opinions of Greg KH and Andy Shevchenko:
 (1) In the structure struct input_dev, the definition of led is
 like this: unsigned long led[BITS_TO_LONGS(LED_CNT)]; If you
 define it like this: unsigned long newstate = *dev->led; I
 always feel that there is still big end and Little endian problem.
 (2) The test_bit function is used to avoid the problem of large
 and small ends, and the current algorithm (v8) also exists
 elsewhere in the kernel: the atkbd_set_leds function (drivers/
 input/keyboard/atkbd.c).
 (3) In the current keyboard.c code, the code is already very good,
 and it is already relatively independent. If you modify the type
 of ledstate to u64 or modify the macro definitions such as
 VC_NUMLOCK, it feels that it is not very meaningful, and this It
 will also cause other related modifications. Of course, this is
 only my current opinion. If everyone still feels that it is
 necessary to modify, I will do it this way. Of course, this
 process may be a bit longer, and I think it is necessary to
 conduct more tests.
 
 v9: Change description information: xorg-->Xorg
 
 drivers/tty/vt/keyboard.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index c7fbbcdcc346..f564fcf61304 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -1130,6 +1130,13 @@ static void kbd_init_leds(void)
 
 #endif
 
+static void kbd_update_ledstate(struct input_dev *dev)
+{
+	ledstate = (test_bit(LED_SCROLLL, dev->led) ? BIT(VC_SCROLLOCK) : 0)
+		 | (test_bit(LED_NUML,    dev->led) ? BIT(VC_NUMLOCK) : 0)
+		 | (test_bit(LED_CAPSL,   dev->led) ? BIT(VC_CAPSLOCK) : 0);
+}
+
 /*
  * The leds display either (i) the status of NumLock, CapsLock, ScrollLock,
  * or (ii) whatever pattern of lights people want to show using KDSETLED,
@@ -1247,9 +1254,14 @@ void vt_kbd_con_stop(unsigned int console)
  */
 static void kbd_bh(struct tasklet_struct *unused)
 {
+	struct kbd_struct *kb;
 	unsigned int leds;
 	unsigned long flags;
 
+	kb = kbd_table + fg_console;
+	if (kb->kbdmode == VC_OFF)
+		return;
+
 	spin_lock_irqsave(&led_lock, flags);
 	leds = getleds();
 	leds |= (unsigned int)kbd->lockstate << 8;
@@ -1524,6 +1536,9 @@ static void kbd_event(struct input_handle *handle, unsigned int event_type,
 	/* We are called with interrupts disabled, just take the lock */
 	spin_lock(&kbd_event_lock);
 
+	if (test_bit(EV_LED, handle->dev->evbit))
+		kbd_update_ledstate(handle->dev);
+
 	if (event_type == EV_MSC && event_code == MSC_RAW &&
 			kbd_is_hw_raw(handle->dev))
 		kbd_rawcode(value);
-- 
2.20.1