[PATCH v2] drm/fb-helper: Call drm_fb_helper_hotplug_event() when releasing drm master

[Jocelyn Falempe]

On 08/11/2021 17:00, Daniel Vetter wrote:
> 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 at redhat.com>
> 
> 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?

I'm investigating if we can fix the userspace (Xorg/logind in this case).

Basically there are 2 ioctl to switch to VT, and the kernel wants to 
have DRM_IOCTL_DROP_MASTER, called before VT_RELDISP.
The issue is that logind monitor /sys/class/tty/tty0/active to drop drm 
master, so it occurs only after Xorg has done VT_RELDISP.

Here are the call stack of both ioctl:

Call stack in Xorg:

ioctl(13, VT_RELDISP, 0x1)        = 0
/usr/lib64/libc.so.6(ioctl+0xb) [0x10739b]
/usr/libexec/Xorg(xf86VTLeave+0x150) [0x9c780]
/usr/libexec/Xorg(WakeupHandler+0xb9) [0x5f969]
/usr/libexec/Xorg(WaitForSomething+0x1ce) [0x1b5aee]
/usr/libexec/Xorg(main+0x4fc) [0x48bbc]
/usr/lib64/libc.so.6(__libc_start_call_main+0x7f) [0x2d55f]
/usr/lib64/libc.so.6(__libc_start_main@@GLIBC_2.34+0x7b) [0x2d60b]
/usr/libexec/Xorg(_start+0x24) [0x49674]

Call stack in logind:

ioctl(33, DRM_IOCTL_DROP_MASTER, 0) = 0
/usr/lib64/libc.so.6(ioctl+0xb) [0x10739b]
/usr/lib/systemd/systemd-logind(session_device_stop+0x4b)
/usr/lib/systemd/systemd-logind(session_device_pause_all+0x67)
/usr/lib/systemd/systemd-logind(seat_set_active.isra.0+0x5d)
/usr/lib/systemd/systemd-logind(seat_read_active_vt.isra.0+0x139)
/usr/lib/systemd/systemd-logind(manager_dispatch_console+0x25)
/usr/lib/systemd/libsystemd-shared-249.so(source_dispatch+0x513)
/usr/lib/systemd/libsystemd-shared-249.so(sd_event_dispatch+0x10c)
/usr/lib/systemd/libsystemd-shared-249.so(sd_event_run+0x117)
/usr/lib/systemd/systemd-logind(main+0x1b12) [0xff02]

We probably have to patch both logind and Xorg, and introduce a new dbus 
message, to fix this in userspace.

Also I didn't see in the kernel documentation, where the order of these 
two ioctl is specified. Maybe we should add a clear statement about this ?

> 
> 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

-- 

Jocelyn