On 2021-11-04 17:32, 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 e2809c7db818df6bbd0edf843e1beb2fbc9d8541 which
> introduced delayed hotplug for MST, and commit
> dc5bdb68b5b369d5bc7d1de96fa64cc1737a6320 which introduced a workaround for
> Xorg and logind, and add a force parameter to
> __drm_fb_helper_restore_fbdev_mode_unlocked() in this case.
>
> 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 in this case, forward the "force" parameter to the hotplug event,
> and ignore if there is a drm master in this case.
I'm worried that this leaves a race condition which allows the still-master (which causes drm_fb_helper_hotplug_event to bail without this patch) to modify the state set by __drm_fb_helper_hotplug_event, which could still result in similar symptoms.
I wonder if something like calling drm_fb_helper_hotplug_event when master is dropped and fb_helper->delayed_hotplug == true could work instead.
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 8e7a124d6c5a..c07080f661b1 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -82,6 +82,8 @@ MODULE_PARM_DESC(drm_leak_fbdev_smem,
> static LIST_HEAD(kernel_fb_helper_list);
> static DEFINE_MUTEX(kernel_fb_helper_lock);
>
> +static int __drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper, bool force);
> +
> /**
> * DOC: fbdev helpers
> *
> @@ -258,7 +260,7 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
> mutex_unlock(&fb_helper->lock);
>
> if (do_delayed)
> - drm_fb_helper_hotplug_event(fb_helper);
> + __drm_fb_helper_hotplug_event(fb_helper, force);
>
> return ret;
> }
> @@ -1930,6 +1932,38 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
> }
> EXPORT_SYMBOL(drm_fb_helper_initial_config);
>
> +static int __drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper, bool force)
> +{
> + int err = 0;
> +
> + if (!drm_fbdev_emulation || !fb_helper)
> + return 0;
> +
> + mutex_lock(&fb_helper->lock);
> + if (fb_helper->deferred_setup) {
> + err = __drm_fb_helper_initial_config_and_unlock(fb_helper,
> + fb_helper->preferred_bpp);
> + return err;
> + }
> + if (!force) {
> + if (!fb_helper->fb || !drm_master_internal_acquire(fb_helper->dev)) {
> + fb_helper->delayed_hotplug = true;
> + mutex_unlock(&fb_helper->lock);
> + return err;
> + }
> + drm_master_internal_release(fb_helper->dev);
> + }
> + drm_dbg_kms(fb_helper->dev, "\n");
> +
> + drm_client_modeset_probe(&fb_helper->client, fb_helper->fb->width, fb_helper->fb->height);
> + drm_setup_crtcs_fb(fb_helper);
> + mutex_unlock(&fb_helper->lock);
> +
> + drm_fb_helper_set_par(fb_helper->fbdev);
> +
> + return 0;
> +}
> +
> /**
> * drm_fb_helper_hotplug_event - respond to a hotplug notification by
> * probing all the outputs attached to the fb
> @@ -1953,35 +1987,7 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
> */
> int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> {
> - int err = 0;
> -
> - if (!drm_fbdev_emulation || !fb_helper)
> - return 0;
> -
> - mutex_lock(&fb_helper->lock);
> - if (fb_helper->deferred_setup) {
> - err = __drm_fb_helper_initial_config_and_unlock(fb_helper,
> - fb_helper->preferred_bpp);
> - return err;
> - }
> -
> - if (!fb_helper->fb || !drm_master_internal_acquire(fb_helper->dev)) {
> - fb_helper->delayed_hotplug = true;
> - mutex_unlock(&fb_helper->lock);
> - return err;
> - }
> -
> - drm_master_internal_release(fb_helper->dev);
> -
> - drm_dbg_kms(fb_helper->dev, "\n");
> -
> - drm_client_modeset_probe(&fb_helper->client, fb_helper->fb->width, fb_helper->fb->height);
> - drm_setup_crtcs_fb(fb_helper);
> - mutex_unlock(&fb_helper->lock);
> -
> - drm_fb_helper_set_par(fb_helper->fbdev);
> -
> - return 0;
> + return __drm_fb_helper_hotplug_event(fb_helper, false);
> }
> EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>
>
--
Earthling Michel Dänzer | https://redhat.com
Libre software enthusiast | Mesa and Xwayland developer
