* [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
@ 2016-12-22 13:43 Star Zeng
2016-12-23 11:53 ` Ni, Ruiyu
0 siblings, 1 reply; 5+ messages in thread
From: Star Zeng @ 2016-12-22 13:43 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Ruiyu Ni, Michael Kinney, Feng Tian
Register key notify for toggle state (CapsLock, NumLock and ScrollLock)
sync between multiple keyboards.
The implementation for this feature requires keyboard driver supports
EFI_KEY_STATE_EXPOSED, and turns on physical TextInEx partial key
report for toggle state sync.
The virtual TextInEx will report the partial key after it is required
by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
Cc: Ruiyu Ni <Ruiyu.ni@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
.../Universal/Console/ConSplitterDxe/ConSplitter.c | 208 ++++++++++++++++++++-
.../Universal/Console/ConSplitterDxe/ConSplitter.h | 5 +-
2 files changed, 206 insertions(+), 7 deletions(-)
diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
index 493bcbafdf39..203ad7b06e20 100644
--- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
+++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
@@ -67,6 +67,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED TEXT_IN_SPLITTER_PRIVATE_DATA mConIn = {
(LIST_ENTRY *) NULL,
(LIST_ENTRY *) NULL
},
+ 0,
+ FALSE,
{
ConSplitterSimplePointerReset,
@@ -301,6 +303,157 @@ EFI_DRIVER_BINDING_PROTOCOL gConSplitterAbsolutePointerDriverBinding =
};
/**
+ Sync current toggle state to new console input device.
+
+ @param TextInEx Simple Text Input Ex Input protocol pointer.
+
+**/
+VOID
+ToggleStateSyncToNewConInDev (
+ IN EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *TextInEx
+ )
+{
+ //
+ // Sync current toggle state to this new console input device.
+ //
+ TextInEx->SetState (TextInEx, &mConIn.PhysicalKeyToggleState);
+}
+
+/**
+ Key notify for toggle state sync.
+
+ @param KeyData A pointer to a buffer that is filled in with
+ the keystroke information for the key that was
+ pressed.
+
+ @retval EFI_SUCCESS Toggle state sync successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+ToggleStateSyncKeyNotify (
+ IN EFI_KEY_DATA *KeyData
+ )
+{
+ UINTN Index;
+
+ if (((KeyData->KeyState.KeyToggleState & KEY_STATE_VALID_EXPOSED) == KEY_STATE_VALID_EXPOSED) &&
+ (KeyData->KeyState.KeyToggleState != mConIn.PhysicalKeyToggleState)) {
+ //
+ // There is toggle state change, sync to other console input devices.
+ //
+ for (Index = 0; Index < mConIn.CurrentNumberOfExConsoles; Index++) {
+ mConIn.TextInExList[Index]->SetState (
+ mConIn.TextInExList[Index],
+ &KeyData->KeyState.KeyToggleState
+ );
+ }
+ mConIn.PhysicalKeyToggleState = KeyData->KeyState.KeyToggleState;
+ DEBUG ((EFI_D_INFO, "Current toggle state is 0x%02x\n", mConIn.PhysicalKeyToggleState));
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Initialization for toggle state sync.
+
+ @param Private Text In Splitter pointer.
+
+**/
+VOID
+ToggleStateSyncInitialization (
+ IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private
+ )
+{
+ EFI_KEY_DATA KeyData;
+ VOID *NotifyHandle;
+
+ //
+ // Initialize PhysicalKeyToggleState that will be synced to new console
+ // input device to turn on physical TextInEx partial key report for
+ // toggle state sync.
+ //
+ Private->PhysicalKeyToggleState = KEY_STATE_VALID_EXPOSED;
+
+ //
+ // Initialize VirtualKeyStateExported to let the virtual TextInEx not report
+ // the partial key even though the physical TextInEx turns on the partial
+ // key report. The virtual TextInEx will report the partial key after it is
+ // required by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
+ //
+ Private->VirtualKeyStateExported = FALSE;
+
+ //
+ // Register key notify for toggle state sync.
+ //
+ KeyData.Key.ScanCode = SCAN_NULL;
+ KeyData.Key.UnicodeChar = CHAR_NULL;
+ KeyData.KeyState.KeyShiftState = 0;
+ KeyData.KeyState.KeyToggleState = 0;
+ Private->TextInEx.RegisterKeyNotify (
+ &Private->TextInEx,
+ &KeyData,
+ ToggleStateSyncKeyNotify,
+ &NotifyHandle
+ );
+}
+
+/**
+ Reinitialization for toggle state sync.
+
+ @param Private Text In Splitter pointer.
+
+**/
+VOID
+ToggleStateSyncReInitialization (
+ IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private
+ )
+{
+ UINTN Index;
+
+ //
+ // Reinitialize PhysicalKeyToggleState that will be synced to new console
+ // input device to turn on physical TextInEx partial key report for
+ // toggle state sync.
+ //
+ Private->PhysicalKeyToggleState = KEY_STATE_VALID_EXPOSED;
+
+ //
+ // Reinitialize VirtualKeyStateExported to let the virtual TextInEx not report
+ // the partial key even though the physical TextInEx turns on the partial
+ // key report. The virtual TextInEx will report the partial key after it is
+ // required by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
+ //
+ Private->VirtualKeyStateExported = FALSE;
+
+ for (Index = 0; Index < Private->CurrentNumberOfExConsoles; Index++) {
+ Private->TextInExList[Index]->SetState (
+ Private->TextInExList[Index],
+ &Private->PhysicalKeyToggleState
+ );
+ }
+}
+
+/**
+ Toggle state sync hook SetState.
+
+ @param KeyToggleState Pointer to key toggle state.
+
+**/
+VOID
+ToggleStateSyncHookSetState (
+ IN OUT EFI_KEY_TOGGLE_STATE *KeyToggleState
+ )
+{
+ //
+ // Always turn on physical TextInEx partial key report for
+ // toggle state sync.
+ //
+ *KeyToggleState |= EFI_KEY_STATE_EXPOSED;
+}
+
+/**
The Entry Point for module ConSplitter. The user code starts with this function.
Installs driver module protocols and. Creates virtual device handles for ConIn,
@@ -538,6 +691,8 @@ ConSplitterTextInConstructor (
InitializeListHead (&ConInPrivate->NotifyList);
+ ToggleStateSyncInitialization (ConInPrivate);
+
ConInPrivate->AbsolutePointer.Mode = &ConInPrivate->AbsolutePointerMode;
//
// Allocate buffer for Absolute Pointer device
@@ -1890,6 +2045,8 @@ ConSplitterTextInExAddDevice (
Private->TextInExList[Private->CurrentNumberOfExConsoles] = TextInEx;
Private->CurrentNumberOfExConsoles++;
+ ToggleStateSyncToNewConInDev (TextInEx);
+
//
// Extra CheckEvent added to reduce the double CheckEvent().
//
@@ -3321,6 +3478,10 @@ ConSplitterTextInReset (
}
}
+ if (!EFI_ERROR (ReturnStatus)) {
+ ToggleStateSyncReInitialization (Private);
+ }
+
return ReturnStatus;
}
@@ -3363,8 +3524,18 @@ ConSplitterTextInPrivateReadKeyStroke (
&CurrentKey
);
if (!EFI_ERROR (Status)) {
- *Key = CurrentKey;
- return Status;
+ //
+ // If it is partial keystroke, skip it.
+ //
+ if ((CurrentKey.ScanCode == CHAR_NULL) && (CurrentKey.UnicodeChar == SCAN_NULL)) {
+ //
+ // Try to read key from this physical console input device again.
+ //
+ Index--;
+ } else {
+ *Key = CurrentKey;
+ return Status;
+ }
}
}
@@ -3542,6 +3713,10 @@ ConSplitterTextInResetEx (
}
}
+ if (!EFI_ERROR (ReturnStatus)) {
+ ToggleStateSyncReInitialization (Private);
+ }
+
return ReturnStatus;
}
@@ -3607,8 +3782,22 @@ ConSplitterTextInReadKeyStrokeEx (
&CurrentKeyData
);
if (!EFI_ERROR (Status)) {
- CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
- return Status;
+ //
+ // If it is partial keystroke, check if virtual KeyState has been required to be exposed.
+ //
+ if ((CurrentKeyData.Key.ScanCode == CHAR_NULL) && (CurrentKeyData.Key.UnicodeChar == SCAN_NULL)) {
+ if (Private->VirtualKeyStateExported) {
+ CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
+ return Status;
+ }
+ //
+ // Try to read key from this physical console input device again.
+ //
+ Index--;
+ } else {
+ CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
+ return Status;
+ }
}
}
@@ -3641,6 +3830,7 @@ ConSplitterTextInSetState (
TEXT_IN_SPLITTER_PRIVATE_DATA *Private;
EFI_STATUS Status;
UINTN Index;
+ EFI_KEY_TOGGLE_STATE PhysicalKeyToggleState;
if (KeyToggleState == NULL) {
return EFI_INVALID_PARAMETER;
@@ -3648,6 +3838,9 @@ ConSplitterTextInSetState (
Private = TEXT_IN_EX_SPLITTER_PRIVATE_DATA_FROM_THIS (This);
+ PhysicalKeyToggleState = *KeyToggleState;
+ ToggleStateSyncHookSetState (&PhysicalKeyToggleState);
+
//
// if no physical console input device exists, return EFI_SUCCESS;
// otherwise return the status of setting state of physical console input device
@@ -3655,13 +3848,16 @@ ConSplitterTextInSetState (
for (Index = 0; Index < Private->CurrentNumberOfExConsoles; Index++) {
Status = Private->TextInExList[Index]->SetState (
Private->TextInExList[Index],
- KeyToggleState
+ &PhysicalKeyToggleState
);
if (EFI_ERROR (Status)) {
return Status;
}
}
+ Private->PhysicalKeyToggleState = PhysicalKeyToggleState;
+ Private->VirtualKeyStateExported = (((*KeyToggleState) & EFI_KEY_STATE_EXPOSED) != 0);
+
return EFI_SUCCESS;
}
@@ -3765,7 +3961,7 @@ ConSplitterTextInRegisterKeyNotify (
}
}
- InsertTailList (&mConIn.NotifyList, &NewNotify->NotifyEntry);
+ InsertTailList (&Private->NotifyList, &NewNotify->NotifyEntry);
*NotifyHandle = NewNotify;
diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
index e32abbaea133..99ac5b55d302 100644
--- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
+++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
@@ -1,7 +1,7 @@
/** @file
Private data structures for the Console Splitter driver
-Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -88,6 +88,7 @@ typedef struct {
UINTN Rows;
} TEXT_OUT_SPLITTER_QUERY_DATA;
+#define KEY_STATE_VALID_EXPOSED (EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED)
#define TEXT_IN_EX_SPLITTER_NOTIFY_SIGNATURE SIGNATURE_32 ('T', 'i', 'S', 'n')
@@ -128,6 +129,8 @@ typedef struct {
EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL **TextInExList;
UINTN TextInExListCount;
LIST_ENTRY NotifyList;
+ EFI_KEY_TOGGLE_STATE PhysicalKeyToggleState;
+ BOOLEAN VirtualKeyStateExported;
EFI_SIMPLE_POINTER_PROTOCOL SimplePointer;
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
2016-12-22 13:43 [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync Star Zeng
@ 2016-12-23 11:53 ` Ni, Ruiyu
2016-12-24 13:52 ` Zeng, Star
0 siblings, 1 reply; 5+ messages in thread
From: Ni, Ruiyu @ 2016-12-23 11:53 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Tian, Feng
Star,
1. ConSplitterTextInPrivateReadKeyStroke(): How about using below loop to eliminate the "Index--"?
for (Index = 0; Index < Private->CurrentNumberOfConsoles;) {
Status = Private->TextInList[Index]->ReadKeyStroke (
Private->TextInList[Index],
&CurrentKey
);
if (!EFI_ERROR (Status)) {
//
// Skip partial keystrokes until read out the non-partial one
//
if ((CurrentKey.ScanCode != CHAR_NULL) || (CurrentKey.UnicodeChar != SCAN_NULL)) {
*Key = CurrentKey;
return Status;
}
} else {
Index++;
}
}
2. Similar logic exists in ConSplitterTextInReadKeyStrokeEx()
3. How about remove ToggleStateSyncHookSetState() and inline the code? It might make the code more readable.
4. How about directly set Private->VirtualKeyStateExported to TRUE in ConSplitterTextInSetState()? It might make the code more readable.
5. How about remove ToggleStateSyncToNewConInDev() and inline the code? Use Private instead of mConIn. It might make the code more readable.
6. I agree ToggleStateSyncKeyNotify() has to use mConIn. Seems like a gap of UEFI spec that doesn't have VOID * Context as the 2nd parameter of the hot key notification function.
Regards,
Ray
>-----Original Message-----
>From: Zeng, Star
>Sent: Thursday, December 22, 2016 9:43 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>Tian, Feng <feng.tian@intel.com>
>Subject: [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
>
>Register key notify for toggle state (CapsLock, NumLock and ScrollLock)
>sync between multiple keyboards.
>The implementation for this feature requires keyboard driver supports
>EFI_KEY_STATE_EXPOSED, and turns on physical TextInEx partial key
>report for toggle state sync.
>The virtual TextInEx will report the partial key after it is required
>by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>
>Cc: Ruiyu Ni <Ruiyu.ni@intel.com>
>Cc: Michael Kinney <michael.d.kinney@intel.com>
>Cc: Feng Tian <feng.tian@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Star Zeng <star.zeng@intel.com>
>---
> .../Universal/Console/ConSplitterDxe/ConSplitter.c | 208 ++++++++++++++++++++-
> .../Universal/Console/ConSplitterDxe/ConSplitter.h | 5 +-
> 2 files changed, 206 insertions(+), 7 deletions(-)
>
>diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>index 493bcbafdf39..203ad7b06e20 100644
>--- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>+++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>@@ -67,6 +67,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED TEXT_IN_SPLITTER_PRIVATE_DATA mConIn = {
> (LIST_ENTRY *) NULL,
> (LIST_ENTRY *) NULL
> },
>+ 0,
>+ FALSE,
>
> {
> ConSplitterSimplePointerReset,
>@@ -301,6 +303,157 @@ EFI_DRIVER_BINDING_PROTOCOL gConSplitterAbsolutePointerDriverBinding =
> };
>
> /**
>+ Sync current toggle state to new console input device.
>+
>+ @param TextInEx Simple Text Input Ex Input protocol pointer.
>+
>+**/
>+VOID
>+ToggleStateSyncToNewConInDev (
>+ IN EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *TextInEx
>+ )
>+{
>+ //
>+ // Sync current toggle state to this new console input device.
>+ //
>+ TextInEx->SetState (TextInEx, &mConIn.PhysicalKeyToggleState);
>+}
>+
>+/**
>+ Key notify for toggle state sync.
>+
>+ @param KeyData A pointer to a buffer that is filled in with
>+ the keystroke information for the key that was
>+ pressed.
>+
>+ @retval EFI_SUCCESS Toggle state sync successfully.
>+
>+**/
>+EFI_STATUS
>+EFIAPI
>+ToggleStateSyncKeyNotify (
>+ IN EFI_KEY_DATA *KeyData
>+ )
>+{
>+ UINTN Index;
>+
>+ if (((KeyData->KeyState.KeyToggleState & KEY_STATE_VALID_EXPOSED) == KEY_STATE_VALID_EXPOSED) &&
>+ (KeyData->KeyState.KeyToggleState != mConIn.PhysicalKeyToggleState)) {
>+ //
>+ // There is toggle state change, sync to other console input devices.
>+ //
>+ for (Index = 0; Index < mConIn.CurrentNumberOfExConsoles; Index++) {
>+ mConIn.TextInExList[Index]->SetState (
>+ mConIn.TextInExList[Index],
>+ &KeyData->KeyState.KeyToggleState
>+ );
>+ }
>+ mConIn.PhysicalKeyToggleState = KeyData->KeyState.KeyToggleState;
>+ DEBUG ((EFI_D_INFO, "Current toggle state is 0x%02x\n", mConIn.PhysicalKeyToggleState));
>+ }
>+
>+ return EFI_SUCCESS;
>+}
>+
>+/**
>+ Initialization for toggle state sync.
>+
>+ @param Private Text In Splitter pointer.
>+
>+**/
>+VOID
>+ToggleStateSyncInitialization (
>+ IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private
>+ )
>+{
>+ EFI_KEY_DATA KeyData;
>+ VOID *NotifyHandle;
>+
>+ //
>+ // Initialize PhysicalKeyToggleState that will be synced to new console
>+ // input device to turn on physical TextInEx partial key report for
>+ // toggle state sync.
>+ //
>+ Private->PhysicalKeyToggleState = KEY_STATE_VALID_EXPOSED;
>+
>+ //
>+ // Initialize VirtualKeyStateExported to let the virtual TextInEx not report
>+ // the partial key even though the physical TextInEx turns on the partial
>+ // key report. The virtual TextInEx will report the partial key after it is
>+ // required by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>+ //
>+ Private->VirtualKeyStateExported = FALSE;
>+
>+ //
>+ // Register key notify for toggle state sync.
>+ //
>+ KeyData.Key.ScanCode = SCAN_NULL;
>+ KeyData.Key.UnicodeChar = CHAR_NULL;
>+ KeyData.KeyState.KeyShiftState = 0;
>+ KeyData.KeyState.KeyToggleState = 0;
>+ Private->TextInEx.RegisterKeyNotify (
>+ &Private->TextInEx,
>+ &KeyData,
>+ ToggleStateSyncKeyNotify,
>+ &NotifyHandle
>+ );
>+}
>+
>+/**
>+ Reinitialization for toggle state sync.
>+
>+ @param Private Text In Splitter pointer.
>+
>+**/
>+VOID
>+ToggleStateSyncReInitialization (
>+ IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private
>+ )
>+{
>+ UINTN Index;
>+
>+ //
>+ // Reinitialize PhysicalKeyToggleState that will be synced to new console
>+ // input device to turn on physical TextInEx partial key report for
>+ // toggle state sync.
>+ //
>+ Private->PhysicalKeyToggleState = KEY_STATE_VALID_EXPOSED;
>+
>+ //
>+ // Reinitialize VirtualKeyStateExported to let the virtual TextInEx not report
>+ // the partial key even though the physical TextInEx turns on the partial
>+ // key report. The virtual TextInEx will report the partial key after it is
>+ // required by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>+ //
>+ Private->VirtualKeyStateExported = FALSE;
>+
>+ for (Index = 0; Index < Private->CurrentNumberOfExConsoles; Index++) {
>+ Private->TextInExList[Index]->SetState (
>+ Private->TextInExList[Index],
>+ &Private->PhysicalKeyToggleState
>+ );
>+ }
>+}
>+
>+/**
>+ Toggle state sync hook SetState.
>+
>+ @param KeyToggleState Pointer to key toggle state.
>+
>+**/
>+VOID
>+ToggleStateSyncHookSetState (
>+ IN OUT EFI_KEY_TOGGLE_STATE *KeyToggleState
>+ )
>+{
>+ //
>+ // Always turn on physical TextInEx partial key report for
>+ // toggle state sync.
>+ //
>+ *KeyToggleState |= EFI_KEY_STATE_EXPOSED;
>+}
>+
>+/**
> The Entry Point for module ConSplitter. The user code starts with this function.
>
> Installs driver module protocols and. Creates virtual device handles for ConIn,
>@@ -538,6 +691,8 @@ ConSplitterTextInConstructor (
>
> InitializeListHead (&ConInPrivate->NotifyList);
>
>+ ToggleStateSyncInitialization (ConInPrivate);
>+
> ConInPrivate->AbsolutePointer.Mode = &ConInPrivate->AbsolutePointerMode;
> //
> // Allocate buffer for Absolute Pointer device
>@@ -1890,6 +2045,8 @@ ConSplitterTextInExAddDevice (
> Private->TextInExList[Private->CurrentNumberOfExConsoles] = TextInEx;
> Private->CurrentNumberOfExConsoles++;
>
>+ ToggleStateSyncToNewConInDev (TextInEx);
>+
> //
> // Extra CheckEvent added to reduce the double CheckEvent().
> //
>@@ -3321,6 +3478,10 @@ ConSplitterTextInReset (
> }
> }
>
>+ if (!EFI_ERROR (ReturnStatus)) {
>+ ToggleStateSyncReInitialization (Private);
>+ }
>+
> return ReturnStatus;
> }
>
>@@ -3363,8 +3524,18 @@ ConSplitterTextInPrivateReadKeyStroke (
> &CurrentKey
> );
> if (!EFI_ERROR (Status)) {
>- *Key = CurrentKey;
>- return Status;
>+ //
>+ // If it is partial keystroke, skip it.
>+ //
>+ if ((CurrentKey.ScanCode == CHAR_NULL) && (CurrentKey.UnicodeChar == SCAN_NULL)) {
>+ //
>+ // Try to read key from this physical console input device again.
>+ //
>+ Index--;
>+ } else {
>+ *Key = CurrentKey;
>+ return Status;
>+ }
> }
> }
>
>@@ -3542,6 +3713,10 @@ ConSplitterTextInResetEx (
> }
> }
>
>+ if (!EFI_ERROR (ReturnStatus)) {
>+ ToggleStateSyncReInitialization (Private);
>+ }
>+
> return ReturnStatus;
>
> }
>@@ -3607,8 +3782,22 @@ ConSplitterTextInReadKeyStrokeEx (
> &CurrentKeyData
> );
> if (!EFI_ERROR (Status)) {
>- CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>- return Status;
>+ //
>+ // If it is partial keystroke, check if virtual KeyState has been required to be exposed.
>+ //
>+ if ((CurrentKeyData.Key.ScanCode == CHAR_NULL) && (CurrentKeyData.Key.UnicodeChar == SCAN_NULL)) {
>+ if (Private->VirtualKeyStateExported) {
>+ CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>+ return Status;
>+ }
>+ //
>+ // Try to read key from this physical console input device again.
>+ //
>+ Index--;
>+ } else {
>+ CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>+ return Status;
>+ }
> }
> }
>
>@@ -3641,6 +3830,7 @@ ConSplitterTextInSetState (
> TEXT_IN_SPLITTER_PRIVATE_DATA *Private;
> EFI_STATUS Status;
> UINTN Index;
>+ EFI_KEY_TOGGLE_STATE PhysicalKeyToggleState;
>
> if (KeyToggleState == NULL) {
> return EFI_INVALID_PARAMETER;
>@@ -3648,6 +3838,9 @@ ConSplitterTextInSetState (
>
> Private = TEXT_IN_EX_SPLITTER_PRIVATE_DATA_FROM_THIS (This);
>
>+ PhysicalKeyToggleState = *KeyToggleState;
>+ ToggleStateSyncHookSetState (&PhysicalKeyToggleState);
>+
> //
> // if no physical console input device exists, return EFI_SUCCESS;
> // otherwise return the status of setting state of physical console input device
>@@ -3655,13 +3848,16 @@ ConSplitterTextInSetState (
> for (Index = 0; Index < Private->CurrentNumberOfExConsoles; Index++) {
> Status = Private->TextInExList[Index]->SetState (
> Private->TextInExList[Index],
>- KeyToggleState
>+ &PhysicalKeyToggleState
> );
> if (EFI_ERROR (Status)) {
> return Status;
> }
> }
>
>+ Private->PhysicalKeyToggleState = PhysicalKeyToggleState;
>+ Private->VirtualKeyStateExported = (((*KeyToggleState) & EFI_KEY_STATE_EXPOSED) != 0);
>+
> return EFI_SUCCESS;
>
> }
>@@ -3765,7 +3961,7 @@ ConSplitterTextInRegisterKeyNotify (
> }
> }
>
>- InsertTailList (&mConIn.NotifyList, &NewNotify->NotifyEntry);
>+ InsertTailList (&Private->NotifyList, &NewNotify->NotifyEntry);
>
> *NotifyHandle = NewNotify;
>
>diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>index e32abbaea133..99ac5b55d302 100644
>--- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>+++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>@@ -1,7 +1,7 @@
> /** @file
> Private data structures for the Console Splitter driver
>
>-Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
>@@ -88,6 +88,7 @@ typedef struct {
> UINTN Rows;
> } TEXT_OUT_SPLITTER_QUERY_DATA;
>
>+#define KEY_STATE_VALID_EXPOSED (EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED)
>
> #define TEXT_IN_EX_SPLITTER_NOTIFY_SIGNATURE SIGNATURE_32 ('T', 'i', 'S', 'n')
>
>@@ -128,6 +129,8 @@ typedef struct {
> EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL **TextInExList;
> UINTN TextInExListCount;
> LIST_ENTRY NotifyList;
>+ EFI_KEY_TOGGLE_STATE PhysicalKeyToggleState;
>+ BOOLEAN VirtualKeyStateExported;
>
>
> EFI_SIMPLE_POINTER_PROTOCOL SimplePointer;
>--
>2.7.0.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
2016-12-23 11:53 ` Ni, Ruiyu
@ 2016-12-24 13:52 ` Zeng, Star
2016-12-26 1:25 ` Ni, Ruiyu
0 siblings, 1 reply; 5+ messages in thread
From: Zeng, Star @ 2016-12-24 13:52 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Tian, Feng, star.zeng
Ray,
On 2016/12/23 19:53, Ni, Ruiyu wrote:
> Star,
> 1. ConSplitterTextInPrivateReadKeyStroke(): How about using below loop to eliminate the "Index--"?
>
> for (Index = 0; Index < Private->CurrentNumberOfConsoles;) {
> Status = Private->TextInList[Index]->ReadKeyStroke (
> Private->TextInList[Index],
> &CurrentKey
> );
> if (!EFI_ERROR (Status)) {
> //
> // Skip partial keystrokes until read out the non-partial one
> //
> if ((CurrentKey.ScanCode != CHAR_NULL) || (CurrentKey.UnicodeChar != SCAN_NULL)) {
> *Key = CurrentKey;
> return Status;
> }
> } else {
> Index++;
> }
> }
I do not object.
>
> 2. Similar logic exists in ConSplitterTextInReadKeyStrokeEx()
Same to 1.
>
> 3. How about remove ToggleStateSyncHookSetState() and inline the code? It might make the code more readable.
In fact, I did it by purpose that is the code can still work as before
if the Toggle State Sync related functions and calling lines are removed.
>
> 4. How about directly set Private->VirtualKeyStateExported to TRUE in ConSplitterTextInSetState()? It might make the code more readable.
I could not get it. Directly set Private->VirtualKeyStateExported to
TRUE even no EFI_KEY_STATE_EXPOSED in the input Key Toggle State?
>
> 5. How about remove ToggleStateSyncToNewConInDev() and inline the code? Use Private instead of mConIn. It might make the code more readable.
Same to 3. And oh, the function can have one more input parameter to
eliminate the using to mConIn directly, I did that for other functions
and forgot this one, my god.
>
> 6. I agree ToggleStateSyncKeyNotify() has to use mConIn. Seems like a gap of UEFI spec that doesn't have VOID * Context as the 2nd parameter of the hot key notification function.
Yes
Thanks for the comments.
Star
>
> Regards,
> Ray
>
>> -----Original Message-----
>> From: Zeng, Star
>> Sent: Thursday, December 22, 2016 9:43 PM
>> To: edk2-devel@lists.01.org
>> Cc: Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>> Tian, Feng <feng.tian@intel.com>
>> Subject: [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
>>
>> Register key notify for toggle state (CapsLock, NumLock and ScrollLock)
>> sync between multiple keyboards.
>> The implementation for this feature requires keyboard driver supports
>> EFI_KEY_STATE_EXPOSED, and turns on physical TextInEx partial key
>> report for toggle state sync.
>> The virtual TextInEx will report the partial key after it is required
>> by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>>
>> Cc: Ruiyu Ni <Ruiyu.ni@intel.com>
>> Cc: Michael Kinney <michael.d.kinney@intel.com>
>> Cc: Feng Tian <feng.tian@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Star Zeng <star.zeng@intel.com>
>> ---
>> .../Universal/Console/ConSplitterDxe/ConSplitter.c | 208 ++++++++++++++++++++-
>> .../Universal/Console/ConSplitterDxe/ConSplitter.h | 5 +-
>> 2 files changed, 206 insertions(+), 7 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> index 493bcbafdf39..203ad7b06e20 100644
>> --- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> +++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> @@ -67,6 +67,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED TEXT_IN_SPLITTER_PRIVATE_DATA mConIn = {
>> (LIST_ENTRY *) NULL,
>> (LIST_ENTRY *) NULL
>> },
>> + 0,
>> + FALSE,
>>
>> {
>> ConSplitterSimplePointerReset,
>> @@ -301,6 +303,157 @@ EFI_DRIVER_BINDING_PROTOCOL gConSplitterAbsolutePointerDriverBinding =
>> };
>>
>> /**
>> + Sync current toggle state to new console input device.
>> +
>> + @param TextInEx Simple Text Input Ex Input protocol pointer.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncToNewConInDev (
>> + IN EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *TextInEx
>> + )
>> +{
>> + //
>> + // Sync current toggle state to this new console input device.
>> + //
>> + TextInEx->SetState (TextInEx, &mConIn.PhysicalKeyToggleState);
>> +}
>> +
>> +/**
>> + Key notify for toggle state sync.
>> +
>> + @param KeyData A pointer to a buffer that is filled in with
>> + the keystroke information for the key that was
>> + pressed.
>> +
>> + @retval EFI_SUCCESS Toggle state sync successfully.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +ToggleStateSyncKeyNotify (
>> + IN EFI_KEY_DATA *KeyData
>> + )
>> +{
>> + UINTN Index;
>> +
>> + if (((KeyData->KeyState.KeyToggleState & KEY_STATE_VALID_EXPOSED) == KEY_STATE_VALID_EXPOSED) &&
>> + (KeyData->KeyState.KeyToggleState != mConIn.PhysicalKeyToggleState)) {
>> + //
>> + // There is toggle state change, sync to other console input devices.
>> + //
>> + for (Index = 0; Index < mConIn.CurrentNumberOfExConsoles; Index++) {
>> + mConIn.TextInExList[Index]->SetState (
>> + mConIn.TextInExList[Index],
>> + &KeyData->KeyState.KeyToggleState
>> + );
>> + }
>> + mConIn.PhysicalKeyToggleState = KeyData->KeyState.KeyToggleState;
>> + DEBUG ((EFI_D_INFO, "Current toggle state is 0x%02x\n", mConIn.PhysicalKeyToggleState));
>> + }
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> + Initialization for toggle state sync.
>> +
>> + @param Private Text In Splitter pointer.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncInitialization (
>> + IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private
>> + )
>> +{
>> + EFI_KEY_DATA KeyData;
>> + VOID *NotifyHandle;
>> +
>> + //
>> + // Initialize PhysicalKeyToggleState that will be synced to new console
>> + // input device to turn on physical TextInEx partial key report for
>> + // toggle state sync.
>> + //
>> + Private->PhysicalKeyToggleState = KEY_STATE_VALID_EXPOSED;
>> +
>> + //
>> + // Initialize VirtualKeyStateExported to let the virtual TextInEx not report
>> + // the partial key even though the physical TextInEx turns on the partial
>> + // key report. The virtual TextInEx will report the partial key after it is
>> + // required by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>> + //
>> + Private->VirtualKeyStateExported = FALSE;
>> +
>> + //
>> + // Register key notify for toggle state sync.
>> + //
>> + KeyData.Key.ScanCode = SCAN_NULL;
>> + KeyData.Key.UnicodeChar = CHAR_NULL;
>> + KeyData.KeyState.KeyShiftState = 0;
>> + KeyData.KeyState.KeyToggleState = 0;
>> + Private->TextInEx.RegisterKeyNotify (
>> + &Private->TextInEx,
>> + &KeyData,
>> + ToggleStateSyncKeyNotify,
>> + &NotifyHandle
>> + );
>> +}
>> +
>> +/**
>> + Reinitialization for toggle state sync.
>> +
>> + @param Private Text In Splitter pointer.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncReInitialization (
>> + IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private
>> + )
>> +{
>> + UINTN Index;
>> +
>> + //
>> + // Reinitialize PhysicalKeyToggleState that will be synced to new console
>> + // input device to turn on physical TextInEx partial key report for
>> + // toggle state sync.
>> + //
>> + Private->PhysicalKeyToggleState = KEY_STATE_VALID_EXPOSED;
>> +
>> + //
>> + // Reinitialize VirtualKeyStateExported to let the virtual TextInEx not report
>> + // the partial key even though the physical TextInEx turns on the partial
>> + // key report. The virtual TextInEx will report the partial key after it is
>> + // required by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>> + //
>> + Private->VirtualKeyStateExported = FALSE;
>> +
>> + for (Index = 0; Index < Private->CurrentNumberOfExConsoles; Index++) {
>> + Private->TextInExList[Index]->SetState (
>> + Private->TextInExList[Index],
>> + &Private->PhysicalKeyToggleState
>> + );
>> + }
>> +}
>> +
>> +/**
>> + Toggle state sync hook SetState.
>> +
>> + @param KeyToggleState Pointer to key toggle state.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncHookSetState (
>> + IN OUT EFI_KEY_TOGGLE_STATE *KeyToggleState
>> + )
>> +{
>> + //
>> + // Always turn on physical TextInEx partial key report for
>> + // toggle state sync.
>> + //
>> + *KeyToggleState |= EFI_KEY_STATE_EXPOSED;
>> +}
>> +
>> +/**
>> The Entry Point for module ConSplitter. The user code starts with this function.
>>
>> Installs driver module protocols and. Creates virtual device handles for ConIn,
>> @@ -538,6 +691,8 @@ ConSplitterTextInConstructor (
>>
>> InitializeListHead (&ConInPrivate->NotifyList);
>>
>> + ToggleStateSyncInitialization (ConInPrivate);
>> +
>> ConInPrivate->AbsolutePointer.Mode = &ConInPrivate->AbsolutePointerMode;
>> //
>> // Allocate buffer for Absolute Pointer device
>> @@ -1890,6 +2045,8 @@ ConSplitterTextInExAddDevice (
>> Private->TextInExList[Private->CurrentNumberOfExConsoles] = TextInEx;
>> Private->CurrentNumberOfExConsoles++;
>>
>> + ToggleStateSyncToNewConInDev (TextInEx);
>> +
>> //
>> // Extra CheckEvent added to reduce the double CheckEvent().
>> //
>> @@ -3321,6 +3478,10 @@ ConSplitterTextInReset (
>> }
>> }
>>
>> + if (!EFI_ERROR (ReturnStatus)) {
>> + ToggleStateSyncReInitialization (Private);
>> + }
>> +
>> return ReturnStatus;
>> }
>>
>> @@ -3363,8 +3524,18 @@ ConSplitterTextInPrivateReadKeyStroke (
>> &CurrentKey
>> );
>> if (!EFI_ERROR (Status)) {
>> - *Key = CurrentKey;
>> - return Status;
>> + //
>> + // If it is partial keystroke, skip it.
>> + //
>> + if ((CurrentKey.ScanCode == CHAR_NULL) && (CurrentKey.UnicodeChar == SCAN_NULL)) {
>> + //
>> + // Try to read key from this physical console input device again.
>> + //
>> + Index--;
>> + } else {
>> + *Key = CurrentKey;
>> + return Status;
>> + }
>> }
>> }
>>
>> @@ -3542,6 +3713,10 @@ ConSplitterTextInResetEx (
>> }
>> }
>>
>> + if (!EFI_ERROR (ReturnStatus)) {
>> + ToggleStateSyncReInitialization (Private);
>> + }
>> +
>> return ReturnStatus;
>>
>> }
>> @@ -3607,8 +3782,22 @@ ConSplitterTextInReadKeyStrokeEx (
>> &CurrentKeyData
>> );
>> if (!EFI_ERROR (Status)) {
>> - CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>> - return Status;
>> + //
>> + // If it is partial keystroke, check if virtual KeyState has been required to be exposed.
>> + //
>> + if ((CurrentKeyData.Key.ScanCode == CHAR_NULL) && (CurrentKeyData.Key.UnicodeChar == SCAN_NULL)) {
>> + if (Private->VirtualKeyStateExported) {
>> + CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>> + return Status;
>> + }
>> + //
>> + // Try to read key from this physical console input device again.
>> + //
>> + Index--;
>> + } else {
>> + CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>> + return Status;
>> + }
>> }
>> }
>>
>> @@ -3641,6 +3830,7 @@ ConSplitterTextInSetState (
>> TEXT_IN_SPLITTER_PRIVATE_DATA *Private;
>> EFI_STATUS Status;
>> UINTN Index;
>> + EFI_KEY_TOGGLE_STATE PhysicalKeyToggleState;
>>
>> if (KeyToggleState == NULL) {
>> return EFI_INVALID_PARAMETER;
>> @@ -3648,6 +3838,9 @@ ConSplitterTextInSetState (
>>
>> Private = TEXT_IN_EX_SPLITTER_PRIVATE_DATA_FROM_THIS (This);
>>
>> + PhysicalKeyToggleState = *KeyToggleState;
>> + ToggleStateSyncHookSetState (&PhysicalKeyToggleState);
>> +
>> //
>> // if no physical console input device exists, return EFI_SUCCESS;
>> // otherwise return the status of setting state of physical console input device
>> @@ -3655,13 +3848,16 @@ ConSplitterTextInSetState (
>> for (Index = 0; Index < Private->CurrentNumberOfExConsoles; Index++) {
>> Status = Private->TextInExList[Index]->SetState (
>> Private->TextInExList[Index],
>> - KeyToggleState
>> + &PhysicalKeyToggleState
>> );
>> if (EFI_ERROR (Status)) {
>> return Status;
>> }
>> }
>>
>> + Private->PhysicalKeyToggleState = PhysicalKeyToggleState;
>> + Private->VirtualKeyStateExported = (((*KeyToggleState) & EFI_KEY_STATE_EXPOSED) != 0);
>> +
>> return EFI_SUCCESS;
>>
>> }
>> @@ -3765,7 +3961,7 @@ ConSplitterTextInRegisterKeyNotify (
>> }
>> }
>>
>> - InsertTailList (&mConIn.NotifyList, &NewNotify->NotifyEntry);
>> + InsertTailList (&Private->NotifyList, &NewNotify->NotifyEntry);
>>
>> *NotifyHandle = NewNotify;
>>
>> diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> index e32abbaea133..99ac5b55d302 100644
>> --- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> +++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> @@ -1,7 +1,7 @@
>> /** @file
>> Private data structures for the Console Splitter driver
>>
>> -Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
>> +Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>> This program and the accompanying materials
>> are licensed and made available under the terms and conditions of the BSD License
>> which accompanies this distribution. The full text of the license may be found at
>> @@ -88,6 +88,7 @@ typedef struct {
>> UINTN Rows;
>> } TEXT_OUT_SPLITTER_QUERY_DATA;
>>
>> +#define KEY_STATE_VALID_EXPOSED (EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED)
>>
>> #define TEXT_IN_EX_SPLITTER_NOTIFY_SIGNATURE SIGNATURE_32 ('T', 'i', 'S', 'n')
>>
>> @@ -128,6 +129,8 @@ typedef struct {
>> EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL **TextInExList;
>> UINTN TextInExListCount;
>> LIST_ENTRY NotifyList;
>> + EFI_KEY_TOGGLE_STATE PhysicalKeyToggleState;
>> + BOOLEAN VirtualKeyStateExported;
>>
>>
>> EFI_SIMPLE_POINTER_PROTOCOL SimplePointer;
>> --
>> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
2016-12-24 13:52 ` Zeng, Star
@ 2016-12-26 1:25 ` Ni, Ruiyu
2016-12-26 1:36 ` Zeng, Star
0 siblings, 1 reply; 5+ messages in thread
From: Ni, Ruiyu @ 2016-12-26 1:25 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Tian, Feng
Star,
Why do you need "code can still work as before if the Toggle State Sync related functions and calling lines are removed"?
Do you have a plan to remove the Toggle State Sync feature?
If no, as long as Toggle State Sync feature cannot be separated very clearly (like VariableAuth), we can embed the feature
In the ConSplitter driver, after all, the two functions I asked to be inline are very simple, containing only one line of code.
We could put comment above the inline code to say it's related to Toggle State Sync.
For #4 comment, I was wrong. Your change is correct.
Thanks/Ray
From: Zeng, Star
Sent: Saturday, December 24, 2016 9:53 PM
To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
Ray,
On 2016/12/23 19:53, Ni, Ruiyu wrote:
> Star,
> 1. ConSplitterTextInPrivateReadKeyStroke(): How about using below loop to eliminate the "Index--"?
>
> for (Index = 0; Index < Private->CurrentNumberOfConsoles;) {
> Status = Private->TextInList[Index]->ReadKeyStroke (
> Private->TextInList[Index],
> &CurrentKey
> );
> if (!EFI_ERROR (Status)) {
> //
> // Skip partial keystrokes until read out the non-partial one
> //
> if ((CurrentKey.ScanCode != CHAR_NULL) || (CurrentKey.UnicodeChar != SCAN_NULL)) {
> *Key = CurrentKey;
> return Status;
> }
> } else {
> Index++;
> }
> }
I do not object.
>
> 2. Similar logic exists in ConSplitterTextInReadKeyStrokeEx()
Same to 1.
>
> 3. How about remove ToggleStateSyncHookSetState() and inline the code? It might make the code more readable.
In fact, I did it by purpose that is the code can still work as before
if the Toggle State Sync related functions and calling lines are removed.
>
> 4. How about directly set Private->VirtualKeyStateExported to TRUE in ConSplitterTextInSetState()? It might make the code more readable.
I could not get it. Directly set Private->VirtualKeyStateExported to
TRUE even no EFI_KEY_STATE_EXPOSED in the input Key Toggle State?
>
> 5. How about remove ToggleStateSyncToNewConInDev() and inline the code? Use Private instead of mConIn. It might make the code more readable.
Same to 3. And oh, the function can have one more input parameter to
eliminate the using to mConIn directly, I did that for other functions
and forgot this one, my god.
>
> 6. I agree ToggleStateSyncKeyNotify() has to use mConIn. Seems like a gap of UEFI spec that doesn't have VOID * Context as the 2nd parameter of the hot key notification function.
Yes
Thanks for the comments.
Star
>
> Regards,
> Ray
>
>> -----Original Message-----
>> From: Zeng, Star
>> Sent: Thursday, December 22, 2016 9:43 PM
>> To: mailto:edk2-devel@lists.01.org
>> Cc: Zeng, Star <mailto:star.zeng@intel.com>; Ni, Ruiyu <mailto:ruiyu.ni@intel.com>; Kinney, Michael D <mailto:michael.d.kinney@intel.com>;
>> Tian, Feng <mailto:feng.tian@intel.com>
>> Subject: [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
>>
>> Register key notify for toggle state (CapsLock, NumLock and ScrollLock)
>> sync between multiple keyboards.
>> The implementation for this feature requires keyboard driver supports
>> EFI_KEY_STATE_EXPOSED, and turns on physical TextInEx partial key
>> report for toggle state sync.
>> The virtual TextInEx will report the partial key after it is required
>> by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>>
>> Cc: Ruiyu Ni <mailto:Ruiyu.ni@intel.com>
>> Cc: Michael Kinney <mailto:michael.d.kinney@intel.com>
>> Cc: Feng Tian <mailto:feng.tian@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Star Zeng <mailto:star.zeng@intel.com>
>> ---
>> .../Universal/Console/ConSplitterDxe/ConSplitter.c | 208 ++++++++++++++++++++-
>> .../Universal/Console/ConSplitterDxe/ConSplitter.h | 5 +-
>> 2 files changed, 206 insertions(+), 7 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> index 493bcbafdf39..203ad7b06e20 100644
>> --- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> +++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> @@ -67,6 +67,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED TEXT_IN_SPLITTER_PRIVATE_DATA mConIn = {
>> (LIST_ENTRY *) NULL,
>> (LIST_ENTRY *) NULL
>> },
>> + 0,
>> + FALSE,
>>
>> {
>> ConSplitterSimplePointerReset,
>> @@ -301,6 +303,157 @@ EFI_DRIVER_BINDING_PROTOCOL gConSplitterAbsolutePointerDriverBinding =
>> };
>>
>> /**
>> + Sync current toggle state to new console input device.
>> +
>> + @param TextInEx Simple Text Input Ex Input protocol pointer.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncToNewConInDev (
>> + IN EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *TextInEx
>> + )
>> +{
>> + //
>> + // Sync current toggle state to this new console input device.
>> + //
>> + TextInEx->SetState (TextInEx, &mConIn.PhysicalKeyToggleState);
>> +}
>> +
>> +/**
>> + Key notify for toggle state sync.
>> +
>> + @param KeyData A pointer to a buffer that is filled in with
>> + the keystroke information for the key that was
>> + pressed.
>> +
>> + @retval EFI_SUCCESS Toggle state sync successfully.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +ToggleStateSyncKeyNotify (
>> + IN EFI_KEY_DATA *KeyData
>> + )
>> +{
>> + UINTN Index;
>> +
>> + if (((KeyData->KeyState.KeyToggleState & KEY_STATE_VALID_EXPOSED) == KEY_STATE_VALID_EXPOSED) &&
>> + (KeyData->KeyState.KeyToggleState != mConIn.PhysicalKeyToggleState)) {
>> + //
>> + // There is toggle state change, sync to other console input devices.
>> + //
>> + for (Index = 0; Index < mConIn.CurrentNumberOfExConsoles; Index++) {
>> + mConIn.TextInExList[Index]->SetState (
>> + mConIn.TextInExList[Index],
>> + &KeyData->KeyState.KeyToggleState
>> + );
>> + }
>> + mConIn.PhysicalKeyToggleState = KeyData->KeyState.KeyToggleState;
>> + DEBUG ((EFI_D_INFO, "Current toggle state is 0x%02x\n", mConIn.PhysicalKeyToggleState));
>> + }
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> + Initialization for toggle state sync.
>> +
>> + @param Private Text In Splitter pointer.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncInitialization (
>> + IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private
>> + )
>> +{
>> + EFI_KEY_DATA KeyData;
>> + VOID *NotifyHandle;
>> +
>> + //
>> + // Initialize PhysicalKeyToggleState that will be synced to new console
>> + // input device to turn on physical TextInEx partial key report for
>> + // toggle state sync.
>> + //
>> + Private->PhysicalKeyToggleState = KEY_STATE_VALID_EXPOSED;
>> +
>> + //
>> + // Initialize VirtualKeyStateExported to let the virtual TextInEx not report
>> + // the partial key even though the physical TextInEx turns on the partial
>> + // key report. The virtual TextInEx will report the partial key after it is
>> + // required by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>> + //
>> + Private->VirtualKeyStateExported = FALSE;
>> +
>> + //
>> + // Register key notify for toggle state sync.
>> + //
>> + KeyData.Key.ScanCode = SCAN_NULL;
>> + KeyData.Key.UnicodeChar = CHAR_NULL;
>> + KeyData.KeyState.KeyShiftState = 0;
>> + KeyData.KeyState.KeyToggleState = 0;
>> + Private->TextInEx.RegisterKeyNotify (
>> + &Private->TextInEx,
>> + &KeyData,
>> + ToggleStateSyncKeyNotify,
>> + &NotifyHandle
>> + );
>> +}
>> +
>> +/**
>> + Reinitialization for toggle state sync.
>> +
>> + @param Private Text In Splitter pointer.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncReInitialization (
>> + IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private
>> + )
>> +{
>> + UINTN Index;
>> +
>> + //
>> + // Reinitialize PhysicalKeyToggleState that will be synced to new console
>> + // input device to turn on physical TextInEx partial key report for
>> + // toggle state sync.
>> + //
>> + Private->PhysicalKeyToggleState = KEY_STATE_VALID_EXPOSED;
>> +
>> + //
>> + // Reinitialize VirtualKeyStateExported to let the virtual TextInEx not report
>> + // the partial key even though the physical TextInEx turns on the partial
>> + // key report. The virtual TextInEx will report the partial key after it is
>> + // required by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>> + //
>> + Private->VirtualKeyStateExported = FALSE;
>> +
>> + for (Index = 0; Index < Private->CurrentNumberOfExConsoles; Index++) {
>> + Private->TextInExList[Index]->SetState (
>> + Private->TextInExList[Index],
>> + &Private->PhysicalKeyToggleState
>> + );
>> + }
>> +}
>> +
>> +/**
>> + Toggle state sync hook SetState.
>> +
>> + @param KeyToggleState Pointer to key toggle state.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncHookSetState (
>> + IN OUT EFI_KEY_TOGGLE_STATE *KeyToggleState
>> + )
>> +{
>> + //
>> + // Always turn on physical TextInEx partial key report for
>> + // toggle state sync.
>> + //
>> + *KeyToggleState |= EFI_KEY_STATE_EXPOSED;
>> +}
>> +
>> +/**
>> The Entry Point for module ConSplitter. The user code starts with this function.
>>
>> Installs driver module protocols and. Creates virtual device handles for ConIn,
>> @@ -538,6 +691,8 @@ ConSplitterTextInConstructor (
>>
>> InitializeListHead (&ConInPrivate->NotifyList);
>>
>> + ToggleStateSyncInitialization (ConInPrivate);
>> +
>> ConInPrivate->AbsolutePointer.Mode = &ConInPrivate->AbsolutePointerMode;
>> //
>> // Allocate buffer for Absolute Pointer device
>> @@ -1890,6 +2045,8 @@ ConSplitterTextInExAddDevice (
>> Private->TextInExList[Private->CurrentNumberOfExConsoles] = TextInEx;
>> Private->CurrentNumberOfExConsoles++;
>>
>> + ToggleStateSyncToNewConInDev (TextInEx);
>> +
>> //
>> // Extra CheckEvent added to reduce the double CheckEvent().
>> //
>> @@ -3321,6 +3478,10 @@ ConSplitterTextInReset (
>> }
>> }
>>
>> + if (!EFI_ERROR (ReturnStatus)) {
>> + ToggleStateSyncReInitialization (Private);
>> + }
>> +
>> return ReturnStatus;
>> }
>>
>> @@ -3363,8 +3524,18 @@ ConSplitterTextInPrivateReadKeyStroke (
>> &CurrentKey
>> );
>> if (!EFI_ERROR (Status)) {
>> - *Key = CurrentKey;
>> - return Status;
>> + //
>> + // If it is partial keystroke, skip it.
>> + //
>> + if ((CurrentKey.ScanCode == CHAR_NULL) && (CurrentKey.UnicodeChar == SCAN_NULL)) {
>> + //
>> + // Try to read key from this physical console input device again.
>> + //
>> + Index--;
>> + } else {
>> + *Key = CurrentKey;
>> + return Status;
>> + }
>> }
>> }
>>
>> @@ -3542,6 +3713,10 @@ ConSplitterTextInResetEx (
>> }
>> }
>>
>> + if (!EFI_ERROR (ReturnStatus)) {
>> + ToggleStateSyncReInitialization (Private);
>> + }
>> +
>> return ReturnStatus;
>>
>> }
>> @@ -3607,8 +3782,22 @@ ConSplitterTextInReadKeyStrokeEx (
>> &CurrentKeyData
>> );
>> if (!EFI_ERROR (Status)) {
>> - CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>> - return Status;
>> + //
>> + // If it is partial keystroke, check if virtual KeyState has been required to be exposed.
>> + //
>> + if ((CurrentKeyData.Key.ScanCode == CHAR_NULL) && (CurrentKeyData.Key.UnicodeChar == SCAN_NULL)) {
>> + if (Private->VirtualKeyStateExported) {
>> + CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>> + return Status;
>> + }
>> + //
>> + // Try to read key from this physical console input device again.
>> + //
>> + Index--;
>> + } else {
>> + CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>> + return Status;
>> + }
>> }
>> }
>>
>> @@ -3641,6 +3830,7 @@ ConSplitterTextInSetState (
>> TEXT_IN_SPLITTER_PRIVATE_DATA *Private;
>> EFI_STATUS Status;
>> UINTN Index;
>> + EFI_KEY_TOGGLE_STATE PhysicalKeyToggleState;
>>
>> if (KeyToggleState == NULL) {
>> return EFI_INVALID_PARAMETER;
>> @@ -3648,6 +3838,9 @@ ConSplitterTextInSetState (
>>
>> Private = TEXT_IN_EX_SPLITTER_PRIVATE_DATA_FROM_THIS (This);
>>
>> + PhysicalKeyToggleState = *KeyToggleState;
>> + ToggleStateSyncHookSetState (&PhysicalKeyToggleState);
>> +
>> //
>> // if no physical console input device exists, return EFI_SUCCESS;
>> // otherwise return the status of setting state of physical console input device
>> @@ -3655,13 +3848,16 @@ ConSplitterTextInSetState (
>> for (Index = 0; Index < Private->CurrentNumberOfExConsoles; Index++) {
>> Status = Private->TextInExList[Index]->SetState (
>> Private->TextInExList[Index],
>> - KeyToggleState
>> + &PhysicalKeyToggleState
>> );
>> if (EFI_ERROR (Status)) {
>> return Status;
>> }
>> }
>>
>> + Private->PhysicalKeyToggleState = PhysicalKeyToggleState;
>> + Private->VirtualKeyStateExported = (((*KeyToggleState) & EFI_KEY_STATE_EXPOSED) != 0);
>> +
>> return EFI_SUCCESS;
>>
>> }
>> @@ -3765,7 +3961,7 @@ ConSplitterTextInRegisterKeyNotify (
>> }
>> }
>>
>> - InsertTailList (&mConIn.NotifyList, &NewNotify->NotifyEntry);
>> + InsertTailList (&Private->NotifyList, &NewNotify->NotifyEntry);
>>
>> *NotifyHandle = NewNotify;
>>
>> diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> index e32abbaea133..99ac5b55d302 100644
>> --- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> +++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> @@ -1,7 +1,7 @@
>> /** @file
>> Private data structures for the Console Splitter driver
>>
>> -Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
>> +Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>> This program and the accompanying materials
>> are licensed and made available under the terms and conditions of the BSD License
>> which accompanies this distribution. The full text of the license may be found at
>> @@ -88,6 +88,7 @@ typedef struct {
>> UINTN Rows;
>> } TEXT_OUT_SPLITTER_QUERY_DATA;
>>
>> +#define KEY_STATE_VALID_EXPOSED (EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED)
>>
>> #define TEXT_IN_EX_SPLITTER_NOTIFY_SIGNATURE SIGNATURE_32 ('T', 'i', 'S', 'n')
>>
>> @@ -128,6 +129,8 @@ typedef struct {
>> EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL **TextInExList;
>> UINTN TextInExListCount;
>> LIST_ENTRY NotifyList;
>> + EFI_KEY_TOGGLE_STATE PhysicalKeyToggleState;
>> + BOOLEAN VirtualKeyStateExported;
>>
>>
>> EFI_SIMPLE_POINTER_PROTOCOL SimplePointer;
>> --
>> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
2016-12-26 1:25 ` Ni, Ruiyu
@ 2016-12-26 1:36 ` Zeng, Star
0 siblings, 0 replies; 5+ messages in thread
From: Zeng, Star @ 2016-12-26 1:36 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Tian, Feng, Zeng, Star
It is going to make the further maintainer to know what kind of code are specific for toggle state sync feature.
If somebody has interest to the feature, he/she can only focus on the related functions and calling lines.
Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu
Sent: Monday, December 26, 2016 9:26 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>
Subject: RE: [edk2] [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
Star,
Why do you need "code can still work as before if the Toggle State Sync related functions and calling lines are removed"?
Do you have a plan to remove the Toggle State Sync feature?
If no, as long as Toggle State Sync feature cannot be separated very clearly (like VariableAuth), we can embed the feature In the ConSplitter driver, after all, the two functions I asked to be inline are very simple, containing only one line of code.
We could put comment above the inline code to say it's related to Toggle State Sync.
For #4 comment, I was wrong. Your change is correct.
Thanks/Ray
From: Zeng, Star
Sent: Saturday, December 24, 2016 9:53 PM
To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
Ray,
On 2016/12/23 19:53, Ni, Ruiyu wrote:
> Star,
> 1. ConSplitterTextInPrivateReadKeyStroke(): How about using below loop to eliminate the "Index--"?
>
> for (Index = 0; Index < Private->CurrentNumberOfConsoles;) {
> Status = Private->TextInList[Index]->ReadKeyStroke (
> Private->TextInList[Index],
> &CurrentKey
> );
> if (!EFI_ERROR (Status)) {
> //
> // Skip partial keystrokes until read out the non-partial one
> //
> if ((CurrentKey.ScanCode != CHAR_NULL) ||
>(CurrentKey.UnicodeChar != SCAN_NULL)) {
> *Key = CurrentKey;
> return Status;
> }
> } else {
> Index++;
> }
> }
I do not object.
>
> 2. Similar logic exists in ConSplitterTextInReadKeyStrokeEx()
Same to 1.
>
> 3. How about remove ToggleStateSyncHookSetState() and inline the code? It might make the code more readable.
In fact, I did it by purpose that is the code can still work as before if the Toggle State Sync related functions and calling lines are removed.
>
> 4. How about directly set Private->VirtualKeyStateExported to TRUE in ConSplitterTextInSetState()? It might make the code more readable.
I could not get it. Directly set Private->VirtualKeyStateExported to TRUE even no EFI_KEY_STATE_EXPOSED in the input Key Toggle State?
>
> 5. How about remove ToggleStateSyncToNewConInDev() and inline the code? Use Private instead of mConIn. It might make the code more readable.
Same to 3. And oh, the function can have one more input parameter to eliminate the using to mConIn directly, I did that for other functions and forgot this one, my god.
>
> 6. I agree ToggleStateSyncKeyNotify() has to use mConIn. Seems like a gap of UEFI spec that doesn't have VOID * Context as the 2nd parameter of the hot key notification function.
Yes
Thanks for the comments.
Star
>
> Regards,
> Ray
>
>> -----Original Message-----
>> From: Zeng, Star
>> Sent: Thursday, December 22, 2016 9:43 PM
>> To: mailto:edk2-devel@lists.01.org
>> Cc: Zeng, Star <mailto:star.zeng@intel.com>; Ni, Ruiyu <mailto:ruiyu.ni@intel.com>; Kinney, Michael D <mailto:michael.d.kinney@intel.com>;
>> Tian, Feng <mailto:feng.tian@intel.com>
>> Subject: [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync
>>
>> Register key notify for toggle state (CapsLock, NumLock and ScrollLock)
>> sync between multiple keyboards.
>> The implementation for this feature requires keyboard driver supports
>> EFI_KEY_STATE_EXPOSED, and turns on physical TextInEx partial key
>> report for toggle state sync.
>> The virtual TextInEx will report the partial key after it is required
>> by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>>
>> Cc: Ruiyu Ni <mailto:Ruiyu.ni@intel.com>
>> Cc: Michael Kinney <mailto:michael.d.kinney@intel.com>
>> Cc: Feng Tian <mailto:feng.tian@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Star Zeng <mailto:star.zeng@intel.com>
>> ---
>> .../Universal/Console/ConSplitterDxe/ConSplitter.c | 208 ++++++++++++++++++++-
>> .../Universal/Console/ConSplitterDxe/ConSplitter.h | 5 +-
>> 2 files changed, 206 insertions(+), 7 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> index 493bcbafdf39..203ad7b06e20 100644
>> --- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> +++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
>> @@ -67,6 +67,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED TEXT_IN_SPLITTER_PRIVATE_DATA mConIn = {
>> (LIST_ENTRY *) NULL,
>> (LIST_ENTRY *) NULL
>> },
>> + 0,
>> + FALSE,
>>
>> {
>> ConSplitterSimplePointerReset,
>> @@ -301,6 +303,157 @@ EFI_DRIVER_BINDING_PROTOCOL gConSplitterAbsolutePointerDriverBinding =
>> };
>>
>> /**
>> + Sync current toggle state to new console input device.
>> +
>> + @param TextInEx Simple Text Input Ex Input protocol pointer.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncToNewConInDev (
>> + IN EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *TextInEx
>> + )
>> +{
>> + //
>> + // Sync current toggle state to this new console input device.
>> + //
>> + TextInEx->SetState (TextInEx, &mConIn.PhysicalKeyToggleState);
>> +}
>> +
>> +/**
>> + Key notify for toggle state sync.
>> +
>> + @param KeyData A pointer to a buffer that is filled in with
>> + the keystroke information for the key that was
>> + pressed.
>> +
>> + @retval EFI_SUCCESS Toggle state sync successfully.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +ToggleStateSyncKeyNotify (
>> + IN EFI_KEY_DATA *KeyData
>> + )
>> +{
>> + UINTN Index;
>> +
>> + if (((KeyData->KeyState.KeyToggleState & KEY_STATE_VALID_EXPOSED) == KEY_STATE_VALID_EXPOSED) &&
>> + (KeyData->KeyState.KeyToggleState != mConIn.PhysicalKeyToggleState)) {
>> + //
>> + // There is toggle state change, sync to other console input devices.
>> + //
>> + for (Index = 0; Index < mConIn.CurrentNumberOfExConsoles; Index++) {
>> + mConIn.TextInExList[Index]->SetState (
>> + mConIn.TextInExList[Index],
>> + &KeyData->KeyState.KeyToggleState
>> + );
>> + }
>> + mConIn.PhysicalKeyToggleState = KeyData->KeyState.KeyToggleState;
>> + DEBUG ((EFI_D_INFO, "Current toggle state is 0x%02x\n", mConIn.PhysicalKeyToggleState));
>> + }
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> + Initialization for toggle state sync.
>> +
>> + @param Private Text In Splitter pointer.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncInitialization (
>> + IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private
>> + )
>> +{
>> + EFI_KEY_DATA KeyData;
>> + VOID *NotifyHandle;
>> +
>> + //
>> + // Initialize PhysicalKeyToggleState that will be synced to new console
>> + // input device to turn on physical TextInEx partial key report for
>> + // toggle state sync.
>> + //
>> + Private->PhysicalKeyToggleState = KEY_STATE_VALID_EXPOSED;
>> +
>> + //
>> + // Initialize VirtualKeyStateExported to let the virtual TextInEx not report
>> + // the partial key even though the physical TextInEx turns on the partial
>> + // key report. The virtual TextInEx will report the partial key after it is
>> + // required by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>> + //
>> + Private->VirtualKeyStateExported = FALSE;
>> +
>> + //
>> + // Register key notify for toggle state sync.
>> + //
>> + KeyData.Key.ScanCode = SCAN_NULL;
>> + KeyData.Key.UnicodeChar = CHAR_NULL;
>> + KeyData.KeyState.KeyShiftState = 0;
>> + KeyData.KeyState.KeyToggleState = 0;
>> + Private->TextInEx.RegisterKeyNotify (
>> + &Private->TextInEx,
>> + &KeyData,
>> + ToggleStateSyncKeyNotify,
>> + &NotifyHandle
>> + );
>> +}
>> +
>> +/**
>> + Reinitialization for toggle state sync.
>> +
>> + @param Private Text In Splitter pointer.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncReInitialization (
>> + IN TEXT_IN_SPLITTER_PRIVATE_DATA *Private
>> + )
>> +{
>> + UINTN Index;
>> +
>> + //
>> + // Reinitialize PhysicalKeyToggleState that will be synced to new console
>> + // input device to turn on physical TextInEx partial key report for
>> + // toggle state sync.
>> + //
>> + Private->PhysicalKeyToggleState = KEY_STATE_VALID_EXPOSED;
>> +
>> + //
>> + // Reinitialize VirtualKeyStateExported to let the virtual TextInEx not report
>> + // the partial key even though the physical TextInEx turns on the partial
>> + // key report. The virtual TextInEx will report the partial key after it is
>> + // required by calling SetState(X | KEY_STATE_VALID_EXPOSED) explicitly.
>> + //
>> + Private->VirtualKeyStateExported = FALSE;
>> +
>> + for (Index = 0; Index < Private->CurrentNumberOfExConsoles; Index++) {
>> + Private->TextInExList[Index]->SetState (
>> + Private->TextInExList[Index],
>> + &Private->PhysicalKeyToggleState
>> + );
>> + }
>> +}
>> +
>> +/**
>> + Toggle state sync hook SetState.
>> +
>> + @param KeyToggleState Pointer to key toggle state.
>> +
>> +**/
>> +VOID
>> +ToggleStateSyncHookSetState (
>> + IN OUT EFI_KEY_TOGGLE_STATE *KeyToggleState
>> + )
>> +{
>> + //
>> + // Always turn on physical TextInEx partial key report for
>> + // toggle state sync.
>> + //
>> + *KeyToggleState |= EFI_KEY_STATE_EXPOSED;
>> +}
>> +
>> +/**
>> The Entry Point for module ConSplitter. The user code starts with this function.
>>
>> Installs driver module protocols and. Creates virtual device handles for ConIn,
>> @@ -538,6 +691,8 @@ ConSplitterTextInConstructor (
>>
>> InitializeListHead (&ConInPrivate->NotifyList);
>>
>> + ToggleStateSyncInitialization (ConInPrivate);
>> +
>> ConInPrivate->AbsolutePointer.Mode = &ConInPrivate->AbsolutePointerMode;
>> //
>> // Allocate buffer for Absolute Pointer device
>> @@ -1890,6 +2045,8 @@ ConSplitterTextInExAddDevice (
>> Private->TextInExList[Private->CurrentNumberOfExConsoles] = TextInEx;
>> Private->CurrentNumberOfExConsoles++;
>>
>> + ToggleStateSyncToNewConInDev (TextInEx);
>> +
>> //
>> // Extra CheckEvent added to reduce the double CheckEvent().
>> //
>> @@ -3321,6 +3478,10 @@ ConSplitterTextInReset (
>> }
>> }
>>
>> + if (!EFI_ERROR (ReturnStatus)) {
>> + ToggleStateSyncReInitialization (Private);
>> + }
>> +
>> return ReturnStatus;
>> }
>>
>> @@ -3363,8 +3524,18 @@ ConSplitterTextInPrivateReadKeyStroke (
>> &CurrentKey
>> );
>> if (!EFI_ERROR (Status)) {
>> - *Key = CurrentKey;
>> - return Status;
>> + //
>> + // If it is partial keystroke, skip it.
>> + //
>> + if ((CurrentKey.ScanCode == CHAR_NULL) && (CurrentKey.UnicodeChar == SCAN_NULL)) {
>> + //
>> + // Try to read key from this physical console input device again.
>> + //
>> + Index--;
>> + } else {
>> + *Key = CurrentKey;
>> + return Status;
>> + }
>> }
>> }
>>
>> @@ -3542,6 +3713,10 @@ ConSplitterTextInResetEx (
>> }
>> }
>>
>> + if (!EFI_ERROR (ReturnStatus)) {
>> + ToggleStateSyncReInitialization (Private);
>> + }
>> +
>> return ReturnStatus;
>>
>> }
>> @@ -3607,8 +3782,22 @@ ConSplitterTextInReadKeyStrokeEx (
>> &CurrentKeyData
>> );
>> if (!EFI_ERROR (Status)) {
>> - CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>> - return Status;
>> + //
>> + // If it is partial keystroke, check if virtual KeyState has been required to be exposed.
>> + //
>> + if ((CurrentKeyData.Key.ScanCode == CHAR_NULL) && (CurrentKeyData.Key.UnicodeChar == SCAN_NULL)) {
>> + if (Private->VirtualKeyStateExported) {
>> + CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>> + return Status;
>> + }
>> + //
>> + // Try to read key from this physical console input device again.
>> + //
>> + Index--;
>> + } else {
>> + CopyMem (KeyData, &CurrentKeyData, sizeof (CurrentKeyData));
>> + return Status;
>> + }
>> }
>> }
>>
>> @@ -3641,6 +3830,7 @@ ConSplitterTextInSetState (
>> TEXT_IN_SPLITTER_PRIVATE_DATA *Private;
>> EFI_STATUS Status;
>> UINTN Index;
>> + EFI_KEY_TOGGLE_STATE PhysicalKeyToggleState;
>>
>> if (KeyToggleState == NULL) {
>> return EFI_INVALID_PARAMETER;
>> @@ -3648,6 +3838,9 @@ ConSplitterTextInSetState (
>>
>> Private = TEXT_IN_EX_SPLITTER_PRIVATE_DATA_FROM_THIS (This);
>>
>> + PhysicalKeyToggleState = *KeyToggleState;
>> + ToggleStateSyncHookSetState (&PhysicalKeyToggleState);
>> +
>> //
>> // if no physical console input device exists, return EFI_SUCCESS;
>> // otherwise return the status of setting state of physical console input device
>> @@ -3655,13 +3848,16 @@ ConSplitterTextInSetState (
>> for (Index = 0; Index < Private->CurrentNumberOfExConsoles; Index++) {
>> Status = Private->TextInExList[Index]->SetState (
>> Private->TextInExList[Index],
>> - KeyToggleState
>> + &PhysicalKeyToggleState
>> );
>> if (EFI_ERROR (Status)) {
>> return Status;
>> }
>> }
>>
>> + Private->PhysicalKeyToggleState = PhysicalKeyToggleState;
>> + Private->VirtualKeyStateExported = (((*KeyToggleState) & EFI_KEY_STATE_EXPOSED) != 0);
>> +
>> return EFI_SUCCESS;
>>
>> }
>> @@ -3765,7 +3961,7 @@ ConSplitterTextInRegisterKeyNotify (
>> }
>> }
>>
>> - InsertTailList (&mConIn.NotifyList, &NewNotify->NotifyEntry);
>> + InsertTailList (&Private->NotifyList, &NewNotify->NotifyEntry);
>>
>> *NotifyHandle = NewNotify;
>>
>> diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> index e32abbaea133..99ac5b55d302 100644
>> --- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> +++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h
>> @@ -1,7 +1,7 @@
>> /** @file
>> Private data structures for the Console Splitter driver
>>
>> -Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
>> +Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>> This program and the accompanying materials
>> are licensed and made available under the terms and conditions of the BSD License
>> which accompanies this distribution. The full text of the license may be found at
>> @@ -88,6 +88,7 @@ typedef struct {
>> UINTN Rows;
>> } TEXT_OUT_SPLITTER_QUERY_DATA;
>>
>> +#define KEY_STATE_VALID_EXPOSED (EFI_TOGGLE_STATE_VALID | EFI_KEY_STATE_EXPOSED)
>>
>> #define TEXT_IN_EX_SPLITTER_NOTIFY_SIGNATURE SIGNATURE_32 ('T', 'i', 'S', 'n')
>>
>> @@ -128,6 +129,8 @@ typedef struct {
>> EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL **TextInExList;
>> UINTN TextInExListCount;
>> LIST_ENTRY NotifyList;
>> + EFI_KEY_TOGGLE_STATE PhysicalKeyToggleState;
>> + BOOLEAN VirtualKeyStateExported;
>>
>>
>> EFI_SIMPLE_POINTER_PROTOCOL SimplePointer;
>> --
>> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-26 1:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-22 13:43 [PATCH] MdeModulePkg ConSplitterDxe: Support toggle state sync Star Zeng
2016-12-23 11:53 ` Ni, Ruiyu
2016-12-24 13:52 ` Zeng, Star
2016-12-26 1:25 ` Ni, Ruiyu
2016-12-26 1:36 ` Zeng, Star
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox