* [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
* 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
* [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 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-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 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
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