Bug 91929

Is your ddx compiled with udev support? Please always attach your Xorg.0.log.

Created attachment 118164 [details]
/var/log/Xorg.0.log

udev in enabled in the DDX, else I wouldn't get the first two events (unless someone was calling XRRGetScreenResources / XRRGetScreenInfo in a loop, which isn't the case).

Oh completely misread what you said. I thought you meant you didn't see any events at all until xrandr (which is a side-effect of not having udev).

When we see a hotplug uevent, we iterate over all connectors looking for new outputs and comparing the connector->status on existing outputs, if the kernel reports that the output->status is different we send a RROutputChanged(output->randr_output, TRUE); and committed with RRTellChanged(screen).

Hmm, what we seem to be missing is setting the RROutputSetConnection().

commit f5aabb7bddc6fc5dc910a983d1291c9864f65f06
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Sep 9 11:18:15 2015 +0100

    sna: On hotplug events, update the output->status
    
    During the hotplug event, we query the current connector status and use
    that to trigger an output changed event to the clients. However, since
    we know the new status, we can set that on the RROutput immediately.
    Note the modelist is left unchanged, and will only be queried when the
    user requests it (though we may want to provide that in the hotplug
    notify as well).
    
    References: https://bugs.freedesktop.org/show_bug.cgi?id=91929#c2
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

should fix the fix issue, but you will still get the RRScreenChangeNotify following xrandr due to the full probe.

The issue is fixed.

Sadly it generates many crashes in kscreen (even sometimes Qt).
Hotplug generates events, then kscreen changes the configuration which generates other events. So any change in the DDX or kscreen can result in new event sequences which weren't tested...

I'm a little surprised to see events like :
RRNotify event, serial 25, synthetic NO, window 0xd5,
    subtype XRRCrtcChangeNotifyEvent
    crtc 63, mode None, rotation RR_Rotate_0
    x 0, y 0, width 0, height 0
but it could be the result of :
[888172.618] (EE) intel(0): sna_mode_check: invalid state found on pipe 0, disabling CRTC:20
[888172.903] (EE) intel(0): sna_mode_check: invalid state found on pipe 1, disabling CRTC:24
which probably means kscreen did set an invalid configuration.

Having KDE crash randomly is not fun either. Looking at mutter (at least, digging around in kwin is a bit trickier), it never seems to do a full reprobe by itself. So if the only source of the updated information is the hotplug uevent and sna_mode_discover(), we need to add the available modes there as well.

Kscreen doesn't run in kwin, but in kded5 (decides which config to apply), and kscreen_backend_launcher (XRandR calls). These two parts communicate over DBus.

So you have XRandR events from hotplug, which generate DBus signals from the backend to the daemon, which calls the backend, which generates other events...


It doesn't use xcb_randr_get_screen_info (only for XRandR 1.1 backend).

It uses xcb_randr_get_screen_resources, but only once. After it uses xcb_randr_get_screen_resources_current (see https://quickgit.kde.org/?p=libkscreen.git&a=blob&h=fae1dcc971d5d3a16e95bfa745d6822c69fa2a55&hb=effd39e2ebe767868660b52a8758873e45e4c656&f=backends%2Fxrandr%2Fxrandr.cpp).
So it doesn't do a full reprobe (RRGetInfo).


On XRROutputChangeNotifyEvent with RR_Connected, it will call XRandROutput::update -> XRandROutput::init -> XRandROutput::updateModes, which uses xcb_randr_get_screen_resources_current.

Unfortunately commit 2c08d72393e4c8ddf5926571b087459aaa225cb1 that was made to resolve this bug introduced infinite recursion if polling udev for events fails (which is always the case on systems without udev).  The scenario is as follows:

1. The server calls RRGetInfo().
2. RRGetInfo() calls sna_randr_getinfo()
3. sna_randr_getinfo() calls sna_uevent_poll(), which returns false
4. sna_randr_getinfo() calls sna_mode_discover()
5. sna_mode_discover() calls RRGetInfo()

and we jump straight back to step 2, until we run out of stack space and crash

It's not obvious to me how to fix this.

Hmm, nasty. So,

diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index 1b1a125..0a59d1d 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -4900,6 +4900,7 @@ void sna_mode_discover(struct sna *sna)
                                DBG(("%s: output %s (id=%d), changed state, reprobing\n",
                                     __FUNCTION__, output->name, sna_output->id));
                                sna_output->last_detect = 0;
+                               changed |= 4;
                        }
                        continue;
                }
@@ -4918,7 +4919,8 @@ void sna_mode_discover(struct sna *sna)
                changed |= 2;
        }
 
-       if (changed) {
+       /* Have the list of available outputs been updated? */
+       if (changed & 3) {
                DBG(("%s: outputs changed, broadcasting\n", __FUNCTION__));
 
                sna_mode_set_primary(sna);
@@ -4933,7 +4935,9 @@ void sna_mode_discover(struct sna *sna)
                xf86RandR12TellChanged(screen);
        }
 
-       RRGetInfo(screen, TRUE);
+       /* If anything has changed, refresh the RandR information */
+       if (changed)
+               RRGetInfo(screen, TRUE);
        RRTellChanged(screen);
 }

It appears that commit 679ee12079a7d2682d41506b81973c7c7d4fa1d8 has not resolved the problem on non udev systems.  Commenting out the call to RRGetInfo() in sna_mode_discover() as suggested by Mark lets X start here on OpenBSD.  So it would seem there is still an infinite recursion in RRGetInfo().

commit 096ddef22d6c57198a424eef00845dc7302b0cfe
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Oct 1 09:47:59 2015 +0100

    sna: Indicate when we expect to call RRGetInfo during discovery

This time with some due diligence and booting with --disable-udev.