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.z...@intel.com>; edk2-devel@lists.01.org Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Tian, Feng <feng.t...@intel.com>; Zeng, Star <star.z...@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-boun...@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...@intel.com>; Kinney, Michael D ><michael.d.kin...@intel.com>; Tian, Feng <feng.t...@intel.com>; Zeng, >Star <star.z...@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...@intel.com> >Cc: Michael Kinney <michael.d.kin...@intel.com> >Cc: Feng Tian <feng.t...@intel.com> >Contributed-under: TianoCore Contribution Agreement 1.0 >Signed-off-by: Star Zeng <star.z...@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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel