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

[Jocelyn Falempe]

On 05/11/2021 16:50, Michel Dänzer wrote:
> 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.
> 

Ok, I will try to make a new patch and call 
drm_fb_helper_hotplug_event() from drm_drop_master() 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);
>>   
>>
> 
>