Re: [PATCH 32/45] tty: vt: use enum for VESA blanking modes

2024-01-18 Thread Thomas Zimmermann

Hi

Am 18.01.24 um 08:57 schrieb Jiri Slaby (SUSE):

Switch VESA macros to an enum and add and use VESA_BLANK_MAX. This
improves type checking in consw::con_blank().

There is a downside of this. The macros were defined twice: in
linux/console.h and uapi/linux/fb.h. We cannot remove the latter (uapi
header), but nor we want to expand them in the kernel too. So protect
them using __KERNEL__. In the kernel case, include linux/console.h
instead. This header dependency is preexisting.

Alternatively, we could create a vesa.h header with that sole enum and
include it. If it turns out linux/console.h is too much for fb.h.


Personally I'd prefer something like include/uapi/video/vesa.h that 
contains the current defines. Fbdev is deprecated and the more we can 
avoid building upon it, the better. If you want an enum type in the 
kernel, maybe create it from the constants like this:


enum vesa_blank_mode {
VESA_BLANK_MODE_NO_BLANKING = VESA_NO_BLANKING,
VESA_BLANK_MODE_VSYNC = VESA_VSYNC_SYSPEND,
[...]
VESA_MAX_BLANK_MODE = ...,
};

Best regards
Thomas



Signed-off-by: Jiri Slaby (SUSE) 
Cc: Helge Deller 
Cc: "James E.J. Bottomley" 
Cc: Daniel Vetter 
Cc: linux-fb...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-par...@vger.kernel.org
---
  drivers/tty/vt/vt.c |  4 ++--
  drivers/video/console/dummycon.c|  6 --
  drivers/video/console/mdacon.c  |  3 ++-
  drivers/video/console/newport_con.c |  3 ++-
  drivers/video/console/sticon.c  |  3 ++-
  drivers/video/console/vgacon.c  |  7 ---
  drivers/video/fbdev/core/fbcon.c|  3 ++-
  include/linux/console.h | 18 +++---
  include/uapi/linux/fb.h |  5 -
  9 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 6f46fefedcfb..756291f37d47 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -175,7 +175,7 @@ int do_poke_blanked_console;
  int console_blanked;
  EXPORT_SYMBOL(console_blanked);
  
-static int vesa_blank_mode; /* 0:none 1:suspendV 2:suspendH 3:powerdown */

+static enum vesa_blank_mode vesa_blank_mode;
  static int vesa_off_interval;
  static int blankinterval;
  core_param(consoleblank, blankinterval, int, 0444);
@@ -4334,7 +4334,7 @@ static int set_vesa_blanking(u8 __user *mode_user)
return -EFAULT;
  
  	console_lock();

-   vesa_blank_mode = (mode < 4) ? mode : VESA_NO_BLANKING;
+   vesa_blank_mode = (mode <= VESA_BLANK_MAX) ? mode : VESA_NO_BLANKING;
console_unlock();
  
  	return 0;

diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
index c8d5aa0e3ed0..d86c1d798690 100644
--- a/drivers/video/console/dummycon.c
+++ b/drivers/video/console/dummycon.c
@@ -79,7 +79,8 @@ static void dummycon_putcs(struct vc_data *vc, const u16 *s, 
unsigned int count,
raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
  }
  
-static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)

+static int dummycon_blank(struct vc_data *vc, enum vesa_blank_mode blank,
+ int mode_switch)
  {
/* Redraw, so that we get putc(s) for output done while blanked */
return 1;
@@ -89,7 +90,8 @@ static void dummycon_putc(struct vc_data *vc, u16 c, unsigned 
int y,
  unsigned int x) { }
  static void dummycon_putcs(struct vc_data *vc, const u16 *s, unsigned int 
count,
   unsigned int ypos, unsigned int xpos) { }
-static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
+static int dummycon_blank(struct vc_data *vc, enum vesa_blank_mode blank,
+ int mode_switch)
  {
return 0;
  }
diff --git a/drivers/video/console/mdacon.c b/drivers/video/console/mdacon.c
index 4485ef923bb3..63e3ce678aab 100644
--- a/drivers/video/console/mdacon.c
+++ b/drivers/video/console/mdacon.c
@@ -451,7 +451,8 @@ static bool mdacon_switch(struct vc_data *c)
return true;/* redrawing needed */
  }
  
-static int mdacon_blank(struct vc_data *c, int blank, int mode_switch)

+static int mdacon_blank(struct vc_data *c, enum vesa_blank_mode blank,
+   int mode_switch)
  {
if (mda_type == TYPE_MDA) {
if (blank)
diff --git a/drivers/video/console/newport_con.c 
b/drivers/video/console/newport_con.c
index ad3a09142770..38437a53b7f1 100644
--- a/drivers/video/console/newport_con.c
+++ b/drivers/video/console/newport_con.c
@@ -476,7 +476,8 @@ static bool newport_switch(struct vc_data *vc)
return true;
  }
  
-static int newport_blank(struct vc_data *c, int blank, int mode_switch)

+static int newport_blank(struct vc_data *c, enum vesa_blank_mode blank,
+int mode_switch)
  {
unsigned short treg;
  
diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c

index 817b89c45e81..e9d5d1f92883 100644
--- a/dri

[PATCH 32/45] tty: vt: use enum for VESA blanking modes

2024-01-17 Thread Jiri Slaby (SUSE)
Switch VESA macros to an enum and add and use VESA_BLANK_MAX. This
improves type checking in consw::con_blank().

There is a downside of this. The macros were defined twice: in
linux/console.h and uapi/linux/fb.h. We cannot remove the latter (uapi
header), but nor we want to expand them in the kernel too. So protect
them using __KERNEL__. In the kernel case, include linux/console.h
instead. This header dependency is preexisting.

Alternatively, we could create a vesa.h header with that sole enum and
include it. If it turns out linux/console.h is too much for fb.h.

Signed-off-by: Jiri Slaby (SUSE) 
Cc: Helge Deller 
Cc: "James E.J. Bottomley" 
Cc: Daniel Vetter 
Cc: linux-fb...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-par...@vger.kernel.org
---
 drivers/tty/vt/vt.c |  4 ++--
 drivers/video/console/dummycon.c|  6 --
 drivers/video/console/mdacon.c  |  3 ++-
 drivers/video/console/newport_con.c |  3 ++-
 drivers/video/console/sticon.c  |  3 ++-
 drivers/video/console/vgacon.c  |  7 ---
 drivers/video/fbdev/core/fbcon.c|  3 ++-
 include/linux/console.h | 18 +++---
 include/uapi/linux/fb.h |  5 -
 9 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 6f46fefedcfb..756291f37d47 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -175,7 +175,7 @@ int do_poke_blanked_console;
 int console_blanked;
 EXPORT_SYMBOL(console_blanked);
 
-static int vesa_blank_mode; /* 0:none 1:suspendV 2:suspendH 3:powerdown */
+static enum vesa_blank_mode vesa_blank_mode;
 static int vesa_off_interval;
 static int blankinterval;
 core_param(consoleblank, blankinterval, int, 0444);
@@ -4334,7 +4334,7 @@ static int set_vesa_blanking(u8 __user *mode_user)
return -EFAULT;
 
console_lock();
-   vesa_blank_mode = (mode < 4) ? mode : VESA_NO_BLANKING;
+   vesa_blank_mode = (mode <= VESA_BLANK_MAX) ? mode : VESA_NO_BLANKING;
console_unlock();
 
return 0;
diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
index c8d5aa0e3ed0..d86c1d798690 100644
--- a/drivers/video/console/dummycon.c
+++ b/drivers/video/console/dummycon.c
@@ -79,7 +79,8 @@ static void dummycon_putcs(struct vc_data *vc, const u16 *s, 
unsigned int count,
raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
 }
 
-static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
+static int dummycon_blank(struct vc_data *vc, enum vesa_blank_mode blank,
+ int mode_switch)
 {
/* Redraw, so that we get putc(s) for output done while blanked */
return 1;
@@ -89,7 +90,8 @@ static void dummycon_putc(struct vc_data *vc, u16 c, unsigned 
int y,
  unsigned int x) { }
 static void dummycon_putcs(struct vc_data *vc, const u16 *s, unsigned int 
count,
   unsigned int ypos, unsigned int xpos) { }
-static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
+static int dummycon_blank(struct vc_data *vc, enum vesa_blank_mode blank,
+ int mode_switch)
 {
return 0;
 }
diff --git a/drivers/video/console/mdacon.c b/drivers/video/console/mdacon.c
index 4485ef923bb3..63e3ce678aab 100644
--- a/drivers/video/console/mdacon.c
+++ b/drivers/video/console/mdacon.c
@@ -451,7 +451,8 @@ static bool mdacon_switch(struct vc_data *c)
return true;/* redrawing needed */
 }
 
-static int mdacon_blank(struct vc_data *c, int blank, int mode_switch)
+static int mdacon_blank(struct vc_data *c, enum vesa_blank_mode blank,
+   int mode_switch)
 {
if (mda_type == TYPE_MDA) {
if (blank) 
diff --git a/drivers/video/console/newport_con.c 
b/drivers/video/console/newport_con.c
index ad3a09142770..38437a53b7f1 100644
--- a/drivers/video/console/newport_con.c
+++ b/drivers/video/console/newport_con.c
@@ -476,7 +476,8 @@ static bool newport_switch(struct vc_data *vc)
return true;
 }
 
-static int newport_blank(struct vc_data *c, int blank, int mode_switch)
+static int newport_blank(struct vc_data *c, enum vesa_blank_mode blank,
+int mode_switch)
 {
unsigned short treg;
 
diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 817b89c45e81..e9d5d1f92883 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c
@@ -298,7 +298,8 @@ static bool sticon_switch(struct vc_data *conp)
 return true;   /* needs refreshing */
 }
 
-static int sticon_blank(struct vc_data *c, int blank, int mode_switch)
+static int sticon_blank(struct vc_data *c, enum vesa_blank_mode blank,
+   int mode_switch)
 {
 if (blank == VESA_NO_BLANKING) {
if (mode_switch)
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 910dc73874b7..a4bd97ab502d 100644
-