[PATCH 1/1] efi_loader: avoid EFI_CALL() for clearing screen

[PATCH 1/1] efi_loader: avoid EFI_CALL() for clearing screen

[Heinrich Schuchardt]

Carve out function efi_clear_screen.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/efi_loader/efi_console.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index 3354b217a9..6d4784e140 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -460,6 +460,20 @@ static efi_status_t EFIAPI efi_cout_set_attribute(
 	return EFI_EXIT(EFI_SUCCESS);
 }
 
+/**
+ * efi_cout_clear_screen() - clear screen
+ */
+static void efi_clear_screen(void)
+{
+	/*
+	 * The Linux console wants both a clear and a home command. The video
+	 * uclass does not support <ESC>[H without coordinates, yet.
+	 */
+	printf(ESC "[2J" ESC "[1;1H");
+	efi_con_mode.cursor_column = 0;
+	efi_con_mode.cursor_row = 0;
+}
+
 /**
  * efi_cout_clear_screen() - clear screen
  *
@@ -475,13 +489,7 @@ static efi_status_t EFIAPI efi_cout_clear_screen(
 {
 	EFI_ENTRY("%p", this);
 
-	/*
-	 * The Linux console wants both a clear and a home command. The video
-	 * uclass does not support <ESC>[H without coordinates, yet.
-	 */
-	printf(ESC "[2J" ESC "[1;1H");
-	efi_con_mode.cursor_column = 0;
-	efi_con_mode.cursor_row = 0;
+	efi_clear_screen();
 
 	return EFI_EXIT(EFI_SUCCESS);
 }
@@ -510,7 +518,7 @@ static efi_status_t EFIAPI efi_cout_set_mode(
 		return EFI_EXIT(EFI_UNSUPPORTED);
 
 	efi_con_mode.mode = mode_number;
-	EFI_CALL(efi_cout_clear_screen(this));
+	efi_clear_screen();
 
 	return EFI_EXIT(EFI_SUCCESS);
 }
@@ -536,7 +544,7 @@ static efi_status_t EFIAPI efi_cout_reset(
 	efi_con_mode.attribute = 0x07;
 	printf(ESC "[0;37;40m");
 	/* Clear screen */
-	EFI_CALL(efi_cout_clear_screen(this));
+	efi_clear_screen();
 
 	return EFI_EXIT(EFI_SUCCESS);
 }
-- 
2.37.2

Re: [PATCH 1/1] efi_loader: avoid EFI_CALL() for clearing screen

[Ilias Apalodimas]

On Sat, Oct 15, 2022 at 12:15:47PM +0200, Heinrich Schuchardt wrote:
> Carve out function efi_clear_screen.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  lib/efi_loader/efi_console.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 3354b217a9..6d4784e140 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -460,6 +460,20 @@ static efi_status_t EFIAPI efi_cout_set_attribute(
>  	return EFI_EXIT(EFI_SUCCESS);
>  }
>  
> +/**
> + * efi_cout_clear_screen() - clear screen
> + */
> +static void efi_clear_screen(void)
> +{
> +	/*
> +	 * The Linux console wants both a clear and a home command. The video
> +	 * uclass does not support <ESC>[H without coordinates, yet.
> +	 */
> +	printf(ESC "[2J" ESC "[1;1H");
> +	efi_con_mode.cursor_column = 0;
> +	efi_con_mode.cursor_row = 0;
> +}
> +
>  /**
>   * efi_cout_clear_screen() - clear screen
>   *
> @@ -475,13 +489,7 @@ static efi_status_t EFIAPI efi_cout_clear_screen(
>  {
>  	EFI_ENTRY("%p", this);
>  
> -	/*
> -	 * The Linux console wants both a clear and a home command. The video
> -	 * uclass does not support <ESC>[H without coordinates, yet.
> -	 */
> -	printf(ESC "[2J" ESC "[1;1H");
> -	efi_con_mode.cursor_column = 0;
> -	efi_con_mode.cursor_row = 0;
> +	efi_clear_screen();
>  
>  	return EFI_EXIT(EFI_SUCCESS);
>  }
> @@ -510,7 +518,7 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>  		return EFI_EXIT(EFI_UNSUPPORTED);
>  
>  	efi_con_mode.mode = mode_number;
> -	EFI_CALL(efi_cout_clear_screen(this));
> +	efi_clear_screen();
>  
>  	return EFI_EXIT(EFI_SUCCESS);
>  }
> @@ -536,7 +544,7 @@ static efi_status_t EFIAPI efi_cout_reset(
>  	efi_con_mode.attribute = 0x07;
>  	printf(ESC "[0;37;40m");
>  	/* Clear screen */
> -	EFI_CALL(efi_cout_clear_screen(this));
> +	efi_clear_screen();
>  
>  	return EFI_EXIT(EFI_SUCCESS);
>  }
> -- 
> 2.37.2
> 
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [PATCH 1/1] efi_loader: avoid EFI_CALL() for clearing screen

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]

On 15.10.22 12:15, Heinrich Schuchardt wrote:
> Carve out function efi_clear_screen.
> 

This does not motivate *why* you prefer to not use efi_cout_clear_screen
anymore.

A side effect of this is that the screen will not longer be filled with
a new background color, leading to lines that are written with that new
background but only for the actual text length. Attached a screenshot to
visualize this. Was that intended? Not saying it's necessarily critical.

Jan

PS: This is with proper console scrolling in place to avoid overwriting.
Will send an update of my related patch soon.

> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  lib/efi_loader/efi_console.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 3354b217a9..6d4784e140 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -460,6 +460,20 @@ static efi_status_t EFIAPI efi_cout_set_attribute(
>  	return EFI_EXIT(EFI_SUCCESS);
>  }
>  
> +/**
> + * efi_cout_clear_screen() - clear screen
> + */
> +static void efi_clear_screen(void)
> +{
> +	/*
> +	 * The Linux console wants both a clear and a home command. The video
> +	 * uclass does not support <ESC>[H without coordinates, yet.
> +	 */
> +	printf(ESC "[2J" ESC "[1;1H");
> +	efi_con_mode.cursor_column = 0;
> +	efi_con_mode.cursor_row = 0;
> +}
> +
>  /**
>   * efi_cout_clear_screen() - clear screen
>   *
> @@ -475,13 +489,7 @@ static efi_status_t EFIAPI efi_cout_clear_screen(
>  {
>  	EFI_ENTRY("%p", this);
>  
> -	/*
> -	 * The Linux console wants both a clear and a home command. The video
> -	 * uclass does not support <ESC>[H without coordinates, yet.
> -	 */
> -	printf(ESC "[2J" ESC "[1;1H");
> -	efi_con_mode.cursor_column = 0;
> -	efi_con_mode.cursor_row = 0;
> +	efi_clear_screen();
>  
>  	return EFI_EXIT(EFI_SUCCESS);
>  }
> @@ -510,7 +518,7 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>  		return EFI_EXIT(EFI_UNSUPPORTED);
>  
>  	efi_con_mode.mode = mode_number;
> -	EFI_CALL(efi_cout_clear_screen(this));
> +	efi_clear_screen();
>  
>  	return EFI_EXIT(EFI_SUCCESS);
>  }
> @@ -536,7 +544,7 @@ static efi_status_t EFIAPI efi_cout_reset(
>  	efi_con_mode.attribute = 0x07;
>  	printf(ESC "[0;37;40m");
>  	/* Clear screen */
> -	EFI_CALL(efi_cout_clear_screen(this));
> +	efi_clear_screen();
>  
>  	return EFI_EXIT(EFI_SUCCESS);
>  }

-- 
Siemens AG, Technology
Competence Center Embedded Linux

[-- Attachment #2: Bildschirmfoto zu 2022-11-04 08-21-28.png --]
[-- Type: image/png, Size: 32395 bytes --]

Re: [PATCH 1/1] efi_loader: avoid EFI_CALL() for clearing screen

[Jan Kiszka]

On 04.11.22 08:25, Jan Kiszka wrote:
> On 15.10.22 12:15, Heinrich Schuchardt wrote:
>> Carve out function efi_clear_screen.
>>
> 
> This does not motivate *why* you prefer to not use efi_cout_clear_screen
> anymore.

Why this in principle still hold...

> 
> A side effect of this is that the screen will not longer be filled with
> a new background color, leading to lines that are written with that new
> background but only for the actual text length. Attached a screenshot to
> visualize this. Was that intended? Not saying it's necessarily critical.

...strike that: I lost some bits from my "efi_loader: Improve console
screen clearing and reset" patch while rebasing, and those were
responsible for also setting the background color consistently. Seems I
need to work on my own commit message too.

Jan

> 
> Jan
> 
> PS: This is with proper console scrolling in place to avoid overwriting.
> Will send an update of my related patch soon.
> 
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>>  lib/efi_loader/efi_console.c | 26 +++++++++++++++++---------
>>  1 file changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>> index 3354b217a9..6d4784e140 100644
>> --- a/lib/efi_loader/efi_console.c
>> +++ b/lib/efi_loader/efi_console.c
>> @@ -460,6 +460,20 @@ static efi_status_t EFIAPI efi_cout_set_attribute(
>>  	return EFI_EXIT(EFI_SUCCESS);
>>  }
>>  
>> +/**
>> + * efi_cout_clear_screen() - clear screen
>> + */
>> +static void efi_clear_screen(void)
>> +{
>> +	/*
>> +	 * The Linux console wants both a clear and a home command. The video
>> +	 * uclass does not support <ESC>[H without coordinates, yet.
>> +	 */
>> +	printf(ESC "[2J" ESC "[1;1H");
>> +	efi_con_mode.cursor_column = 0;
>> +	efi_con_mode.cursor_row = 0;
>> +}
>> +
>>  /**
>>   * efi_cout_clear_screen() - clear screen
>>   *
>> @@ -475,13 +489,7 @@ static efi_status_t EFIAPI efi_cout_clear_screen(
>>  {
>>  	EFI_ENTRY("%p", this);
>>  
>> -	/*
>> -	 * The Linux console wants both a clear and a home command. The video
>> -	 * uclass does not support <ESC>[H without coordinates, yet.
>> -	 */
>> -	printf(ESC "[2J" ESC "[1;1H");
>> -	efi_con_mode.cursor_column = 0;
>> -	efi_con_mode.cursor_row = 0;
>> +	efi_clear_screen();
>>  
>>  	return EFI_EXIT(EFI_SUCCESS);
>>  }
>> @@ -510,7 +518,7 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>>  		return EFI_EXIT(EFI_UNSUPPORTED);
>>  
>>  	efi_con_mode.mode = mode_number;
>> -	EFI_CALL(efi_cout_clear_screen(this));
>> +	efi_clear_screen();
>>  
>>  	return EFI_EXIT(EFI_SUCCESS);
>>  }
>> @@ -536,7 +544,7 @@ static efi_status_t EFIAPI efi_cout_reset(
>>  	efi_con_mode.attribute = 0x07;
>>  	printf(ESC "[0;37;40m");
>>  	/* Clear screen */
>> -	EFI_CALL(efi_cout_clear_screen(this));
>> +	efi_clear_screen();
>>  
>>  	return EFI_EXIT(EFI_SUCCESS);
>>  }
> 

-- 
Siemens AG, Technology
Competence Center Embedded Linux