Hi Michael,

I still have questions about measuring timer tick by event:

1. How can we measure the time elapsed? TimerLib is not in UEFI spec, and shall we use gBS->Stall in a loop to measure the time? 2. By using a periodic timer event, I think we need to drop the 1st trigger as it is random and determined by the time when event is created, isn't it? 3. For the timer event with period of 1*100ns, I think it might be triggered twice in one timer tick by below code:
Timer.c, line #143:
Event->Timer.TriggerTime = Event->Timer.TriggerTime + Event->Timer.Period;

      //
      // If that's before now, then reset the timer to start from now
      //
      if (Event->Timer.TriggerTime <= SystemTime) {
        Event->Timer.TriggerTime = SystemTime;
        CoreSignalEvent (mEfiCheckTimerEvent);
      }
4. How about setting TriggerTime=0 when calling SetTimer so that the period of event will be exactly the timer tick, as stated in UEFI spec?

Thanks and regards.

Heyi


On 04/27/2016 12:21 AM, Kinney, Michael D wrote:
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:[email protected]] On Behalf Of Heyi Guo
Sent: Tuesday, April 26, 2016 8:14 AM
To: Kinney, Michael D <[email protected]>; [email protected]
Cc: Tian, Feng <[email protected]>; Zeng, Star <[email protected]>
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:[email protected]]
Sent: Saturday, April 23, 2016 1:54 AM
To: [email protected]
Cc: Heyi Guo <[email protected]>; Tian, Feng <[email protected]>; Zeng, Star
<[email protected]>; Kinney, Michael D <[email protected]>
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 <[email protected]>
Cc: Feng Tian <[email protected]>
Cc: Star Zeng <[email protected]>
Cc: Michael D Kinney <[email protected]>
---
   .../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
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to