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

[Helge Deller <deller@gmx.de>]

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