[PATCH] drm/fb-helper: Fix restore_fbdev when there are pending delayed hotplug

[Michel Dänzer]

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