public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/5] Execute key notify function at TPL_CALLBACK
@ 2016-12-22 13:45 Star Zeng
  2016-12-22 13:45 ` [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func " Star Zeng
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Star Zeng @ 2016-12-22 13:45 UTC (permalink / raw)
  To: edk2-devel
  Cc: Star Zeng, Ruiyu Ni, Michael Kinney, Feng Tian, Jeff Fan,
	Michael Zimmermann

Current implementation of keyboard driver executes key notify
function in TimerHandler at TPL_NOTIFY. The code change is to
make key notify function executed at TPL_CALLBACK to reduce
the time occupied at TPL_NOTIFY.

The code will signal KeyNotify process event if the key pressed
matches any key registered and insert the KeyData to the EFI Key
queue for notify, then the KeyNotify process handler will invoke
key notify functions at TPL_CALLBACK.

It can also address the problem raised at
https://lists.01.org/pipermail/edk2-devel/2016-December/005644.html

Cc: Ruiyu Ni <Ruiyu.ni@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael Zimmermann <sigmaepsilon92@gmail.com>

Star Zeng (5):
  MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK
  MdeModulePkg TerminalDxe: Execute key notify func at TPL_CALLBACK
  MdeModulePkg Ps2KbDxe: Execute key notify func at TPL_CALLBACK
  IntelFrameworkModulePkg Ps2KbDxe: Execute key notify func at
    TPL_CALLBACK
  IntelFrameworkModulePkg KbDxe: Execute key notify func at TPL_CALLBACK

 .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c         |   9 +-
 .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c          |  47 +++++-
 .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c           |  23 ++-
 .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h           |  15 ++
 .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c       |  75 ++++++++-
 .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h       |  17 +-
 .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c         |   7 +-
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c |  45 ++++++
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c  |  20 +++
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h  |  15 ++
 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c             |  64 +++++++-
 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h             |  17 +-
 MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c           |  10 +-
 .../Universal/Console/TerminalDxe/Terminal.c       |  25 ++-
 .../Universal/Console/TerminalDxe/Terminal.h       |  80 +++++++++-
 .../Universal/Console/TerminalDxe/TerminalConIn.c  | 177 ++++++++++++++++++++-
 16 files changed, 622 insertions(+), 24 deletions(-)

-- 
2.7.0.windows.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK
  2016-12-22 13:45 [PATCH 0/5] Execute key notify function at TPL_CALLBACK Star Zeng
@ 2016-12-22 13:45 ` Star Zeng
  2016-12-23  7:01   ` Ni, Ruiyu
  2016-12-22 13:45 ` [PATCH 2/5] MdeModulePkg TerminalDxe: " Star Zeng
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Star Zeng @ 2016-12-22 13:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng, Ruiyu Ni, Michael Kinney, Feng Tian

Current implementation executes key notify function in TimerHandler
at TPL_NOTIFY. The code change is to make key notify function
executed at TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.

The code will signal KeyNotify process event if the key pressed
matches any key registered and insert the KeyData to the EFI Key
queue for notify, then the KeyNotify process handler will invoke
key notify functions at TPL_CALLBACK.

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>
---
 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c   | 64 +++++++++++++++++++++++++++++++-
 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h   | 17 ++++++++-
 MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 10 +++--
 3 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
index fb7558b73070..b1e1657173c3 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
@@ -2,7 +2,7 @@
   USB Keyboard Driver that manages USB keyboard and produces Simple Text Input
   Protocol and Simple Text Input Ex Protocol.
 
-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -312,6 +312,17 @@ USBKeyboardDriverBindingStart (
     goto ErrorExit;
   }
 
+  Status = gBS->CreateEvent (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  KeyNotifyProcessHandler,
+                  UsbKeyboardDevice,
+                  &UsbKeyboardDevice->KeyNotifyProcessEvent
+                  );
+  if (EFI_ERROR (Status)) {
+    goto ErrorExit;
+  }
+
   //
   // Install Simple Text Input Protocol and Simple Text Input Ex Protocol
   // for the USB keyboard device.
@@ -427,6 +438,9 @@ ErrorExit:
     if (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx != NULL) {
       gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx);
     }
+    if (UsbKeyboardDevice->KeyNotifyProcessEvent != NULL) {
+      gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
+    }
     if (UsbKeyboardDevice->KeyboardLayoutEvent != NULL) {
       ReleaseKeyboardLayoutResources (UsbKeyboardDevice);
       gBS->CloseEvent (UsbKeyboardDevice->KeyboardLayoutEvent);
@@ -548,6 +562,7 @@ USBKeyboardDriverBindingStop (
   gBS->CloseEvent (UsbKeyboardDevice->DelayedRecoveryEvent);
   gBS->CloseEvent (UsbKeyboardDevice->SimpleInput.WaitForKey);
   gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx);
+  gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
   KbdFreeNotifyList (&UsbKeyboardDevice->NotifyList);
 
   ReleaseKeyboardLayoutResources (UsbKeyboardDevice);
@@ -559,6 +574,7 @@ USBKeyboardDriverBindingStop (
 
   DestroyQueue (&UsbKeyboardDevice->UsbKeyQueue);
   DestroyQueue (&UsbKeyboardDevice->EfiKeyQueue);
+  DestroyQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify);
 
   FreePool (UsbKeyboardDevice);
 
@@ -647,6 +663,7 @@ USBKeyboardReset (
     //
     InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY));
     InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
+    InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof (EFI_KEY_DATA));
 
     return EFI_SUCCESS;
   }
@@ -1170,3 +1187,48 @@ USBKeyboardUnregisterKeyNotify (
   return EFI_INVALID_PARAMETER;
 }
 
+/**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  )
+{
+  USB_KB_DEV                    *UsbKeyboardDevice;
+  EFI_KEY_DATA                  KeyData;
+  LIST_ENTRY                    *Link;
+  LIST_ENTRY                    *NotifyList;
+  KEYBOARD_CONSOLE_IN_EX_NOTIFY *CurrentNotify;
+  EFI_TPL                       OldTpl;
+
+  UsbKeyboardDevice = (USB_KB_DEV *) Context;
+
+  //
+  // Invoke notification functions.
+  //
+  NotifyList = &UsbKeyboardDevice->NotifyList;
+  while (!IsQueueEmpty (&UsbKeyboardDevice->EfiKeyQueueForNotify)) {
+    //
+    // Enter critical section
+    //  
+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
+    Dequeue (&UsbKeyboardDevice->EfiKeyQueueForNotify, &KeyData, sizeof (KeyData));
+    //
+    // Leave critical section
+    //
+    gBS->RestoreTPL (OldTpl);
+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
+      CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY, NotifyEntry, USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
+        CurrentNotify->KeyNotificationFn (&KeyData);
+      }
+    }
+  }
+}
+
diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
index 58edb3f65f2b..089f113d5fd4 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
@@ -1,7 +1,7 @@
 /** @file
   Header file for USB Keyboard Driver's Data Structures.
 
-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -114,6 +114,7 @@ typedef struct {
 
   USB_SIMPLE_QUEUE                  UsbKeyQueue;
   USB_SIMPLE_QUEUE                  EfiKeyQueue;
+  USB_SIMPLE_QUEUE                  EfiKeyQueueForNotify;
   BOOLEAN                           CtrlOn;
   BOOLEAN                           AltOn;
   BOOLEAN                           ShiftOn;
@@ -149,6 +150,7 @@ typedef struct {
   // Notification function list
   //
   LIST_ENTRY                        NotifyList;
+  EFI_EVENT                         KeyNotifyProcessEvent;
 
   //
   // Non-spacing key list
@@ -596,5 +598,18 @@ USBKeyboardTimerHandler (
   IN  VOID                      *Context
   );
 
+/**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  );
+
 #endif
 
diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
index fef1449e3c35..a017d51e3b97 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
@@ -820,6 +820,7 @@ InitUSBKeyboard (
 
   InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY));
   InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
+  InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof (EFI_KEY_DATA));
 
   //
   // Use the config out of the descriptor
@@ -1665,20 +1666,23 @@ UsbKeyCodeToEfiInputKey (
     KeyData->KeyState.KeyToggleState |= EFI_KEY_STATE_EXPOSED;
   }
   //
-  // Invoke notification functions if the key is registered.
+  // Signal KeyNotify process event if this key pressed matches any key registered.
   //
   NotifyList = &UsbKeyboardDevice->NotifyList;
   for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
     CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY, NotifyEntry, USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
     if (IsKeyRegistered (&CurrentNotify->KeyData, KeyData)) {
-      CurrentNotify->KeyNotificationFn (KeyData);
+      //
+      // Insert to the EFI Key queue for notify.
+      //
+      Enqueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, KeyData, sizeof (*KeyData));
+      gBS->SignalEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
     }
   }
 
   return EFI_SUCCESS;
 }
 
-
 /**
   Create the queue.
 
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/5] MdeModulePkg TerminalDxe: Execute key notify func at TPL_CALLBACK
  2016-12-22 13:45 [PATCH 0/5] Execute key notify function at TPL_CALLBACK Star Zeng
  2016-12-22 13:45 ` [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func " Star Zeng
@ 2016-12-22 13:45 ` Star Zeng
  2016-12-22 13:45 ` [PATCH 3/5] MdeModulePkg Ps2KbDxe: " Star Zeng
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Star Zeng @ 2016-12-22 13:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng, Ruiyu Ni, Michael Kinney, Feng Tian

Current implementation executes key notify function in TimerHandler
at TPL_NOTIFY. The code change is to make key notify function
executed at TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.

The code will signal KeyNotify process event if the key pressed
matches any key registered and insert the KeyData to the EFI Key
queue for notify, then the KeyNotify process handler will invoke
key notify functions at TPL_CALLBACK.

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/TerminalDxe/Terminal.c       |  25 ++-
 .../Universal/Console/TerminalDxe/Terminal.h       |  80 +++++++++-
 .../Universal/Console/TerminalDxe/TerminalConIn.c  | 177 ++++++++++++++++++++-
 3 files changed, 275 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
index 3f445f02721f..a209bf3cacf8 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
@@ -2,7 +2,7 @@
   Produces Simple Text Input Protocol, Simple Text Input Extended Protocol and
   Simple Text Output Protocol upon Serial IO Protocol.
 
-Copyright (c) 2006 - 2014, 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
@@ -75,6 +75,7 @@ TERMINAL_DEV  mTerminalDevTemplate = {
   NULL, // RawFifo
   NULL, // UnicodeFiFo
   NULL, // EfiKeyFiFo
+  NULL, // EfiKeyFiFoForNotify
 
   NULL, // ControllerNameTable
   NULL, // TimerEvent
@@ -99,7 +100,8 @@ TERMINAL_DEV  mTerminalDevTemplate = {
   {   // NotifyList
     NULL,
     NULL,
-  }
+  },
+  NULL // KeyNotifyProcessEvent
 };
 
 TERMINAL_CONSOLE_MODE_DATA mTerminalConsoleModeData[] = {
@@ -791,6 +793,10 @@ TerminalDriverBindingStart (
     if (TerminalDevice->EfiKeyFiFo == NULL) {
       goto Error;
     }
+    TerminalDevice->EfiKeyFiFoForNotify = AllocateZeroPool (sizeof (EFI_KEY_FIFO));
+    if (TerminalDevice->EfiKeyFiFoForNotify == NULL) {
+      goto Error;
+    }
 
     //
     // Set the timeout value of serial buffer for
@@ -1000,6 +1006,15 @@ TerminalDriverBindingStart (
                     );
     ASSERT_EFI_ERROR (Status);
 
+    Status = gBS->CreateEvent (
+                    EVT_NOTIFY_SIGNAL,
+                    TPL_CALLBACK,
+                    KeyNotifyProcessHandler,
+                    TerminalDevice,
+                    &TerminalDevice->KeyNotifyProcessEvent
+                    );
+    ASSERT_EFI_ERROR (Status);
+
     Status = gBS->InstallProtocolInterface (
                     &TerminalDevice->Handle,
                     &gEfiDevicePathProtocolGuid,
@@ -1222,7 +1237,10 @@ Error:
       if (TerminalDevice->EfiKeyFiFo != NULL) {
         FreePool (TerminalDevice->EfiKeyFiFo);
       }
-
+      if (TerminalDevice->EfiKeyFiFoForNotify != NULL) {
+        FreePool (TerminalDevice->EfiKeyFiFoForNotify);
+      }
+  
       if (TerminalDevice->ControllerNameTable != NULL) {
         FreeUnicodeStringTable (TerminalDevice->ControllerNameTable);
       }
@@ -1400,6 +1418,7 @@ TerminalDriverBindingStop (
         gBS->CloseEvent (TerminalDevice->TwoSecondTimeOut);
         gBS->CloseEvent (TerminalDevice->SimpleInput.WaitForKey);
         gBS->CloseEvent (TerminalDevice->SimpleInputEx.WaitForKeyEx);
+        gBS->CloseEvent (TerminalDevice->KeyNotifyProcessEvent);
         TerminalFreeNotifyList (&TerminalDevice->NotifyList);
         FreePool (TerminalDevice->DevicePath);
         if (TerminalDevice->TerminalConsoleModeData != NULL) {
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 3ee396984e6a..e16b89c264ce 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -1,7 +1,7 @@
 /** @file
   Header file for Terminal driver.
 
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
 Copyright (C) 2016 Silicon Graphics, Inc. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
@@ -95,6 +95,7 @@ typedef struct {
   RAW_DATA_FIFO                       *RawFiFo;
   UNICODE_FIFO                        *UnicodeFiFo;
   EFI_KEY_FIFO                        *EfiKeyFiFo;
+  EFI_KEY_FIFO                        *EfiKeyFiFoForNotify;
   EFI_UNICODE_STRING_TABLE            *ControllerNameTable;
   EFI_EVENT                           TimerEvent;
   EFI_EVENT                           TwoSecondTimeOut;
@@ -113,6 +114,7 @@ typedef struct {
   BOOLEAN                             OutputEscChar;
   EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL   SimpleInputEx;
   LIST_ENTRY                          NotifyList;
+  EFI_EVENT                           KeyNotifyProcessEvent;
 } TERMINAL_DEV;
 
 #define INPUT_STATE_DEFAULT               0x00
@@ -944,6 +946,67 @@ IsRawFiFoFull (
 /**
   Insert one pre-fetched key into the FIFO buffer.
 
+  @param  EfiKeyFiFo            Pointer to instance of EFI_KEY_FIFO.
+  @param  Input                 The key will be input.
+
+  @retval TRUE                  If insert successfully.
+  @retval FALSE                 If FIFO buffer is full before key insertion,
+                                and the key is lost.
+
+**/
+BOOLEAN
+EfiKeyFiFoForNotifyInsertOneKey (
+  EFI_KEY_FIFO                  *EfiKeyFiFo,
+  EFI_INPUT_KEY                 *Input
+  );
+
+/**
+  Remove one pre-fetched key out of the FIFO buffer.
+
+  @param  EfiKeyFiFo            Pointer to instance of EFI_KEY_FIFO.
+  @param  Output                The key will be removed.
+
+  @retval TRUE                  If insert successfully.
+  @retval FALSE                 If FIFO buffer is empty before remove operation.
+
+**/
+BOOLEAN
+EfiKeyFiFoForNotifyRemoveOneKey (
+  EFI_KEY_FIFO                  *EfiKeyFiFo,
+  EFI_INPUT_KEY                 *Output
+  );
+
+/**
+  Clarify whether FIFO buffer is empty.
+
+  @param  EfiKeyFiFo            Pointer to instance of EFI_KEY_FIFO.
+
+  @retval TRUE                  If FIFO buffer is empty.
+  @retval FALSE                 If FIFO buffer is not empty.
+
+**/
+BOOLEAN
+IsEfiKeyFiFoForNotifyEmpty (
+  IN EFI_KEY_FIFO               *EfiKeyFiFo
+  );
+
+/**
+  Clarify whether FIFO buffer is full.
+
+  @param  EfiKeyFiFo            Pointer to instance of EFI_KEY_FIFO.
+
+  @retval TRUE                  If FIFO buffer is full.
+  @retval FALSE                 If FIFO buffer is not full.
+
+**/
+BOOLEAN
+IsEfiKeyFiFoForNotifyFull (
+  EFI_KEY_FIFO                  *EfiKeyFiFo
+  );
+
+/**
+  Insert one pre-fetched key into the FIFO buffer.
+
   @param  TerminalDevice       Terminal driver private structure.
   @param  Key                  The key will be input.
 
@@ -1360,4 +1423,19 @@ TerminalConInTimerHandler (
   IN EFI_EVENT            Event,
   IN VOID                 *Context
   );
+
+
+/**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  );
+
 #endif
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 5c3ea86fe104..799aabe3f0bc 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -2,7 +2,7 @@
   Implementation for EFI_SIMPLE_TEXT_INPUT_PROTOCOL protocol.
 
 (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
 Copyright (C) 2016 Silicon Graphics, Inc. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
@@ -94,6 +94,7 @@ TerminalConInReset (
   TerminalDevice->RawFiFo->Head     = TerminalDevice->RawFiFo->Tail;
   TerminalDevice->UnicodeFiFo->Head = TerminalDevice->UnicodeFiFo->Tail;
   TerminalDevice->EfiKeyFiFo->Head  = TerminalDevice->EfiKeyFiFo->Tail;
+  TerminalDevice->EfiKeyFiFoForNotify->Head = TerminalDevice->EfiKeyFiFoForNotify->Tail;
 
   if (EFI_ERROR (Status)) {
     REPORT_STATUS_CODE_WITH_DEVICE_PATH (
@@ -599,6 +600,55 @@ TerminalConInTimerHandler (
 }
 
 /**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  )
+{
+  TERMINAL_DEV                  *TerminalDevice;
+  EFI_INPUT_KEY                 Key;
+  EFI_KEY_DATA                  KeyData;
+  LIST_ENTRY                    *Link;
+  LIST_ENTRY                    *NotifyList;
+  TERMINAL_CONSOLE_IN_EX_NOTIFY *CurrentNotify;
+  EFI_TPL                       OldTpl;
+
+  TerminalDevice = (TERMINAL_DEV *) Context;
+
+  //
+  // Invoke notification functions.
+  //
+  NotifyList = &TerminalDevice->NotifyList;
+  while (!IsEfiKeyFiFoForNotifyEmpty (TerminalDevice->EfiKeyFiFoForNotify)) {
+    //
+    // Enter critical section
+    //  
+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
+    EfiKeyFiFoForNotifyRemoveOneKey (TerminalDevice->EfiKeyFiFoForNotify, &Key);
+    CopyMem (&KeyData.Key, &Key, sizeof (EFI_INPUT_KEY));
+    KeyData.KeyState.KeyShiftState  = 0;
+    KeyData.KeyState.KeyToggleState = 0;
+    //
+    // Leave critical section
+    //
+    gBS->RestoreTPL (OldTpl);
+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
+      CurrentNotify = CR (Link, TERMINAL_CONSOLE_IN_EX_NOTIFY, NotifyEntry, TERMINAL_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
+        CurrentNotify->KeyNotificationFn (&KeyData);
+      }
+    }
+  }
+}
+
+/**
   Get one key out of serial buffer.
 
   @param  SerialIo           Serial I/O protocol attached to the serial device.
@@ -766,6 +816,126 @@ IsRawFiFoFull (
 /**
   Insert one pre-fetched key into the FIFO buffer.
 
+  @param  EfiKeyFiFo            Pointer to instance of EFI_KEY_FIFO.
+  @param  Input                 The key will be input.
+
+  @retval TRUE                  If insert successfully.
+  @retval FALSE                 If FIFO buffer is full before key insertion,
+                                and the key is lost.
+
+**/
+BOOLEAN
+EfiKeyFiFoForNotifyInsertOneKey (
+  EFI_KEY_FIFO                  *EfiKeyFiFo,
+  EFI_INPUT_KEY                 *Input
+  )
+{
+  UINT8                         Tail;
+
+  Tail = EfiKeyFiFo->Tail;
+
+  if (IsEfiKeyFiFoForNotifyFull (EfiKeyFiFo)) {
+    //
+    // FIFO is full
+    //
+    return FALSE;
+  }
+
+  CopyMem (&EfiKeyFiFo->Data[Tail], Input, sizeof (EFI_INPUT_KEY));
+
+  EfiKeyFiFo->Tail = (UINT8) ((Tail + 1) % (FIFO_MAX_NUMBER + 1));
+
+  return TRUE;
+}
+
+/**
+  Remove one pre-fetched key out of the FIFO buffer.
+
+  @param  EfiKeyFiFo            Pointer to instance of EFI_KEY_FIFO.
+  @param  Output                The key will be removed.
+
+  @retval TRUE                  If insert successfully.
+  @retval FALSE                 If FIFO buffer is empty before remove operation.
+
+**/
+BOOLEAN
+EfiKeyFiFoForNotifyRemoveOneKey (
+  EFI_KEY_FIFO                  *EfiKeyFiFo,
+  EFI_INPUT_KEY                 *Output
+  )
+{
+  UINT8                         Head;
+
+  Head = EfiKeyFiFo->Head;
+  ASSERT (Head < FIFO_MAX_NUMBER + 1);
+
+  if (IsEfiKeyFiFoForNotifyEmpty (EfiKeyFiFo)) {
+    //
+    // FIFO is empty
+    //
+    Output->ScanCode    = SCAN_NULL;
+    Output->UnicodeChar = 0;
+    return FALSE;
+  }
+
+  CopyMem (Output, &EfiKeyFiFo->Data[Head], sizeof (EFI_INPUT_KEY));
+
+  EfiKeyFiFo->Head = (UINT8) ((Head + 1) % (FIFO_MAX_NUMBER + 1));
+
+  return TRUE;
+}
+
+/**
+  Clarify whether FIFO buffer is empty.
+
+  @param  EfiKeyFiFo            Pointer to instance of EFI_KEY_FIFO.
+
+  @retval TRUE                  If FIFO buffer is empty.
+  @retval FALSE                 If FIFO buffer is not empty.
+
+**/
+BOOLEAN
+IsEfiKeyFiFoForNotifyEmpty (
+  EFI_KEY_FIFO                  *EfiKeyFiFo
+  )
+{
+  if (EfiKeyFiFo->Head == EfiKeyFiFo->Tail) {
+    return TRUE;
+  } else {
+    return FALSE;
+  }
+}
+
+/**
+  Clarify whether FIFO buffer is full.
+
+  @param  EfiKeyFiFo            Pointer to instance of EFI_KEY_FIFO.
+
+  @retval TRUE                  If FIFO buffer is full.
+  @retval FALSE                 If FIFO buffer is not full.
+
+**/
+BOOLEAN
+IsEfiKeyFiFoForNotifyFull (
+  EFI_KEY_FIFO                  *EfiKeyFiFo
+  )
+{
+  UINT8                         Tail;
+  UINT8                         Head;
+
+  Tail = EfiKeyFiFo->Tail;
+  Head = EfiKeyFiFo->Head;
+
+  if (((Tail + 1) % (FIFO_MAX_NUMBER + 1)) == Head) {
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+/**
+  Insert one pre-fetched key into the FIFO buffer.
+
   @param  TerminalDevice       Terminal driver private structure.
   @param  Key                  The key will be input.
 
@@ -793,7 +963,7 @@ EfiKeyFiFoInsertOneKey (
   KeyData.KeyState.KeyToggleState = 0;
 
   //
-  // Invoke notification functions if exist
+  // Signal KeyNotify process event if this key pressed matches any key registered.
   //
   NotifyList = &TerminalDevice->NotifyList;
   for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList,Link); Link = GetNextNode (NotifyList,Link)) {
@@ -804,7 +974,8 @@ EfiKeyFiFoInsertOneKey (
                       TERMINAL_CONSOLE_IN_EX_NOTIFY_SIGNATURE
                       );
     if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
-      CurrentNotify->KeyNotificationFn (&KeyData);
+      EfiKeyFiFoForNotifyInsertOneKey (TerminalDevice->EfiKeyFiFoForNotify, Key);
+      gBS->SignalEvent (TerminalDevice->KeyNotifyProcessEvent);
     }
   }
   if (IsEfiKeyFiFoFull (TerminalDevice)) {
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/5] MdeModulePkg Ps2KbDxe: Execute key notify func at TPL_CALLBACK
  2016-12-22 13:45 [PATCH 0/5] Execute key notify function at TPL_CALLBACK Star Zeng
  2016-12-22 13:45 ` [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func " Star Zeng
  2016-12-22 13:45 ` [PATCH 2/5] MdeModulePkg TerminalDxe: " Star Zeng
@ 2016-12-22 13:45 ` Star Zeng
  2016-12-22 13:45 ` [PATCH 4/5] IntelFrameworkModulePkg " Star Zeng
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Star Zeng @ 2016-12-22 13:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng, Ruiyu Ni, Michael Kinney, Feng Tian

Current implementation executes key notify function in TimerHandler
at TPL_NOTIFY. The code change is to make key notify function
executed at TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.

The code will signal KeyNotify process event if the key pressed
matches any key registered and insert the KeyData to the EFI Key
queue for notify, then the KeyNotify process handler will invoke
key notify functions at TPL_CALLBACK.

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>
---
 .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c         |  7 +++-
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c | 45 ++++++++++++++++++++++
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c  | 20 ++++++++++
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h  | 15 ++++++++
 4 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
index eeb7de34abc8..df36c9063b7a 100644
--- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
+++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
@@ -1420,7 +1420,7 @@ KeyGetchar (
   }
 
   //
-  // Invoke notification functions if exist
+  // Signal KeyNotify process event if this key pressed matches any key registered.
   //
   for (Link = GetFirstNode (&ConsoleIn->NotifyList); !IsNull (&ConsoleIn->NotifyList, Link); Link = GetNextNode (&ConsoleIn->NotifyList, Link)) {
     CurrentNotify = CR (
@@ -1430,7 +1430,8 @@ KeyGetchar (
                       KEYBOARD_CONSOLE_IN_EX_NOTIFY_SIGNATURE
                       );
     if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
-      CurrentNotify->KeyNotificationFn (&KeyData);
+      PushEfikeyBufTail (&ConsoleIn->EfiKeyQueueForNotify, &KeyData);
+      gBS->SignalEvent (ConsoleIn->KeyNotifyProcessEvent);
     }
   }
 
@@ -1629,6 +1630,8 @@ InitKeyboard (
   ConsoleIn->ScancodeQueue.Tail = 0;
   ConsoleIn->EfiKeyQueue.Head   = 0;
   ConsoleIn->EfiKeyQueue.Tail   = 0;
+  ConsoleIn->EfiKeyQueueForNotify.Head = 0;
+  ConsoleIn->EfiKeyQueueForNotify.Tail = 0;
 
   //
   // Reset the status indicators
diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
index f6ccd9598fb0..aee78a391cc1 100644
--- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
+++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
@@ -681,3 +681,48 @@ Exit:
   return Status;
 }
 
+/**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  )
+{
+  KEYBOARD_CONSOLE_IN_DEV       *ConsoleIn;
+  EFI_KEY_DATA                  KeyData;
+  LIST_ENTRY                    *Link;
+  LIST_ENTRY                    *NotifyList;
+  KEYBOARD_CONSOLE_IN_EX_NOTIFY *CurrentNotify;
+  EFI_TPL                       OldTpl;
+
+  ConsoleIn = (KEYBOARD_CONSOLE_IN_DEV *) Context;
+
+  //
+  // Invoke notification functions.
+  //
+  NotifyList = &ConsoleIn->NotifyList;
+  while (!IsEfikeyBufEmpty (&ConsoleIn->EfiKeyQueueForNotify)) {
+    //
+    // Enter critical section
+    //  
+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
+    PopEfikeyBufHead (&ConsoleIn->EfiKeyQueueForNotify, &KeyData);
+    //
+    // Leave critical section
+    //
+    gBS->RestoreTPL (OldTpl);
+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
+      CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY, NotifyEntry, KEYBOARD_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
+        CurrentNotify->KeyNotificationFn (&KeyData);
+      }
+    }
+  }
+}
+
diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c
index 4df1b890e62f..6121d29fb791 100644
--- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c
+++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c
@@ -351,6 +351,19 @@ KbdControllerDriverStart (
     goto ErrorExit;
   }
 
+  Status = gBS->CreateEvent (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  KeyNotifyProcessHandler,
+                  ConsoleIn,
+                  &ConsoleIn->KeyNotifyProcessEvent
+                  );
+  if (EFI_ERROR (Status)) {
+    Status      = EFI_OUT_OF_RESOURCES;
+    StatusCode  = EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_CONTROLLER_ERROR;
+    goto ErrorExit;
+  }
+
   REPORT_STATUS_CODE_WITH_DEVICE_PATH (
     EFI_PROGRESS_CODE,
     EFI_PERIPHERAL_KEYBOARD | EFI_P_PC_PRESENCE_DETECT,
@@ -430,6 +443,9 @@ ErrorExit:
   if ((ConsoleIn != NULL) && (ConsoleIn->ConInEx.WaitForKeyEx != NULL)) {
     gBS->CloseEvent (ConsoleIn->ConInEx.WaitForKeyEx);
   }
+  if ((ConsoleIn != NULL) && (ConsoleIn->KeyNotifyProcessEvent != NULL)) {
+    gBS->CloseEvent (ConsoleIn->KeyNotifyProcessEvent);
+  }
   KbdFreeNotifyList (&ConsoleIn->NotifyList);
   if ((ConsoleIn != NULL) && (ConsoleIn->ControllerNameTable != NULL)) {
     FreeUnicodeStringTable (ConsoleIn->ControllerNameTable);
@@ -570,6 +586,10 @@ KbdControllerDriverStop (
     gBS->CloseEvent (ConsoleIn->ConInEx.WaitForKeyEx);
     ConsoleIn->ConInEx.WaitForKeyEx = NULL;
   }
+  if (ConsoleIn->KeyNotifyProcessEvent != NULL) {
+    gBS->CloseEvent (ConsoleIn->KeyNotifyProcessEvent);
+    ConsoleIn->KeyNotifyProcessEvent = NULL;
+  }
   KbdFreeNotifyList (&ConsoleIn->NotifyList);
   FreeUnicodeStringTable (ConsoleIn->ControllerNameTable);
   gBS->FreePool (ConsoleIn);
diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
index d0aecfb08788..e41c1980fc6f 100644
--- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
+++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
@@ -105,6 +105,7 @@ typedef struct {
   //
   SCAN_CODE_QUEUE                     ScancodeQueue;
   EFI_KEY_QUEUE                       EfiKeyQueue;
+  EFI_KEY_QUEUE                       EfiKeyQueueForNotify;
 
   //
   // Error state
@@ -118,6 +119,7 @@ typedef struct {
   // Notification Function List
   //
   LIST_ENTRY                          NotifyList;
+  EFI_EVENT                           KeyNotifyProcessEvent;
 } KEYBOARD_CONSOLE_IN_DEV;
 
 #define KEYBOARD_CONSOLE_IN_DEV_FROM_THIS(a)  CR (a, KEYBOARD_CONSOLE_IN_DEV, ConIn, KEYBOARD_CONSOLE_IN_DEV_SIGNATURE)
@@ -269,6 +271,19 @@ KeyGetchar (
   );
 
 /**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  );
+
+/**
   Perform 8042 controller and keyboard Initialization.
   If ExtendedVerification is TRUE, do additional test for
   the keyboard interface
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/5] IntelFrameworkModulePkg Ps2KbDxe: Execute key notify func at TPL_CALLBACK
  2016-12-22 13:45 [PATCH 0/5] Execute key notify function at TPL_CALLBACK Star Zeng
                   ` (2 preceding siblings ...)
  2016-12-22 13:45 ` [PATCH 3/5] MdeModulePkg Ps2KbDxe: " Star Zeng
@ 2016-12-22 13:45 ` Star Zeng
  2016-12-23  2:16   ` Fan, Jeff
  2016-12-22 13:45 ` [PATCH 5/5] IntelFrameworkModulePkg KbDxe: " Star Zeng
  2016-12-23  7:11 ` [PATCH 0/5] Execute key notify function " Ni, Ruiyu
  5 siblings, 1 reply; 14+ messages in thread
From: Star Zeng @ 2016-12-22 13:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng, Ruiyu Ni, Michael Kinney, Feng Tian, Jeff Fan

Current implementation executes key notify function in TimerHandler
at TPL_NOTIFY. The code change is to make key notify function
executed at TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.

The code will signal KeyNotify process event if the key pressed
matches any key registered and insert the KeyData to the EFI Key
queue for notify, then the KeyNotify process handler will invoke
key notify functions at TPL_CALLBACK.

Cc: Ruiyu Ni <Ruiyu.ni@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c         |  9 +++--
 .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c          | 47 +++++++++++++++++++++-
 .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c           | 23 ++++++++++-
 .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h           | 15 +++++++
 4 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
index 9bde538d10c7..ea0875996753 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
@@ -1,7 +1,7 @@
 /** @file
   Routines that access 8042 keyboard controller
 
-Copyright (c) 2006 - 2011, 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
@@ -1456,7 +1456,7 @@ KeyGetchar (
   }
 
   //
-  // Invoke notification functions if exist
+  // Signal KeyNotify process event if this key pressed matches any key registered.
   //
   for (Link = GetFirstNode (&ConsoleIn->NotifyList); !IsNull (&ConsoleIn->NotifyList, Link); Link = GetNextNode (&ConsoleIn->NotifyList, Link)) {
     CurrentNotify = CR (
@@ -1466,7 +1466,8 @@ KeyGetchar (
                       KEYBOARD_CONSOLE_IN_EX_NOTIFY_SIGNATURE
                       );
     if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
-      CurrentNotify->KeyNotificationFn (&KeyData);
+      PushEfikeyBufTail (&ConsoleIn->EfiKeyQueueForNotify, &KeyData);
+      gBS->SignalEvent (ConsoleIn->KeyNotifyProcessEvent);
     }
   }
 
@@ -1665,6 +1666,8 @@ InitKeyboard (
   ConsoleIn->ScancodeQueue.Tail = 0;
   ConsoleIn->EfiKeyQueue.Head   = 0;
   ConsoleIn->EfiKeyQueue.Tail   = 0;
+  ConsoleIn->EfiKeyQueueForNotify.Head = 0;
+  ConsoleIn->EfiKeyQueueForNotify.Tail = 0;
 
   //
   // Reset the status indicators
diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
index 0efeb39e0da4..9803cde50ebf 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
@@ -2,7 +2,7 @@
   Routines implements SIMPLE_TEXT_IN protocol's interfaces based on 8042 interfaces
   provided by Ps2KbdCtrller.c.
 
-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
@@ -681,3 +681,48 @@ Exit:
   return Status;
 }
 
+/**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  )
+{
+  KEYBOARD_CONSOLE_IN_DEV       *ConsoleIn;
+  EFI_KEY_DATA                  KeyData;
+  LIST_ENTRY                    *Link;
+  LIST_ENTRY                    *NotifyList;
+  KEYBOARD_CONSOLE_IN_EX_NOTIFY *CurrentNotify;
+  EFI_TPL                       OldTpl;
+
+  ConsoleIn = (KEYBOARD_CONSOLE_IN_DEV *) Context;
+
+  //
+  // Invoke notification functions.
+  //
+  NotifyList = &ConsoleIn->NotifyList;
+  while (!IsEfikeyBufEmpty (&ConsoleIn->EfiKeyQueueForNotify)) {
+    //
+    // Enter critical section
+    //  
+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
+    PopEfikeyBufHead (&ConsoleIn->EfiKeyQueueForNotify, &KeyData);
+    //
+    // Leave critical section
+    //
+    gBS->RestoreTPL (OldTpl);
+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
+      CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY, NotifyEntry, KEYBOARD_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
+        CurrentNotify->KeyNotificationFn (&KeyData);
+      }
+    }
+  }
+}
+
diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c
index b6a9b70ca95a..ff562b2698d6 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c
@@ -3,7 +3,7 @@
   PS/2 Keyboard driver. Routines that interacts with callers,
   conforming to EFI driver model
 
-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
@@ -306,6 +306,7 @@ KbdControllerDriverStart (
     StatusCode  = EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_CONTROLLER_ERROR;
     goto ErrorExit;
   }
+  
   // Setup a periodic timer, used for reading keystrokes at a fixed interval
   //
   Status = gBS->CreateEvent (
@@ -332,6 +333,19 @@ KbdControllerDriverStart (
     goto ErrorExit;
   }
 
+  Status = gBS->CreateEvent (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  KeyNotifyProcessHandler,
+                  ConsoleIn,
+                  &ConsoleIn->KeyNotifyProcessEvent
+                  );
+  if (EFI_ERROR (Status)) {
+    Status      = EFI_OUT_OF_RESOURCES;
+    StatusCode  = EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_CONTROLLER_ERROR;
+    goto ErrorExit;
+  }
+
   REPORT_STATUS_CODE_WITH_DEVICE_PATH (
     EFI_PROGRESS_CODE,
     EFI_PERIPHERAL_KEYBOARD | EFI_P_PC_PRESENCE_DETECT,
@@ -411,6 +425,9 @@ ErrorExit:
   if ((ConsoleIn != NULL) && (ConsoleIn->ConInEx.WaitForKeyEx != NULL)) {
     gBS->CloseEvent (ConsoleIn->ConInEx.WaitForKeyEx);
   }
+  if ((ConsoleIn != NULL) && (ConsoleIn->KeyNotifyProcessEvent != NULL)) {
+    gBS->CloseEvent (ConsoleIn->KeyNotifyProcessEvent);
+  }
   KbdFreeNotifyList (&ConsoleIn->NotifyList);
   if ((ConsoleIn != NULL) && (ConsoleIn->ControllerNameTable != NULL)) {
     FreeUnicodeStringTable (ConsoleIn->ControllerNameTable);
@@ -565,6 +582,10 @@ KbdControllerDriverStop (
     gBS->CloseEvent (ConsoleIn->ConInEx.WaitForKeyEx);
     ConsoleIn->ConInEx.WaitForKeyEx = NULL;
   }
+  if (ConsoleIn->KeyNotifyProcessEvent != NULL) {
+    gBS->CloseEvent (ConsoleIn->KeyNotifyProcessEvent);
+    ConsoleIn->KeyNotifyProcessEvent = NULL;
+  }
   KbdFreeNotifyList (&ConsoleIn->NotifyList);
   FreeUnicodeStringTable (ConsoleIn->ControllerNameTable);
   gBS->FreePool (ConsoleIn);
diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
index 62d0e78c4d5a..82aa5a64faea 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
@@ -104,6 +104,7 @@ typedef struct {
   //
   SCAN_CODE_QUEUE                     ScancodeQueue;
   EFI_KEY_QUEUE                       EfiKeyQueue;
+  EFI_KEY_QUEUE                       EfiKeyQueueForNotify;
 
   //
   // Error state
@@ -117,6 +118,7 @@ typedef struct {
   // Notification Function List
   //
   LIST_ENTRY                          NotifyList;
+  EFI_EVENT                           KeyNotifyProcessEvent;
 } KEYBOARD_CONSOLE_IN_DEV;
 
 #define KEYBOARD_CONSOLE_IN_DEV_FROM_THIS(a)  CR (a, KEYBOARD_CONSOLE_IN_DEV, ConIn, KEYBOARD_CONSOLE_IN_DEV_SIGNATURE)
@@ -268,6 +270,19 @@ KeyGetchar (
   );
 
 /**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  );
+
+/**
   Perform 8042 controller and keyboard Initialization.
   If ExtendedVerification is TRUE, do additional test for
   the keyboard interface
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/5] IntelFrameworkModulePkg KbDxe: Execute key notify func at TPL_CALLBACK
  2016-12-22 13:45 [PATCH 0/5] Execute key notify function at TPL_CALLBACK Star Zeng
                   ` (3 preceding siblings ...)
  2016-12-22 13:45 ` [PATCH 4/5] IntelFrameworkModulePkg " Star Zeng
@ 2016-12-22 13:45 ` Star Zeng
  2016-12-23  2:16   ` Fan, Jeff
  2016-12-23  7:11 ` [PATCH 0/5] Execute key notify function " Ni, Ruiyu
  5 siblings, 1 reply; 14+ messages in thread
From: Star Zeng @ 2016-12-22 13:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng, Ruiyu Ni, Michael Kinney, Feng Tian, Jeff Fan

Current implementation executes key notify function in TimerHandler
at TPL_NOTIFY. The code change is to make key notify function
executed at TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.

The code will signal KeyNotify process event if the key pressed
matches any key registered and insert the KeyData to the EFI Key
queue for notify, then the KeyNotify process handler will invoke
key notify functions at TPL_CALLBACK.

Cc: Ruiyu Ni <Ruiyu.ni@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c       | 75 ++++++++++++++++++++--
 .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h       | 17 ++++-
 2 files changed, 87 insertions(+), 5 deletions(-)

diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
index 8dcb13177881..640079fa299a 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
@@ -1,7 +1,7 @@
 /** @file
   ConsoleOut Routines that speak VGA.
 
-Copyright (c) 2006 - 2014, 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
@@ -272,6 +272,8 @@ BiosKeyboardDriverBindingStart (
   
   BiosKeyboardPrivate->Queue.Front                = 0;
   BiosKeyboardPrivate->Queue.Rear                 = 0;
+  BiosKeyboardPrivate->QueueForNotify.Front       = 0;
+  BiosKeyboardPrivate->QueueForNotify.Rear        = 0;
   BiosKeyboardPrivate->SimpleTextInputEx.Reset               = BiosKeyboardResetEx;
   BiosKeyboardPrivate->SimpleTextInputEx.ReadKeyStrokeEx     = BiosKeyboardReadKeyStrokeEx;
   BiosKeyboardPrivate->SimpleTextInputEx.SetState            = BiosKeyboardSetState;
@@ -339,7 +341,20 @@ BiosKeyboardDriverBindingStart (
     StatusCode  = EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_CONTROLLER_ERROR;
     goto Done;
   }
-  
+
+  Status = gBS->CreateEvent (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  KeyNotifyProcessHandler,
+                  BiosKeyboardPrivate,
+                  &BiosKeyboardPrivate->KeyNotifyProcessEvent
+                  );
+  if (EFI_ERROR (Status)) {
+    Status      = EFI_OUT_OF_RESOURCES;
+    StatusCode  = EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_CONTROLLER_ERROR;
+    goto Done;
+  }
+
   //
   // Report a Progress Code for an attempt to detect the precense of the keyboard device in the system
   //
@@ -462,6 +477,11 @@ Done:
       if ((BiosKeyboardPrivate->SimpleTextInputEx).WaitForKeyEx != NULL) {
         gBS->CloseEvent ((BiosKeyboardPrivate->SimpleTextInputEx).WaitForKeyEx);    
       }
+
+      if (BiosKeyboardPrivate->KeyNotifyProcessEvent != NULL) {
+        gBS->CloseEvent (BiosKeyboardPrivate->KeyNotifyProcessEvent);
+      }
+
       BiosKeyboardFreeNotifyList (&BiosKeyboardPrivate->NotifyList);
 
       if (BiosKeyboardPrivate->TimerEvent != NULL) {
@@ -566,6 +586,7 @@ BiosKeyboardDriverBindingStop (
   gBS->CloseEvent ((BiosKeyboardPrivate->SimpleTextIn).WaitForKey);
   gBS->CloseEvent (BiosKeyboardPrivate->TimerEvent);
   gBS->CloseEvent (BiosKeyboardPrivate->SimpleTextInputEx.WaitForKeyEx);
+  gBS->CloseEvent (BiosKeyboardPrivate->KeyNotifyProcessEvent);
   BiosKeyboardFreeNotifyList (&BiosKeyboardPrivate->NotifyList);
 
   FreePool (BiosKeyboardPrivate);
@@ -1941,7 +1962,7 @@ BiosKeyboardTimerHandler (
   }
 
   //
-  // Invoke notification functions if exist
+  // Signal KeyNotify process event if this key pressed matches any key registered.
   //
   for (Link = BiosKeyboardPrivate->NotifyList.ForwardLink; Link != &BiosKeyboardPrivate->NotifyList; Link = Link->ForwardLink) {
     CurrentNotify = CR (
@@ -1951,7 +1972,8 @@ BiosKeyboardTimerHandler (
                       BIOS_KEYBOARD_CONSOLE_IN_EX_NOTIFY_SIGNATURE
                       );
     if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) { 
-      CurrentNotify->KeyNotificationFn (&KeyData);
+      Enqueue (&BiosKeyboardPrivate->QueueForNotify, &KeyData);
+      gBS->SignalEvent (BiosKeyboardPrivate->KeyNotifyProcessEvent);
     }
   }
 
@@ -1965,6 +1987,51 @@ BiosKeyboardTimerHandler (
 }
 
 /**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  )
+{
+  BIOS_KEYBOARD_DEV                     *BiosKeyboardPrivate;
+  EFI_KEY_DATA                          KeyData;
+  LIST_ENTRY                            *Link;
+  LIST_ENTRY                            *NotifyList;
+  BIOS_KEYBOARD_CONSOLE_IN_EX_NOTIFY    *CurrentNotify;
+  EFI_TPL                               OldTpl;
+
+  BiosKeyboardPrivate = (BIOS_KEYBOARD_DEV *) Context;
+
+  //
+  // Invoke notification functions.
+  //
+  NotifyList = &BiosKeyboardPrivate->NotifyList;
+  while (!EFI_ERROR (CheckQueue (&BiosKeyboardPrivate->QueueForNotify))) {
+    //
+    // Enter critical section
+    //  
+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
+    Dequeue (&BiosKeyboardPrivate->QueueForNotify, &KeyData);
+    //
+    // Leave critical section
+    //
+    gBS->RestoreTPL (OldTpl);
+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
+      CurrentNotify = CR (Link, BIOS_KEYBOARD_CONSOLE_IN_EX_NOTIFY, NotifyEntry, BIOS_KEYBOARD_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
+        CurrentNotify->KeyNotificationFn (&KeyData);
+      }
+    }
+  }
+}
+
+/**
   Free keyboard notify list.
 
   @param  ListHead   The list head
diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
index e83608659bc1..f38411aaa645 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
@@ -1,6 +1,6 @@
 /** @file
 
-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
@@ -223,11 +223,13 @@ typedef struct {
   // Buffer storing EFI_KEY_DATA
   //
   SIMPLE_QUEUE                                Queue;
+  SIMPLE_QUEUE                                QueueForNotify;
 
   //
   // Notification Function List
   //
   LIST_ENTRY                                  NotifyList;
+  EFI_EVENT                                   KeyNotifyProcessEvent;
   EFI_EVENT                                   TimerEvent;
   
 } BIOS_KEYBOARD_DEV;
@@ -555,6 +557,19 @@ BiosKeyboardTimerHandler (
   );
 
 /**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  );
+
+/**
   Reset the input device and optionaly run diagnostics
  
   @param  This                  Protocol instance pointer.
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/5] IntelFrameworkModulePkg Ps2KbDxe: Execute key notify func at TPL_CALLBACK
  2016-12-22 13:45 ` [PATCH 4/5] IntelFrameworkModulePkg " Star Zeng
@ 2016-12-23  2:16   ` Fan, Jeff
  0 siblings, 0 replies; 14+ messages in thread
From: Fan, Jeff @ 2016-12-23  2:16 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org
  Cc: Ni, Ruiyu, Kinney, Michael D, Tian, Feng, Zeng, Star

Reviewed-by: Jeff Fan <jeff.fan@intel.com>

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star Zeng
Sent: Thursday, December 22, 2016 9:45 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu; Kinney, Michael D; Tian, Feng; Fan, Jeff; Zeng, Star
Subject: [edk2] [PATCH 4/5] IntelFrameworkModulePkg Ps2KbDxe: Execute key notify func at TPL_CALLBACK

Current implementation executes key notify function in TimerHandler at TPL_NOTIFY. The code change is to make key notify function executed at TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.

The code will signal KeyNotify process event if the key pressed matches any key registered and insert the KeyData to the EFI Key queue for notify, then the KeyNotify process handler will invoke key notify functions at TPL_CALLBACK.

Cc: Ruiyu Ni <Ruiyu.ni@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c         |  9 +++--
 .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c          | 47 +++++++++++++++++++++-
 .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c           | 23 ++++++++++-
 .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h           | 15 +++++++
 4 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
index 9bde538d10c7..ea0875996753 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
@@ -1,7 +1,7 @@
 /** @file
   Routines that access 8042 keyboard controller
 
-Copyright (c) 2006 - 2011, 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 @@ -1456,7 +1456,7 @@ KeyGetchar (
   }
 
   //
-  // Invoke notification functions if exist
+  // Signal KeyNotify process event if this key pressed matches any key registered.
   //
   for (Link = GetFirstNode (&ConsoleIn->NotifyList); !IsNull (&ConsoleIn->NotifyList, Link); Link = GetNextNode (&ConsoleIn->NotifyList, Link)) {
     CurrentNotify = CR (
@@ -1466,7 +1466,8 @@ KeyGetchar (
                       KEYBOARD_CONSOLE_IN_EX_NOTIFY_SIGNATURE
                       );
     if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
-      CurrentNotify->KeyNotificationFn (&KeyData);
+      PushEfikeyBufTail (&ConsoleIn->EfiKeyQueueForNotify, &KeyData);
+      gBS->SignalEvent (ConsoleIn->KeyNotifyProcessEvent);
     }
   }
 
@@ -1665,6 +1666,8 @@ InitKeyboard (
   ConsoleIn->ScancodeQueue.Tail = 0;
   ConsoleIn->EfiKeyQueue.Head   = 0;
   ConsoleIn->EfiKeyQueue.Tail   = 0;
+  ConsoleIn->EfiKeyQueueForNotify.Head = 0;  
+ ConsoleIn->EfiKeyQueueForNotify.Tail = 0;
 
   //
   // Reset the status indicators
diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
index 0efeb39e0da4..9803cde50ebf 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
@@ -2,7 +2,7 @@
   Routines implements SIMPLE_TEXT_IN protocol's interfaces based on 8042 interfaces
   provided by Ps2KbdCtrller.c.
 
-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 @@ -681,3 +681,48 @@ Exit:
   return Status;
 }
 
+/**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  )
+{
+  KEYBOARD_CONSOLE_IN_DEV       *ConsoleIn;
+  EFI_KEY_DATA                  KeyData;
+  LIST_ENTRY                    *Link;
+  LIST_ENTRY                    *NotifyList;
+  KEYBOARD_CONSOLE_IN_EX_NOTIFY *CurrentNotify;
+  EFI_TPL                       OldTpl;
+
+  ConsoleIn = (KEYBOARD_CONSOLE_IN_DEV *) Context;
+
+  //
+  // Invoke notification functions.
+  //
+  NotifyList = &ConsoleIn->NotifyList;
+  while (!IsEfikeyBufEmpty (&ConsoleIn->EfiKeyQueueForNotify)) {
+    //
+    // Enter critical section
+    //  
+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
+    PopEfikeyBufHead (&ConsoleIn->EfiKeyQueueForNotify, &KeyData);
+    //
+    // Leave critical section
+    //
+    gBS->RestoreTPL (OldTpl);
+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
+      CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY, NotifyEntry, KEYBOARD_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
+        CurrentNotify->KeyNotificationFn (&KeyData);
+      }
+    }
+  }
+}
+
diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c
index b6a9b70ca95a..ff562b2698d6 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c
@@ -3,7 +3,7 @@
   PS/2 Keyboard driver. Routines that interacts with callers,
   conforming to EFI driver model
 
-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 @@ -306,6 +306,7 @@ KbdControllerDriverStart (
     StatusCode  = EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_CONTROLLER_ERROR;
     goto ErrorExit;
   }
+  
   // Setup a periodic timer, used for reading keystrokes at a fixed interval
   //
   Status = gBS->CreateEvent (
@@ -332,6 +333,19 @@ KbdControllerDriverStart (
     goto ErrorExit;
   }
 
+  Status = gBS->CreateEvent (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  KeyNotifyProcessHandler,
+                  ConsoleIn,
+                  &ConsoleIn->KeyNotifyProcessEvent
+                  );
+  if (EFI_ERROR (Status)) {
+    Status      = EFI_OUT_OF_RESOURCES;
+    StatusCode  = EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_CONTROLLER_ERROR;
+    goto ErrorExit;
+  }
+
   REPORT_STATUS_CODE_WITH_DEVICE_PATH (
     EFI_PROGRESS_CODE,
     EFI_PERIPHERAL_KEYBOARD | EFI_P_PC_PRESENCE_DETECT, @@ -411,6 +425,9 @@ ErrorExit:
   if ((ConsoleIn != NULL) && (ConsoleIn->ConInEx.WaitForKeyEx != NULL)) {
     gBS->CloseEvent (ConsoleIn->ConInEx.WaitForKeyEx);
   }
+  if ((ConsoleIn != NULL) && (ConsoleIn->KeyNotifyProcessEvent != NULL)) {
+    gBS->CloseEvent (ConsoleIn->KeyNotifyProcessEvent);
+  }
   KbdFreeNotifyList (&ConsoleIn->NotifyList);
   if ((ConsoleIn != NULL) && (ConsoleIn->ControllerNameTable != NULL)) {
     FreeUnicodeStringTable (ConsoleIn->ControllerNameTable); @@ -565,6 +582,10 @@ KbdControllerDriverStop (
     gBS->CloseEvent (ConsoleIn->ConInEx.WaitForKeyEx);
     ConsoleIn->ConInEx.WaitForKeyEx = NULL;
   }
+  if (ConsoleIn->KeyNotifyProcessEvent != NULL) {
+    gBS->CloseEvent (ConsoleIn->KeyNotifyProcessEvent);
+    ConsoleIn->KeyNotifyProcessEvent = NULL;  }
   KbdFreeNotifyList (&ConsoleIn->NotifyList);
   FreeUnicodeStringTable (ConsoleIn->ControllerNameTable);
   gBS->FreePool (ConsoleIn);
diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
index 62d0e78c4d5a..82aa5a64faea 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
@@ -104,6 +104,7 @@ typedef struct {
   //
   SCAN_CODE_QUEUE                     ScancodeQueue;
   EFI_KEY_QUEUE                       EfiKeyQueue;
+  EFI_KEY_QUEUE                       EfiKeyQueueForNotify;
 
   //
   // Error state
@@ -117,6 +118,7 @@ typedef struct {
   // Notification Function List
   //
   LIST_ENTRY                          NotifyList;
+  EFI_EVENT                           KeyNotifyProcessEvent;
 } KEYBOARD_CONSOLE_IN_DEV;
 
 #define KEYBOARD_CONSOLE_IN_DEV_FROM_THIS(a)  CR (a, KEYBOARD_CONSOLE_IN_DEV, ConIn, KEYBOARD_CONSOLE_IN_DEV_SIGNATURE)
@@ -268,6 +270,19 @@ KeyGetchar (
   );
 
 /**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  );
+
+/**
   Perform 8042 controller and keyboard Initialization.
   If ExtendedVerification is TRUE, do additional test for
   the keyboard interface
--
2.7.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] IntelFrameworkModulePkg KbDxe: Execute key notify func at TPL_CALLBACK
  2016-12-22 13:45 ` [PATCH 5/5] IntelFrameworkModulePkg KbDxe: " Star Zeng
@ 2016-12-23  2:16   ` Fan, Jeff
  0 siblings, 0 replies; 14+ messages in thread
From: Fan, Jeff @ 2016-12-23  2:16 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org
  Cc: Ni, Ruiyu, Kinney, Michael D, Tian, Feng

Reviewed-by: Jeff Fan <jeff.fan@intel.com>

-----Original Message-----
From: Zeng, Star 
Sent: Thursday, December 22, 2016 9:45 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star; Ni, Ruiyu; Kinney, Michael D; Tian, Feng; Fan, Jeff
Subject: [PATCH 5/5] IntelFrameworkModulePkg KbDxe: Execute key notify func at TPL_CALLBACK

Current implementation executes key notify function in TimerHandler at TPL_NOTIFY. The code change is to make key notify function executed at TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.

The code will signal KeyNotify process event if the key pressed matches any key registered and insert the KeyData to the EFI Key queue for notify, then the KeyNotify process handler will invoke key notify functions at TPL_CALLBACK.

Cc: Ruiyu Ni <Ruiyu.ni@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c       | 75 ++++++++++++++++++++--
 .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h       | 17 ++++-
 2 files changed, 87 insertions(+), 5 deletions(-)

diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
index 8dcb13177881..640079fa299a 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
@@ -1,7 +1,7 @@
 /** @file
   ConsoleOut Routines that speak VGA.
 
-Copyright (c) 2006 - 2014, 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 @@ -272,6 +272,8 @@ BiosKeyboardDriverBindingStart (
   
   BiosKeyboardPrivate->Queue.Front                = 0;
   BiosKeyboardPrivate->Queue.Rear                 = 0;
+  BiosKeyboardPrivate->QueueForNotify.Front       = 0;
+  BiosKeyboardPrivate->QueueForNotify.Rear        = 0;
   BiosKeyboardPrivate->SimpleTextInputEx.Reset               = BiosKeyboardResetEx;
   BiosKeyboardPrivate->SimpleTextInputEx.ReadKeyStrokeEx     = BiosKeyboardReadKeyStrokeEx;
   BiosKeyboardPrivate->SimpleTextInputEx.SetState            = BiosKeyboardSetState;
@@ -339,7 +341,20 @@ BiosKeyboardDriverBindingStart (
     StatusCode  = EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_CONTROLLER_ERROR;
     goto Done;
   }
-  
+
+  Status = gBS->CreateEvent (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  KeyNotifyProcessHandler,
+                  BiosKeyboardPrivate,
+                  &BiosKeyboardPrivate->KeyNotifyProcessEvent
+                  );
+  if (EFI_ERROR (Status)) {
+    Status      = EFI_OUT_OF_RESOURCES;
+    StatusCode  = EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_CONTROLLER_ERROR;
+    goto Done;
+  }
+
   //
   // Report a Progress Code for an attempt to detect the precense of the keyboard device in the system
   //
@@ -462,6 +477,11 @@ Done:
       if ((BiosKeyboardPrivate->SimpleTextInputEx).WaitForKeyEx != NULL) {
         gBS->CloseEvent ((BiosKeyboardPrivate->SimpleTextInputEx).WaitForKeyEx);    
       }
+
+      if (BiosKeyboardPrivate->KeyNotifyProcessEvent != NULL) {
+        gBS->CloseEvent (BiosKeyboardPrivate->KeyNotifyProcessEvent);
+      }
+
       BiosKeyboardFreeNotifyList (&BiosKeyboardPrivate->NotifyList);
 
       if (BiosKeyboardPrivate->TimerEvent != NULL) { @@ -566,6 +586,7 @@ BiosKeyboardDriverBindingStop (
   gBS->CloseEvent ((BiosKeyboardPrivate->SimpleTextIn).WaitForKey);
   gBS->CloseEvent (BiosKeyboardPrivate->TimerEvent);
   gBS->CloseEvent (BiosKeyboardPrivate->SimpleTextInputEx.WaitForKeyEx);
+  gBS->CloseEvent (BiosKeyboardPrivate->KeyNotifyProcessEvent);
   BiosKeyboardFreeNotifyList (&BiosKeyboardPrivate->NotifyList);
 
   FreePool (BiosKeyboardPrivate);
@@ -1941,7 +1962,7 @@ BiosKeyboardTimerHandler (
   }
 
   //
-  // Invoke notification functions if exist
+  // Signal KeyNotify process event if this key pressed matches any key registered.
   //
   for (Link = BiosKeyboardPrivate->NotifyList.ForwardLink; Link != &BiosKeyboardPrivate->NotifyList; Link = Link->ForwardLink) {
     CurrentNotify = CR (
@@ -1951,7 +1972,8 @@ BiosKeyboardTimerHandler (
                       BIOS_KEYBOARD_CONSOLE_IN_EX_NOTIFY_SIGNATURE
                       );
     if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) { 
-      CurrentNotify->KeyNotificationFn (&KeyData);
+      Enqueue (&BiosKeyboardPrivate->QueueForNotify, &KeyData);
+      gBS->SignalEvent (BiosKeyboardPrivate->KeyNotifyProcessEvent);
     }
   }
 
@@ -1965,6 +1987,51 @@ BiosKeyboardTimerHandler (  }
 
 /**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  )
+{
+  BIOS_KEYBOARD_DEV                     *BiosKeyboardPrivate;
+  EFI_KEY_DATA                          KeyData;
+  LIST_ENTRY                            *Link;
+  LIST_ENTRY                            *NotifyList;
+  BIOS_KEYBOARD_CONSOLE_IN_EX_NOTIFY    *CurrentNotify;
+  EFI_TPL                               OldTpl;
+
+  BiosKeyboardPrivate = (BIOS_KEYBOARD_DEV *) Context;
+
+  //
+  // Invoke notification functions.
+  //
+  NotifyList = &BiosKeyboardPrivate->NotifyList;
+  while (!EFI_ERROR (CheckQueue (&BiosKeyboardPrivate->QueueForNotify))) {
+    //
+    // Enter critical section
+    //  
+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
+    Dequeue (&BiosKeyboardPrivate->QueueForNotify, &KeyData);
+    //
+    // Leave critical section
+    //
+    gBS->RestoreTPL (OldTpl);
+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
+      CurrentNotify = CR (Link, BIOS_KEYBOARD_CONSOLE_IN_EX_NOTIFY, NotifyEntry, BIOS_KEYBOARD_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
+        CurrentNotify->KeyNotificationFn (&KeyData);
+      }
+    }
+  }
+}
+
+/**
   Free keyboard notify list.
 
   @param  ListHead   The list head
diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
index e83608659bc1..f38411aaa645 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
@@ -1,6 +1,6 @@
 /** @file
 
-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 @@ -223,11 +223,13 @@ typedef struct {
   // Buffer storing EFI_KEY_DATA
   //
   SIMPLE_QUEUE                                Queue;
+  SIMPLE_QUEUE                                QueueForNotify;
 
   //
   // Notification Function List
   //
   LIST_ENTRY                                  NotifyList;
+  EFI_EVENT                                   KeyNotifyProcessEvent;
   EFI_EVENT                                   TimerEvent;
   
 } BIOS_KEYBOARD_DEV;
@@ -555,6 +557,19 @@ BiosKeyboardTimerHandler (
   );
 
 /**
+  Process key notify.
+
+  @param  Event                 Indicates the event that invoke this function.
+  @param  Context               Indicates the calling context.
+**/
+VOID
+EFIAPI
+KeyNotifyProcessHandler (
+  IN  EFI_EVENT                 Event,
+  IN  VOID                      *Context
+  );
+
+/**
   Reset the input device and optionaly run diagnostics
  
   @param  This                  Protocol instance pointer.
--
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK
  2016-12-22 13:45 ` [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func " Star Zeng
@ 2016-12-23  7:01   ` Ni, Ruiyu
  2016-12-23  7:05     ` Zeng, Star
  0 siblings, 1 reply; 14+ messages in thread
From: Ni, Ruiyu @ 2016-12-23  7:01 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Tian, Feng, Zeng, Star

Star,
I have one comment:
When call Enque(), can we put comments above the call to say that the key notification
function needs to run at TPL_CALLBACK while current TPL is TPL_NOTIFY. It will be called
in KeyNotifyProcessHandler() which runs at TPL_CALLBACK?

Regards,
Ray

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star Zeng
>Sent: Thursday, December 22, 2016 9:45 PM
>To: edk2-devel@lists.01.org
>Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>;
>Zeng, Star <star.zeng@intel.com>
>Subject: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK
>
>Current implementation executes key notify function in TimerHandler
>at TPL_NOTIFY. The code change is to make key notify function
>executed at TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.
>
>The code will signal KeyNotify process event if the key pressed
>matches any key registered and insert the KeyData to the EFI Key
>queue for notify, then the KeyNotify process handler will invoke
>key notify functions at TPL_CALLBACK.
>
>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>
>---
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c   | 64 +++++++++++++++++++++++++++++++-
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h   | 17 ++++++++-
> MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 10 +++--
> 3 files changed, 86 insertions(+), 5 deletions(-)
>
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>index fb7558b73070..b1e1657173c3 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>@@ -2,7 +2,7 @@
>   USB Keyboard Driver that manages USB keyboard and produces Simple Text Input
>   Protocol and Simple Text Input Ex Protocol.
>
>-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2004 - 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
>@@ -312,6 +312,17 @@ USBKeyboardDriverBindingStart (
>     goto ErrorExit;
>   }
>
>+  Status = gBS->CreateEvent (
>+                  EVT_NOTIFY_SIGNAL,
>+                  TPL_CALLBACK,
>+                  KeyNotifyProcessHandler,
>+                  UsbKeyboardDevice,
>+                  &UsbKeyboardDevice->KeyNotifyProcessEvent
>+                  );
>+  if (EFI_ERROR (Status)) {
>+    goto ErrorExit;
>+  }
>+
>   //
>   // Install Simple Text Input Protocol and Simple Text Input Ex Protocol
>   // for the USB keyboard device.
>@@ -427,6 +438,9 @@ ErrorExit:
>     if (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx != NULL) {
>       gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx);
>     }
>+    if (UsbKeyboardDevice->KeyNotifyProcessEvent != NULL) {
>+      gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>+    }
>     if (UsbKeyboardDevice->KeyboardLayoutEvent != NULL) {
>       ReleaseKeyboardLayoutResources (UsbKeyboardDevice);
>       gBS->CloseEvent (UsbKeyboardDevice->KeyboardLayoutEvent);
>@@ -548,6 +562,7 @@ USBKeyboardDriverBindingStop (
>   gBS->CloseEvent (UsbKeyboardDevice->DelayedRecoveryEvent);
>   gBS->CloseEvent (UsbKeyboardDevice->SimpleInput.WaitForKey);
>   gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx);
>+  gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>   KbdFreeNotifyList (&UsbKeyboardDevice->NotifyList);
>
>   ReleaseKeyboardLayoutResources (UsbKeyboardDevice);
>@@ -559,6 +574,7 @@ USBKeyboardDriverBindingStop (
>
>   DestroyQueue (&UsbKeyboardDevice->UsbKeyQueue);
>   DestroyQueue (&UsbKeyboardDevice->EfiKeyQueue);
>+  DestroyQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify);
>
>   FreePool (UsbKeyboardDevice);
>
>@@ -647,6 +663,7 @@ USBKeyboardReset (
>     //
>     InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY));
>     InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
>+    InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof (EFI_KEY_DATA));
>
>     return EFI_SUCCESS;
>   }
>@@ -1170,3 +1187,48 @@ USBKeyboardUnregisterKeyNotify (
>   return EFI_INVALID_PARAMETER;
> }
>
>+/**
>+  Process key notify.
>+
>+  @param  Event                 Indicates the event that invoke this function.
>+  @param  Context               Indicates the calling context.
>+**/
>+VOID
>+EFIAPI
>+KeyNotifyProcessHandler (
>+  IN  EFI_EVENT                 Event,
>+  IN  VOID                      *Context
>+  )
>+{
>+  USB_KB_DEV                    *UsbKeyboardDevice;
>+  EFI_KEY_DATA                  KeyData;
>+  LIST_ENTRY                    *Link;
>+  LIST_ENTRY                    *NotifyList;
>+  KEYBOARD_CONSOLE_IN_EX_NOTIFY *CurrentNotify;
>+  EFI_TPL                       OldTpl;
>+
>+  UsbKeyboardDevice = (USB_KB_DEV *) Context;
>+
>+  //
>+  // Invoke notification functions.
>+  //
>+  NotifyList = &UsbKeyboardDevice->NotifyList;
>+  while (!IsQueueEmpty (&UsbKeyboardDevice->EfiKeyQueueForNotify)) {
>+    //
>+    // Enter critical section
>+    //
>+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
>+    Dequeue (&UsbKeyboardDevice->EfiKeyQueueForNotify, &KeyData, sizeof (KeyData));
>+    //
>+    // Leave critical section
>+    //
>+    gBS->RestoreTPL (OldTpl);
>+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
>+      CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY, NotifyEntry,
>USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
>+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
>+        CurrentNotify->KeyNotificationFn (&KeyData);
>+      }
>+    }
>+  }
>+}
>+
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>index 58edb3f65f2b..089f113d5fd4 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>@@ -1,7 +1,7 @@
> /** @file
>   Header file for USB Keyboard Driver's Data Structures.
>
>-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2004 - 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
>@@ -114,6 +114,7 @@ typedef struct {
>
>   USB_SIMPLE_QUEUE                  UsbKeyQueue;
>   USB_SIMPLE_QUEUE                  EfiKeyQueue;
>+  USB_SIMPLE_QUEUE                  EfiKeyQueueForNotify;
>   BOOLEAN                           CtrlOn;
>   BOOLEAN                           AltOn;
>   BOOLEAN                           ShiftOn;
>@@ -149,6 +150,7 @@ typedef struct {
>   // Notification function list
>   //
>   LIST_ENTRY                        NotifyList;
>+  EFI_EVENT                         KeyNotifyProcessEvent;
>
>   //
>   // Non-spacing key list
>@@ -596,5 +598,18 @@ USBKeyboardTimerHandler (
>   IN  VOID                      *Context
>   );
>
>+/**
>+  Process key notify.
>+
>+  @param  Event                 Indicates the event that invoke this function.
>+  @param  Context               Indicates the calling context.
>+**/
>+VOID
>+EFIAPI
>+KeyNotifyProcessHandler (
>+  IN  EFI_EVENT                 Event,
>+  IN  VOID                      *Context
>+  );
>+
> #endif
>
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>index fef1449e3c35..a017d51e3b97 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>@@ -820,6 +820,7 @@ InitUSBKeyboard (
>
>   InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY));
>   InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
>+  InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof (EFI_KEY_DATA));
>
>   //
>   // Use the config out of the descriptor
>@@ -1665,20 +1666,23 @@ UsbKeyCodeToEfiInputKey (
>     KeyData->KeyState.KeyToggleState |= EFI_KEY_STATE_EXPOSED;
>   }
>   //
>-  // Invoke notification functions if the key is registered.
>+  // Signal KeyNotify process event if this key pressed matches any key registered.
>   //
>   NotifyList = &UsbKeyboardDevice->NotifyList;
>   for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
>     CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY, NotifyEntry,
>USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
>     if (IsKeyRegistered (&CurrentNotify->KeyData, KeyData)) {
>-      CurrentNotify->KeyNotificationFn (KeyData);
>+      //
>+      // Insert to the EFI Key queue for notify.
>+      //
>+      Enqueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, KeyData, sizeof (*KeyData));
>+      gBS->SignalEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>     }
>   }
>
>   return EFI_SUCCESS;
> }
>
>-
> /**
>   Create the queue.
>
>--
>2.7.0.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK
  2016-12-23  7:01   ` Ni, Ruiyu
@ 2016-12-23  7:05     ` Zeng, Star
  2016-12-23  7:09       ` Ni, Ruiyu
  0 siblings, 1 reply; 14+ messages in thread
From: Zeng, Star @ 2016-12-23  7:05 UTC (permalink / raw)
  To: Ni, Ruiyu, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Tian, Feng, Zeng, Star

Ray,

Good comment, it can make the code more clear, I agree it.

Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu 
Sent: Friday, December 23, 2016 3:01 PM
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>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK

Star,
I have one comment:
When call Enque(), can we put comments above the call to say that the key notification function needs to run at TPL_CALLBACK while current TPL is TPL_NOTIFY. It will be called in KeyNotifyProcessHandler() which runs at TPL_CALLBACK?

Regards,
Ray

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
>Star Zeng
>Sent: Thursday, December 22, 2016 9:45 PM
>To: edk2-devel@lists.01.org
>Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Kinney, Michael D 
><michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>; Zeng, 
>Star <star.zeng@intel.com>
>Subject: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify 
>func at TPL_CALLBACK
>
>Current implementation executes key notify function in TimerHandler at 
>TPL_NOTIFY. The code change is to make key notify function executed at 
>TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.
>
>The code will signal KeyNotify process event if the key pressed matches 
>any key registered and insert the KeyData to the EFI Key queue for 
>notify, then the KeyNotify process handler will invoke key notify 
>functions at TPL_CALLBACK.
>
>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>
>---
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c   | 64 +++++++++++++++++++++++++++++++-
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h   | 17 ++++++++-
> MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 10 +++--
> 3 files changed, 86 insertions(+), 5 deletions(-)
>
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c 
>b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>index fb7558b73070..b1e1657173c3 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>@@ -2,7 +2,7 @@
>   USB Keyboard Driver that manages USB keyboard and produces Simple Text Input
>   Protocol and Simple Text Input Ex Protocol.
>
>-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2004 - 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 @@ -312,6 +312,17 @@ USBKeyboardDriverBindingStart (
>     goto ErrorExit;
>   }
>
>+  Status = gBS->CreateEvent (
>+                  EVT_NOTIFY_SIGNAL,
>+                  TPL_CALLBACK,
>+                  KeyNotifyProcessHandler,
>+                  UsbKeyboardDevice,
>+                  &UsbKeyboardDevice->KeyNotifyProcessEvent
>+                  );
>+  if (EFI_ERROR (Status)) {
>+    goto ErrorExit;
>+  }
>+
>   //
>   // Install Simple Text Input Protocol and Simple Text Input Ex Protocol
>   // for the USB keyboard device.
>@@ -427,6 +438,9 @@ ErrorExit:
>     if (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx != NULL) {
>       gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx);
>     }
>+    if (UsbKeyboardDevice->KeyNotifyProcessEvent != NULL) {
>+      gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>+    }
>     if (UsbKeyboardDevice->KeyboardLayoutEvent != NULL) {
>       ReleaseKeyboardLayoutResources (UsbKeyboardDevice);
>       gBS->CloseEvent (UsbKeyboardDevice->KeyboardLayoutEvent);
>@@ -548,6 +562,7 @@ USBKeyboardDriverBindingStop (
>   gBS->CloseEvent (UsbKeyboardDevice->DelayedRecoveryEvent);
>   gBS->CloseEvent (UsbKeyboardDevice->SimpleInput.WaitForKey);
>   gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx);
>+  gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>   KbdFreeNotifyList (&UsbKeyboardDevice->NotifyList);
>
>   ReleaseKeyboardLayoutResources (UsbKeyboardDevice); @@ -559,6 +574,7 
>@@ USBKeyboardDriverBindingStop (
>
>   DestroyQueue (&UsbKeyboardDevice->UsbKeyQueue);
>   DestroyQueue (&UsbKeyboardDevice->EfiKeyQueue);
>+  DestroyQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify);
>
>   FreePool (UsbKeyboardDevice);
>
>@@ -647,6 +663,7 @@ USBKeyboardReset (
>     //
>     InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY));
>     InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof 
>(EFI_KEY_DATA));
>+    InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof 
>+ (EFI_KEY_DATA));
>
>     return EFI_SUCCESS;
>   }
>@@ -1170,3 +1187,48 @@ USBKeyboardUnregisterKeyNotify (
>   return EFI_INVALID_PARAMETER;
> }
>
>+/**
>+  Process key notify.
>+
>+  @param  Event                 Indicates the event that invoke this function.
>+  @param  Context               Indicates the calling context.
>+**/
>+VOID
>+EFIAPI
>+KeyNotifyProcessHandler (
>+  IN  EFI_EVENT                 Event,
>+  IN  VOID                      *Context
>+  )
>+{
>+  USB_KB_DEV                    *UsbKeyboardDevice;
>+  EFI_KEY_DATA                  KeyData;
>+  LIST_ENTRY                    *Link;
>+  LIST_ENTRY                    *NotifyList;
>+  KEYBOARD_CONSOLE_IN_EX_NOTIFY *CurrentNotify;
>+  EFI_TPL                       OldTpl;
>+
>+  UsbKeyboardDevice = (USB_KB_DEV *) Context;
>+
>+  //
>+  // Invoke notification functions.
>+  //
>+  NotifyList = &UsbKeyboardDevice->NotifyList;  while (!IsQueueEmpty 
>+ (&UsbKeyboardDevice->EfiKeyQueueForNotify)) {
>+    //
>+    // Enter critical section
>+    //
>+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
>+    Dequeue (&UsbKeyboardDevice->EfiKeyQueueForNotify, &KeyData, sizeof (KeyData));
>+    //
>+    // Leave critical section
>+    //
>+    gBS->RestoreTPL (OldTpl);
>+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
>+      CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY, 
>+ NotifyEntry,
>USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
>+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
>+        CurrentNotify->KeyNotificationFn (&KeyData);
>+      }
>+    }
>+  }
>+}
>+
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h 
>b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>index 58edb3f65f2b..089f113d5fd4 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>@@ -1,7 +1,7 @@
> /** @file
>   Header file for USB Keyboard Driver's Data Structures.
>
>-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2004 - 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 @@ -114,6 +114,7 @@ typedef struct {
>
>   USB_SIMPLE_QUEUE                  UsbKeyQueue;
>   USB_SIMPLE_QUEUE                  EfiKeyQueue;
>+  USB_SIMPLE_QUEUE                  EfiKeyQueueForNotify;
>   BOOLEAN                           CtrlOn;
>   BOOLEAN                           AltOn;
>   BOOLEAN                           ShiftOn;
>@@ -149,6 +150,7 @@ typedef struct {
>   // Notification function list
>   //
>   LIST_ENTRY                        NotifyList;
>+  EFI_EVENT                         KeyNotifyProcessEvent;
>
>   //
>   // Non-spacing key list
>@@ -596,5 +598,18 @@ USBKeyboardTimerHandler (
>   IN  VOID                      *Context
>   );
>
>+/**
>+  Process key notify.
>+
>+  @param  Event                 Indicates the event that invoke this function.
>+  @param  Context               Indicates the calling context.
>+**/
>+VOID
>+EFIAPI
>+KeyNotifyProcessHandler (
>+  IN  EFI_EVENT                 Event,
>+  IN  VOID                      *Context
>+  );
>+
> #endif
>
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c 
>b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>index fef1449e3c35..a017d51e3b97 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>@@ -820,6 +820,7 @@ InitUSBKeyboard (
>
>   InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY));
>   InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
>+  InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof 
>+ (EFI_KEY_DATA));
>
>   //
>   // Use the config out of the descriptor @@ -1665,20 +1666,23 @@ 
>UsbKeyCodeToEfiInputKey (
>     KeyData->KeyState.KeyToggleState |= EFI_KEY_STATE_EXPOSED;
>   }
>   //
>-  // Invoke notification functions if the key is registered.
>+  // Signal KeyNotify process event if this key pressed matches any key registered.
>   //
>   NotifyList = &UsbKeyboardDevice->NotifyList;
>   for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
>     CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY, 
>NotifyEntry, USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
>     if (IsKeyRegistered (&CurrentNotify->KeyData, KeyData)) {
>-      CurrentNotify->KeyNotificationFn (KeyData);
>+      //
>+      // Insert to the EFI Key queue for notify.
>+      //
>+      Enqueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, KeyData, sizeof (*KeyData));
>+      gBS->SignalEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>     }
>   }
>
>   return EFI_SUCCESS;
> }
>
>-
> /**
>   Create the queue.
>
>--
>2.7.0.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK
  2016-12-23  7:05     ` Zeng, Star
@ 2016-12-23  7:09       ` Ni, Ruiyu
  2016-12-23  7:28         ` Zeng, Star
  0 siblings, 1 reply; 14+ messages in thread
From: Ni, Ruiyu @ 2016-12-23  7:09 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Tian, Feng

Star,
Another comment: I think we need to use TPL_NOTIFY to guard the call to IsQueueEmpty().

To simplify the code, how about create a EFI_LOCK (TPL_NOTIFY) to guard the access to EfiKeyQueueForNotify?
EfiAcquireLock() can be used to replace the RaiseTPL().


Regards,
Ray

From: Zeng, Star
Sent: Friday, December 23, 2016 3:05 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 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK

Ray,

Good comment, it can make the code more clear, I agree it.

Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu
Sent: Friday, December 23, 2016 3:01 PM
To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
Subject: RE: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK

Star,
I have one comment:
When call Enque(), can we put comments above the call to say that the key notification function needs to run at TPL_CALLBACK while current TPL is TPL_NOTIFY. It will be called in KeyNotifyProcessHandler() which runs at TPL_CALLBACK?

Regards,
Ray

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>Star Zeng
>Sent: Thursday, December 22, 2016 9:45 PM
>To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>Cc: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; Kinney, Michael D
><michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Zeng,
>Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
>Subject: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify
>func at TPL_CALLBACK
>
>Current implementation executes key notify function in TimerHandler at
>TPL_NOTIFY. The code change is to make key notify function executed at
>TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.
>
>The code will signal KeyNotify process event if the key pressed matches
>any key registered and insert the KeyData to the EFI Key queue for
>notify, then the KeyNotify process handler will invoke key notify
>functions at TPL_CALLBACK.
>
>Cc: Ruiyu Ni <Ruiyu.ni@intel.com<mailto:Ruiyu.ni@intel.com>>
>Cc: Michael Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
>Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com>>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
>---
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c   | 64 +++++++++++++++++++++++++++++++-
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h   | 17 ++++++++-
> MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 10 +++--
> 3 files changed, 86 insertions(+), 5 deletions(-)
>
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>index fb7558b73070..b1e1657173c3 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>@@ -2,7 +2,7 @@
>   USB Keyboard Driver that manages USB keyboard and produces Simple Text Input
>   Protocol and Simple Text Input Ex Protocol.
>
>-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2004 - 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 @@ -312,6 +312,17 @@ USBKeyboardDriverBindingStart (
>     goto ErrorExit;
>   }
>
>+  Status = gBS->CreateEvent (
>+                  EVT_NOTIFY_SIGNAL,
>+                  TPL_CALLBACK,
>+                  KeyNotifyProcessHandler,
>+                  UsbKeyboardDevice,
>+                  &UsbKeyboardDevice->KeyNotifyProcessEvent
>+                  );
>+  if (EFI_ERROR (Status)) {
>+    goto ErrorExit;
>+  }
>+
>   //
>   // Install Simple Text Input Protocol and Simple Text Input Ex Protocol
>   // for the USB keyboard device.
>@@ -427,6 +438,9 @@ ErrorExit:
>     if (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx != NULL) {
>       gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx);
>     }
>+    if (UsbKeyboardDevice->KeyNotifyProcessEvent != NULL) {
>+      gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>+    }
>     if (UsbKeyboardDevice->KeyboardLayoutEvent != NULL) {
>       ReleaseKeyboardLayoutResources (UsbKeyboardDevice);
>       gBS->CloseEvent (UsbKeyboardDevice->KeyboardLayoutEvent);
>@@ -548,6 +562,7 @@ USBKeyboardDriverBindingStop (
>   gBS->CloseEvent (UsbKeyboardDevice->DelayedRecoveryEvent);
>   gBS->CloseEvent (UsbKeyboardDevice->SimpleInput.WaitForKey);
>   gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx);
>+  gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>   KbdFreeNotifyList (&UsbKeyboardDevice->NotifyList);
>
>   ReleaseKeyboardLayoutResources (UsbKeyboardDevice); @@ -559,6 +574,7
>@@ USBKeyboardDriverBindingStop (
>
>   DestroyQueue (&UsbKeyboardDevice->UsbKeyQueue);
>   DestroyQueue (&UsbKeyboardDevice->EfiKeyQueue);
>+  DestroyQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify);
>
>   FreePool (UsbKeyboardDevice);
>
>@@ -647,6 +663,7 @@ USBKeyboardReset (
>     //
>     InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY));
>     InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof
>(EFI_KEY_DATA));
>+    InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof
>+ (EFI_KEY_DATA));
>
>     return EFI_SUCCESS;
>   }
>@@ -1170,3 +1187,48 @@ USBKeyboardUnregisterKeyNotify (
>   return EFI_INVALID_PARAMETER;
> }
>
>+/**
>+  Process key notify.
>+
>+  @param  Event                 Indicates the event that invoke this function.
>+  @param  Context               Indicates the calling context.
>+**/
>+VOID
>+EFIAPI
>+KeyNotifyProcessHandler (
>+  IN  EFI_EVENT                 Event,
>+  IN  VOID                      *Context
>+  )
>+{
>+  USB_KB_DEV                    *UsbKeyboardDevice;
>+  EFI_KEY_DATA                  KeyData;
>+  LIST_ENTRY                    *Link;
>+  LIST_ENTRY                    *NotifyList;
>+  KEYBOARD_CONSOLE_IN_EX_NOTIFY *CurrentNotify;
>+  EFI_TPL                       OldTpl;
>+
>+  UsbKeyboardDevice = (USB_KB_DEV *) Context;
>+
>+  //
>+  // Invoke notification functions.
>+  //
>+  NotifyList = &UsbKeyboardDevice->NotifyList;  while (!IsQueueEmpty
>+ (&UsbKeyboardDevice->EfiKeyQueueForNotify)) {
>+    //
>+    // Enter critical section
>+    //
>+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
>+    Dequeue (&UsbKeyboardDevice->EfiKeyQueueForNotify, &KeyData, sizeof (KeyData));
>+    //
>+    // Leave critical section
>+    //
>+    gBS->RestoreTPL (OldTpl);
>+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
>+      CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY,
>+ NotifyEntry,
>USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
>+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
>+        CurrentNotify->KeyNotificationFn (&KeyData);
>+      }
>+    }
>+  }
>+}
>+
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>index 58edb3f65f2b..089f113d5fd4 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>@@ -1,7 +1,7 @@
> /** @file
>   Header file for USB Keyboard Driver's Data Structures.
>
>-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2004 - 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 @@ -114,6 +114,7 @@ typedef struct {
>
>   USB_SIMPLE_QUEUE                  UsbKeyQueue;
>   USB_SIMPLE_QUEUE                  EfiKeyQueue;
>+  USB_SIMPLE_QUEUE                  EfiKeyQueueForNotify;
>   BOOLEAN                           CtrlOn;
>   BOOLEAN                           AltOn;
>   BOOLEAN                           ShiftOn;
>@@ -149,6 +150,7 @@ typedef struct {
>   // Notification function list
>   //
>   LIST_ENTRY                        NotifyList;
>+  EFI_EVENT                         KeyNotifyProcessEvent;
>
>   //
>   // Non-spacing key list
>@@ -596,5 +598,18 @@ USBKeyboardTimerHandler (
>   IN  VOID                      *Context
>   );
>
>+/**
>+  Process key notify.
>+
>+  @param  Event                 Indicates the event that invoke this function.
>+  @param  Context               Indicates the calling context.
>+**/
>+VOID
>+EFIAPI
>+KeyNotifyProcessHandler (
>+  IN  EFI_EVENT                 Event,
>+  IN  VOID                      *Context
>+  );
>+
> #endif
>
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>index fef1449e3c35..a017d51e3b97 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>@@ -820,6 +820,7 @@ InitUSBKeyboard (
>
>   InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY));
>   InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
>+  InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof
>+ (EFI_KEY_DATA));
>
>   //
>   // Use the config out of the descriptor @@ -1665,20 +1666,23 @@
>UsbKeyCodeToEfiInputKey (
>     KeyData->KeyState.KeyToggleState |= EFI_KEY_STATE_EXPOSED;
>   }
>   //
>-  // Invoke notification functions if the key is registered.
>+  // Signal KeyNotify process event if this key pressed matches any key registered.
>   //
>   NotifyList = &UsbKeyboardDevice->NotifyList;
>   for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
>     CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY,
>NotifyEntry, USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
>     if (IsKeyRegistered (&CurrentNotify->KeyData, KeyData)) {
>-      CurrentNotify->KeyNotificationFn (KeyData);
>+      //
>+      // Insert to the EFI Key queue for notify.
>+      //
>+      Enqueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, KeyData, sizeof (*KeyData));
>+      gBS->SignalEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>     }
>   }
>
>   return EFI_SUCCESS;
> }
>
>-
> /**
>   Create the queue.
>
>--
>2.7.0.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] Execute key notify function at TPL_CALLBACK
  2016-12-22 13:45 [PATCH 0/5] Execute key notify function at TPL_CALLBACK Star Zeng
                   ` (4 preceding siblings ...)
  2016-12-22 13:45 ` [PATCH 5/5] IntelFrameworkModulePkg KbDxe: " Star Zeng
@ 2016-12-23  7:11 ` Ni, Ruiyu
  2016-12-23  8:14   ` Zeng, Star
  5 siblings, 1 reply; 14+ messages in thread
From: Ni, Ruiyu @ 2016-12-23  7:11 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org
  Cc: Tian, Feng, Kinney, Michael D, Michael Zimmermann, Fan, Jeff,
	Zeng, Star

Star,
I checked all your patches.
My comments to Patch 1/5 also apply to your other patches.

Regards,
Ray

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star Zeng
>Sent: Thursday, December 22, 2016 9:45 PM
>To: edk2-devel@lists.01.org
>Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Tian, Feng <feng.tian@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>Michael Zimmermann <sigmaepsilon92@gmail.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
>Subject: [edk2] [PATCH 0/5] Execute key notify function at TPL_CALLBACK
>
>Current implementation of keyboard driver executes key notify
>function in TimerHandler at TPL_NOTIFY. The code change is to
>make key notify function executed at TPL_CALLBACK to reduce
>the time occupied at TPL_NOTIFY.
>
>The code will signal KeyNotify process event if the key pressed
>matches any key registered and insert the KeyData to the EFI Key
>queue for notify, then the KeyNotify process handler will invoke
>key notify functions at TPL_CALLBACK.
>
>It can also address the problem raised at
>https://lists.01.org/pipermail/edk2-devel/2016-December/005644.html
>
>Cc: Ruiyu Ni <Ruiyu.ni@intel.com>
>Cc: Michael Kinney <michael.d.kinney@intel.com>
>Cc: Feng Tian <feng.tian@intel.com>
>Cc: Jeff Fan <jeff.fan@intel.com>
>Cc: Michael Zimmermann <sigmaepsilon92@gmail.com>
>
>Star Zeng (5):
>  MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK
>  MdeModulePkg TerminalDxe: Execute key notify func at TPL_CALLBACK
>  MdeModulePkg Ps2KbDxe: Execute key notify func at TPL_CALLBACK
>  IntelFrameworkModulePkg Ps2KbDxe: Execute key notify func at
>    TPL_CALLBACK
>  IntelFrameworkModulePkg KbDxe: Execute key notify func at TPL_CALLBACK
>
> .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c         |   9 +-
> .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c          |  47 +++++-
> .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c           |  23 ++-
> .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h           |  15 ++
> .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c       |  75 ++++++++-
> .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h       |  17 +-
> .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c         |   7 +-
> MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c |  45 ++++++
> MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c  |  20 +++
> MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h  |  15 ++
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c             |  64 +++++++-
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h             |  17 +-
> MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c           |  10 +-
> .../Universal/Console/TerminalDxe/Terminal.c       |  25 ++-
> .../Universal/Console/TerminalDxe/Terminal.h       |  80 +++++++++-
> .../Universal/Console/TerminalDxe/TerminalConIn.c  | 177 ++++++++++++++++++++-
> 16 files changed, 622 insertions(+), 24 deletions(-)
>
>--
>2.7.0.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK
  2016-12-23  7:09       ` Ni, Ruiyu
@ 2016-12-23  7:28         ` Zeng, Star
  0 siblings, 0 replies; 14+ messages in thread
From: Zeng, Star @ 2016-12-23  7:28 UTC (permalink / raw)
  To: Ni, Ruiyu, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Tian, Feng, Zeng, Star

Ray,

Yes, it is a problem.
After discussion with you, we can eliminate the call to IsQueueEmpty() as Dequeue() has included the logic of IsQueueEmpty() and return status accordingly.
I will resend the V2 patch series.

Thanks again for the comments. :)

Star
From: Ni, Ruiyu
Sent: Friday, December 23, 2016 3:10 PM
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 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK

Star,
Another comment: I think we need to use TPL_NOTIFY to guard the call to IsQueueEmpty().

To simplify the code, how about create a EFI_LOCK (TPL_NOTIFY) to guard the access to EfiKeyQueueForNotify?
EfiAcquireLock() can be used to replace the RaiseTPL().


Regards,
Ray

From: Zeng, Star
Sent: Friday, December 23, 2016 3:05 PM
To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
Subject: RE: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK

Ray,

Good comment, it can make the code more clear, I agree it.

Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu
Sent: Friday, December 23, 2016 3:01 PM
To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
Subject: RE: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK

Star,
I have one comment:
When call Enque(), can we put comments above the call to say that the key notification function needs to run at TPL_CALLBACK while current TPL is TPL_NOTIFY. It will be called in KeyNotifyProcessHandler() which runs at TPL_CALLBACK?

Regards,
Ray

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>Star Zeng
>Sent: Thursday, December 22, 2016 9:45 PM
>To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>Cc: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; Kinney, Michael D
><michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Zeng,
>Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
>Subject: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify
>func at TPL_CALLBACK
>
>Current implementation executes key notify function in TimerHandler at
>TPL_NOTIFY. The code change is to make key notify function executed at
>TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY.
>
>The code will signal KeyNotify process event if the key pressed matches
>any key registered and insert the KeyData to the EFI Key queue for
>notify, then the KeyNotify process handler will invoke key notify
>functions at TPL_CALLBACK.
>
>Cc: Ruiyu Ni <Ruiyu.ni@intel.com<mailto:Ruiyu.ni@intel.com>>
>Cc: Michael Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
>Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com>>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
>---
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c   | 64 +++++++++++++++++++++++++++++++-
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h   | 17 ++++++++-
> MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 10 +++--
> 3 files changed, 86 insertions(+), 5 deletions(-)
>
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>index fb7558b73070..b1e1657173c3 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
>@@ -2,7 +2,7 @@
>   USB Keyboard Driver that manages USB keyboard and produces Simple Text Input
>   Protocol and Simple Text Input Ex Protocol.
>
>-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2004 - 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 @@ -312,6 +312,17 @@ USBKeyboardDriverBindingStart (
>     goto ErrorExit;
>   }
>
>+  Status = gBS->CreateEvent (
>+                  EVT_NOTIFY_SIGNAL,
>+                  TPL_CALLBACK,
>+                  KeyNotifyProcessHandler,
>+                  UsbKeyboardDevice,
>+                  &UsbKeyboardDevice->KeyNotifyProcessEvent
>+                  );
>+  if (EFI_ERROR (Status)) {
>+    goto ErrorExit;
>+  }
>+
>   //
>   // Install Simple Text Input Protocol and Simple Text Input Ex Protocol
>   // for the USB keyboard device.
>@@ -427,6 +438,9 @@ ErrorExit:
>     if (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx != NULL) {
>       gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx);
>     }
>+    if (UsbKeyboardDevice->KeyNotifyProcessEvent != NULL) {
>+      gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>+    }
>     if (UsbKeyboardDevice->KeyboardLayoutEvent != NULL) {
>       ReleaseKeyboardLayoutResources (UsbKeyboardDevice);
>       gBS->CloseEvent (UsbKeyboardDevice->KeyboardLayoutEvent);
>@@ -548,6 +562,7 @@ USBKeyboardDriverBindingStop (
>   gBS->CloseEvent (UsbKeyboardDevice->DelayedRecoveryEvent);
>   gBS->CloseEvent (UsbKeyboardDevice->SimpleInput.WaitForKey);
>   gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx);
>+  gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>   KbdFreeNotifyList (&UsbKeyboardDevice->NotifyList);
>
>   ReleaseKeyboardLayoutResources (UsbKeyboardDevice); @@ -559,6 +574,7
>@@ USBKeyboardDriverBindingStop (
>
>   DestroyQueue (&UsbKeyboardDevice->UsbKeyQueue);
>   DestroyQueue (&UsbKeyboardDevice->EfiKeyQueue);
>+  DestroyQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify);
>
>   FreePool (UsbKeyboardDevice);
>
>@@ -647,6 +663,7 @@ USBKeyboardReset (
>     //
>     InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY));
>     InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof
>(EFI_KEY_DATA));
>+    InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof
>+ (EFI_KEY_DATA));
>
>     return EFI_SUCCESS;
>   }
>@@ -1170,3 +1187,48 @@ USBKeyboardUnregisterKeyNotify (
>   return EFI_INVALID_PARAMETER;
> }
>
>+/**
>+  Process key notify.
>+
>+  @param  Event                 Indicates the event that invoke this function.
>+  @param  Context               Indicates the calling context.
>+**/
>+VOID
>+EFIAPI
>+KeyNotifyProcessHandler (
>+  IN  EFI_EVENT                 Event,
>+  IN  VOID                      *Context
>+  )
>+{
>+  USB_KB_DEV                    *UsbKeyboardDevice;
>+  EFI_KEY_DATA                  KeyData;
>+  LIST_ENTRY                    *Link;
>+  LIST_ENTRY                    *NotifyList;
>+  KEYBOARD_CONSOLE_IN_EX_NOTIFY *CurrentNotify;
>+  EFI_TPL                       OldTpl;
>+
>+  UsbKeyboardDevice = (USB_KB_DEV *) Context;
>+
>+  //
>+  // Invoke notification functions.
>+  //
>+  NotifyList = &UsbKeyboardDevice->NotifyList;  while (!IsQueueEmpty
>+ (&UsbKeyboardDevice->EfiKeyQueueForNotify)) {
>+    //
>+    // Enter critical section
>+    //
>+    OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
>+    Dequeue (&UsbKeyboardDevice->EfiKeyQueueForNotify, &KeyData, sizeof (KeyData));
>+    //
>+    // Leave critical section
>+    //
>+    gBS->RestoreTPL (OldTpl);
>+    for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
>+      CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY,
>+ NotifyEntry,
>USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
>+      if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) {
>+        CurrentNotify->KeyNotificationFn (&KeyData);
>+      }
>+    }
>+  }
>+}
>+
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>index 58edb3f65f2b..089f113d5fd4 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
>@@ -1,7 +1,7 @@
> /** @file
>   Header file for USB Keyboard Driver's Data Structures.
>
>-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2004 - 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 @@ -114,6 +114,7 @@ typedef struct {
>
>   USB_SIMPLE_QUEUE                  UsbKeyQueue;
>   USB_SIMPLE_QUEUE                  EfiKeyQueue;
>+  USB_SIMPLE_QUEUE                  EfiKeyQueueForNotify;
>   BOOLEAN                           CtrlOn;
>   BOOLEAN                           AltOn;
>   BOOLEAN                           ShiftOn;
>@@ -149,6 +150,7 @@ typedef struct {
>   // Notification function list
>   //
>   LIST_ENTRY                        NotifyList;
>+  EFI_EVENT                         KeyNotifyProcessEvent;
>
>   //
>   // Non-spacing key list
>@@ -596,5 +598,18 @@ USBKeyboardTimerHandler (
>   IN  VOID                      *Context
>   );
>
>+/**
>+  Process key notify.
>+
>+  @param  Event                 Indicates the event that invoke this function.
>+  @param  Context               Indicates the calling context.
>+**/
>+VOID
>+EFIAPI
>+KeyNotifyProcessHandler (
>+  IN  EFI_EVENT                 Event,
>+  IN  VOID                      *Context
>+  );
>+
> #endif
>
>diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>index fef1449e3c35..a017d51e3b97 100644
>--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
>@@ -820,6 +820,7 @@ InitUSBKeyboard (
>
>   InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY));
>   InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA));
>+  InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof
>+ (EFI_KEY_DATA));
>
>   //
>   // Use the config out of the descriptor @@ -1665,20 +1666,23 @@
>UsbKeyCodeToEfiInputKey (
>     KeyData->KeyState.KeyToggleState |= EFI_KEY_STATE_EXPOSED;
>   }
>   //
>-  // Invoke notification functions if the key is registered.
>+  // Signal KeyNotify process event if this key pressed matches any key registered.
>   //
>   NotifyList = &UsbKeyboardDevice->NotifyList;
>   for (Link = GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Link = GetNextNode (NotifyList, Link)) {
>     CurrentNotify = CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY,
>NotifyEntry, USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE);
>     if (IsKeyRegistered (&CurrentNotify->KeyData, KeyData)) {
>-      CurrentNotify->KeyNotificationFn (KeyData);
>+      //
>+      // Insert to the EFI Key queue for notify.
>+      //
>+      Enqueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, KeyData, sizeof (*KeyData));
>+      gBS->SignalEvent (UsbKeyboardDevice->KeyNotifyProcessEvent);
>     }
>   }
>
>   return EFI_SUCCESS;
> }
>
>-
> /**
>   Create the queue.
>
>--
>2.7.0.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] Execute key notify function at TPL_CALLBACK
  2016-12-23  7:11 ` [PATCH 0/5] Execute key notify function " Ni, Ruiyu
@ 2016-12-23  8:14   ` Zeng, Star
  0 siblings, 0 replies; 14+ messages in thread
From: Zeng, Star @ 2016-12-23  8:14 UTC (permalink / raw)
  To: Ni, Ruiyu, edk2-devel@lists.01.org
  Cc: Tian, Feng, Kinney, Michael D, Michael Zimmermann, Fan, Jeff,
	Zeng, Star

Ray,

Thanks for the comments, I just sent out the V2 patch series based on the disucssion, please help review them again. :)

Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu 
Sent: Friday, December 23, 2016 3:12 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Tian, Feng <feng.tian@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Michael Zimmermann <sigmaepsilon92@gmail.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH 0/5] Execute key notify function at TPL_CALLBACK

Star,
I checked all your patches.
My comments to Patch 1/5 also apply to your other patches.

Regards,
Ray

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
>Star Zeng
>Sent: Thursday, December 22, 2016 9:45 PM
>To: edk2-devel@lists.01.org
>Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Tian, Feng <feng.tian@intel.com>; 
>Kinney, Michael D <michael.d.kinney@intel.com>; Michael Zimmermann 
><sigmaepsilon92@gmail.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star 
><star.zeng@intel.com>
>Subject: [edk2] [PATCH 0/5] Execute key notify function at TPL_CALLBACK
>
>Current implementation of keyboard driver executes key notify function 
>in TimerHandler at TPL_NOTIFY. The code change is to make key notify 
>function executed at TPL_CALLBACK to reduce the time occupied at 
>TPL_NOTIFY.
>
>The code will signal KeyNotify process event if the key pressed matches 
>any key registered and insert the KeyData to the EFI Key queue for 
>notify, then the KeyNotify process handler will invoke key notify 
>functions at TPL_CALLBACK.
>
>It can also address the problem raised at 
>https://lists.01.org/pipermail/edk2-devel/2016-December/005644.html
>
>Cc: Ruiyu Ni <Ruiyu.ni@intel.com>
>Cc: Michael Kinney <michael.d.kinney@intel.com>
>Cc: Feng Tian <feng.tian@intel.com>
>Cc: Jeff Fan <jeff.fan@intel.com>
>Cc: Michael Zimmermann <sigmaepsilon92@gmail.com>
>
>Star Zeng (5):
>  MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK
>  MdeModulePkg TerminalDxe: Execute key notify func at TPL_CALLBACK
>  MdeModulePkg Ps2KbDxe: Execute key notify func at TPL_CALLBACK
>  IntelFrameworkModulePkg Ps2KbDxe: Execute key notify func at
>    TPL_CALLBACK
>  IntelFrameworkModulePkg KbDxe: Execute key notify func at 
>TPL_CALLBACK
>
> .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c         |   9 +-
> .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c          |  47 +++++-
> .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c           |  23 ++-
> .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h           |  15 ++
> .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c       |  75 ++++++++-
> .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h       |  17 +-
> .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c         |   7 +-
> MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c |  45 ++++++ 
> MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.c  |  20 +++ 
> MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h  |  15 ++
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c             |  64 +++++++-
> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h             |  17 +-
> MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c           |  10 +-
> .../Universal/Console/TerminalDxe/Terminal.c       |  25 ++-
> .../Universal/Console/TerminalDxe/Terminal.h       |  80 +++++++++-
> .../Universal/Console/TerminalDxe/TerminalConIn.c  | 177 
> ++++++++++++++++++++-
> 16 files changed, 622 insertions(+), 24 deletions(-)
>
>--
>2.7.0.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-12-23  8:14 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-22 13:45 [PATCH 0/5] Execute key notify function at TPL_CALLBACK Star Zeng
2016-12-22 13:45 ` [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func " Star Zeng
2016-12-23  7:01   ` Ni, Ruiyu
2016-12-23  7:05     ` Zeng, Star
2016-12-23  7:09       ` Ni, Ruiyu
2016-12-23  7:28         ` Zeng, Star
2016-12-22 13:45 ` [PATCH 2/5] MdeModulePkg TerminalDxe: " Star Zeng
2016-12-22 13:45 ` [PATCH 3/5] MdeModulePkg Ps2KbDxe: " Star Zeng
2016-12-22 13:45 ` [PATCH 4/5] IntelFrameworkModulePkg " Star Zeng
2016-12-23  2:16   ` Fan, Jeff
2016-12-22 13:45 ` [PATCH 5/5] IntelFrameworkModulePkg KbDxe: " Star Zeng
2016-12-23  2:16   ` Fan, Jeff
2016-12-23  7:11 ` [PATCH 0/5] Execute key notify function " Ni, Ruiyu
2016-12-23  8:14   ` Zeng, Star

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox