drm/fb-helper: Fix vt restore

Commit Message

Daniel Vetter June 23, 2020, 3:54 p.m. UTC

In the past we had a pile of hacks to orchestrate access between fbdev
emulation and native kms clients. We've tried to streamline this, by
always preferring the kms side above fbdev calls when a drm master
exists, because drm master controls access to the display resources.

Unfortunately this breaks existing userspace, specifically Xorg. When
exiting Xorg first restores the console to text mode using the KDSET
ioctl on the vt. This does nothing, because a drm master is still
around. Then it drops the drm master status, which again does nothing,
because logind is keeping additional drm fd open to be able to
orchestrate vt switches. In the past this is the point where fbdev was
restored, as part of the ->lastclose hook on the drm side.

Now to fix this regression we don't want to go back to letting fbdev
restore things whenever it feels like, or to the pile of hacks we've
had before. Instead try and go with a minimal exception to make the
KDSET case work again, and nothing else.

This means that if userspace does a KDSET call when switching between
graphical compositors, there will be some flickering with fbcon
showing up for a bit. But a) that's not a regression and b) userspace
can fix it by improving the vt switching dance - logind should have
all the information it needs.

While pondering all this I'm also wondering wheter we should have a
SWITCH_MASTER ioctl to allow race-free master status handover. But
that's for another day.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=208179
Cc: [e-mail]
Reported-and-Tested-by: [e-mail]
Cc: Michel Dänzer <[e-mail]>
Fixes: 64914da24ea9 ("drm/fbdev-helper: don't force restores")
Cc: Noralf Trønnes <[e-mail]>
Cc: Thomas Zimmermann <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
Cc: Maarten Lankhorst <[e-mail]>
Cc: Maxime Ripard <[e-mail]>
Cc: David Airlie <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
Cc: [e-mail]
Cc: <[e-mail]> # v5.7+
Signed-off-by: Daniel Vetter <[e-mail]>
---
 drivers/gpu/drm/drm_fb_helper.c  | 63 +++++++++++++++++++++++++-------
 drivers/video/fbdev/core/fbcon.c |  3 +-
 include/uapi/linux/fb.h          |  1 +
 3 files changed, 52 insertions(+), 15 deletions(-)

Comments

Thomas Zimmermann July 10, 2020, 11:31 a.m. UTC | #1

Hi Daniel,

this patch might not be enougth. I started Xorg and then did 'kill -9'
on the Xorg process. Xorg went away, but the console did not come back.

Best regards
Thomas

Am 23.06.20 um 17:54 schrieb Daniel Vetter:
> In the past we had a pile of hacks to orchestrate access between fbdev
> emulation and native kms clients. We've tried to streamline this, by
> always preferring the kms side above fbdev calls when a drm master
> exists, because drm master controls access to the display resources.
> 
> Unfortunately this breaks existing userspace, specifically Xorg. When
> exiting Xorg first restores the console to text mode using the KDSET
> ioctl on the vt. This does nothing, because a drm master is still
> around. Then it drops the drm master status, which again does nothing,
> because logind is keeping additional drm fd open to be able to
> orchestrate vt switches. In the past this is the point where fbdev was
> restored, as part of the ->lastclose hook on the drm side.
> 
> Now to fix this regression we don't want to go back to letting fbdev
> restore things whenever it feels like, or to the pile of hacks we've
> had before. Instead try and go with a minimal exception to make the
> KDSET case work again, and nothing else.
> 
> This means that if userspace does a KDSET call when switching between
> graphical compositors, there will be some flickering with fbcon
> showing up for a bit. But a) that's not a regression and b) userspace
> can fix it by improving the vt switching dance - logind should have
> all the information it needs.
> 
> While pondering all this I'm also wondering wheter we should have a
> SWITCH_MASTER ioctl to allow race-free master status handover. But
> that's for another day.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=208179
> Cc: [e-mail]
> Reported-and-Tested-by: [e-mail]
> Cc: Michel Dänzer <[e-mail]>
> Fixes: 64914da24ea9 ("drm/fbdev-helper: don't force restores")
> Cc: Noralf Trønnes <[e-mail]>
> Cc: Thomas Zimmermann <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: [e-mail]
> Cc: <[e-mail]> # v5.7+
> Signed-off-by: Daniel Vetter <[e-mail]>
> ---
>  drivers/gpu/drm/drm_fb_helper.c  | 63 +++++++++++++++++++++++++-------
>  drivers/video/fbdev/core/fbcon.c |  3 +-
>  include/uapi/linux/fb.h          |  1 +
>  3 files changed, 52 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 170aa7689110..ae69bf8e9bcc 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -227,18 +227,9 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
>  }
>  EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>  
> -/**
> - * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
> - * @fb_helper: driver-allocated fbdev helper, can be NULL
> - *
> - * This should be called from driver's drm &drm_driver.lastclose callback
> - * when implementing an fbcon on top of kms using this helper. This ensures that
> - * the user isn't greeted with a black screen when e.g. X dies.
> - *
> - * RETURNS:
> - * Zero if everything went ok, negative error code otherwise.
> - */
> -int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> +static int
> +__drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
> +					    bool force)
>  {
>  	bool do_delayed;
>  	int ret;
> @@ -250,7 +241,16 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>  		return 0;
>  
>  	mutex_lock(&fb_helper->lock);
> -	ret = drm_client_modeset_commit(&fb_helper->client);
> +	if (force) {
> +		/*
> +		 * Yes this is the _locked version which expects the master lock
> +		 * to be held. But for forced restores we're intentionally
> +		 * racing here, see drm_fb_helper_set_par().
> +		 */
> +		ret = drm_client_modeset_commit_locked(&fb_helper->client);
> +	} else {
> +		ret = drm_client_modeset_commit(&fb_helper->client);
> +	}
>  
>  	do_delayed = fb_helper->delayed_hotplug;
>  	if (do_delayed)
> @@ -262,6 +262,22 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>  
>  	return ret;
>  }
> +
> +/**
> + * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
> + * @fb_helper: driver-allocated fbdev helper, can be NULL
> + *
> + * This should be called from driver's drm &drm_driver.lastclose callback
> + * when implementing an fbcon on top of kms using this helper. This ensures that
> + * the user isn't greeted with a black screen when e.g. X dies.
> + *
> + * RETURNS:
> + * Zero if everything went ok, negative error code otherwise.
> + */
> +int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> +{
> +	return __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, false);
> +}
>  EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
>  
>  #ifdef CONFIG_MAGIC_SYSRQ
> @@ -1318,6 +1334,7 @@ int drm_fb_helper_set_par(struct fb_info *info)
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
>  	struct fb_var_screeninfo *var = &info->var;
> +	bool force;
>  
>  	if (oops_in_progress)
>  		return -EBUSY;
> @@ -1327,7 +1344,25 @@ int drm_fb_helper_set_par(struct fb_info *info)
>  		return -EINVAL;
>  	}
>  
> -	drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
> +	/*
> +	 * Normally we want to make sure that a kms master takes
> +	 * precedence over fbdev, to avoid fbdev flickering and
> +	 * occasionally stealing the display status. But Xorg first sets
> +	 * the vt back to text mode using the KDSET IOCTL with KD_TEXT,
> +	 * and only after that drops the master status when exiting.
> +	 *
> +	 * In the past this was caught by drm_fb_helper_lastclose(), but
> +	 * on modern systems where logind always keeps a drm fd open to
> +	 * orchestrate the vt switching, this doesn't work.
> +	 *
> +	 * To no break the userspace ABI we have this special case here,
> +	 * which is only used for the above case. Everything else uses
> +	 * the normal commit function, which ensures that we never steal
> +	 * the display from an active drm master.
> +	 */
> +	force = var->activate & FB_ACTIVATE_KD_TEXT;
> +
> +	__drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, force);
>  
>  	return 0;
>  }
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 9d28a8e3328f..e2a490c5ae08 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -2402,7 +2402,8 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
>  		ops->graphics = 1;
>  
>  		if (!blank) {
> -			var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE;
> +			var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE |
> +				FB_ACTIVATE_KD_TEXT;
>  			fb_set_var(info, &var);
>  			ops->graphics = 0;
>  			ops->var = info->var;
> diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
> index b6aac7ee1f67..4c14e8be7267 100644
> --- a/include/uapi/linux/fb.h
> +++ b/include/uapi/linux/fb.h
> @@ -205,6 +205,7 @@ struct fb_bitfield {
>  #define FB_ACTIVATE_ALL	       64	/* change all VCs on this fb	*/
>  #define FB_ACTIVATE_FORCE     128	/* force apply even when no change*/
>  #define FB_ACTIVATE_INV_MODE  256       /* invalidate videomode */
> +#define FB_ACTIVATE_KD_TEXT   512       /* for KDSET vt ioctl */
>  
>  #define FB_ACCELF_TEXT		1	/* (OBSOLETE) see fb_info.flags and vc_mode */
>  
>

Daniel Vetter July 10, 2020, 12:38 p.m. UTC | #2

On Fri, Jul 10, 2020 at 1:31 PM Thomas Zimmermann <[e-mail]> wrote:
>
> Hi Daniel,
>
> this patch might not be enougth. I started Xorg and then did 'kill -9'
> on the Xorg process. Xorg went away, but the console did not come back.

Hm don't you need to reset your terminal to text mode in that case
first? Iirc there's a sysrq. All again assuming not terribly modern
system where that stuff is all done by logind.
-Daniel

>
> Best regards
> Thomas
>
> Am 23.06.20 um 17:54 schrieb Daniel Vetter:
> > In the past we had a pile of hacks to orchestrate access between fbdev
> > emulation and native kms clients. We've tried to streamline this, by
> > always preferring the kms side above fbdev calls when a drm master
> > exists, because drm master controls access to the display resources.
> >
> > Unfortunately this breaks existing userspace, specifically Xorg. When
> > exiting Xorg first restores the console to text mode using the KDSET
> > ioctl on the vt. This does nothing, because a drm master is still
> > around. Then it drops the drm master status, which again does nothing,
> > because logind is keeping additional drm fd open to be able to
> > orchestrate vt switches. In the past this is the point where fbdev was
> > restored, as part of the ->lastclose hook on the drm side.
> >
> > Now to fix this regression we don't want to go back to letting fbdev
> > restore things whenever it feels like, or to the pile of hacks we've
> > had before. Instead try and go with a minimal exception to make the
> > KDSET case work again, and nothing else.
> >
> > This means that if userspace does a KDSET call when switching between
> > graphical compositors, there will be some flickering with fbcon
> > showing up for a bit. But a) that's not a regression and b) userspace
> > can fix it by improving the vt switching dance - logind should have
> > all the information it needs.
> >
> > While pondering all this I'm also wondering wheter we should have a
> > SWITCH_MASTER ioctl to allow race-free master status handover. But
> > that's for another day.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=208179
> > Cc: [e-mail]
> > Reported-and-Tested-by: [e-mail]
> > Cc: Michel Dänzer <[e-mail]>
> > Fixes: 64914da24ea9 ("drm/fbdev-helper: don't force restores")
> > Cc: Noralf Trønnes <[e-mail]>
> > Cc: Thomas Zimmermann <[e-mail]>
> > Cc: Daniel Vetter <[e-mail]>
> > Cc: Maarten Lankhorst <[e-mail]>
> > Cc: Maxime Ripard <[e-mail]>
> > Cc: David Airlie <[e-mail]>
> > Cc: Daniel Vetter <[e-mail]>
> > Cc: [e-mail]
> > Cc: <[e-mail]> # v5.7+
> > Signed-off-by: Daniel Vetter <[e-mail]>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c  | 63 +++++++++++++++++++++++++-------
> >  drivers/video/fbdev/core/fbcon.c |  3 +-
> >  include/uapi/linux/fb.h          |  1 +
> >  3 files changed, 52 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index 170aa7689110..ae69bf8e9bcc 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -227,18 +227,9 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
> >  }
> >  EXPORT_SYMBOL(drm_fb_helper_debug_leave);
> >
> > -/**
> > - * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
> > - * @fb_helper: driver-allocated fbdev helper, can be NULL
> > - *
> > - * This should be called from driver's drm &drm_driver.lastclose callback
> > - * when implementing an fbcon on top of kms using this helper. This ensures that
> > - * the user isn't greeted with a black screen when e.g. X dies.
> > - *
> > - * RETURNS:
> > - * Zero if everything went ok, negative error code otherwise.
> > - */
> > -int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> > +static int
> > +__drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
> > +                                         bool force)
> >  {
> >       bool do_delayed;
> >       int ret;
> > @@ -250,7 +241,16 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> >               return 0;
> >
> >       mutex_lock(&fb_helper->lock);
> > -     ret = drm_client_modeset_commit(&fb_helper->client);
> > +     if (force) {
> > +             /*
> > +              * Yes this is the _locked version which expects the master lock
> > +              * to be held. But for forced restores we're intentionally
> > +              * racing here, see drm_fb_helper_set_par().
> > +              */
> > +             ret = drm_client_modeset_commit_locked(&fb_helper->client);
> > +     } else {
> > +             ret = drm_client_modeset_commit(&fb_helper->client);
> > +     }
> >
> >       do_delayed = fb_helper->delayed_hotplug;
> >       if (do_delayed)
> > @@ -262,6 +262,22 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> >
> >       return ret;
> >  }
> > +
> > +/**
> > + * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
> > + * @fb_helper: driver-allocated fbdev helper, can be NULL
> > + *
> > + * This should be called from driver's drm &drm_driver.lastclose callback
> > + * when implementing an fbcon on top of kms using this helper. This ensures that
> > + * the user isn't greeted with a black screen when e.g. X dies.
> > + *
> > + * RETURNS:
> > + * Zero if everything went ok, negative error code otherwise.
> > + */
> > +int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> > +{
> > +     return __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, false);
> > +}
> >  EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
> >
> >  #ifdef CONFIG_MAGIC_SYSRQ
> > @@ -1318,6 +1334,7 @@ int drm_fb_helper_set_par(struct fb_info *info)
> >  {
> >       struct drm_fb_helper *fb_helper = info->par;
> >       struct fb_var_screeninfo *var = &info->var;
> > +     bool force;
> >
> >       if (oops_in_progress)
> >               return -EBUSY;
> > @@ -1327,7 +1344,25 @@ int drm_fb_helper_set_par(struct fb_info *info)
> >               return -EINVAL;
> >       }
> >
> > -     drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
> > +     /*
> > +      * Normally we want to make sure that a kms master takes
> > +      * precedence over fbdev, to avoid fbdev flickering and
> > +      * occasionally stealing the display status. But Xorg first sets
> > +      * the vt back to text mode using the KDSET IOCTL with KD_TEXT,
> > +      * and only after that drops the master status when exiting.
> > +      *
> > +      * In the past this was caught by drm_fb_helper_lastclose(), but
> > +      * on modern systems where logind always keeps a drm fd open to
> > +      * orchestrate the vt switching, this doesn't work.
> > +      *
> > +      * To no break the userspace ABI we have this special case here,
> > +      * which is only used for the above case. Everything else uses
> > +      * the normal commit function, which ensures that we never steal
> > +      * the display from an active drm master.
> > +      */
> > +     force = var->activate & FB_ACTIVATE_KD_TEXT;
> > +
> > +     __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, force);
> >
> >       return 0;
> >  }
> > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> > index 9d28a8e3328f..e2a490c5ae08 100644
> > --- a/drivers/video/fbdev/core/fbcon.c
> > +++ b/drivers/video/fbdev/core/fbcon.c
> > @@ -2402,7 +2402,8 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
> >               ops->graphics = 1;
> >
> >               if (!blank) {
> > -                     var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE;
> > +                     var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE |
> > +                             FB_ACTIVATE_KD_TEXT;
> >                       fb_set_var(info, &var);
> >                       ops->graphics = 0;
> >                       ops->var = info->var;
> > diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
> > index b6aac7ee1f67..4c14e8be7267 100644
> > --- a/include/uapi/linux/fb.h
> > +++ b/include/uapi/linux/fb.h
> > @@ -205,6 +205,7 @@ struct fb_bitfield {
> >  #define FB_ACTIVATE_ALL             64       /* change all VCs on this fb    */
> >  #define FB_ACTIVATE_FORCE     128    /* force apply even when no change*/
> >  #define FB_ACTIVATE_INV_MODE  256       /* invalidate videomode */
> > +#define FB_ACTIVATE_KD_TEXT   512       /* for KDSET vt ioctl */
> >
> >  #define FB_ACCELF_TEXT               1       /* (OBSOLETE) see fb_info.flags and vc_mode */
> >
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>

Thomas Zimmermann July 10, 2020, 1:09 p.m. UTC | #3

Hi

Am 10.07.20 um 14:38 schrieb Daniel Vetter:
> On Fri, Jul 10, 2020 at 1:31 PM Thomas Zimmermann <[e-mail]> wrote:
>>
>> Hi Daniel,
>>
>> this patch might not be enougth. I started Xorg and then did 'kill -9'
>> on the Xorg process. Xorg went away, but the console did not come back.
> 
> Hm don't you need to reset your terminal to text mode in that case
> first? Iirc there's a sysrq. All again assuming not terribly modern
> system where that stuff is all done by logind.

I don't know. I just noticed that kill -9 did not restore the terminal
and apparently it was related to these patches. Maybe it did not even
work before?

Best regards
Thomas

> -Daniel
> 
>>
>> Best regards
>> Thomas
>>
>> Am 23.06.20 um 17:54 schrieb Daniel Vetter:
>>> In the past we had a pile of hacks to orchestrate access between fbdev
>>> emulation and native kms clients. We've tried to streamline this, by
>>> always preferring the kms side above fbdev calls when a drm master
>>> exists, because drm master controls access to the display resources.
>>>
>>> Unfortunately this breaks existing userspace, specifically Xorg. When
>>> exiting Xorg first restores the console to text mode using the KDSET
>>> ioctl on the vt. This does nothing, because a drm master is still
>>> around. Then it drops the drm master status, which again does nothing,
>>> because logind is keeping additional drm fd open to be able to
>>> orchestrate vt switches. In the past this is the point where fbdev was
>>> restored, as part of the ->lastclose hook on the drm side.
>>>
>>> Now to fix this regression we don't want to go back to letting fbdev
>>> restore things whenever it feels like, or to the pile of hacks we've
>>> had before. Instead try and go with a minimal exception to make the
>>> KDSET case work again, and nothing else.
>>>
>>> This means that if userspace does a KDSET call when switching between
>>> graphical compositors, there will be some flickering with fbcon
>>> showing up for a bit. But a) that's not a regression and b) userspace
>>> can fix it by improving the vt switching dance - logind should have
>>> all the information it needs.
>>>
>>> While pondering all this I'm also wondering wheter we should have a
>>> SWITCH_MASTER ioctl to allow race-free master status handover. But
>>> that's for another day.
>>>
>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=208179
>>> Cc: [e-mail]
>>> Reported-and-Tested-by: [e-mail]
>>> Cc: Michel Dänzer <[e-mail]>
>>> Fixes: 64914da24ea9 ("drm/fbdev-helper: don't force restores")
>>> Cc: Noralf Trønnes <[e-mail]>
>>> Cc: Thomas Zimmermann <[e-mail]>
>>> Cc: Daniel Vetter <[e-mail]>
>>> Cc: Maarten Lankhorst <[e-mail]>
>>> Cc: Maxime Ripard <[e-mail]>
>>> Cc: David Airlie <[e-mail]>
>>> Cc: Daniel Vetter <[e-mail]>
>>> Cc: [e-mail]
>>> Cc: <[e-mail]> # v5.7+
>>> Signed-off-by: Daniel Vetter <[e-mail]>
>>> ---
>>>  drivers/gpu/drm/drm_fb_helper.c  | 63 +++++++++++++++++++++++++-------
>>>  drivers/video/fbdev/core/fbcon.c |  3 +-
>>>  include/uapi/linux/fb.h          |  1 +
>>>  3 files changed, 52 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>>> index 170aa7689110..ae69bf8e9bcc 100644
>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>> @@ -227,18 +227,9 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
>>>  }
>>>  EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>>>
>>> -/**
>>> - * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
>>> - * @fb_helper: driver-allocated fbdev helper, can be NULL
>>> - *
>>> - * This should be called from driver's drm &drm_driver.lastclose callback
>>> - * when implementing an fbcon on top of kms using this helper. This ensures that
>>> - * the user isn't greeted with a black screen when e.g. X dies.
>>> - *
>>> - * RETURNS:
>>> - * Zero if everything went ok, negative error code otherwise.
>>> - */
>>> -int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>>> +static int
>>> +__drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
>>> +                                         bool force)
>>>  {
>>>       bool do_delayed;
>>>       int ret;
>>> @@ -250,7 +241,16 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>>>               return 0;
>>>
>>>       mutex_lock(&fb_helper->lock);
>>> -     ret = drm_client_modeset_commit(&fb_helper->client);
>>> +     if (force) {
>>> +             /*
>>> +              * Yes this is the _locked version which expects the master lock
>>> +              * to be held. But for forced restores we're intentionally
>>> +              * racing here, see drm_fb_helper_set_par().
>>> +              */
>>> +             ret = drm_client_modeset_commit_locked(&fb_helper->client);
>>> +     } else {
>>> +             ret = drm_client_modeset_commit(&fb_helper->client);
>>> +     }
>>>
>>>       do_delayed = fb_helper->delayed_hotplug;
>>>       if (do_delayed)
>>> @@ -262,6 +262,22 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>>>
>>>       return ret;
>>>  }
>>> +
>>> +/**
>>> + * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
>>> + * @fb_helper: driver-allocated fbdev helper, can be NULL
>>> + *
>>> + * This should be called from driver's drm &drm_driver.lastclose callback
>>> + * when implementing an fbcon on top of kms using this helper. This ensures that
>>> + * the user isn't greeted with a black screen when e.g. X dies.
>>> + *
>>> + * RETURNS:
>>> + * Zero if everything went ok, negative error code otherwise.
>>> + */
>>> +int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>>> +{
>>> +     return __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, false);
>>> +}
>>>  EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
>>>
>>>  #ifdef CONFIG_MAGIC_SYSRQ
>>> @@ -1318,6 +1334,7 @@ int drm_fb_helper_set_par(struct fb_info *info)
>>>  {
>>>       struct drm_fb_helper *fb_helper = info->par;
>>>       struct fb_var_screeninfo *var = &info->var;
>>> +     bool force;
>>>
>>>       if (oops_in_progress)
>>>               return -EBUSY;
>>> @@ -1327,7 +1344,25 @@ int drm_fb_helper_set_par(struct fb_info *info)
>>>               return -EINVAL;
>>>       }
>>>
>>> -     drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
>>> +     /*
>>> +      * Normally we want to make sure that a kms master takes
>>> +      * precedence over fbdev, to avoid fbdev flickering and
>>> +      * occasionally stealing the display status. But Xorg first sets
>>> +      * the vt back to text mode using the KDSET IOCTL with KD_TEXT,
>>> +      * and only after that drops the master status when exiting.
>>> +      *
>>> +      * In the past this was caught by drm_fb_helper_lastclose(), but
>>> +      * on modern systems where logind always keeps a drm fd open to
>>> +      * orchestrate the vt switching, this doesn't work.
>>> +      *
>>> +      * To no break the userspace ABI we have this special case here,
>>> +      * which is only used for the above case. Everything else uses
>>> +      * the normal commit function, which ensures that we never steal
>>> +      * the display from an active drm master.
>>> +      */
>>> +     force = var->activate & FB_ACTIVATE_KD_TEXT;
>>> +
>>> +     __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, force);
>>>
>>>       return 0;
>>>  }
>>> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
>>> index 9d28a8e3328f..e2a490c5ae08 100644
>>> --- a/drivers/video/fbdev/core/fbcon.c
>>> +++ b/drivers/video/fbdev/core/fbcon.c
>>> @@ -2402,7 +2402,8 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
>>>               ops->graphics = 1;
>>>
>>>               if (!blank) {
>>> -                     var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE;
>>> +                     var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE |
>>> +                             FB_ACTIVATE_KD_TEXT;
>>>                       fb_set_var(info, &var);
>>>                       ops->graphics = 0;
>>>                       ops->var = info->var;
>>> diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
>>> index b6aac7ee1f67..4c14e8be7267 100644
>>> --- a/include/uapi/linux/fb.h
>>> +++ b/include/uapi/linux/fb.h
>>> @@ -205,6 +205,7 @@ struct fb_bitfield {
>>>  #define FB_ACTIVATE_ALL             64       /* change all VCs on this fb    */
>>>  #define FB_ACTIVATE_FORCE     128    /* force apply even when no change*/
>>>  #define FB_ACTIVATE_INV_MODE  256       /* invalidate videomode */
>>> +#define FB_ACTIVATE_KD_TEXT   512       /* for KDSET vt ioctl */
>>>
>>>  #define FB_ACCELF_TEXT               1       /* (OBSOLETE) see fb_info.flags and vc_mode */
>>>
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
> 
>

Patch

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 170aa7689110..ae69bf8e9bcc 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -227,18 +227,9 @@  int drm_fb_helper_debug_leave(struct fb_info *info)
 }
 EXPORT_SYMBOL(drm_fb_helper_debug_leave);
 
-/**
- * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
- * @fb_helper: driver-allocated fbdev helper, can be NULL
- *
- * This should be called from driver's drm &drm_driver.lastclose callback
- * when implementing an fbcon on top of kms using this helper. This ensures that
- * the user isn't greeted with a black screen when e.g. X dies.
- *
- * RETURNS:
- * Zero if everything went ok, negative error code otherwise.
- */
-int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
+static int
+__drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
+					    bool force)
 {
 	bool do_delayed;
 	int ret;
@@ -250,7 +241,16 @@  int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
-	ret = drm_client_modeset_commit(&fb_helper->client);
+	if (force) {
+		/*
+		 * Yes this is the _locked version which expects the master lock
+		 * to be held. But for forced restores we're intentionally
+		 * racing here, see drm_fb_helper_set_par().
+		 */
+		ret = drm_client_modeset_commit_locked(&fb_helper->client);
+	} else {
+		ret = drm_client_modeset_commit(&fb_helper->client);
+	}
 
 	do_delayed = fb_helper->delayed_hotplug;
 	if (do_delayed)
@@ -262,6 +262,22 @@  int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 
 	return ret;
 }
+
+/**
+ * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
+ * @fb_helper: driver-allocated fbdev helper, can be NULL
+ *
+ * This should be called from driver's drm &drm_driver.lastclose callback
+ * when implementing an fbcon on top of kms using this helper. This ensures that
+ * the user isn't greeted with a black screen when e.g. X dies.
+ *
+ * RETURNS:
+ * Zero if everything went ok, negative error code otherwise.
+ */
+int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
+{
+	return __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, false);
+}
 EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
 
 #ifdef CONFIG_MAGIC_SYSRQ
@@ -1318,6 +1334,7 @@  int drm_fb_helper_set_par(struct fb_info *info)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 	struct fb_var_screeninfo *var = &info->var;
+	bool force;
 
 	if (oops_in_progress)
 		return -EBUSY;
@@ -1327,7 +1344,25 @@  int drm_fb_helper_set_par(struct fb_info *info)
 		return -EINVAL;
 	}
 
-	drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
+	/*
+	 * Normally we want to make sure that a kms master takes
+	 * precedence over fbdev, to avoid fbdev flickering and
+	 * occasionally stealing the display status. But Xorg first sets
+	 * the vt back to text mode using the KDSET IOCTL with KD_TEXT,
+	 * and only after that drops the master status when exiting.
+	 *
+	 * In the past this was caught by drm_fb_helper_lastclose(), but
+	 * on modern systems where logind always keeps a drm fd open to
+	 * orchestrate the vt switching, this doesn't work.
+	 *
+	 * To no break the userspace ABI we have this special case here,
+	 * which is only used for the above case. Everything else uses
+	 * the normal commit function, which ensures that we never steal
+	 * the display from an active drm master.
+	 */
+	force = var->activate & FB_ACTIVATE_KD_TEXT;
+
+	__drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper, force);
 
 	return 0;
 }
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 9d28a8e3328f..e2a490c5ae08 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2402,7 +2402,8 @@  static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
 		ops->graphics = 1;
 
 		if (!blank) {
-			var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE;
+			var.activate = FB_ACTIVATE_NOW | FB_ACTIVATE_FORCE |
+				FB_ACTIVATE_KD_TEXT;
 			fb_set_var(info, &var);
 			ops->graphics = 0;
 			ops->var = info->var;
diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
index b6aac7ee1f67..4c14e8be7267 100644
--- a/include/uapi/linux/fb.h
+++ b/include/uapi/linux/fb.h
@@ -205,6 +205,7 @@  struct fb_bitfield {
 #define FB_ACTIVATE_ALL	       64	/* change all VCs on this fb	*/
 #define FB_ACTIVATE_FORCE     128	/* force apply even when no change*/
 #define FB_ACTIVATE_INV_MODE  256       /* invalidate videomode */
+#define FB_ACTIVATE_KD_TEXT   512       /* for KDSET vt ioctl */
 
 #define FB_ACCELF_TEXT		1	/* (OBSOLETE) see fb_info.flags and vc_mode */