On Mon, Nov 08, 2021 at 04:34:53PM +0100, Jocelyn Falempe wrote:
> When using Xorg/Logind and an external monitor connected with an MST dock.
> After disconnecting the external monitor, switching to VT may not work,
> the (internal) monitor sill display Xorg, and you can't see what you are
> typing in the VT.
>
> This is related to commit e2809c7db818 ("drm/fb_helper: move deferred fb
> checking into restore mode (v2)")
>
> When switching to VT, with Xorg and logind, if there
> are pending hotplug event (like MST unplugged), the hotplug path
> may not be fulfilled, because logind may drop the master a bit later.
> It leads to the console not showing up on the monitor.
>
> So when dropping the drm master, call the delayed hotplug function if
> needed.
>
> v2: rewrote the patch by calling the hotplug function after dropping master
>
> Signed-off-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>
Lastclose console restore is a very gross hack, and generally doesn't work
well.
The way this is supposed to work is:
- userspace drops drm master (because drm master always wins)
- userspace switches the console back to text mode (which will restore the
console)
I guess we could also do this from dropmaster once more (like from
lastclose), but that really feels like papering over userspace bugs. And
given what a massive mess this entire area is already, I'm not eager to
add more hacks here.
So ... can't we fix userspace?
Ofc if it's a regression then that's different, but then I think we need a
bit clearer implementation. I'm not clear on why you clear the callback
(plus the locking looks busted).
-Daniel
> ---
> drivers/gpu/drm/drm_auth.c | 7 +++++++
> drivers/gpu/drm/drm_fb_helper.c | 6 +++---
> include/drm/drm_fb_helper.h | 4 +++-
> 3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index 60a6b21474b1..73acf1994d49 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -35,6 +35,7 @@
> #include <drm/drm_file.h>
> #include <drm/drm_lease.h>
> #include <drm/drm_print.h>
> +#include <drm/drm_fb_helper.h>
>
> #include "drm_internal.h"
> #include "drm_legacy.h"
> @@ -321,6 +322,12 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
> }
>
> drm_drop_master(dev, file_priv);
> +
> + mutex_unlock(&dev->master_mutex);
> + if (dev->fb_helper && dev->fb_helper->delayed_hotplug)
> + dev->fb_helper->delayed_hotplug(dev->fb_helper);
> + return ret;
> +
> out_unlock:
> mutex_unlock(&dev->master_mutex);
> return ret;
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 8e7a124d6c5a..e8d8cc3f47c3 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -252,9 +252,9 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
> ret = drm_client_modeset_commit(&fb_helper->client);
> }
>
> - do_delayed = fb_helper->delayed_hotplug;
> + do_delayed = (fb_helper->delayed_hotplug != NULL);
> if (do_delayed)
> - fb_helper->delayed_hotplug = false;
> + fb_helper->delayed_hotplug = NULL;
> mutex_unlock(&fb_helper->lock);
>
> if (do_delayed)
> @@ -1966,7 +1966,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> }
>
> if (!fb_helper->fb || !drm_master_internal_acquire(fb_helper->dev)) {
> - fb_helper->delayed_hotplug = true;
> + fb_helper->delayed_hotplug = drm_fb_helper_hotplug_event;
> mutex_unlock(&fb_helper->lock);
> return err;
> }
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 3af4624368d8..c2131d1a0e23 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -160,8 +160,10 @@ struct drm_fb_helper {
> * A hotplug was received while fbdev wasn't in control of the DRM
> * device, i.e. another KMS master was active. The output configuration
> * needs to be reprobe when fbdev is in control again.
> + * NULL, or a pointer to the hotplug function, so it can be called
> + * by the drm drop function directly.
> */
> - bool delayed_hotplug;
> + int (*delayed_hotplug)(struct drm_fb_helper *helper);
>
> /**
> * @deferred_setup:
> --
> 2.33.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
