On Mon, Sep 23, 2024 at 11:04:55PM +0200, Helge Deller wrote:
> On 9/23/24 17:57, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxx>
> > ---
> > 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...?!?
There is just a single fbcon device in sysfs, so not really.
It's true that it only ever operated on the fg_console, so
on a first glane it may look like it might be some kind of
per-fb thing. But the state was only recorded in the
fbcon_cursor_noblink singleton, so when another vt becomes
active it'll look at that an not start the blinker.
So I think the per-fb aspect was just an illusion.
The whole interface is a bit of a mess. But I don't
really see why anyone would want to use this on a
per-fb device basis anyway. Either you want blinking
stuff and extra power draw, or you don't.
I think there are ways to turn of blinking via some escape
sequences or something as well, so those could probably
be used on a per-console basis. But I like to use this
sysfs instead because it can't accidentally be re-enabled
when random programs mess around with ttys.
--
Ville Syrjälä
Intel
