Heyi, I agree the source code required to detect the current tick rate using only UEFI services is more complex.
However, a UEFI driver (especially ones on an add-in devices such as a PCI adapter) should not use a PCD for the system tick rate because the add-in card can be used in systems with different system tick rates. If you are concerned about complexity, we could consider adding a new lib function to the UefiLib that returns the current system tick rate using UEFI services to detect it. This way, A UEFI driver can be kept simple and we move the complexity into a single new lib function. Best regards, Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Heyi > Guo > Sent: Tuesday, April 26, 2016 8:14 AM > To: Kinney, Michael D <michael.d.kin...@intel.com>; edk2-devel@lists.01.org > Cc: Tian, Feng <feng.t...@intel.com>; Zeng, Star <star.z...@intel.com> > Subject: Re: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by > serial IO > mode > > Hi Michael, > > It seems we are making the implementation more and more complicated. How > about just creating a PCD for polling rate which can be set freely by > platforms? > > Regards. > > Heyi > > On 04/24/2016 12:11 AM, Kinney, Michael D wrote: > > Heyi Guo, > > > > The TerminalDxe driver is intended to be a UEFI Driver. The Timer Arch > > Protocol is > > a PI Protocol that is intended to be used by the PI DXE Core. In order to > > determine > > the timer rate in a UEFI way, create a periodic timer event with a period > > of 1, 100ns > > unit, and then measure the time between timer event notification functions. > > > > Mike > > > >> -----Original Message----- > >> From: Heyi Guo [mailto:heyi....@linaro.org] > >> Sent: Saturday, April 23, 2016 1:54 AM > >> To: edk2-devel@lists.01.org > >> Cc: Heyi Guo <heyi....@linaro.org>; Tian, Feng <feng.t...@intel.com>; > >> Zeng, Star > >> <star.z...@intel.com>; Kinney, Michael D <michael.d.kin...@intel.com> > >> Subject: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by > >> serial IO mode > >> > >> Calculate serial input polling rate according to parameters from > >> serial IO mode as below, to fix potential input truncation. > >> > >> Polling interval (100ns) = > >> FifoDepth * (StartBit + DataBits + StopBits + ParityBits) * 10,000,000 > >> / BaudRate > >> (StopBits is assumed to be 1 and ParityBits is ignored for simplicity. > >> > >> However, as the event is time sensitive, we need to align the interval > >> to timer interrupt period to make sure the event will be triggered at > >> the expected rate. E.g. if the interval is 2.7ms and timer interrupt > >> period is 1ms, the event will be triggered by time slice sequence as > >> below: > >> 3ms 3ms 2ms 3ms 3ms 2ms... > >> > >> In such case we will adjust the polling interval to be 2ms. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Heyi Guo <heyi....@linaro.org> > >> Cc: Feng Tian <feng.t...@intel.com> > >> Cc: Star Zeng <star.z...@intel.com> > >> Cc: Michael D Kinney <michael.d.kin...@intel.com> > >> --- > >> .../Universal/Console/TerminalDxe/Terminal.c | 5 +- > >> .../Universal/Console/TerminalDxe/Terminal.h | 28 ++++++- > >> .../Universal/Console/TerminalDxe/TerminalConIn.c | 92 > >> ++++++++++++++++++++++ > >> .../Universal/Console/TerminalDxe/TerminalDxe.inf | 1 + > >> 4 files changed, 123 insertions(+), 3 deletions(-) > >> > >> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c > >> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c > >> index 6fde3b2..2944707 100644 > >> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c > >> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c > >> @@ -71,6 +71,7 @@ TERMINAL_DEV mTerminalDevTemplate = { > >> }, > >> NULL, // TerminalConsoleModeData > >> 0, // SerialInTimeOut > >> + 0, // KeyboardTimerInterval > >> > >> NULL, // RawFifo > >> NULL, // UnicodeFiFo > >> @@ -984,10 +985,12 @@ TerminalDriverBindingStart ( > >> ); > >> ASSERT_EFI_ERROR (Status); > >> > >> + TerminalDevice->KeyboardTimerInterval = GetKeyboardTimerInterval > >> (Mode); > >> + > >> Status = gBS->SetTimer ( > >> TerminalDevice->TimerEvent, > >> TimerPeriodic, > >> - KEYBOARD_TIMER_INTERVAL > >> + TerminalDevice->KeyboardTimerInterval > >> ); > >> ASSERT_EFI_ERROR (Status); > >> > >> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h > >> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h > >> index 269d2ae..a1ff595 100644 > >> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h > >> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h > >> @@ -28,6 +28,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > >> EITHER EXPRESS > >> OR IMPLIED. > >> #include <Protocol/DevicePath.h> > >> #include <Protocol/SimpleTextIn.h> > >> #include <Protocol/SimpleTextInEx.h> > >> +#include <Protocol/Timer.h> > >> > >> #include <Library/DebugLib.h> > >> #include <Library/UefiDriverEntryPoint.h> > >> @@ -68,8 +69,6 @@ typedef struct { > >> UINTN Rows; > >> } TERMINAL_CONSOLE_MODE_DATA; > >> > >> -#define KEYBOARD_TIMER_INTERVAL 200000 // 0.02s > >> - > >> #define TERMINAL_DEV_SIGNATURE SIGNATURE_32 ('t', 'm', 'n', 'l') > >> > >> #define TERMINAL_CONSOLE_IN_EX_NOTIFY_SIGNATURE SIGNATURE_32 ('t', 'm', > >> 'e', 'n') > >> @@ -91,6 +90,7 @@ typedef struct { > >> EFI_SIMPLE_TEXT_OUTPUT_MODE SimpleTextOutputMode; > >> TERMINAL_CONSOLE_MODE_DATA *TerminalConsoleModeData; > >> UINTN SerialInTimeOut; > >> + UINT64 KeyboardTimerInterval; > >> RAW_DATA_FIFO *RawFiFo; > >> UNICODE_FIFO *UnicodeFiFo; > >> EFI_KEY_FIFO *EfiKeyFiFo; > >> @@ -1358,4 +1358,28 @@ TerminalConInTimerHandler ( > >> IN EFI_EVENT Event, > >> IN VOID *Context > >> ); > >> + > >> +/** > >> + Calculate input polling timer interval by serial IO mode. > >> + > >> + @param Mode Pointer to serial IO mode. > >> + > >> + @retval The required polling timer interval in 100ns. > >> + > >> +**/ > >> +UINT64 > >> +GetKeyboardTimerInterval ( > >> + IN EFI_SERIAL_IO_MODE *Mode > >> + ); > >> + > >> +/** > >> + Update period of polling timer event. > >> + > >> + @param TerminalDevice The terminal device to update. > >> +**/ > >> +VOID > >> +UpdatePollingRate ( > >> + IN TERMINAL_DEV *TerminalDevice > >> + ); > >> + > >> #endif > >> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c > >> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c > >> index 3be877b..e7788a0 100644 > >> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c > >> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c > >> @@ -15,6 +15,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > >> EITHER EXPRESS > >> OR IMPLIED. > >> > >> #include "Terminal.h" > >> > >> +EFI_TIMER_ARCH_PROTOCOL *gTimer; > >> > >> /** > >> Reads the next keystroke from the input device. The WaitForKey Event > >> can > >> @@ -502,6 +503,94 @@ TerminalConInWaitForKey ( > >> } > >> > >> /** > >> + Calculate input polling timer interval by serial IO mode. > >> + > >> + @param Mode Pointer to serial IO mode. > >> + > >> + @retval The required polling timer interval in 100ns. > >> + > >> +**/ > >> +UINT64 > >> +GetKeyboardTimerInterval ( > >> + IN EFI_SERIAL_IO_MODE *Mode > >> + ) > >> +{ > >> + UINT32 FifoDepth; > >> + UINT64 BaudRate; > >> + UINT64 Interval; > >> + UINT64 TimerPeriod; > >> + EFI_STATUS Status; > >> + > >> + // Make some assumption if the values are not suitable for calculating. > >> + BaudRate = Mode->BaudRate; > >> + if (BaudRate == 0) { > >> + BaudRate = 115200; > >> + } > >> + FifoDepth = Mode->ReceiveFifoDepth; > >> + if (FifoDepth == 0) { > >> + FifoDepth = 1; > >> + } > >> + > >> + // We assume stop bits to be 1 and ignore parity bit to make it simple > >> + // and fast enough to poll. > >> + Interval = DivU64x64Remainder ( > >> + FifoDepth * (1 + Mode->DataBits + 1) * 10000000, > >> + Mode->BaudRate, > >> + NULL > >> + ); > >> + > >> + // As this is a time sensitive event, we still need to align the > >> + // interval to timer interrupt period. > >> + if (gTimer == NULL) { > >> + Status = gBS->LocateProtocol ( > >> + &gEfiTimerArchProtocolGuid, > >> + EFI_NATIVE_INTERFACE, > >> + (VOID **) &gTimer > >> + ); > >> + ASSERT_EFI_ERROR (Status); > >> + } > >> + > >> + Status = gTimer->GetTimerPeriod (gTimer, &TimerPeriod); > >> + ASSERT_EFI_ERROR (Status); > >> + > >> + if (Interval <= TimerPeriod) { > >> + return TimerPeriod; > >> + } > >> + return MultU64x64 (DivU64x64Remainder (Interval, TimerPeriod, NULL), > TimerPeriod); > >> +} > >> + > >> + > >> +/** > >> + Update period of polling timer event. > >> + > >> + @param TerminalDevice The terminal device to update. > >> +**/ > >> +VOID > >> +UpdatePollingRate ( > >> + IN TERMINAL_DEV *TerminalDevice > >> + ) > >> +{ > >> + UINT64 NewInterval; > >> + EFI_STATUS Status; > >> + > >> + NewInterval = GetKeyboardTimerInterval (TerminalDevice->SerialIo->Mode); > >> + > >> + if (TerminalDevice->KeyboardTimerInterval == NewInterval) { > >> + return; > >> + } > >> + > >> + Status = gBS->SetTimer ( > >> + TerminalDevice->TimerEvent, > >> + TimerPeriodic, > >> + NewInterval > >> + ); > >> + ASSERT_EFI_ERROR (Status); > >> + > >> + TerminalDevice->KeyboardTimerInterval = NewInterval; > >> +} > >> + > >> + > >> +/** > >> Timer handler to poll the key from serial. > >> > >> @param Event Indicates the event that invoke this > >> function. > >> @@ -560,6 +649,9 @@ TerminalConInTimerHandler ( > >> TerminalDevice->SerialInTimeOut = SerialInTimeOut; > >> } > >> } > >> + > >> + UpdatePollingRate (TerminalDevice); > >> + > >> // > >> // Check whether serial buffer is empty. > >> // Skip the key transfer loop only if the SerialIo protocol instance > >> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf > >> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf > >> index 0780296..dfd5035 100644 > >> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf > >> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf > >> @@ -84,6 +84,7 @@ > >> gEfiSimpleTextInProtocolGuid ## BY_START > >> gEfiSimpleTextInputExProtocolGuid ## BY_START > >> gEfiSimpleTextOutProtocolGuid ## BY_START > >> + gEfiTimerArchProtocolGuid ## CONSUMES > >> > >> [Pcd] > >> gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType ## > >> SOMETIMES_CONSUMES > >> -- > >> 2.7.0 > > _______________________________________________ > 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
