* [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements
@ 2016-10-07 14:53 Brian J. Johnson
2016-10-07 14:53 ` [PATCH 1/3] MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking Brian J. Johnson
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Brian J. Johnson @ 2016-10-07 14:53 UTC (permalink / raw)
To: edk2-devel; +Cc: Brian J. Johnson, Feng Tian, Star Zeng
This patch series implements some improvements to the TtyTerm terminal
type in the TerminalDxe driver. It fixes an end case with cursor
position tracking, and uses that to optimize cursor motion escape
sequences. It also adds support for the page up, page down, insert,
home, and end keys on some additional common terminal emulators.
The result is improved performance, especially at the shell prompt,
and better compatibility with common terminal emulators. In
particular, as a side effect of the optimized cursor motion, terminal
windows which are taller than the current mode setting (eg. 25 lines)
work much better than before.
Most of these fixes have been in production in some form on SGI's
servers for years.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brian Johnson <bjohnson@sgi.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Brian J. Johnson (3):
MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
.../Universal/Console/TerminalDxe/Terminal.h | 2 +
.../Universal/Console/TerminalDxe/TerminalConIn.c | 24 +++++++--
.../Universal/Console/TerminalDxe/TerminalConOut.c | 61 ++++++++++++++++++++--
3 files changed, 79 insertions(+), 8 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
2016-10-07 14:53 [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements Brian J. Johnson
@ 2016-10-07 14:53 ` Brian J. Johnson
2016-10-07 14:53 ` [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion Brian J. Johnson
` (2 subsequent siblings)
3 siblings, 0 replies; 18+ messages in thread
From: Brian J. Johnson @ 2016-10-07 14:53 UTC (permalink / raw)
To: edk2-devel; +Cc: Brian J. Johnson, Feng Tian, Star Zeng
When we print the last character on a line, the terminal driver wraps
CursorRow/CursorColumn to the beginning of the next line. But the
terminal itself doesn't wrap its cursor until the next character is
printed. That throws off the driver's cursor position tracking.
So when we have printed the last character on a line, and are not in
the middle of outputing an escape sequence, synchronize the terminal
with the driver by outputing CR+LF. This matches the expected
behavior, and the behavior of the VGA console driver.
Only change the behavior of TtyTerm, not the other terminal types.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brian Johnson <bjohnson@sgi.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
.../Universal/Console/TerminalDxe/TerminalConOut.c | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
index 9fa952a..b11e83f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
@@ -2,6 +2,7 @@
Implementation for EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL protocol.
Copyright (c) 2006 - 2010, 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
which accompanies this distribution. The full text of the license may be found at
@@ -313,6 +314,30 @@ TerminalConOutOutputString (
Mode->CursorRow++;
}
+ if (TerminalDevice->TerminalType == TTYTERMTYPE &&
+ !TerminalDevice->OutputEscChar) {
+ //
+ // We've written the last character on the line. The
+ // terminal doesn't actually wrap its cursor until we print
+ // the next character, but the driver thinks it has wrapped
+ // already. Print CR LF to synchronize the terminal with
+ // the driver, but only if we're not in the middle of
+ // printing an escape sequence.
+ //
+ CHAR8 CrLfStr[] = {'\r', '\n'};
+
+ Length = sizeof(CrLfStr);
+
+ Status = TerminalDevice->SerialIo->Write (
+ TerminalDevice->SerialIo,
+ &Length,
+ CrLfStr
+ );
+
+ if (EFI_ERROR (Status)) {
+ goto OutputError;
+ }
+ }
}
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
2016-10-07 14:53 [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements Brian J. Johnson
2016-10-07 14:53 ` [PATCH 1/3] MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking Brian J. Johnson
@ 2016-10-07 14:53 ` Brian J. Johnson
2016-10-27 3:09 ` Kinney, Michael D
2016-10-07 14:54 ` [PATCH 3/3] MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm Brian J. Johnson
2016-10-07 15:56 ` [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements Laszlo Ersek
3 siblings, 1 reply; 18+ messages in thread
From: Brian J. Johnson @ 2016-10-07 14:53 UTC (permalink / raw)
To: edk2-devel; +Cc: Brian J. Johnson, Feng Tian, Star Zeng
For TtyTerm terminals, output a shorter escape sequence when possible
to move the cursor within the current line, and don't print any escape
sequence if the cursor is already at the correct position. This
removes extra cursor motion activity at the EFI shell prompt,
improving performance. It also makes it possible in many cases to
successfully use a terminal window which is taller than the driver's
mode setting (eg. 80x25.)
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brian Johnson <bjohnson@sgi.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
.../Universal/Console/TerminalDxe/Terminal.h | 2 ++
.../Universal/Console/TerminalDxe/TerminalConOut.c | 36 +++++++++++++++++++---
2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 269d2ae..3ee3969 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -2,6 +2,7 @@
Header file for Terminal driver.
Copyright (c) 2006 - 2014, 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
which accompanies this distribution. The full text of the license may be found at
@@ -157,6 +158,7 @@ typedef union {
#define BACKGROUND_CONTROL_OFFSET 11
#define ROW_OFFSET 2
#define COLUMN_OFFSET 5
+#define FW_BACK_OFFSET 2
typedef struct {
UINT16 Unicode;
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
index b11e83f..e9b5ed0 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
@@ -83,6 +83,8 @@ CHAR16 mSetModeString[] = { ESC, '[', '=', '3', 'h', 0 };
CHAR16 mSetAttributeString[] = { ESC, '[', '0', 'm', ESC, '[', '4', '0', 'm', ESC, '[', '4', '0', 'm', 0 };
CHAR16 mClearScreenString[] = { ESC, '[', '2', 'J', 0 };
CHAR16 mSetCursorPositionString[] = { ESC, '[', '0', '0', ';', '0', '0', 'H', 0 };
+CHAR16 mCursorForwardString[] = { ESC, '[', '0', '0', 'C', 0 };
+CHAR16 mCursorBackwardString[] = { ESC, '[', '0', '0', 'D', 0 };
//
// Body of the ConOut functions
@@ -755,6 +757,7 @@ TerminalConOutSetCursorPosition (
UINTN MaxRow;
EFI_STATUS Status;
TERMINAL_DEV *TerminalDevice;
+ CHAR16 *String;
TerminalDevice = TERMINAL_CON_OUT_DEV_FROM_THIS (This);
@@ -782,13 +785,36 @@ TerminalConOutSetCursorPosition (
//
// control sequence to move the cursor
//
- mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) / 10));
- mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) % 10));
- mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' + ((Column + 1) / 10));
- mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' + ((Column + 1) % 10));
+ // Optimize cursor motion control sequences for TtyTerm. Move
+ // within the current line if possible, and don't output anyting if
+ // it isn't necessary.
+ //
+ if (TerminalDevice->TerminalType == TTYTERMTYPE &&
+ Mode->CursorRow == Row) {
+ if (Mode->CursorColumn > Column) {
+ mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Mode->CursorColumn - Column) / 10));
+ mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Mode->CursorColumn - Column) % 10));
+ String = mCursorBackwardString;
+ }
+ else if (Column > Mode->CursorColumn) {
+ mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Column - Mode->CursorColumn) / 10));
+ mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Column - Mode->CursorColumn) % 10));
+ String = mCursorForwardString;
+ }
+ else {
+ String = L""; // No cursor motion necessary
+ }
+ }
+ else {
+ mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) / 10));
+ mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) % 10));
+ mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' + ((Column + 1) / 10));
+ mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' + ((Column + 1) % 10));
+ String = mSetCursorPositionString;
+ }
TerminalDevice->OutputEscChar = TRUE;
- Status = This->OutputString (This, mSetCursorPositionString);
+ Status = This->OutputString (This, String);
TerminalDevice->OutputEscChar = FALSE;
if (EFI_ERROR (Status)) {
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
2016-10-07 14:53 [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements Brian J. Johnson
2016-10-07 14:53 ` [PATCH 1/3] MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking Brian J. Johnson
2016-10-07 14:53 ` [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion Brian J. Johnson
@ 2016-10-07 14:54 ` Brian J. Johnson
2016-10-14 20:53 ` Roy Franz (HPE)
2016-10-07 15:56 ` [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements Laszlo Ersek
3 siblings, 1 reply; 18+ messages in thread
From: Brian J. Johnson @ 2016-10-07 14:54 UTC (permalink / raw)
To: edk2-devel; +Cc: Brian J. Johnson, Feng Tian, Star Zeng
The TtyTerm terminal driver is missing support for sequences produced
by the page up, page down, insert, home, and end keys in some terimnal
emulators. Add them.
Tested under Ubuntu 16.04 using xterm 322-1ubuntu1, GNOME terminal
3.18.3-1ubuntu1, and XFCE terminal 0.6.3-2ubuntu1.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Kyle Roberts <kyroberts@sgi.com>
Signed-off-by: Brian Johnson <bjohnson@sgi.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
.../Universal/Console/TerminalDxe/TerminalConIn.c | 24 +++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 3be877b..5c3ea86 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -3,6 +3,7 @@
(C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
Copyright (c) 2006 - 2015, 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
which accompanies this distribution. The full text of the license may be found at
@@ -1374,7 +1375,7 @@ UnicodeToEfiKey (
break;
}
} else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
- /* Also accept VT100 escape codes for F1-F4 for TTY term */
+ /* Also accept VT100 escape codes for F1-F4, HOME and END for TTY term */
switch (UnicodeChar) {
case 'P':
Key.ScanCode = SCAN_F1;
@@ -1388,6 +1389,12 @@ UnicodeToEfiKey (
case 'S':
Key.ScanCode = SCAN_F4;
break;
+ case 'H':
+ Key.ScanCode = SCAN_HOME;
+ break;
+ case 'F':
+ Key.ScanCode = SCAN_END;
+ break;
}
}
@@ -1429,12 +1436,14 @@ UnicodeToEfiKey (
break;
case 'H':
if (TerminalDevice->TerminalType == PCANSITYPE ||
- TerminalDevice->TerminalType == VT100TYPE) {
+ TerminalDevice->TerminalType == VT100TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE) {
Key.ScanCode = SCAN_HOME;
}
break;
case 'F':
- if (TerminalDevice->TerminalType == PCANSITYPE) {
+ if (TerminalDevice->TerminalType == PCANSITYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE) {
Key.ScanCode = SCAN_END;
}
break;
@@ -1573,9 +1582,18 @@ UnicodeToEfiKey (
TerminalDevice->TtyEscapeStr[TerminalDevice->TtyEscapeIndex] = 0; /* Terminate string */
EscCode = (UINT16) StrDecimalToUintn(TerminalDevice->TtyEscapeStr);
switch (EscCode) {
+ case 2:
+ Key.ScanCode = SCAN_INSERT;
+ break;
case 3:
Key.ScanCode = SCAN_DELETE;
break;
+ case 5:
+ Key.ScanCode = SCAN_PAGE_UP;
+ break;
+ case 6:
+ Key.ScanCode = SCAN_PAGE_DOWN;
+ break;
case 11:
case 12:
case 13:
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements
2016-10-07 14:53 [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements Brian J. Johnson
` (2 preceding siblings ...)
2016-10-07 14:54 ` [PATCH 3/3] MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm Brian J. Johnson
@ 2016-10-07 15:56 ` Laszlo Ersek
2016-10-07 15:59 ` Leif Lindholm
3 siblings, 1 reply; 18+ messages in thread
From: Laszlo Ersek @ 2016-10-07 15:56 UTC (permalink / raw)
To: Roy Franz, Ryan Harkin; +Cc: Brian J. Johnson, edk2-devel, Feng Tian, Star Zeng
Roy, Ryan,
On 10/07/16 16:53, Brian J. Johnson wrote:
> This patch series implements some improvements to the TtyTerm terminal
> type in the TerminalDxe driver. It fixes an end case with cursor
> position tracking, and uses that to optimize cursor motion escape
> sequences. It also adds support for the page up, page down, insert,
> home, and end keys on some additional common terminal emulators.
>
> The result is improved performance, especially at the shell prompt,
> and better compatibility with common terminal emulators. In
> particular, as a side effect of the optimized cursor motion, terminal
> windows which are taller than the current mode setting (eg. 25 lines)
> work much better than before.
>
> Most of these fixes have been in production in some form on SGI's
> servers for years.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
>
> Brian J. Johnson (3):
> MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
> MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
> MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
>
> .../Universal/Console/TerminalDxe/Terminal.h | 2 +
> .../Universal/Console/TerminalDxe/TerminalConIn.c | 24 +++++++--
> .../Universal/Console/TerminalDxe/TerminalConOut.c | 61 ++++++++++++++++++++--
> 3 files changed, 79 insertions(+), 8 deletions(-)
>
can you please provide feedback (testing or otherwise) on this series?
Thanks
Laszlo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements
2016-10-07 15:56 ` [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements Laszlo Ersek
@ 2016-10-07 15:59 ` Leif Lindholm
2016-10-12 8:17 ` Ryan Harkin
0 siblings, 1 reply; 18+ messages in thread
From: Leif Lindholm @ 2016-10-07 15:59 UTC (permalink / raw)
To: Laszlo Ersek
Cc: Roy Franz, Ryan Harkin, Brian J. Johnson, edk2-devel, Feng Tian,
Star Zeng
Roy can now be found at Roy Franz <roy.franz@hpe.com> (cc:d).
On Fri, Oct 07, 2016 at 05:56:26PM +0200, Laszlo Ersek wrote:
> Roy, Ryan,
>
> On 10/07/16 16:53, Brian J. Johnson wrote:
> > This patch series implements some improvements to the TtyTerm terminal
> > type in the TerminalDxe driver. It fixes an end case with cursor
> > position tracking, and uses that to optimize cursor motion escape
> > sequences. It also adds support for the page up, page down, insert,
> > home, and end keys on some additional common terminal emulators.
> >
> > The result is improved performance, especially at the shell prompt,
> > and better compatibility with common terminal emulators. In
> > particular, as a side effect of the optimized cursor motion, terminal
> > windows which are taller than the current mode setting (eg. 25 lines)
> > work much better than before.
> >
> > Most of these fixes have been in production in some form on SGI's
> > servers for years.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Brian Johnson <bjohnson@sgi.com>
> > Cc: Feng Tian <feng.tian@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> >
> > Brian J. Johnson (3):
> > MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
> > MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
> > MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
> >
> > .../Universal/Console/TerminalDxe/Terminal.h | 2 +
> > .../Universal/Console/TerminalDxe/TerminalConIn.c | 24 +++++++--
> > .../Universal/Console/TerminalDxe/TerminalConOut.c | 61 ++++++++++++++++++++--
> > 3 files changed, 79 insertions(+), 8 deletions(-)
> >
>
> can you please provide feedback (testing or otherwise) on this series?
>
> Thanks
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements
2016-10-07 15:59 ` Leif Lindholm
@ 2016-10-12 8:17 ` Ryan Harkin
2016-10-14 19:39 ` Brian J. Johnson
0 siblings, 1 reply; 18+ messages in thread
From: Ryan Harkin @ 2016-10-12 8:17 UTC (permalink / raw)
To: Leif Lindholm
Cc: Laszlo Ersek, Roy Franz, Brian J. Johnson,
edk2-devel@lists.01.org, Feng Tian, Star Zeng
On 7 October 2016 at 16:59, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> Roy can now be found at Roy Franz <roy.franz@hpe.com> (cc:d).
>
> On Fri, Oct 07, 2016 at 05:56:26PM +0200, Laszlo Ersek wrote:
>> Roy, Ryan,
>>
>> On 10/07/16 16:53, Brian J. Johnson wrote:
>> > This patch series implements some improvements to the TtyTerm terminal
>> > type in the TerminalDxe driver. It fixes an end case with cursor
>> > position tracking, and uses that to optimize cursor motion escape
>> > sequences. It also adds support for the page up, page down, insert,
>> > home, and end keys on some additional common terminal emulators.
>> >
>> > The result is improved performance, especially at the shell prompt,
>> > and better compatibility with common terminal emulators. In
>> > particular, as a side effect of the optimized cursor motion, terminal
>> > windows which are taller than the current mode setting (eg. 25 lines)
>> > work much better than before.
>> >
>> > Most of these fixes have been in production in some form on SGI's
>> > servers for years.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Brian Johnson <bjohnson@sgi.com>
>> > Cc: Feng Tian <feng.tian@intel.com>
>> > Cc: Star Zeng <star.zeng@intel.com>
>> >
>> > Brian J. Johnson (3):
>> > MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
>> > MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
>> > MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
>> >
>> > .../Universal/Console/TerminalDxe/Terminal.h | 2 +
>> > .../Universal/Console/TerminalDxe/TerminalConIn.c | 24 +++++++--
>> > .../Universal/Console/TerminalDxe/TerminalConOut.c | 61 ++++++++++++++++++++--
>> > 3 files changed, 79 insertions(+), 8 deletions(-)
>> >
>>
>> can you please provide feedback (testing or otherwise) on this series?
>>
Well, they "work" for me and I'd be happy with them being submitted.
Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
The only curious effect I can see is the Print(L"xxx"); lines that
expect the \n to be missing will no longer "work". For example, I
carry a patch by Daniil Egranov titled
"IntelFrameworkModulePkg/BdsDxe: Show boot timeout message" and it no
longer displays the countdown on the same line each time, it prints
each message on a new line.
However, I don't see that as a blocking point, Daniil's patch could be
changed easily and there are other advantages to this series that make
it worthwhile, IMO, eg, Shell commands with lots of output (like
"help" or "dir fs0:") no longer create an awful mess on the serial
console.
>> Thanks
>> Laszlo
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements
2016-10-12 8:17 ` Ryan Harkin
@ 2016-10-14 19:39 ` Brian J. Johnson
2016-10-14 20:37 ` Laszlo Ersek
0 siblings, 1 reply; 18+ messages in thread
From: Brian J. Johnson @ 2016-10-14 19:39 UTC (permalink / raw)
To: Ryan Harkin, Leif Lindholm
Cc: Laszlo Ersek, Roy Franz, edk2-devel@lists.01.org, Feng Tian,
Star Zeng
On 10/12/2016 03:17 AM, Ryan Harkin wrote:
> On 7 October 2016 at 16:59, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> Roy can now be found at Roy Franz <roy.franz@hpe.com> (cc:d).
>>
>> On Fri, Oct 07, 2016 at 05:56:26PM +0200, Laszlo Ersek wrote:
>>> Roy, Ryan,
>>>
>>> On 10/07/16 16:53, Brian J. Johnson wrote:
>>>> This patch series implements some improvements to the TtyTerm terminal
>>>> type in the TerminalDxe driver. It fixes an end case with cursor
>>>> position tracking, and uses that to optimize cursor motion escape
>>>> sequences. It also adds support for the page up, page down, insert,
>>>> home, and end keys on some additional common terminal emulators.
>>>>
>>>> The result is improved performance, especially at the shell prompt,
>>>> and better compatibility with common terminal emulators. In
>>>> particular, as a side effect of the optimized cursor motion, terminal
>>>> windows which are taller than the current mode setting (eg. 25 lines)
>>>> work much better than before.
>>>>
>>>> Most of these fixes have been in production in some form on SGI's
>>>> servers for years.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
>>>> Cc: Feng Tian <feng.tian@intel.com>
>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>>
>>>> Brian J. Johnson (3):
>>>> MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
>>>> MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
>>>> MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
>>>>
>>>> .../Universal/Console/TerminalDxe/Terminal.h | 2 +
>>>> .../Universal/Console/TerminalDxe/TerminalConIn.c | 24 +++++++--
>>>> .../Universal/Console/TerminalDxe/TerminalConOut.c | 61 ++++++++++++++++++++--
>>>> 3 files changed, 79 insertions(+), 8 deletions(-)
>>>>
>>>
>>> can you please provide feedback (testing or otherwise) on this series?
>>>
>
> Well, they "work" for me and I'd be happy with them being submitted.
>
> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
>
> The only curious effect I can see is the Print(L"xxx"); lines that
> expect the \n to be missing will no longer "work". For example, I
> carry a patch by Daniil Egranov titled
> "IntelFrameworkModulePkg/BdsDxe: Show boot timeout message" and it no
> longer displays the countdown on the same line each time, it prints
> each message on a new line.
>
> However, I don't see that as a blocking point, Daniil's patch could be
> changed easily and there are other advantages to this series that make
> it worthwhile, IMO, eg, Shell commands with lots of output (like
> "help" or "dir fs0:") no longer create an awful mess on the serial
> console.
>
So, is this just waiting for a maintainer's review?
--
Brian J. Johnson
--------------------------------------------------------------------
My statements are my own, are not authorized by SGI, and do not
necessarily represent SGI’s positions.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements
2016-10-14 19:39 ` Brian J. Johnson
@ 2016-10-14 20:37 ` Laszlo Ersek
2016-10-18 15:34 ` Brian J. Johnson
0 siblings, 1 reply; 18+ messages in thread
From: Laszlo Ersek @ 2016-10-14 20:37 UTC (permalink / raw)
To: Brian J. Johnson, Ryan Harkin, Leif Lindholm
Cc: Roy Franz, edk2-devel@lists.01.org, Feng Tian, Star Zeng
On 10/14/16 21:39, Brian J. Johnson wrote:
> On 10/12/2016 03:17 AM, Ryan Harkin wrote:
>> On 7 October 2016 at 16:59, Leif Lindholm <leif.lindholm@linaro.org>
>> wrote:
>>> Roy can now be found at Roy Franz <roy.franz@hpe.com> (cc:d).
>>>
>>> On Fri, Oct 07, 2016 at 05:56:26PM +0200, Laszlo Ersek wrote:
>>>> Roy, Ryan,
>>>>
>>>> On 10/07/16 16:53, Brian J. Johnson wrote:
>>>>> This patch series implements some improvements to the TtyTerm terminal
>>>>> type in the TerminalDxe driver. It fixes an end case with cursor
>>>>> position tracking, and uses that to optimize cursor motion escape
>>>>> sequences. It also adds support for the page up, page down, insert,
>>>>> home, and end keys on some additional common terminal emulators.
>>>>>
>>>>> The result is improved performance, especially at the shell prompt,
>>>>> and better compatibility with common terminal emulators. In
>>>>> particular, as a side effect of the optimized cursor motion, terminal
>>>>> windows which are taller than the current mode setting (eg. 25 lines)
>>>>> work much better than before.
>>>>>
>>>>> Most of these fixes have been in production in some form on SGI's
>>>>> servers for years.
>>>>>
>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
>>>>> Cc: Feng Tian <feng.tian@intel.com>
>>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>>>
>>>>> Brian J. Johnson (3):
>>>>> MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
>>>>> MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
>>>>> MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
>>>>>
>>>>> .../Universal/Console/TerminalDxe/Terminal.h | 2 +
>>>>> .../Universal/Console/TerminalDxe/TerminalConIn.c | 24 +++++++--
>>>>> .../Universal/Console/TerminalDxe/TerminalConOut.c | 61
>>>>> ++++++++++++++++++++--
>>>>> 3 files changed, 79 insertions(+), 8 deletions(-)
>>>>>
>>>>
>>>> can you please provide feedback (testing or otherwise) on this series?
>>>>
>>
>> Well, they "work" for me and I'd be happy with them being submitted.
>>
>> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
>>
>> The only curious effect I can see is the Print(L"xxx"); lines that
>> expect the \n to be missing will no longer "work". For example, I
>> carry a patch by Daniil Egranov titled
>> "IntelFrameworkModulePkg/BdsDxe: Show boot timeout message" and it no
>> longer displays the countdown on the same line each time, it prints
>> each message on a new line.
>>
>> However, I don't see that as a blocking point, Daniil's patch could be
>> changed easily and there are other advantages to this series that make
>> it worthwhile, IMO, eg, Shell commands with lots of output (like
>> "help" or "dir fs0:") no longer create an awful mess on the serial
>> console.
>>
>
> So, is this just waiting for a maintainer's review?
That's my understanding, yes.
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
2016-10-07 14:54 ` [PATCH 3/3] MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm Brian J. Johnson
@ 2016-10-14 20:53 ` Roy Franz (HPE)
0 siblings, 0 replies; 18+ messages in thread
From: Roy Franz (HPE) @ 2016-10-14 20:53 UTC (permalink / raw)
To: Brian J. Johnson; +Cc: edk2-devel, Feng Tian, Star Zeng, roy.franz
On Fri, Oct 7, 2016 at 7:54 AM, Brian J. Johnson <bjohnson@sgi.com> wrote:
> The TtyTerm terminal driver is missing support for sequences produced
> by the page up, page down, insert, home, and end keys in some terimnal
> emulators. Add them.
>
> Tested under Ubuntu 16.04 using xterm 322-1ubuntu1, GNOME terminal
> 3.18.3-1ubuntu1, and XFCE terminal 0.6.3-2ubuntu1.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Kyle Roberts <kyroberts@sgi.com>
> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
> .../Universal/Console/TerminalDxe/TerminalConIn.c | 24 +++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> index 3be877b..5c3ea86 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> @@ -3,6 +3,7 @@
>
> (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> Copyright (c) 2006 - 2015, 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
> which accompanies this distribution. The full text of the license may be found at
> @@ -1374,7 +1375,7 @@ UnicodeToEfiKey (
> break;
> }
> } else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
> - /* Also accept VT100 escape codes for F1-F4 for TTY term */
> + /* Also accept VT100 escape codes for F1-F4, HOME and END for TTY term */
> switch (UnicodeChar) {
> case 'P':
> Key.ScanCode = SCAN_F1;
> @@ -1388,6 +1389,12 @@ UnicodeToEfiKey (
> case 'S':
> Key.ScanCode = SCAN_F4;
> break;
> + case 'H':
> + Key.ScanCode = SCAN_HOME;
> + break;
> + case 'F':
> + Key.ScanCode = SCAN_END;
> + break;
> }
> }
>
> @@ -1429,12 +1436,14 @@ UnicodeToEfiKey (
> break;
> case 'H':
> if (TerminalDevice->TerminalType == PCANSITYPE ||
> - TerminalDevice->TerminalType == VT100TYPE) {
> + TerminalDevice->TerminalType == VT100TYPE ||
> + TerminalDevice->TerminalType == TTYTERMTYPE) {
> Key.ScanCode = SCAN_HOME;
> }
> break;
> case 'F':
> - if (TerminalDevice->TerminalType == PCANSITYPE) {
> + if (TerminalDevice->TerminalType == PCANSITYPE ||
> + TerminalDevice->TerminalType == TTYTERMTYPE) {
> Key.ScanCode = SCAN_END;
> }
> break;
> @@ -1573,9 +1582,18 @@ UnicodeToEfiKey (
> TerminalDevice->TtyEscapeStr[TerminalDevice->TtyEscapeIndex] = 0; /* Terminate string */
> EscCode = (UINT16) StrDecimalToUintn(TerminalDevice->TtyEscapeStr);
> switch (EscCode) {
> + case 2:
> + Key.ScanCode = SCAN_INSERT;
> + break;
> case 3:
> Key.ScanCode = SCAN_DELETE;
> break;
> + case 5:
> + Key.ScanCode = SCAN_PAGE_UP;
> + break;
> + case 6:
> + Key.ScanCode = SCAN_PAGE_DOWN;
> + break;
> case 11:
> case 12:
> case 13:
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
Reviewed-by: Roy Franz <roy.franz@hpe.com>
This patch looks good to me - I'm not enough of a terminal expert to
review patches 1/2.
Roy
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements
2016-10-14 20:37 ` Laszlo Ersek
@ 2016-10-18 15:34 ` Brian J. Johnson
2016-10-26 15:00 ` Brian J. Johnson
0 siblings, 1 reply; 18+ messages in thread
From: Brian J. Johnson @ 2016-10-18 15:34 UTC (permalink / raw)
To: Feng Tian, Star Zeng
Cc: Laszlo Ersek, Ryan Harkin, Leif Lindholm, Roy Franz,
edk2-devel@lists.01.org
On 10/14/2016 03:37 PM, Laszlo Ersek wrote:
> On 10/14/16 21:39, Brian J. Johnson wrote:
>> On 10/12/2016 03:17 AM, Ryan Harkin wrote:
>>> On 7 October 2016 at 16:59, Leif Lindholm <leif.lindholm@linaro.org>
>>> wrote:
>>>> Roy can now be found at Roy Franz <roy.franz@hpe.com> (cc:d).
>>>>
>>>> On Fri, Oct 07, 2016 at 05:56:26PM +0200, Laszlo Ersek wrote:
>>>>> Roy, Ryan,
>>>>>
>>>>> On 10/07/16 16:53, Brian J. Johnson wrote:
>>>>>> This patch series implements some improvements to the TtyTerm terminal
>>>>>> type in the TerminalDxe driver. It fixes an end case with cursor
>>>>>> position tracking, and uses that to optimize cursor motion escape
>>>>>> sequences. It also adds support for the page up, page down, insert,
>>>>>> home, and end keys on some additional common terminal emulators.
>>>>>>
>>>>>> The result is improved performance, especially at the shell prompt,
>>>>>> and better compatibility with common terminal emulators. In
>>>>>> particular, as a side effect of the optimized cursor motion, terminal
>>>>>> windows which are taller than the current mode setting (eg. 25 lines)
>>>>>> work much better than before.
>>>>>>
>>>>>> Most of these fixes have been in production in some form on SGI's
>>>>>> servers for years.
>>>>>>
>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
>>>>>> Cc: Feng Tian <feng.tian@intel.com>
>>>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>>>>
>>>>>> Brian J. Johnson (3):
>>>>>> MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
>>>>>> MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
>>>>>> MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
>>>>>>
>>>>>> .../Universal/Console/TerminalDxe/Terminal.h | 2 +
>>>>>> .../Universal/Console/TerminalDxe/TerminalConIn.c | 24 +++++++--
>>>>>> .../Universal/Console/TerminalDxe/TerminalConOut.c | 61
>>>>>> ++++++++++++++++++++--
>>>>>> 3 files changed, 79 insertions(+), 8 deletions(-)
>>>>>>
>>>>>
>>>>> can you please provide feedback (testing or otherwise) on this series?
>>>>>
>>>
>>> Well, they "work" for me and I'd be happy with them being submitted.
>>>
>>> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
>>>
>>> The only curious effect I can see is the Print(L"xxx"); lines that
>>> expect the \n to be missing will no longer "work". For example, I
>>> carry a patch by Daniil Egranov titled
>>> "IntelFrameworkModulePkg/BdsDxe: Show boot timeout message" and it no
>>> longer displays the countdown on the same line each time, it prints
>>> each message on a new line.
>>>
>>> However, I don't see that as a blocking point, Daniil's patch could be
>>> changed easily and there are other advantages to this series that make
>>> it worthwhile, IMO, eg, Shell commands with lots of output (like
>>> "help" or "dir fs0:") no longer create an awful mess on the serial
>>> console.
>>>
>>
>> So, is this just waiting for a maintainer's review?
>
> That's my understanding, yes.
Feng or Star, could you please review these changes?
Thanks,
--
Brian J. Johnson
--------------------------------------------------------------------
My statements are my own, are not authorized by SGI, and do not
necessarily represent SGI’s positions.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements
2016-10-18 15:34 ` Brian J. Johnson
@ 2016-10-26 15:00 ` Brian J. Johnson
2016-10-27 1:01 ` Tian, Feng
0 siblings, 1 reply; 18+ messages in thread
From: Brian J. Johnson @ 2016-10-26 15:00 UTC (permalink / raw)
To: Feng Tian, Star Zeng
Cc: Roy Franz, Ryan Harkin, Laszlo Ersek, edk2-devel@lists.01.org,
Leif Lindholm
On 10/18/2016 10:34 AM, Brian J. Johnson wrote:
> On 10/14/2016 03:37 PM, Laszlo Ersek wrote:
>> On 10/14/16 21:39, Brian J. Johnson wrote:
>>> On 10/12/2016 03:17 AM, Ryan Harkin wrote:
>>>> On 7 October 2016 at 16:59, Leif Lindholm <leif.lindholm@linaro.org>
>>>> wrote:
>>>>> Roy can now be found at Roy Franz <roy.franz@hpe.com> (cc:d).
>>>>>
>>>>> On Fri, Oct 07, 2016 at 05:56:26PM +0200, Laszlo Ersek wrote:
>>>>>> Roy, Ryan,
>>>>>>
>>>>>> On 10/07/16 16:53, Brian J. Johnson wrote:
>>>>>>> This patch series implements some improvements to the TtyTerm
>>>>>>> terminal
>>>>>>> type in the TerminalDxe driver. It fixes an end case with cursor
>>>>>>> position tracking, and uses that to optimize cursor motion escape
>>>>>>> sequences. It also adds support for the page up, page down, insert,
>>>>>>> home, and end keys on some additional common terminal emulators.
>>>>>>>
>>>>>>> The result is improved performance, especially at the shell prompt,
>>>>>>> and better compatibility with common terminal emulators. In
>>>>>>> particular, as a side effect of the optimized cursor motion,
>>>>>>> terminal
>>>>>>> windows which are taller than the current mode setting (eg. 25
>>>>>>> lines)
>>>>>>> work much better than before.
>>>>>>>
>>>>>>> Most of these fixes have been in production in some form on SGI's
>>>>>>> servers for years.
>>>>>>>
>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>>> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
>>>>>>> Cc: Feng Tian <feng.tian@intel.com>
>>>>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>>>>>
>>>>>>> Brian J. Johnson (3):
>>>>>>> MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
>>>>>>> MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
>>>>>>> MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
>>>>>>>
>>>>>>> .../Universal/Console/TerminalDxe/Terminal.h | 2 +
>>>>>>> .../Universal/Console/TerminalDxe/TerminalConIn.c | 24 +++++++--
>>>>>>> .../Universal/Console/TerminalDxe/TerminalConOut.c | 61
>>>>>>> ++++++++++++++++++++--
>>>>>>> 3 files changed, 79 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>
>>>>>> can you please provide feedback (testing or otherwise) on this
>>>>>> series?
>>>>>>
>>>>
>>>> Well, they "work" for me and I'd be happy with them being submitted.
>>>>
>>>> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
>>>>
>>>> The only curious effect I can see is the Print(L"xxx"); lines that
>>>> expect the \n to be missing will no longer "work". For example, I
>>>> carry a patch by Daniil Egranov titled
>>>> "IntelFrameworkModulePkg/BdsDxe: Show boot timeout message" and it no
>>>> longer displays the countdown on the same line each time, it prints
>>>> each message on a new line.
>>>>
>>>> However, I don't see that as a blocking point, Daniil's patch could be
>>>> changed easily and there are other advantages to this series that make
>>>> it worthwhile, IMO, eg, Shell commands with lots of output (like
>>>> "help" or "dir fs0:") no longer create an awful mess on the serial
>>>> console.
>>>>
>>>
>>> So, is this just waiting for a maintainer's review?
>>
>> That's my understanding, yes.
>
> Feng or Star, could you please review these changes?
Ping?
--
Brian J. Johnson
--------------------------------------------------------------------
My statements are my own, are not authorized by SGI, and do not
necessarily represent SGI’s positions.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements
2016-10-26 15:00 ` Brian J. Johnson
@ 2016-10-27 1:01 ` Tian, Feng
0 siblings, 0 replies; 18+ messages in thread
From: Tian, Feng @ 2016-10-27 1:01 UTC (permalink / raw)
To: Brian J. Johnson, Zeng, Star
Cc: Roy Franz, Ryan Harkin, Laszlo Ersek, edk2-devel@lists.01.org,
Leif Lindholm, Tian, Feng
Sorry for missing this patch.
Reviewed-by: Feng Tian <feng.tian@intel.com>
I will help push it in.
Thanks
Feng
-----Original Message-----
From: Brian J. Johnson [mailto:bjohnson@sgi.com]
Sent: Wednesday, October 26, 2016 11:00 PM
To: Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
Cc: Roy Franz <roy.franz@hpe.com>; Ryan Harkin <ryan.harkin@linaro.org>; Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [edk2] [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements
On 10/18/2016 10:34 AM, Brian J. Johnson wrote:
> On 10/14/2016 03:37 PM, Laszlo Ersek wrote:
>> On 10/14/16 21:39, Brian J. Johnson wrote:
>>> On 10/12/2016 03:17 AM, Ryan Harkin wrote:
>>>> On 7 October 2016 at 16:59, Leif Lindholm
>>>> <leif.lindholm@linaro.org>
>>>> wrote:
>>>>> Roy can now be found at Roy Franz <roy.franz@hpe.com> (cc:d).
>>>>>
>>>>> On Fri, Oct 07, 2016 at 05:56:26PM +0200, Laszlo Ersek wrote:
>>>>>> Roy, Ryan,
>>>>>>
>>>>>> On 10/07/16 16:53, Brian J. Johnson wrote:
>>>>>>> This patch series implements some improvements to the TtyTerm
>>>>>>> terminal type in the TerminalDxe driver. It fixes an end case
>>>>>>> with cursor position tracking, and uses that to optimize cursor
>>>>>>> motion escape sequences. It also adds support for the page up,
>>>>>>> page down, insert, home, and end keys on some additional common
>>>>>>> terminal emulators.
>>>>>>>
>>>>>>> The result is improved performance, especially at the shell
>>>>>>> prompt, and better compatibility with common terminal emulators.
>>>>>>> In particular, as a side effect of the optimized cursor motion,
>>>>>>> terminal windows which are taller than the current mode setting
>>>>>>> (eg. 25
>>>>>>> lines)
>>>>>>> work much better than before.
>>>>>>>
>>>>>>> Most of these fixes have been in production in some form on
>>>>>>> SGI's servers for years.
>>>>>>>
>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>>> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
>>>>>>> Cc: Feng Tian <feng.tian@intel.com>
>>>>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>>>>>
>>>>>>> Brian J. Johnson (3):
>>>>>>> MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
>>>>>>> MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
>>>>>>> MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
>>>>>>>
>>>>>>> .../Universal/Console/TerminalDxe/Terminal.h | 2 +
>>>>>>> .../Universal/Console/TerminalDxe/TerminalConIn.c | 24
>>>>>>> +++++++-- .../Universal/Console/TerminalDxe/TerminalConOut.c |
>>>>>>> 61
>>>>>>> ++++++++++++++++++++--
>>>>>>> 3 files changed, 79 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>
>>>>>> can you please provide feedback (testing or otherwise) on this
>>>>>> series?
>>>>>>
>>>>
>>>> Well, they "work" for me and I'd be happy with them being submitted.
>>>>
>>>> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
>>>>
>>>> The only curious effect I can see is the Print(L"xxx"); lines that
>>>> expect the \n to be missing will no longer "work". For example, I
>>>> carry a patch by Daniil Egranov titled
>>>> "IntelFrameworkModulePkg/BdsDxe: Show boot timeout message" and it
>>>> no longer displays the countdown on the same line each time, it
>>>> prints each message on a new line.
>>>>
>>>> However, I don't see that as a blocking point, Daniil's patch could
>>>> be changed easily and there are other advantages to this series
>>>> that make it worthwhile, IMO, eg, Shell commands with lots of
>>>> output (like "help" or "dir fs0:") no longer create an awful mess
>>>> on the serial console.
>>>>
>>>
>>> So, is this just waiting for a maintainer's review?
>>
>> That's my understanding, yes.
>
> Feng or Star, could you please review these changes?
Ping?
--
Brian J. Johnson
--------------------------------------------------------------------
My statements are my own, are not authorized by SGI, and do not
necessarily represent SGI’s positions.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
2016-10-07 14:53 ` [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion Brian J. Johnson
@ 2016-10-27 3:09 ` Kinney, Michael D
2016-10-27 5:06 ` Tian, Feng
2016-10-27 18:06 ` Brian J. Johnson
0 siblings, 2 replies; 18+ messages in thread
From: Kinney, Michael D @ 2016-10-27 3:09 UTC (permalink / raw)
To: Brian J. Johnson, edk2-devel@lists.01.org, Kinney, Michael D
Cc: Tian, Feng, Zeng, Star
Tian Feng,
Unfortunately, this patch that was pushed to edk2/master today
breaks on IA32 VS2015x86 builds with a signed/unsigned mismatch
on 3 lines. I think the right fix might be:
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
index e9b5ed0..9625f4d 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
@@ -790,13 +790,13 @@ TerminalConOutSetCursorPosition (
// it isn't necessary.
//
if (TerminalDevice->TerminalType == TTYTERMTYPE &&
- Mode->CursorRow == Row) {
- if (Mode->CursorColumn > Column) {
+ (UINTN)Mode->CursorRow == Row) {
+ if ((UINTN)Mode->CursorColumn > Column) {
mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Mode->CursorColumn - Column) / 10));
mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Mode->CursorColumn - Column) % 10));
String = mCursorBackwardString;
}
- else if (Column > Mode->CursorColumn) {
+ else if (Column > (UINTN)Mode->CursorColumn) {
mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Column - Mode->CursorColumn) / 10));
mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Column - Mode->CursorColumn) % 10));
String = mCursorForwardString;
Please see if you can reproduce this issue.
Thanks,
Mike
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Brian J. Johnson
> Sent: Friday, October 7, 2016 7:54 AM
> To: edk2-devel@lists.01.org
> Cc: Brian J. Johnson <bjohnson@sgi.com>; Tian, Feng <feng.tian@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: [edk2] [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
>
> For TtyTerm terminals, output a shorter escape sequence when possible
> to move the cursor within the current line, and don't print any escape
> sequence if the cursor is already at the correct position. This
> removes extra cursor motion activity at the EFI shell prompt,
> improving performance. It also makes it possible in many cases to
> successfully use a terminal window which is taller than the driver's
> mode setting (eg. 80x25.)
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
> .../Universal/Console/TerminalDxe/Terminal.h | 2 ++
> .../Universal/Console/TerminalDxe/TerminalConOut.c | 36 +++++++++++++++++++---
> 2 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> index 269d2ae..3ee3969 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> @@ -2,6 +2,7 @@
> Header file for Terminal driver.
>
> Copyright (c) 2006 - 2014, 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
> which accompanies this distribution. The full text of the license may be found at
> @@ -157,6 +158,7 @@ typedef union {
> #define BACKGROUND_CONTROL_OFFSET 11
> #define ROW_OFFSET 2
> #define COLUMN_OFFSET 5
> +#define FW_BACK_OFFSET 2
>
> typedef struct {
> UINT16 Unicode;
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> index b11e83f..e9b5ed0 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> @@ -83,6 +83,8 @@ CHAR16 mSetModeString[] = { ESC, '[', '=', '3', 'h', 0 };
> CHAR16 mSetAttributeString[] = { ESC, '[', '0', 'm', ESC, '[', '4', '0', 'm',
> ESC, '[', '4', '0', 'm', 0 };
> CHAR16 mClearScreenString[] = { ESC, '[', '2', 'J', 0 };
> CHAR16 mSetCursorPositionString[] = { ESC, '[', '0', '0', ';', '0', '0', 'H', 0 };
> +CHAR16 mCursorForwardString[] = { ESC, '[', '0', '0', 'C', 0 };
> +CHAR16 mCursorBackwardString[] = { ESC, '[', '0', '0', 'D', 0 };
>
> //
> // Body of the ConOut functions
> @@ -755,6 +757,7 @@ TerminalConOutSetCursorPosition (
> UINTN MaxRow;
> EFI_STATUS Status;
> TERMINAL_DEV *TerminalDevice;
> + CHAR16 *String;
>
> TerminalDevice = TERMINAL_CON_OUT_DEV_FROM_THIS (This);
>
> @@ -782,13 +785,36 @@ TerminalConOutSetCursorPosition (
> //
> // control sequence to move the cursor
> //
> - mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) / 10));
> - mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) % 10));
> - mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' + ((Column + 1) / 10));
> - mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' + ((Column + 1) % 10));
> + // Optimize cursor motion control sequences for TtyTerm. Move
> + // within the current line if possible, and don't output anyting if
> + // it isn't necessary.
> + //
> + if (TerminalDevice->TerminalType == TTYTERMTYPE &&
> + Mode->CursorRow == Row) {
> + if (Mode->CursorColumn > Column) {
> + mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Mode->CursorColumn
> - Column) / 10));
> + mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Mode->CursorColumn
> - Column) % 10));
> + String = mCursorBackwardString;
> + }
> + else if (Column > Mode->CursorColumn) {
> + mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Column - Mode-
> >CursorColumn) / 10));
> + mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Column - Mode-
> >CursorColumn) % 10));
> + String = mCursorForwardString;
> + }
> + else {
> + String = L""; // No cursor motion necessary
> + }
> + }
> + else {
> + mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) / 10));
> + mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) % 10));
> + mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' + ((Column + 1) /
> 10));
> + mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' + ((Column + 1) %
> 10));
> + String = mSetCursorPositionString;
> + }
>
> TerminalDevice->OutputEscChar = TRUE;
> - Status = This->OutputString (This, mSetCursorPositionString);
> + Status = This->OutputString (This, String);
> TerminalDevice->OutputEscChar = FALSE;
>
> if (EFI_ERROR (Status)) {
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
2016-10-27 3:09 ` Kinney, Michael D
@ 2016-10-27 5:06 ` Tian, Feng
2016-10-27 18:06 ` Brian J. Johnson
1 sibling, 0 replies; 18+ messages in thread
From: Tian, Feng @ 2016-10-27 5:06 UTC (permalink / raw)
To: Kinney, Michael D, Brian J. Johnson, edk2-devel@lists.01.org
Cc: Zeng, Star, Tian, Feng
Mike,
You are right, the VS2015x64 IA32 build would fail.
With your fix, the code could pass build.
If there is no objection, I will help to push this fix tomorrow after adding EDKII commit log with your Sign-off and my Review-by.
Thanks
Feng
-----Original Message-----
From: Kinney, Michael D
Sent: Thursday, October 27, 2016 11:10 AM
To: Brian J. Johnson <bjohnson@sgi.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
Tian Feng,
Unfortunately, this patch that was pushed to edk2/master today breaks on IA32 VS2015x86 builds with a signed/unsigned mismatch on 3 lines. I think the right fix might be:
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
index e9b5ed0..9625f4d 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
@@ -790,13 +790,13 @@ TerminalConOutSetCursorPosition (
// it isn't necessary.
//
if (TerminalDevice->TerminalType == TTYTERMTYPE &&
- Mode->CursorRow == Row) {
- if (Mode->CursorColumn > Column) {
+ (UINTN)Mode->CursorRow == Row) {
+ if ((UINTN)Mode->CursorColumn > Column) {
mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Mode->CursorColumn - Column) / 10));
mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Mode->CursorColumn - Column) % 10));
String = mCursorBackwardString;
}
- else if (Column > Mode->CursorColumn) {
+ else if (Column > (UINTN)Mode->CursorColumn) {
mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Column - Mode->CursorColumn) / 10));
mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Column - Mode->CursorColumn) % 10));
String = mCursorForwardString;
Please see if you can reproduce this issue.
Thanks,
Mike
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Brian J. Johnson
> Sent: Friday, October 7, 2016 7:54 AM
> To: edk2-devel@lists.01.org
> Cc: Brian J. Johnson <bjohnson@sgi.com>; Tian, Feng
> <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm
> cursor motion
>
> For TtyTerm terminals, output a shorter escape sequence when possible
> to move the cursor within the current line, and don't print any escape
> sequence if the cursor is already at the correct position. This
> removes extra cursor motion activity at the EFI shell prompt,
> improving performance. It also makes it possible in many cases to
> successfully use a terminal window which is taller than the driver's
> mode setting (eg. 80x25.)
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
> .../Universal/Console/TerminalDxe/Terminal.h | 2 ++
> .../Universal/Console/TerminalDxe/TerminalConOut.c | 36
> +++++++++++++++++++---
> 2 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> index 269d2ae..3ee3969 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> @@ -2,6 +2,7 @@
> Header file for Terminal driver.
>
> Copyright (c) 2006 - 2014, 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 which
> accompanies this distribution. The full text of the license may be
> found at @@ -157,6 +158,7 @@ typedef union { #define
> BACKGROUND_CONTROL_OFFSET 11
> #define ROW_OFFSET 2
> #define COLUMN_OFFSET 5
> +#define FW_BACK_OFFSET 2
>
> typedef struct {
> UINT16 Unicode;
> diff --git
> a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> index b11e83f..e9b5ed0 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> @@ -83,6 +83,8 @@ CHAR16 mSetModeString[] = { ESC, '[', '=', '3', 'h', 0 };
> CHAR16 mSetAttributeString[] = { ESC, '[', '0', 'm', ESC, '[', '4', '0', 'm',
> ESC, '[', '4', '0', 'm', 0 };
> CHAR16 mClearScreenString[] = { ESC, '[', '2', 'J', 0 };
> CHAR16 mSetCursorPositionString[] = { ESC, '[', '0', '0', ';', '0',
> '0', 'H', 0 };
> +CHAR16 mCursorForwardString[] = { ESC, '[', '0', '0', 'C', 0 };
> +CHAR16 mCursorBackwardString[] = { ESC, '[', '0', '0', 'D', 0 };
>
> //
> // Body of the ConOut functions
> @@ -755,6 +757,7 @@ TerminalConOutSetCursorPosition (
> UINTN MaxRow;
> EFI_STATUS Status;
> TERMINAL_DEV *TerminalDevice;
> + CHAR16 *String;
>
> TerminalDevice = TERMINAL_CON_OUT_DEV_FROM_THIS (This);
>
> @@ -782,13 +785,36 @@ TerminalConOutSetCursorPosition (
> //
> // control sequence to move the cursor
> //
> - mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) / 10));
> - mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) % 10));
> - mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' +
> ((Column + 1) / 10));
> - mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' +
> ((Column + 1) % 10));
> + // Optimize cursor motion control sequences for TtyTerm. Move //
> + within the current line if possible, and don't output anyting if //
> + it isn't necessary.
> + //
> + if (TerminalDevice->TerminalType == TTYTERMTYPE &&
> + Mode->CursorRow == Row) {
> + if (Mode->CursorColumn > Column) {
> + mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' +
> + ((Mode->CursorColumn
> - Column) / 10));
> + mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' +
> + ((Mode->CursorColumn
> - Column) % 10));
> + String = mCursorBackwardString;
> + }
> + else if (Column > Mode->CursorColumn) {
> + mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' +
> + ((Column - Mode-
> >CursorColumn) / 10));
> + mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' +
> + ((Column - Mode-
> >CursorColumn) % 10));
> + String = mCursorForwardString;
> + }
> + else {
> + String = L""; // No cursor motion necessary
> + }
> + }
> + else {
> + mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) / 10));
> + mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) % 10));
> + mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' +
> + ((Column + 1) /
> 10));
> + mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' +
> + ((Column + 1) %
> 10));
> + String = mSetCursorPositionString; }
>
> TerminalDevice->OutputEscChar = TRUE;
> - Status = This->OutputString (This, mSetCursorPositionString);
> + Status = This->OutputString (This, String);
> TerminalDevice->OutputEscChar = FALSE;
>
> if (EFI_ERROR (Status)) {
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
2016-10-27 3:09 ` Kinney, Michael D
2016-10-27 5:06 ` Tian, Feng
@ 2016-10-27 18:06 ` Brian J. Johnson
2016-10-27 18:14 ` Kinney, Michael D
1 sibling, 1 reply; 18+ messages in thread
From: Brian J. Johnson @ 2016-10-27 18:06 UTC (permalink / raw)
To: Kinney, Michael D, edk2-devel@lists.01.org; +Cc: Tian, Feng, Zeng, Star
On 10/26/2016 10:09 PM, Kinney, Michael D wrote:
> Tian Feng,
>
> Unfortunately, this patch that was pushed to edk2/master today
> breaks on IA32 VS2015x86 builds with a signed/unsigned mismatch
> on 3 lines. I think the right fix might be:
>
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> index e9b5ed0..9625f4d 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> @@ -790,13 +790,13 @@ TerminalConOutSetCursorPosition (
> // it isn't necessary.
> //
> if (TerminalDevice->TerminalType == TTYTERMTYPE &&
> - Mode->CursorRow == Row) {
> - if (Mode->CursorColumn > Column) {
> + (UINTN)Mode->CursorRow == Row) {
> + if ((UINTN)Mode->CursorColumn > Column) {
> mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Mode->CursorColumn - Column) / 10));
> mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Mode->CursorColumn - Column) % 10));
> String = mCursorBackwardString;
> }
> - else if (Column > Mode->CursorColumn) {
> + else if (Column > (UINTN)Mode->CursorColumn) {
> mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Column - Mode->CursorColumn) / 10));
> mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Column - Mode->CursorColumn) % 10));
> String = mCursorForwardString;
>
> Please see if you can reproduce this issue.
>
> Thanks,
>
> Mike
The changes look fine to me. Unfortunately, I don't have that compiler
to test with.
If it's needed,
Reviewed-by: Brian Johnson <bjohnson@sgi.com>
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Brian J. Johnson
>> Sent: Friday, October 7, 2016 7:54 AM
>> To: edk2-devel@lists.01.org
>> Cc: Brian J. Johnson <bjohnson@sgi.com>; Tian, Feng <feng.tian@intel.com>; Zeng, Star
>> <star.zeng@intel.com>
>> Subject: [edk2] [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
>>
>> For TtyTerm terminals, output a shorter escape sequence when possible
>> to move the cursor within the current line, and don't print any escape
>> sequence if the cursor is already at the correct position. This
>> removes extra cursor motion activity at the EFI shell prompt,
>> improving performance. It also makes it possible in many cases to
>> successfully use a terminal window which is taller than the driver's
>> mode setting (eg. 80x25.)
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
>> Cc: Feng Tian <feng.tian@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> ---
>> .../Universal/Console/TerminalDxe/Terminal.h | 2 ++
>> .../Universal/Console/TerminalDxe/TerminalConOut.c | 36 +++++++++++++++++++---
>> 2 files changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>> index 269d2ae..3ee3969 100644
>> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>> @@ -2,6 +2,7 @@
>> Header file for Terminal driver.
>>
>> Copyright (c) 2006 - 2014, 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
>> which accompanies this distribution. The full text of the license may be found at
>> @@ -157,6 +158,7 @@ typedef union {
>> #define BACKGROUND_CONTROL_OFFSET 11
>> #define ROW_OFFSET 2
>> #define COLUMN_OFFSET 5
>> +#define FW_BACK_OFFSET 2
>>
>> typedef struct {
>> UINT16 Unicode;
>> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
>> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
>> index b11e83f..e9b5ed0 100644
>> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
>> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
>> @@ -83,6 +83,8 @@ CHAR16 mSetModeString[] = { ESC, '[', '=', '3', 'h', 0 };
>> CHAR16 mSetAttributeString[] = { ESC, '[', '0', 'm', ESC, '[', '4', '0', 'm',
>> ESC, '[', '4', '0', 'm', 0 };
>> CHAR16 mClearScreenString[] = { ESC, '[', '2', 'J', 0 };
>> CHAR16 mSetCursorPositionString[] = { ESC, '[', '0', '0', ';', '0', '0', 'H', 0 };
>> +CHAR16 mCursorForwardString[] = { ESC, '[', '0', '0', 'C', 0 };
>> +CHAR16 mCursorBackwardString[] = { ESC, '[', '0', '0', 'D', 0 };
>>
>> //
>> // Body of the ConOut functions
>> @@ -755,6 +757,7 @@ TerminalConOutSetCursorPosition (
>> UINTN MaxRow;
>> EFI_STATUS Status;
>> TERMINAL_DEV *TerminalDevice;
>> + CHAR16 *String;
>>
>> TerminalDevice = TERMINAL_CON_OUT_DEV_FROM_THIS (This);
>>
>> @@ -782,13 +785,36 @@ TerminalConOutSetCursorPosition (
>> //
>> // control sequence to move the cursor
>> //
>> - mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) / 10));
>> - mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) % 10));
>> - mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' + ((Column + 1) / 10));
>> - mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' + ((Column + 1) % 10));
>> + // Optimize cursor motion control sequences for TtyTerm. Move
>> + // within the current line if possible, and don't output anyting if
>> + // it isn't necessary.
>> + //
>> + if (TerminalDevice->TerminalType == TTYTERMTYPE &&
>> + Mode->CursorRow == Row) {
>> + if (Mode->CursorColumn > Column) {
>> + mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Mode->CursorColumn
>> - Column) / 10));
>> + mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Mode->CursorColumn
>> - Column) % 10));
>> + String = mCursorBackwardString;
>> + }
>> + else if (Column > Mode->CursorColumn) {
>> + mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Column - Mode-
>>> CursorColumn) / 10));
>> + mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Column - Mode-
>>> CursorColumn) % 10));
>> + String = mCursorForwardString;
>> + }
>> + else {
>> + String = L""; // No cursor motion necessary
>> + }
>> + }
>> + else {
>> + mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) / 10));
>> + mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) % 10));
>> + mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' + ((Column + 1) /
>> 10));
>> + mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' + ((Column + 1) %
>> 10));
>> + String = mSetCursorPositionString;
>> + }
>>
>> TerminalDevice->OutputEscChar = TRUE;
>> - Status = This->OutputString (This, mSetCursorPositionString);
>> + Status = This->OutputString (This, String);
>> TerminalDevice->OutputEscChar = FALSE;
>>
>> if (EFI_ERROR (Status)) {
>> --
>> 2.7.4
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
--
Brian
--------------------------------------------------------------------
"I have discovered that all human evil comes from this, man's being
unable to sit still in a room."
-- Blaise Pascal
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
2016-10-27 18:06 ` Brian J. Johnson
@ 2016-10-27 18:14 ` Kinney, Michael D
2016-10-27 18:32 ` Kinney, Michael D
0 siblings, 1 reply; 18+ messages in thread
From: Kinney, Michael D @ 2016-10-27 18:14 UTC (permalink / raw)
To: Brian J. Johnson, edk2-devel@lists.01.org, Kinney, Michael D
Cc: Tian, Feng, Zeng, Star
Brian,
Thanks. I will do the commit with your rb.
Mike
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Brian J. Johnson
> Sent: Thursday, October 27, 2016 11:06 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor
> motion
>
> On 10/26/2016 10:09 PM, Kinney, Michael D wrote:
> > Tian Feng,
> >
> > Unfortunately, this patch that was pushed to edk2/master today
> > breaks on IA32 VS2015x86 builds with a signed/unsigned mismatch
> > on 3 lines. I think the right fix might be:
> >
> > diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > index e9b5ed0..9625f4d 100644
> > --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > @@ -790,13 +790,13 @@ TerminalConOutSetCursorPosition (
> > // it isn't necessary.
> > //
> > if (TerminalDevice->TerminalType == TTYTERMTYPE &&
> > - Mode->CursorRow == Row) {
> > - if (Mode->CursorColumn > Column) {
> > + (UINTN)Mode->CursorRow == Row) {
> > + if ((UINTN)Mode->CursorColumn > Column) {
> > mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Mode-
> >CursorColumn - Column) / 10));
> > mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Mode-
> >CursorColumn - Column) % 10));
> > String = mCursorBackwardString;
> > }
> > - else if (Column > Mode->CursorColumn) {
> > + else if (Column > (UINTN)Mode->CursorColumn) {
> > mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Column - Mode-
> >CursorColumn) / 10));
> > mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Column - Mode-
> >CursorColumn) % 10));
> > String = mCursorForwardString;
> >
> > Please see if you can reproduce this issue.
> >
> > Thanks,
> >
> > Mike
>
> The changes look fine to me. Unfortunately, I don't have that compiler
> to test with.
>
> If it's needed,
> Reviewed-by: Brian Johnson <bjohnson@sgi.com>
>
> >
> >> -----Original Message-----
> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Brian J.
> Johnson
> >> Sent: Friday, October 7, 2016 7:54 AM
> >> To: edk2-devel@lists.01.org
> >> Cc: Brian J. Johnson <bjohnson@sgi.com>; Tian, Feng <feng.tian@intel.com>; Zeng,
> Star
> >> <star.zeng@intel.com>
> >> Subject: [edk2] [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
> >>
> >> For TtyTerm terminals, output a shorter escape sequence when possible
> >> to move the cursor within the current line, and don't print any escape
> >> sequence if the cursor is already at the correct position. This
> >> removes extra cursor motion activity at the EFI shell prompt,
> >> improving performance. It also makes it possible in many cases to
> >> successfully use a terminal window which is taller than the driver's
> >> mode setting (eg. 80x25.)
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
> >> Cc: Feng Tian <feng.tian@intel.com>
> >> Cc: Star Zeng <star.zeng@intel.com>
> >> ---
> >> .../Universal/Console/TerminalDxe/Terminal.h | 2 ++
> >> .../Universal/Console/TerminalDxe/TerminalConOut.c | 36 +++++++++++++++++++---
> >> 2 files changed, 33 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> >> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> >> index 269d2ae..3ee3969 100644
> >> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> >> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> >> @@ -2,6 +2,7 @@
> >> Header file for Terminal driver.
> >>
> >> Copyright (c) 2006 - 2014, 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
> >> which accompanies this distribution. The full text of the license may be found at
> >> @@ -157,6 +158,7 @@ typedef union {
> >> #define BACKGROUND_CONTROL_OFFSET 11
> >> #define ROW_OFFSET 2
> >> #define COLUMN_OFFSET 5
> >> +#define FW_BACK_OFFSET 2
> >>
> >> typedef struct {
> >> UINT16 Unicode;
> >> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> >> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> >> index b11e83f..e9b5ed0 100644
> >> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> >> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> >> @@ -83,6 +83,8 @@ CHAR16 mSetModeString[] = { ESC, '[', '=', '3', 'h', 0
> };
> >> CHAR16 mSetAttributeString[] = { ESC, '[', '0', 'm', ESC, '[', '4', '0', 'm',
> >> ESC, '[', '4', '0', 'm', 0 };
> >> CHAR16 mClearScreenString[] = { ESC, '[', '2', 'J', 0 };
> >> CHAR16 mSetCursorPositionString[] = { ESC, '[', '0', '0', ';', '0', '0', 'H', 0 };
> >> +CHAR16 mCursorForwardString[] = { ESC, '[', '0', '0', 'C', 0 };
> >> +CHAR16 mCursorBackwardString[] = { ESC, '[', '0', '0', 'D', 0 };
> >>
> >> //
> >> // Body of the ConOut functions
> >> @@ -755,6 +757,7 @@ TerminalConOutSetCursorPosition (
> >> UINTN MaxRow;
> >> EFI_STATUS Status;
> >> TERMINAL_DEV *TerminalDevice;
> >> + CHAR16 *String;
> >>
> >> TerminalDevice = TERMINAL_CON_OUT_DEV_FROM_THIS (This);
> >>
> >> @@ -782,13 +785,36 @@ TerminalConOutSetCursorPosition (
> >> //
> >> // control sequence to move the cursor
> >> //
> >> - mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) / 10));
> >> - mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) % 10));
> >> - mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' + ((Column + 1) /
> 10));
> >> - mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' + ((Column + 1) %
> 10));
> >> + // Optimize cursor motion control sequences for TtyTerm. Move
> >> + // within the current line if possible, and don't output anyting if
> >> + // it isn't necessary.
> >> + //
> >> + if (TerminalDevice->TerminalType == TTYTERMTYPE &&
> >> + Mode->CursorRow == Row) {
> >> + if (Mode->CursorColumn > Column) {
> >> + mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Mode-
> >CursorColumn
> >> - Column) / 10));
> >> + mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Mode-
> >CursorColumn
> >> - Column) % 10));
> >> + String = mCursorBackwardString;
> >> + }
> >> + else if (Column > Mode->CursorColumn) {
> >> + mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Column - Mode-
> >>> CursorColumn) / 10));
> >> + mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Column - Mode-
> >>> CursorColumn) % 10));
> >> + String = mCursorForwardString;
> >> + }
> >> + else {
> >> + String = L""; // No cursor motion necessary
> >> + }
> >> + }
> >> + else {
> >> + mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) /
> 10));
> >> + mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) %
> 10));
> >> + mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' + ((Column + 1) /
> >> 10));
> >> + mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' + ((Column + 1) %
> >> 10));
> >> + String = mSetCursorPositionString;
> >> + }
> >>
> >> TerminalDevice->OutputEscChar = TRUE;
> >> - Status = This->OutputString (This, mSetCursorPositionString);
> >> + Status = This->OutputString (This, String);
> >> TerminalDevice->OutputEscChar = FALSE;
> >>
> >> if (EFI_ERROR (Status)) {
> >> --
> >> 2.7.4
> >>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
>
>
> --
>
> Brian
>
> --------------------------------------------------------------------
>
> "I have discovered that all human evil comes from this, man's being
> unable to sit still in a room."
> -- Blaise Pascal
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
2016-10-27 18:14 ` Kinney, Michael D
@ 2016-10-27 18:32 ` Kinney, Michael D
0 siblings, 0 replies; 18+ messages in thread
From: Kinney, Michael D @ 2016-10-27 18:32 UTC (permalink / raw)
To: Brian J. Johnson, edk2-devel@lists.01.org, Kinney, Michael D
Cc: Tian, Feng, Zeng, Star
Committed at
https://github.com/tianocore/edk2/commit/d1b757e2cd034e32676c5cc2d542f785e74f8c5d
Mike
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Thursday, October 27, 2016 11:14 AM
> To: Brian J. Johnson <bjohnson@sgi.com>; edk2-devel@lists.01.org; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor
> motion
>
> Brian,
>
> Thanks. I will do the commit with your rb.
>
> Mike
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Brian J.
> Johnson
> > Sent: Thursday, October 27, 2016 11:06 AM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
> > Cc: Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: Re: [edk2] [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor
> > motion
> >
> > On 10/26/2016 10:09 PM, Kinney, Michael D wrote:
> > > Tian Feng,
> > >
> > > Unfortunately, this patch that was pushed to edk2/master today
> > > breaks on IA32 VS2015x86 builds with a signed/unsigned mismatch
> > > on 3 lines. I think the right fix might be:
> > >
> > > diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > > index e9b5ed0..9625f4d 100644
> > > --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > > +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > > @@ -790,13 +790,13 @@ TerminalConOutSetCursorPosition (
> > > // it isn't necessary.
> > > //
> > > if (TerminalDevice->TerminalType == TTYTERMTYPE &&
> > > - Mode->CursorRow == Row) {
> > > - if (Mode->CursorColumn > Column) {
> > > + (UINTN)Mode->CursorRow == Row) {
> > > + if ((UINTN)Mode->CursorColumn > Column) {
> > > mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Mode-
> > >CursorColumn - Column) / 10));
> > > mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Mode-
> > >CursorColumn - Column) % 10));
> > > String = mCursorBackwardString;
> > > }
> > > - else if (Column > Mode->CursorColumn) {
> > > + else if (Column > (UINTN)Mode->CursorColumn) {
> > > mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Column - Mode-
> > >CursorColumn) / 10));
> > > mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Column - Mode-
> > >CursorColumn) % 10));
> > > String = mCursorForwardString;
> > >
> > > Please see if you can reproduce this issue.
> > >
> > > Thanks,
> > >
> > > Mike
> >
> > The changes look fine to me. Unfortunately, I don't have that compiler
> > to test with.
> >
> > If it's needed,
> > Reviewed-by: Brian Johnson <bjohnson@sgi.com>
> >
> > >
> > >> -----Original Message-----
> > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Brian J.
> > Johnson
> > >> Sent: Friday, October 7, 2016 7:54 AM
> > >> To: edk2-devel@lists.01.org
> > >> Cc: Brian J. Johnson <bjohnson@sgi.com>; Tian, Feng <feng.tian@intel.com>; Zeng,
> > Star
> > >> <star.zeng@intel.com>
> > >> Subject: [edk2] [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor
> motion
> > >>
> > >> For TtyTerm terminals, output a shorter escape sequence when possible
> > >> to move the cursor within the current line, and don't print any escape
> > >> sequence if the cursor is already at the correct position. This
> > >> removes extra cursor motion activity at the EFI shell prompt,
> > >> improving performance. It also makes it possible in many cases to
> > >> successfully use a terminal window which is taller than the driver's
> > >> mode setting (eg. 80x25.)
> > >>
> > >> Contributed-under: TianoCore Contribution Agreement 1.0
> > >> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
> > >> Cc: Feng Tian <feng.tian@intel.com>
> > >> Cc: Star Zeng <star.zeng@intel.com>
> > >> ---
> > >> .../Universal/Console/TerminalDxe/Terminal.h | 2 ++
> > >> .../Universal/Console/TerminalDxe/TerminalConOut.c | 36 +++++++++++++++++++---
> > >> 2 files changed, 33 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > >> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > >> index 269d2ae..3ee3969 100644
> > >> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > >> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> > >> @@ -2,6 +2,7 @@
> > >> Header file for Terminal driver.
> > >>
> > >> Copyright (c) 2006 - 2014, 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
> > >> which accompanies this distribution. The full text of the license may be found
> at
> > >> @@ -157,6 +158,7 @@ typedef union {
> > >> #define BACKGROUND_CONTROL_OFFSET 11
> > >> #define ROW_OFFSET 2
> > >> #define COLUMN_OFFSET 5
> > >> +#define FW_BACK_OFFSET 2
> > >>
> > >> typedef struct {
> > >> UINT16 Unicode;
> > >> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > >> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > >> index b11e83f..e9b5ed0 100644
> > >> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > >> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> > >> @@ -83,6 +83,8 @@ CHAR16 mSetModeString[] = { ESC, '[', '=', '3', 'h',
> 0
> > };
> > >> CHAR16 mSetAttributeString[] = { ESC, '[', '0', 'm', ESC, '[', '4', '0',
> 'm',
> > >> ESC, '[', '4', '0', 'm', 0 };
> > >> CHAR16 mClearScreenString[] = { ESC, '[', '2', 'J', 0 };
> > >> CHAR16 mSetCursorPositionString[] = { ESC, '[', '0', '0', ';', '0', '0', 'H', 0
> };
> > >> +CHAR16 mCursorForwardString[] = { ESC, '[', '0', '0', 'C', 0 };
> > >> +CHAR16 mCursorBackwardString[] = { ESC, '[', '0', '0', 'D', 0 };
> > >>
> > >> //
> > >> // Body of the ConOut functions
> > >> @@ -755,6 +757,7 @@ TerminalConOutSetCursorPosition (
> > >> UINTN MaxRow;
> > >> EFI_STATUS Status;
> > >> TERMINAL_DEV *TerminalDevice;
> > >> + CHAR16 *String;
> > >>
> > >> TerminalDevice = TERMINAL_CON_OUT_DEV_FROM_THIS (This);
> > >>
> > >> @@ -782,13 +785,36 @@ TerminalConOutSetCursorPosition (
> > >> //
> > >> // control sequence to move the cursor
> > >> //
> > >> - mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) /
> 10));
> > >> - mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) %
> 10));
> > >> - mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' + ((Column + 1) /
> > 10));
> > >> - mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' + ((Column + 1) %
> > 10));
> > >> + // Optimize cursor motion control sequences for TtyTerm. Move
> > >> + // within the current line if possible, and don't output anyting if
> > >> + // it isn't necessary.
> > >> + //
> > >> + if (TerminalDevice->TerminalType == TTYTERMTYPE &&
> > >> + Mode->CursorRow == Row) {
> > >> + if (Mode->CursorColumn > Column) {
> > >> + mCursorBackwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Mode-
> > >CursorColumn
> > >> - Column) / 10));
> > >> + mCursorBackwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Mode-
> > >CursorColumn
> > >> - Column) % 10));
> > >> + String = mCursorBackwardString;
> > >> + }
> > >> + else if (Column > Mode->CursorColumn) {
> > >> + mCursorForwardString[FW_BACK_OFFSET + 0] = (CHAR16) ('0' + ((Column - Mode-
> > >>> CursorColumn) / 10));
> > >> + mCursorForwardString[FW_BACK_OFFSET + 1] = (CHAR16) ('0' + ((Column - Mode-
> > >>> CursorColumn) % 10));
> > >> + String = mCursorForwardString;
> > >> + }
> > >> + else {
> > >> + String = L""; // No cursor motion necessary
> > >> + }
> > >> + }
> > >> + else {
> > >> + mSetCursorPositionString[ROW_OFFSET + 0] = (CHAR16) ('0' + ((Row + 1) /
> > 10));
> > >> + mSetCursorPositionString[ROW_OFFSET + 1] = (CHAR16) ('0' + ((Row + 1) %
> > 10));
> > >> + mSetCursorPositionString[COLUMN_OFFSET + 0] = (CHAR16) ('0' + ((Column + 1) /
> > >> 10));
> > >> + mSetCursorPositionString[COLUMN_OFFSET + 1] = (CHAR16) ('0' + ((Column + 1) %
> > >> 10));
> > >> + String = mSetCursorPositionString;
> > >> + }
> > >>
> > >> TerminalDevice->OutputEscChar = TRUE;
> > >> - Status = This->OutputString (This, mSetCursorPositionString);
> > >> + Status = This->OutputString (This, String);
> > >> TerminalDevice->OutputEscChar = FALSE;
> > >>
> > >> if (EFI_ERROR (Status)) {
> > >> --
> > >> 2.7.4
> > >>
> > >> _______________________________________________
> > >> edk2-devel mailing list
> > >> edk2-devel@lists.01.org
> > >> https://lists.01.org/mailman/listinfo/edk2-devel
> >
> >
> > --
> >
> > Brian
> >
> > --------------------------------------------------------------------
> >
> > "I have discovered that all human evil comes from this, man's being
> > unable to sit still in a room."
> > -- Blaise Pascal
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-10-27 18:32 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-07 14:53 [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements Brian J. Johnson
2016-10-07 14:53 ` [PATCH 1/3] MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking Brian J. Johnson
2016-10-07 14:53 ` [PATCH 2/3] MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion Brian J. Johnson
2016-10-27 3:09 ` Kinney, Michael D
2016-10-27 5:06 ` Tian, Feng
2016-10-27 18:06 ` Brian J. Johnson
2016-10-27 18:14 ` Kinney, Michael D
2016-10-27 18:32 ` Kinney, Michael D
2016-10-07 14:54 ` [PATCH 3/3] MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm Brian J. Johnson
2016-10-14 20:53 ` Roy Franz (HPE)
2016-10-07 15:56 ` [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements Laszlo Ersek
2016-10-07 15:59 ` Leif Lindholm
2016-10-12 8:17 ` Ryan Harkin
2016-10-14 19:39 ` Brian J. Johnson
2016-10-14 20:37 ` Laszlo Ersek
2016-10-18 15:34 ` Brian J. Johnson
2016-10-26 15:00 ` Brian J. Johnson
2016-10-27 1:01 ` Tian, Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox