Re: [PATCH 2/2] vt: Distinguish between blanking and VESA blank mode in con_blank

[Thomas Zimmermann]

Hi

Am 12.10.26 um 13:29 schrieb Greg KH:
> On Mon, Oct 12, 2026 at 11:27:39AM +0200, Thomas Zimmermann wrote:
> > The con_blank callback in struct consw encodes blanking and display
> > blank mode in a single parameter named blank. Its semantics equal
> > the FB_BLANK constants from the fbdev subsystems. Yet it is of type
> > enum vesa_blank_mode, which describes VESA DPMS levels of the attached
> > monitor. Although related, this is something else.
> >
> > For this reason, calls to con_blank fill the blank parameter with
> > values that appear non-sensical at first. For example, to blank the
> > console, but keep the monitor on, vt passes VESA_VSYNC_SUSPEND (eq '1').
> > This is because consoles interpret '1' as pixels-off-but-monitor-on.
> > The constant VESA_VSYNC_SUSPEND actually means monitor-vsync-off. And
> > the argument used for monitor-vsync-off is VESA_VSYNC_SUSPEND+1.
> >
> > To fix this by splitting blank into two parameters, blank and blank_mode.
> > The new blank is a boolean to control the display blanking. If cleared,
> > the console creates display output, if set the console does not. The
> > new parameter blank_mode specifies the VESA blank mode for the monitor
> > and equals VESA DPMS levels. If blank is set, blank_mode gives a hint
> > to the console, on how to set up the signal to the monitor.
> >
> > The actual interpretation is up to the console. Newportcon and sticon
> > only handle blank or not-blank. Fbcon and vgacon implement blank plus
> > the VESA DPMS handling. The implementation of both consoles is again
> > somewhat confusing because types and values do not match. Clean up
> > both with the new blank/blank_mode logic.
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > ---
> >   drivers/tty/vt/vt.c                 | 11 ++++----
> >   drivers/video/console/dummycon.c    |  4 +--
> >   drivers/video/console/newport_con.c |  4 +--
> >   drivers/video/console/sticon.c      |  4 +--
> >   drivers/video/console/vgacon.c      | 44 +++++++++++++++--------------
> >   drivers/video/fbdev/core/fbcon.c    | 19 +++++++------
> >   include/linux/console.h             |  9 ++++--
> >   7 files changed, 51 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> > index 9e7b4419b422..f762c965351d 100644
> > --- a/drivers/tty/vt/vt.c
> > +++ b/drivers/tty/vt/vt.c
> > @@ -4671,7 +4671,7 @@ void do_blank_screen(bool entering_gfx)
> >   	if (console_blanked) {
> >   		if (blank_state == blank_vesa_wait) {
> >   			blank_state = blank_off;
> > -			vc->vc_sw->con_blank(vc, vesa_blank_mode + 1, false);
> > +			vc->vc_sw->con_blank(vc, true, vesa_blank_mode, false);
> I hate adding random bool to function parameters as now you need to go
> look them up to try to determine exactly what happened.
>
> Why not just 2 different callbacks:

Sounds like a cleaner design.


> > -static bool vgacon_blank(struct vc_data *c, enum vesa_blank_mode blank,
> > +static bool vgacon_blank(struct vc_data *c, bool blank, enum vesa_blank_mode blank_mode,
> >   			 bool mode_switch)
> >   {
> > -	switch (blank) {
> > -	case VESA_NO_BLANKING:		/* Unblank */
> > +	if (blank) {
> > +		switch (blank_mode) {
> > +		case VESA_NO_BLANKING:	/* Normal blanking */
> > +			if (!mode_switch && vga_video_type == VIDEO_TYPE_VGAC) {
> > +				vga_pal_blank(&vgastate);
> > +				vga_palette_blanked = true;
> > +				return 0;
> > +			}
> > +			vgacon_set_origin(c);
> > +			scr_memsetw((void *)vga_vram_base, BLANK,
> > +				    c->vc_screenbuf_size);
> > +			if (mode_switch)
> > +				vga_is_gfx = true;
> > +			return 1;
> > +		default:		/* VESA blanking */
> > +			if (vga_video_type == VIDEO_TYPE_VGAC) {
> > +				vga_vesa_blank(&vgastate, blank_mode);
> > +				vga_vesa_blanked = blank_mode;
> > +			}
> > +			return 0;
> > +		}
> > +	} else { /* Unblank */
> >   		if (vga_vesa_blanked) {
> >   			vga_vesa_unblank(&vgastate);
> >   			vga_vesa_blanked = VESA_NO_BLANKING;
> > @@ -814,24 +834,6 @@ static bool vgacon_blank(struct vc_data *c, enum vesa_blank_mode blank,
> >   		vga_is_gfx = false;
> >   		/* Tell console.c that it has to restore the screen itself */
> >   		return 1;
> > -	case VESA_VSYNC_SUSPEND:	/* Normal blanking */
> > -		if (!mode_switch && vga_video_type == VIDEO_TYPE_VGAC) {
> > -			vga_pal_blank(&vgastate);
> > -			vga_palette_blanked = true;
> > -			return 0;
> > -		}
> > -		vgacon_set_origin(c);
> > -		scr_memsetw((void *) vga_vram_base, BLANK,
> > -			    c->vc_screenbuf_size);
> > -		if (mode_switch)
> > -			vga_is_gfx = true;
> > -		return 1;
> > -	default:		/* VESA blanking */
> > -		if (vga_video_type == VIDEO_TYPE_VGAC) {
> > -			vga_vesa_blank(&vgastate, blank - 1);
> > -			vga_vesa_blanked = blank;
> > -		}
> > -		return 0;
> >   	}
> This can be vgacon_blank() and vgacon_unblank(), right?  That way we
> have a hint as to what is happening and don't have to try to remember
> that a bool as parameter 2 is blank/unblank.

Yeah, we can do that.  We'd also not need the VESA mode in the unblank 
case. It's always VESA_NO_BLANKING there.

Best regards
Thomas

> thanks,
>
> greg k-h

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)