[PATCH 1/6] fbcon: Make cursor_blink=0 work when configured before fb devices appear

[Helge Deller]

On 9/23/24 17:57, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Currently setting cursor_blink attribute to 0 before any fb
> devices are around does absolutely nothing. When fb devices appear
> and fbcon becomes active the cursor starts blinking. Fix the problem
> by recoding the desired state of the attribute even if no fb devices
> are present at the time.
>
> Also adjust the show() method such that it shows the current
> state of the attribute even when no fb devices are in use.
>
> Note that store_cursor_blink() is still a bit dodgy:
> - seems to be missing some of the other checks that we do
>    elsewhere when deciding whether the cursor should be
>    blinking or not
> - when set to 0 when the cursor is currently invisible due
>    to blinking, the cursor will remains invisible. We should
>    either explicitly make it visible here, or wait until the
>    full blink cycle has finished.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>   drivers/video/fbdev/core/fbcon.c | 34 +++++++-------------------------
>   1 file changed, 7 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 2e093535884b..8936fa79b9e0 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -3217,26 +3217,7 @@ static ssize_t show_rotate(struct device *device,
>   static ssize_t show_cursor_blink(struct device *device,
>   				 struct device_attribute *attr, char *buf)
>   {
> -	struct fb_info *info;
> -	struct fbcon_ops *ops;
> -	int idx, blink = -1;
> -
> -	console_lock();
> -	idx = con2fb_map[fg_console];
> -
> -	if (idx == -1 || fbcon_registered_fb[idx] == NULL)
> -		goto err;
> -
> -	info = fbcon_registered_fb[idx];
> -	ops = info->fbcon_par;
> -
> -	if (!ops)
> -		goto err;
> -
> -	blink = delayed_work_pending(&ops->cursor_work);
> -err:
> -	console_unlock();
> -	return sysfs_emit(buf, "%d\n", blink);
> +	return sysfs_emit(buf, "%d\n", !fbcon_cursor_noblink);

I might be wrong and mix up things, but I think the previous code allowed
to show/set the blink value *per* registered framebuffer console,
while now you generally enable/disable blinking for all
framebuffer drivers at once?
Just thinking of a multiseat setup with different fb drivers
attached to different monitors with own mouse/keyboards...?!?


>   }
>
>   static ssize_t store_cursor_blink(struct device *device,
> @@ -3247,9 +3228,13 @@ static ssize_t store_cursor_blink(struct device *device,
>   	int blink, idx;
>   	char **last = NULL;
>
> +	blink = simple_strtoul(buf, last, 0);
> +
>   	console_lock();
>   	idx = con2fb_map[fg_console];
>
> +	fbcon_cursor_noblink = !blink;
> +
>   	if (idx == -1 || fbcon_registered_fb[idx] == NULL)
>   		goto err;
>
> @@ -3258,15 +3243,10 @@ static ssize_t store_cursor_blink(struct device *device,
>   	if (!info->fbcon_par)
>   		goto err;
>
> -	blink = simple_strtoul(buf, last, 0);
> -
> -	if (blink) {
> -		fbcon_cursor_noblink = 0;
> +	if (blink)
>   		fbcon_add_cursor_work(info);
> -	} else {
> -		fbcon_cursor_noblink = 1;
> +	else
>   		fbcon_del_cursor_work(info);
> -	}
>
>   err:
>   	console_unlock();