Re: [PATCH v2 5/9] vgacon: remove screen_info dependency

2023-07-20 Thread Khalid Aziz 

On 7/19/23 6:39 AM, Arnd Bergmann wrote:

From: Arnd Bergmann 

The vga console driver is fairly self-contained, and only used by
architectures that explicitly initialize the screen_info settings.

Chance every instance that picks the vga console by setting conswitchp
to call a function instead, and pass a reference to the screen_info
there.

Signed-off-by: Arnd Bergmann 


PCDP and ia64 changes look good to me.

Acked-by: Khalid Azzi 


---
  arch/alpha/kernel/setup.c  |  2 +-
  arch/arm/kernel/setup.c|  2 +-
  arch/ia64/kernel/setup.c   |  2 +-
  arch/mips/kernel/setup.c   |  2 +-
  arch/x86/kernel/setup.c|  2 +-
  drivers/firmware/pcdp.c|  2 +-
  drivers/video/console/vgacon.c | 68 --
  include/linux/console.h|  7 
  8 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index b4d2297765c02..d73b685fe9852 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -655,7 +655,7 @@ setup_arch(char **cmdline_p)
  
  #ifdef CONFIG_VT

  #if defined(CONFIG_VGA_CONSOLE)
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
  #endif
  #endif
  
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c

index 40326a35a179b..5d8a7fb3eba45 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1192,7 +1192,7 @@ void __init setup_arch(char **cmdline_p)
  
  #ifdef CONFIG_VT

  #if defined(CONFIG_VGA_CONSOLE)
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
  #endif
  #endif
  
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c

index d2c66efdde560..2c9283fcd3759 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -619,7 +619,7 @@ setup_arch (char **cmdline_p)
 * memory so we can avoid this problem.
 */
if (efi_mem_type(0xA) != EFI_CONVENTIONAL_MEMORY)
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
  # endif
}
  #endif
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 1aba7dc95132c..6c3fae62a9f6b 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -794,7 +794,7 @@ void __init setup_arch(char **cmdline_p)
  
  #if defined(CONFIG_VT)

  #if defined(CONFIG_VGA_CONSOLE)
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
  #endif
  #endif
  
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c

index fd975a4a52006..b1ea77d504615 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1293,7 +1293,7 @@ void __init setup_arch(char **cmdline_p)
  #ifdef CONFIG_VT
  #if defined(CONFIG_VGA_CONSOLE)
if (!efi_enabled(EFI_BOOT) || (efi_mem_type(0xa) != 
EFI_CONVENTIONAL_MEMORY))
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
  #endif
  #endif
x86_init.oem.banner();
diff --git a/drivers/firmware/pcdp.c b/drivers/firmware/pcdp.c
index 715a45442d1cf..667a595373b2d 100644
--- a/drivers/firmware/pcdp.c
+++ b/drivers/firmware/pcdp.c
@@ -72,7 +72,7 @@ setup_vga_console(struct pcdp_device *dev)
return -ENODEV;
}
  
-	conswitchp = &vga_con;

+   vgacon_register_screen(&screen_info);
printk(KERN_INFO "PCDP: VGA console\n");
return 0;
  #else
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index e25ba523892e5..3d7fedf27ffc1 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -97,6 +97,8 @@ static intvga_video_font_height;
  static intvga_scan_lines  __read_mostly;
  static unsigned int   vga_rolled_over; /* last vc_origin offset before wrap */
  
+static struct screen_info *vga_si;

+
  static bool vga_hardscroll_enabled;
  static bool vga_hardscroll_user_enable = true;
  
@@ -161,8 +163,9 @@ static const char *vgacon_startup(void)

u16 saved1, saved2;
volatile u16 *p;
  
-	if (screen_info.orig_video_isVGA == VIDEO_TYPE_VLFB ||

-   screen_info.orig_video_isVGA == VIDEO_TYPE_EFI) {
+   if (!vga_si ||
+   vga_si->orig_video_isVGA == VIDEO_TYPE_VLFB ||
+   vga_si->orig_video_isVGA == VIDEO_TYPE_EFI) {
  no_vga:
  #ifdef CONFIG_DUMMY_CONSOLE
conswitchp = &dummy_con;
@@ -172,29 +175,29 @@ static const char *vgacon_startup(void)
  #endif
}
  
-	/* boot_params.screen_info reasonably initialized? */

-   if ((screen_info.orig_video_lines == 0) ||
-   (screen_info.orig_video_cols  == 0))
+   /* vga_si reasonably initialized? */
+   if ((vga_si->orig_video_lines == 0) ||
+   (vga_si->orig_video_cols  == 0))
goto no_vga;
  
  	/* VGA16 modes are not handled by VGACON */

-   if ((screen_info.orig_video_mode == 0x0D) ||/* 320x200/4 */
-   (sc

Re: [PATCH v2 5/9] vgacon: remove screen_info dependency

2023-07-19 Thread Arnd Bergmann 
On Wed, Jul 19, 2023, at 15:49, Philippe Mathieu-Daudé wrote:
> On 19/7/23 14:39, Arnd Bergmann wrote:

>> @@ -1074,13 +1077,13 @@ static int vgacon_resize(struct vc_data *c, unsigned 
>> int width,
>>   * Ho ho!  Someone (svgatextmode, eh?) may have reprogrammed
>>   * the video mode!  Set the new defaults then and go away.
>>   */
>> -screen_info.orig_video_cols = width;
>> -screen_info.orig_video_lines = height;
>> +vga_si->orig_video_cols = width;
>> +vga_si->orig_video_lines = height;
>>  vga_default_font_height = c->vc_cell_height;
>>  return 0;
>>  }
>> -if (width % 2 || width > screen_info.orig_video_cols ||
>> -height > (screen_info.orig_video_lines * vga_default_font_height)/
>> +if (width % 2 || width > vga_si->orig_video_cols ||
>> +height > (vga_si->orig_video_lines * vga_default_font_height)/
>>  c->vc_cell_height)
>>  return -EINVAL;
>>   
>> @@ -1110,8 +1113,8 @@ static void vgacon_save_screen(struct vc_data *c)
>>   * console initialization routines.
>>   */
>>  vga_bootup_console = 1;
>> -c->state.x = screen_info.orig_x;
>> -c->state.y = screen_info.orig_y;
>> +c->state.x = vga_si->orig_x;
>> +c->state.y = vga_si->orig_y;
>
> Not really my area, so bare with me if this is obviously not
> possible :) If using DUMMY_CONSOLE, can we trigger a save_screen
> / resize? If so, we'd reach here with vga_si=NULL.
>

I think it cannot happen because the only way that anything calls
into vgacon.c is through the "conswitchp = &vga_con;" that now happens
at the same time as the "vga_si = &screen_info;". It's definitely
possible that I'm missing something as well here.

 Arnd

Re: [PATCH v2 5/9] vgacon: remove screen_info dependency

2023-07-19 Thread Philippe Mathieu-Daudé 

Hi Arnd,

On 19/7/23 14:39, Arnd Bergmann wrote:

From: Arnd Bergmann 

The vga console driver is fairly self-contained, and only used by
architectures that explicitly initialize the screen_info settings.

Chance every instance that picks the vga console by setting conswitchp
to call a function instead, and pass a reference to the screen_info
there.

Signed-off-by: Arnd Bergmann 
---
  arch/alpha/kernel/setup.c  |  2 +-
  arch/arm/kernel/setup.c|  2 +-
  arch/ia64/kernel/setup.c   |  2 +-
  arch/mips/kernel/setup.c   |  2 +-
  arch/x86/kernel/setup.c|  2 +-
  drivers/firmware/pcdp.c|  2 +-
  drivers/video/console/vgacon.c | 68 --
  include/linux/console.h|  7 
  8 files changed, 53 insertions(+), 34 deletions(-)




@@ -1074,13 +1077,13 @@ static int vgacon_resize(struct vc_data *c, unsigned 
int width,
 * Ho ho!  Someone (svgatextmode, eh?) may have reprogrammed
 * the video mode!  Set the new defaults then and go away.
 */
-   screen_info.orig_video_cols = width;
-   screen_info.orig_video_lines = height;
+   vga_si->orig_video_cols = width;
+   vga_si->orig_video_lines = height;
vga_default_font_height = c->vc_cell_height;
return 0;
}
-   if (width % 2 || width > screen_info.orig_video_cols ||
-   height > (screen_info.orig_video_lines * vga_default_font_height)/
+   if (width % 2 || width > vga_si->orig_video_cols ||
+   height > (vga_si->orig_video_lines * vga_default_font_height)/
c->vc_cell_height)
return -EINVAL;
  
@@ -1110,8 +1113,8 @@ static void vgacon_save_screen(struct vc_data *c)

 * console initialization routines.
 */
vga_bootup_console = 1;
-   c->state.x = screen_info.orig_x;
-   c->state.y = screen_info.orig_y;
+   c->state.x = vga_si->orig_x;
+   c->state.y = vga_si->orig_y;


Not really my area, so bare with me if this is obviously not
possible :) If using DUMMY_CONSOLE, can we trigger a save_screen
/ resize? If so, we'd reach here with vga_si=NULL.


}
  
  	/* We can't copy in more than the size of the video buffer,

@@ -1204,4 +1207,13 @@ const struct consw vga_con = {
  };
  EXPORT_SYMBOL(vga_con);
  
+void vgacon_register_screen(struct screen_info *si)

+{
+   if (!si || vga_si)
+   return;
+
+   conswitchp = &vga_con;
+   vga_si = si;
+}

Re: [PATCH v2 5/9] vgacon: remove screen_info dependency

2023-07-19 Thread Javier Martinez Canillas 
Arnd Bergmann  writes:

> From: Arnd Bergmann 
>
> The vga console driver is fairly self-contained, and only used by
> architectures that explicitly initialize the screen_info settings.
>
> Chance every instance that picks the vga console by setting conswitchp
> to call a function instead, and pass a reference to the screen_info
> there.
>
> Signed-off-by: Arnd Bergmann 
> ---

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

[PATCH v2 5/9] vgacon: remove screen_info dependency

2023-07-19 Thread Arnd Bergmann 
From: Arnd Bergmann 

The vga console driver is fairly self-contained, and only used by
architectures that explicitly initialize the screen_info settings.

Chance every instance that picks the vga console by setting conswitchp
to call a function instead, and pass a reference to the screen_info
there.

Signed-off-by: Arnd Bergmann 
---
 arch/alpha/kernel/setup.c  |  2 +-
 arch/arm/kernel/setup.c|  2 +-
 arch/ia64/kernel/setup.c   |  2 +-
 arch/mips/kernel/setup.c   |  2 +-
 arch/x86/kernel/setup.c|  2 +-
 drivers/firmware/pcdp.c|  2 +-
 drivers/video/console/vgacon.c | 68 --
 include/linux/console.h|  7 
 8 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index b4d2297765c02..d73b685fe9852 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -655,7 +655,7 @@ setup_arch(char **cmdline_p)
 
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
 #endif
 #endif
 
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 40326a35a179b..5d8a7fb3eba45 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1192,7 +1192,7 @@ void __init setup_arch(char **cmdline_p)
 
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
 #endif
 #endif
 
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index d2c66efdde560..2c9283fcd3759 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -619,7 +619,7 @@ setup_arch (char **cmdline_p)
 * memory so we can avoid this problem.
 */
if (efi_mem_type(0xA) != EFI_CONVENTIONAL_MEMORY)
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
 # endif
}
 #endif
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 1aba7dc95132c..6c3fae62a9f6b 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -794,7 +794,7 @@ void __init setup_arch(char **cmdline_p)
 
 #if defined(CONFIG_VT)
 #if defined(CONFIG_VGA_CONSOLE)
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
 #endif
 #endif
 
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index fd975a4a52006..b1ea77d504615 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1293,7 +1293,7 @@ void __init setup_arch(char **cmdline_p)
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
if (!efi_enabled(EFI_BOOT) || (efi_mem_type(0xa) != 
EFI_CONVENTIONAL_MEMORY))
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
 #endif
 #endif
x86_init.oem.banner();
diff --git a/drivers/firmware/pcdp.c b/drivers/firmware/pcdp.c
index 715a45442d1cf..667a595373b2d 100644
--- a/drivers/firmware/pcdp.c
+++ b/drivers/firmware/pcdp.c
@@ -72,7 +72,7 @@ setup_vga_console(struct pcdp_device *dev)
return -ENODEV;
}
 
-   conswitchp = &vga_con;
+   vgacon_register_screen(&screen_info);
printk(KERN_INFO "PCDP: VGA console\n");
return 0;
 #else
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index e25ba523892e5..3d7fedf27ffc1 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -97,6 +97,8 @@ static intvga_video_font_height;
 static int vga_scan_lines  __read_mostly;
 static unsigned intvga_rolled_over; /* last vc_origin offset before wrap */
 
+static struct screen_info *vga_si;
+
 static bool vga_hardscroll_enabled;
 static bool vga_hardscroll_user_enable = true;
 
@@ -161,8 +163,9 @@ static const char *vgacon_startup(void)
u16 saved1, saved2;
volatile u16 *p;
 
-   if (screen_info.orig_video_isVGA == VIDEO_TYPE_VLFB ||
-   screen_info.orig_video_isVGA == VIDEO_TYPE_EFI) {
+   if (!vga_si ||
+   vga_si->orig_video_isVGA == VIDEO_TYPE_VLFB ||
+   vga_si->orig_video_isVGA == VIDEO_TYPE_EFI) {
  no_vga:
 #ifdef CONFIG_DUMMY_CONSOLE
conswitchp = &dummy_con;
@@ -172,29 +175,29 @@ static const char *vgacon_startup(void)
 #endif
}
 
-   /* boot_params.screen_info reasonably initialized? */
-   if ((screen_info.orig_video_lines == 0) ||
-   (screen_info.orig_video_cols  == 0))
+   /* vga_si reasonably initialized? */
+   if ((vga_si->orig_video_lines == 0) ||
+   (vga_si->orig_video_cols  == 0))
goto no_vga;
 
/* VGA16 modes are not handled by VGACON */
-   if ((screen_info.orig_video_mode == 0x0D) ||/* 320x200/4 */
-   (screen_info.orig_video_mode == 0x0E) ||/* 640x200/4 */
-   (screen_info.orig_video_mode == 0x10) ||/* 640x350/4 */
-   (s