linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nicolas Pitre <nico@fluxnic.net>
To: Lukas Wunner <lukas@wunner.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>,
	Mikulas Patocka <mpatocka@redhat.com>,
	Matthew Whitehead <tedheadster@gmail.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Arvind Sankar <nivedita@alum.mit.edu>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] vt: Fix non-blinking cursor regression
Date: Wed, 22 Jan 2020 11:40:38 -0500 (EST)	[thread overview]
Message-ID: <nycvar.YSQ.7.76.2001221032210.1655@knanqh.ubzr> (raw)
In-Reply-To: <575cb82102aa59a7a8e34248821b78e1dd844777.1579701673.git.lukas@wunner.de>

On Wed, 22 Jan 2020, Lukas Wunner wrote:

> Since commit a6dbe4427559 ("vt: perform safe console erase in the right
> order"), when userspace clears both the scrollback buffer and the screen
> by writing "\e[3J" to an fbdev virtual console, the cursor stops blinking
> if that virtual console is not in the foreground.  I'm witnessing this
> on every boot of Raspbian since updating to v4.19.37+ because agetty
> writes the sequence to /dev/tty6 while the console is still switched to
> /dev/tty1.  Switching consoles once makes the cursor blink again.
> 
> The commit added an invocation of ->con_switch() to flush_scrollback().
> Normally this is only invoked from switch_screen() to switch consoles.
> switch_screen() updates *vc->vc_display_fg to the new console and
> fbcon_switch() updates ops->currcon.  Because the commit only invokes
> fbcon_switch() but doesn't update *vc->vc_display_fg, it performs an
> incomplete console switch.
> 
> When fb_flashcursor() subsequently blinks the cursor, it retrieves the
> foreground console from ops->currcon.  Because *vc->vc_display_fg wasn't
> updated, con_is_visible() incorrectly returns false and as a result,
> fb_flashcursor() bails out without blinking the cursor.
> 
> The invocation of ->con_switch() appears to have been erroneous.  After
> all, why should a console switch be performed when clearing the screen?
> The commit message doesn't provide a rationale either.  So delete it.

The problem here is that only vgacon provides a con_flush_scrollback 
method. When not provided, the only way to flush the scrollback buffer 
is to invoke the switch method. If you remove it the scrollback buffer 
of the foreground console won't be flushed in the fb case and possibly 
others.

Originally the con_switch method was invoked via update_screen(). The 
code rationalization in commit a6dbe4427559 failed to carry over a few 
details though.

So I think the actual fix should instead be like this:

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 34aa39d1ae..93314a2f26 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -936,10 +936,13 @@ static void flush_scrollback(struct vc_data *vc)
 	WARN_CONSOLE_UNLOCKED();
 
 	set_origin(vc);
-	if (vc->vc_sw->con_flush_scrollback)
+	if (vc->vc_sw->con_flush_scrollback) {
 		vc->vc_sw->con_flush_scrollback(vc);
-	else
+	} else if (con_is_visible(vc)) {
+		hide_cursor(vc);
 		vc->vc_sw->con_switch(vc);
+		set_cursor(vc);
+	}
 }
 
 /*


Nicolas

  reply	other threads:[~2020-01-22 16:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-22 14:17 [PATCH] vt: Fix non-blinking cursor regression Lukas Wunner
2020-01-22 16:40 ` Nicolas Pitre [this message]
2020-01-26 12:48   ` Lukas Wunner
2020-01-26 17:32     ` Nicolas Pitre
2020-01-28 11:11       ` Lukas Wunner
2020-01-28 16:23         ` Nicolas Pitre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=nycvar.YSQ.7.76.2001221032210.1655@knanqh.ubzr \
    --to=nico@fluxnic.net \
    --cc=daniel.vetter@ffwll.ch \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mpatocka@redhat.com \
    --cc=nivedita@alum.mit.edu \
    --cc=tedheadster@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).