[PATCH 0/5] fbdev: Remove use of VESA constants

[PATCH 0/5] fbdev: Remove use of VESA constants

[Thomas Zimmermann]

Fbdev and its drivers use FB_BLANK constants for controlling the display
blanking state in the fb_blank callback. Several drivers implement the
callback with VESA constants; thus mixing up semantics.

Update the drivers to use VESA constants instead.

Some background: the VESA constants describe DPMS levels for monitor
singal generation. VESA_NO_BLANKING means that the sync signals are on,
although the display adapter might not transfer meaningful pixel data.
The constants FB_BLANK_NORMAL represents this case in fbdev, while
FB_BLANK_UNBLANK represents enabled sync signal generation plus pixel
data. Fbdev drivers using VESA constants therefore often contain
off-by-one errors.

Thomas Zimmermann (5):
  fbdev/au1100fb: Use FB blanking constants
  fbdev/i810fb: Use FB blanking constants
  fbdev/sa1100fb: Remove comment on VESA DPMS modes
  fbdev/platinumfb: Use FB blanking constants
  fbdev/xilinxfb: Use FB blanking constants

 drivers/video/fbdev/au1100fb.c       | 18 ++++++++---------
 drivers/video/fbdev/i810/i810_main.c |  2 +-
 drivers/video/fbdev/platinumfb.c     |  8 +++++---
 drivers/video/fbdev/sa1100fb.c       | 29 ----------------------------
 drivers/video/fbdev/xilinxfb.c       |  2 +-
 5 files changed, 16 insertions(+), 43 deletions(-)

-- 
2.55.0

[PATCH 1/5] fbdev/au1100fb: Use FB blanking constants

[Thomas Zimmermann]

Fbdev and its drivers use FB_BLANK constants for controlling the
display's blanking state. Semantics differ to similar VESA blanking
constants.

Replace the VESA constants in au1100fb with FB_BLANK.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/au1100fb.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
index 39f86ffb1ae3..6c375f389fbd 100644
--- a/drivers/video/fbdev/au1100fb.c
+++ b/drivers/video/fbdev/au1100fb.c
@@ -424,15 +424,15 @@ static int au1100fb_fb_blank(int blank_mode, struct fb_info *fbi)
 
 	switch (blank_mode) {
 
-	case VESA_NO_BLANKING:
+	case FB_BLANK_UNBLANK:
 		/* Turn on panel */
 		fbdev->regs->lcd_control |= LCD_CONTROL_GO;
 		wmb(); /* drain writebuffer */
 		break;
-
-	case VESA_VSYNC_SUSPEND:
-	case VESA_HSYNC_SUSPEND:
-	case VESA_POWERDOWN:
+	case FB_BLANK_NORMAL:
+	case FB_BLANK_VSYNC_SUSPEND:
+	case FB_BLANK_HSYNC_SUSPEND:
+	case FB_BLANK_POWERDOWN:
 		/* Turn off panel */
 		fbdev->regs->lcd_control &= ~LCD_CONTROL_GO;
 		wmb(); /* drain writebuffer */
@@ -539,7 +539,7 @@ static int au1100fb_setmode(struct au1100fb_device *fbdev)
 	/* Resume controller */
 	fbdev->regs->lcd_control |= LCD_CONTROL_GO;
 	mdelay(10);
-	au1100fb_fb_blank(VESA_NO_BLANKING, info);
+	au1100fb_fb_blank(FB_BLANK_UNBLANK, info);
 
 	return 0;
 }
@@ -856,7 +856,7 @@ static void au1100fb_drv_remove(struct platform_device *dev)
 	fbdev = platform_get_drvdata(dev);
 
 #if !defined(CONFIG_FRAMEBUFFER_CONSOLE) && defined(CONFIG_LOGO)
-	au1100fb_fb_blank(VESA_POWERDOWN, &fbdev->info);
+	au1100fb_fb_blank(FB_BLANK_POWERDOWN, &fbdev->info);
 #endif
 	fbdev->regs->lcd_control &= ~LCD_CONTROL_GO;
 
@@ -880,7 +880,7 @@ static int au1100fb_drv_suspend(struct platform_device *dev, pm_message_t state)
 		return 0;
 
 	/* Blank the LCD */
-	au1100fb_fb_blank(VESA_POWERDOWN, &fbdev->info);
+	au1100fb_fb_blank(FB_BLANK_POWERDOWN, &fbdev->info);
 
 	clk_disable(fbdev->lcdclk);
 
@@ -904,7 +904,7 @@ static int au1100fb_drv_resume(struct platform_device *dev)
 		return ret;
 
 	/* Unblank the LCD */
-	au1100fb_fb_blank(VESA_NO_BLANKING, &fbdev->info);
+	au1100fb_fb_blank(FB_BLANK_UNBLANK, &fbdev->info);
 
 	return 0;
 }
-- 
2.55.0

[PATCH 2/5] fbdev/i810fb: Use FB blanking constants

[Thomas Zimmermann]

Fbdev and its drivers use FB_BLANK constants for controlling the
display's blanking state. Semantics differ to similar VESA blanking
constants.

Replace the VESA constant in i810fb with FB_BLANK.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/i810/i810_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/i810/i810_main.c b/drivers/video/fbdev/i810/i810_main.c
index 10b914a24114..f5780fd8afea 100644
--- a/drivers/video/fbdev/i810/i810_main.c
+++ b/drivers/video/fbdev/i810/i810_main.c
@@ -1623,7 +1623,7 @@ static int i810fb_resume(struct pci_dev *dev)
 			par->cursor_heap.offset);
 	i810fb_set_par(info);
 	fb_set_suspend (info, 0);
-	info->fbops->fb_blank(VESA_NO_BLANKING, info);
+	info->fbops->fb_blank(FB_BLANK_UNBLANK, info);
 fail:
 	console_unlock();
 	return 0;
-- 
2.55.0

[PATCH 3/5] fbdev/sa1100fb: Remove comment on VESA DPMS modes

[Thomas Zimmermann]

Fbdev and its drivers use FB_BLANK constants for controlling the
display's blanking state. Semantics differ to similar VESA blanking
constants.

Remove the comment on VESA constants from sa1100fb. It's misleading at
best, as the driver should never use the VESA constants for blanking.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/sa1100fb.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/drivers/video/fbdev/sa1100fb.c b/drivers/video/fbdev/sa1100fb.c
index fea0172c45b5..8102ab7d51e6 100644
--- a/drivers/video/fbdev/sa1100fb.c
+++ b/drivers/video/fbdev/sa1100fb.c
@@ -491,35 +491,6 @@ sa1100fb_set_cmap(struct fb_cmap *cmap, int kspc, int con,
 }
 #endif
 
-/*
- * Formal definition of the VESA spec:
- *  On
- *  	This refers to the state of the display when it is in full operation
- *  Stand-By
- *  	This defines an optional operating state of minimal power reduction with
- *  	the shortest recovery time
- *  Suspend
- *  	This refers to a level of power management in which substantial power
- *  	reduction is achieved by the display.  The display can have a longer
- *  	recovery time from this state than from the Stand-by state
- *  Off
- *  	This indicates that the display is consuming the lowest level of power
- *  	and is non-operational. Recovery from this state may optionally require
- *  	the user to manually power on the monitor
- *
- *  Now, the fbdev driver adds an additional state, (blank), where they
- *  turn off the video (maybe by colormap tricks), but don't mess with the
- *  video itself: think of it semantically between on and Stand-By.
- *
- *  So here's what we should do in our fbdev blank routine:
- *
- *  	VESA_NO_BLANKING (mode 0)	Video on,  front/back light on
- *  	VESA_VSYNC_SUSPEND (mode 1)  	Video on,  front/back light off
- *  	VESA_HSYNC_SUSPEND (mode 2)  	Video on,  front/back light off
- *  	VESA_POWERDOWN (mode 3)		Video off, front/back light off
- *
- *  This will match the matrox implementation.
- */
 /*
  * sa1100fb_blank():
  *	Blank the display by setting all palette values to zero.  Note, the
-- 
2.55.0

[PATCH 4/5] fbdev/platinumfb: Use FB blanking constants

[Thomas Zimmermann]

Fbdev and its drivers use FB_BLANK constants for controlling the
display's blanking state. Semantics differ to similar VESA blanking
constants.

Replace the VESA constants with FB_BLANK in platinumfb. The VESA
constants can be interpreted bitwise, while the FB_BLANK constants
can not. Therefore turn the bit tests in an if-else sequence that
handles the individual cases. By default, the display is unblanked.

The modified code is within a comment block, so it will not run. I
still changed it to keep the driver up to standard. This might also
resolve the nearby TODO comment on incorrect hardware blanking.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/platinumfb.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/platinumfb.c b/drivers/video/fbdev/platinumfb.c
index 81fdaa3541db..b55a7b70d31d 100644
--- a/drivers/video/fbdev/platinumfb.c
+++ b/drivers/video/fbdev/platinumfb.c
@@ -167,10 +167,12 @@ static int platinumfb_blank(int blank,  struct fb_info *fb)
 	ctrl = le32_to_cpup(&info->platinum_regs->ctrl.r) | 0x33;
 	if (blank)
 		--blank_mode;
-	if (blank & VESA_VSYNC_SUSPEND)
-		ctrl &= ~3;
-	if (blank & VESA_HSYNC_SUSPEND)
+	if (blank == FB_BLANK_VSYNC_SUSPEND)
+		ctrl &= ~0x03;
+	else if (blank == FB_BLANK_HSYNC_SUSPEND)
 		ctrl &= ~0x30;
+	else if (blank == FB_BLANK_POWERDOWN)
+		ctrl &= ~0x33;
 	out_le32(&info->platinum_regs->ctrl.r, ctrl);
 */
 /* TODO: Figure out how the heck to powerdown this thing! */
-- 
2.55.0

[PATCH 5/5] fbdev/xilinxfb: Use FB blanking constants

[Thomas Zimmermann]

Fbdev and its drivers use FB_BLANK constants for controlling the
display's blanking state. Semantics differ to similar VESA blanking
constants.

Replace the VESA constant in xilinxfb with FB_BLANK.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/xilinxfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/xilinxfb.c b/drivers/video/fbdev/xilinxfb.c
index 0a6e05cd155a..462fe2e94d55 100644
--- a/drivers/video/fbdev/xilinxfb.c
+++ b/drivers/video/fbdev/xilinxfb.c
@@ -376,7 +376,7 @@ static void xilinxfb_release(struct device *dev)
 	struct xilinxfb_drvdata *drvdata = dev_get_drvdata(dev);
 
 #if !defined(CONFIG_FRAMEBUFFER_CONSOLE) && defined(CONFIG_LOGO)
-	xilinx_fb_blank(VESA_POWERDOWN, &drvdata->info);
+	xilinx_fb_blank(FB_BLANK_POWERDOWN, &drvdata->info);
 #endif
 
 	unregister_framebuffer(&drvdata->info);
-- 
2.55.0

Re: [PATCH 1/5] fbdev/au1100fb: Use FB blanking constants

[Helge Deller]

Hi Thomas,

On 10/9/26 18:05, Thomas Zimmermann wrote:
> Fbdev and its drivers use FB_BLANK constants for controlling the
> display's blanking state. Semantics differ to similar VESA blanking
> constants.
> 
> Replace the VESA constants in au1100fb with FB_BLANK.

I understand that this series drops the VESA constants.
That seems logical and correct, but....

  
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/video/fbdev/au1100fb.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
> index 39f86ffb1ae3..6c375f389fbd 100644
> --- a/drivers/video/fbdev/au1100fb.c
> +++ b/drivers/video/fbdev/au1100fb.c
> @@ -424,15 +424,15 @@ static int au1100fb_fb_blank(int blank_mode, struct fb_info *fbi)
>   
>   	switch (blank_mode) {
>   
> -	case VESA_NO_BLANKING:
> +	case FB_BLANK_UNBLANK:
>   		/* Turn on panel */
>   		fbdev->regs->lcd_control |= LCD_CONTROL_GO;
>   		wmb(); /* drain writebuffer */
>   		break;
> -
> -	case VESA_VSYNC_SUSPEND:
> -	case VESA_HSYNC_SUSPEND:
> -	case VESA_POWERDOWN:
> +	case FB_BLANK_NORMAL:
> +	case FB_BLANK_VSYNC_SUSPEND:
> +	case FB_BLANK_HSYNC_SUSPEND:
> +	case FB_BLANK_POWERDOWN:
I don't understand (this is not the fault of this/your patch),
how this enum in fb.h works:

enum {
         /* screen: unblanked, hsync: on,  vsync: on */
         FB_BLANK_UNBLANK       = VESA_NO_BLANKING,

         /* screen: blanked,   hsync: on,  vsync: on */
         FB_BLANK_NORMAL        = VESA_NO_BLANKING + 1,

         /* screen: blanked,   hsync: on,  vsync: off */
         FB_BLANK_VSYNC_SUSPEND = VESA_VSYNC_SUSPEND + 1,

         /* screen: blanked,   hsync: off, vsync: on */
         FB_BLANK_HSYNC_SUSPEND = VESA_HSYNC_SUSPEND + 1,

         /* screen: blanked,   hsync: off, vsync: off */
         FB_BLANK_POWERDOWN     = VESA_POWERDOWN + 1


In vesa.h we have:
/* VESA Blanking Levels */
enum vesa_blank_mode {
         VESA_NO_BLANKING        = 0,
#define VESA_NO_BLANKING        VESA_NO_BLANKING
         VESA_VSYNC_SUSPEND      = 1,
#define VESA_VSYNC_SUSPEND      VESA_VSYNC_SUSPEND
         VESA_HSYNC_SUSPEND      = 2,
#define VESA_HSYNC_SUSPEND      VESA_HSYNC_SUSPEND
         VESA_POWERDOWN          = VESA_VSYNC_SUSPEND | VESA_HSYNC_SUSPEND,
#define VESA_POWERDOWN          VESA_POWERDOWN
         VESA_BLANK_MAX          = VESA_POWERDOWN,


for example this one:
         /* screen: blanked,   hsync: on,  vsync: off */
         FB_BLANK_VSYNC_SUSPEND = VESA_VSYNC_SUSPEND + 1,
which is effectively:
         FB_BLANK_VSYNC_SUSPEND = VESA_HSYNC_SUSPEND,
which seems wrong.

Am I missing something?

Helge

Re: [PATCH 1/5] fbdev/au1100fb: Use FB blanking constants

[Thomas Zimmermann]

Hi

Am 18.10.26 um 15:52 schrieb Helge Deller:
> Hi Thomas,
>
> On 10/9/26 18:05, Thomas Zimmermann wrote:
>> Fbdev and its drivers use FB_BLANK constants for controlling the
>> display's blanking state. Semantics differ to similar VESA blanking
>> constants.
>>
>> Replace the VESA constants in au1100fb with FB_BLANK.
>
> I understand that this series drops the VESA constants.
> That seems logical and correct, but....
>
>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/video/fbdev/au1100fb.c | 18 +++++++++---------
>>   1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/au1100fb.c 
>> b/drivers/video/fbdev/au1100fb.c
>> index 39f86ffb1ae3..6c375f389fbd 100644
>> --- a/drivers/video/fbdev/au1100fb.c
>> +++ b/drivers/video/fbdev/au1100fb.c
>> @@ -424,15 +424,15 @@ static int au1100fb_fb_blank(int blank_mode, 
>> struct fb_info *fbi)
>>         switch (blank_mode) {
>>   -    case VESA_NO_BLANKING:
>> +    case FB_BLANK_UNBLANK:
>>           /* Turn on panel */
>>           fbdev->regs->lcd_control |= LCD_CONTROL_GO;
>>           wmb(); /* drain writebuffer */
>>           break;
>> -
>> -    case VESA_VSYNC_SUSPEND:
>> -    case VESA_HSYNC_SUSPEND:
>> -    case VESA_POWERDOWN:
>> +    case FB_BLANK_NORMAL:
>> +    case FB_BLANK_VSYNC_SUSPEND:
>> +    case FB_BLANK_HSYNC_SUSPEND:
>> +    case FB_BLANK_POWERDOWN:
> I don't understand (this is not the fault of this/your patch),
> how this enum in fb.h works:
>
> enum {
>         /* screen: unblanked, hsync: on,  vsync: on */
>         FB_BLANK_UNBLANK       = VESA_NO_BLANKING,
>
>         /* screen: blanked,   hsync: on,  vsync: on */
>         FB_BLANK_NORMAL        = VESA_NO_BLANKING + 1,
>
>         /* screen: blanked,   hsync: on,  vsync: off */
>         FB_BLANK_VSYNC_SUSPEND = VESA_VSYNC_SUSPEND + 1,
>
>         /* screen: blanked,   hsync: off, vsync: on */
>         FB_BLANK_HSYNC_SUSPEND = VESA_HSYNC_SUSPEND + 1,
>
>         /* screen: blanked,   hsync: off, vsync: off */
>         FB_BLANK_POWERDOWN     = VESA_POWERDOWN + 1
>
>
> In vesa.h we have:
> /* VESA Blanking Levels */
> enum vesa_blank_mode {
>         VESA_NO_BLANKING        = 0,
> #define VESA_NO_BLANKING        VESA_NO_BLANKING
>         VESA_VSYNC_SUSPEND      = 1,
> #define VESA_VSYNC_SUSPEND      VESA_VSYNC_SUSPEND
>         VESA_HSYNC_SUSPEND      = 2,
> #define VESA_HSYNC_SUSPEND      VESA_HSYNC_SUSPEND
>         VESA_POWERDOWN          = VESA_VSYNC_SUSPEND | 
> VESA_HSYNC_SUSPEND,
> #define VESA_POWERDOWN          VESA_POWERDOWN
>         VESA_BLANK_MAX          = VESA_POWERDOWN,
>
>
> for example this one:
>         /* screen: blanked,   hsync: on,  vsync: off */
>         FB_BLANK_VSYNC_SUSPEND = VESA_VSYNC_SUSPEND + 1,
> which is effectively:
>         FB_BLANK_VSYNC_SUSPEND = VESA_HSYNC_SUSPEND,
> which seems wrong.
>
> Am I missing something?

No worries, it's super-confusing.

- FB_BLANK_ describes CRTC state plus monitor state. VESA_ only 
describes monitor state and assumes that the CRTC is off.

- FB_BLANK_UNBLANK means "monitor on and send pixels".  There is no VESA 
equivalent for this mode, because all VESA constants assume that the 
stream if pixel data has been disabled.

- FB_BLANK_NORMAL means "monitor on, but do not send pixels". This 
corresponds to VESA_BLANK_NO_BLANKING, but has a different integer value.

- The FB_BLANK_*_SUSPEND and _POWERDOWN constants correspond to the VESA 
constants of similar name. But they are all off by one.

Therefore some of the drivers (au1100fb) use VESA_NO_BLANKING when they 
mean FB_BLANK_UNBLANK. Others (xilinxfb) get it wrong and use 
VESA_POWERDOWN when they should use FB_BLANK_POWERDOWN.

Best regards
Thomas


>
> Helge
>

-- 
--
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)