Bug 448778

Does this happen when *not* running with that merge request? If not, can you comment in that merge request instead of here?

(In reply to Nate Graham from comment #1)
> Does this happen when *not* running with that merge request? If not, can you
> comment in that merge request instead of here?

Yes: https://pastebin.com/nns7Jn5b I could comment there but I was told to create a new report.

I added a bunch of debugging prints to the MR. Can you trigger the crash with that and then attach the logs for kwin_wayland_drm here?
You can get them with either
$ grep kwin_wayland_drm ~/.local/share/sddm/wayland-session.log
or
$ journalctl --boot 0 | grep kwin_wayland_drm
(which depends on your distro)

I must be doing something wrong because neither of those commands produce any output.
What I did:
I cloned kwin branch Plasma/5.24 and applied your patch.
Then I built and installed kwin with: "cmake -B build  -DCMAKE_INSTALL_LIBEXECDIR=lib -DBUILD_TESTING=OFF".
Then I set QT_LOGGING_RULES="kwin_wayland_drm.debug=true;kwin_core.debug=true;kwin_wayland_backend.debug=true" in /etc/environment
Finally, I re-logged in and triggered the crash.

Maybe logging in the legacy session is still broken... Try 
$ kwriteconfig5 --file startkderc --group General --key systemdBoot true
and reboot.

OK, now it gives an output: https://pastebin.com/Jb6T2m5F

Did you reproduce the crash before getting the log? It only shows the external output being connected, not removed. Up to that point everything looks correct

In the log I provided I unplug the cable before 08:52:41, is that what you are asking? After that only a cursor appears on my laptop screen and after a few seconds Plasma is restarted (only with systemd startup).

Sorry, didn't scroll up enough. KWin reads the configuration from KScreen, it is correct (eDP-1 is supposed to be enabled again) and then it gets applied... But it silently fails, which it really should not. 
I could now reproduce it myself though; at least on my laptop KScreen somehow uses a different refresh rate number (60001) than KWin (60000) and that makes applying the changes fail.

I didn't find the underlying bug yet but I pushed a commit to the MR that makes KWin not depend on the refresh rate to match in order to not crash.

Latest version of the MR fixes this issue. Unplugging the cable doesn't make Kwin crash anymore.

Git commit 74b1fb0ab5344d3189e77538fc03c60c45b02707 by Xaver Hugl.
Committed on 27/01/2022 at 18:40.
Pushed by zamundaaa into branch 'master'.

backends/drm: round refresh rate values from KScreen configs

The implicit cast effectively rounds the value down, which make the
refresh rate be different from what KScreen actually wrote.
A better fix would be to use integers instead of floating point numbers
but that needs to happen in KScreen.

M  +2    -1    src/backends/drm/drm_backend.cpp

https://invent.kde.org/plasma/kwin/commit/74b1fb0ab5344d3189e77538fc03c60c45b02707

Git commit 9f9ad7036518f0d0df984a3eda28e2b4c57607d3 by Xaver Hugl.
Committed on 27/01/2022 at 19:13.
Pushed by zamundaaa into branch 'Plasma/5.24'.

backends/drm: round refresh rate values from KScreen configs

The implicit cast effectively rounds the value down, which make the
refresh rate be different from what KScreen actually wrote.
A better fix would be to use integers instead of floating point numbers
but that needs to happen in KScreen.


(cherry picked from commit 74b1fb0ab5344d3189e77538fc03c60c45b02707)

M  +2    -1    src/backends/drm/drm_backend.cpp

https://invent.kde.org/plasma/kwin/commit/9f9ad7036518f0d0df984a3eda28e2b4c57607d3

I believe I am having this same issue, in KDE version 5.25 on Wayland.  I have a PC with 2 monitors connected, and I have a projector on which I watch films/TV shows.  When I turn off the monitors and connect the projector the display sometimes freezes without the rest of the system crashing - music remains playing indefinitely in the background.  The same crash also occasionally happens when waking from sleep.  
 
I presume the fix mentioned in this thread is enough for most use cases, however I have one old monitor and one new monitor so my main setup is 1680*1050 @ 60hz on DVI and a gaming monitor using 1920*1050 @ up to 144hz (with AMD freesync) on HDMI so I think it is testing this fix quite hard (along with many other parts of the system - it took me a while to get it working at all).  It is not a dealbreaker as I can restart and retry when it breaks which is about 1 in 3 attempts, however I wanted to log the fact that this is still an issue.

Please open a new bug report for that issue, it doesn't sound related at a first glance; KWin crashing does not make your monitors hang

(In reply to Zamundaaa from comment #14)
> Please open a new bug report for that issue, it doesn't sound related at a
> first glance; KWin crashing does not make your monitors hang

The monitors are not actually hanging, I can use ctrl+alt+fX and login on another tty.  I think it might actually be a bug within wayland itself so I may make a report there.

Wayland is not involved with what KWin displays. Please create a new bug report here

Hello @Zamundaaa,

SOFTWARE/OS VERSIONS
OS : Linux Deepin(Debian based)
KERNEL:
5.15.45-amd64-desktop
KWIN:
kwin 5.15.5

Seems like even i am facing the same issue, from a pretty long time that too ...

STEPS TO REPRODUCE
1. Log into the laptop with an external monitor attached
2. Works fine when external monitor is attached
3. Unplug monitor/dock(with monitor attached)
4. grey screen with only the cursor showing on the laptop screen cursor responds but crashed screen.

OBSERVED RESULT
Kwin crashes with only a grey screen with only the mouse cursor visible which responds fine when moved but the screen is unresponsive. a perpetual state until the monitor/dock is reconnected. 

My situation is so bad that I have to CONNECT an external monitor or atleast the VGA port to my laptop to get it come back from the freeze.
STRANGELY THOUGH WHEN I TURN 'WINDOWS EFFECT' OFF THEN THE LAPTOP IS USABLE WITH THE UGLY UI OF COURSE, FOR WINDOWS EFFECT IS THE REAL UX WITHOUT WHICH IT LIKE TRAVELLING IN A CARRIAGE IN THE AGE OF SPACE TRAVEL.


EXPECTED RESULT
It doesn't crash ON DISCONNECTING THE EXTERNAL MONITOR OR DOCKING STATION.

@zamundaaa, I will open another bug if you so wish, but the symptoms are the same as reported by @Matej Mrenica.

(as replied by Matej Mrenica from comment #8)
> In the log I provided I unplug the cable before 08:52:41, is that what you
> are asking? After that only a cursor appears on my laptop screen ...

(In reply to aiamuzz from comment #18)
> @zamundaaa, I will open another bug if you so wish, but the symptoms are the
> same as reported by @Matej Mrenica.

Can you test if it happens on Wayland as well?
Also, can you test if it happens with a newer version of KWin? Or is 5.15.5 a typo? It's been unsupported for more than three years now.

(In reply to Zamundaaa from comment #19)
> (In reply to aiamuzz from comment #18)
> > @zamundaaa, I will open another bug if you so wish, but the symptoms are the
> > same as reported by @Matej Mrenica.
> 
> Can you test if it happens on Wayland as well?
> Also, can you test if it happens with a newer version of KWin? Or is 5.15.5
> a typo? It's been unsupported for more than three years now.

I know, I've heard that its 3 years old, unfortunately its true Linux Deepin are lagging way behind ...

> Can you test if it happens on Wayland as well ? 

I am sorry I am just a Linux power user with limited understanding, so i'll not know the answer to that question.
If you have some commands whose outputs you want, I can run them so you can help diagnose the issue i am facing.

It would sound amusing, But i have basically pulled a VGA Cable from an old CRT monitor along with the PCB its connected to and i kind of attach this dangling cable with the PCB to my laptop to get my laptop working with 'Windows Effect' enabled ... the alternative being turning off 'Windows Effect' and using an ugly interface ... with the choice between a deep sea and the devil, i've chosen a dangling cable at my office as am used to the features 'Windows Effect' has to offer.


FYI ... https://github.com/linuxdeepin/deepin-kwin ... this is the repo of the package that's probably running on my system.

I was just hoping you are able to identify the issue i am facing and raise a MR/PR to this repo with the patch/code correction so i can install it and rectify my problem.

Please let me know the set of commands you want me to run on my affected system.

Thanks for the quick and prompt response.

I notice the following ...

Extended monitor ...
Resolution : 1920 x 1080 (Recommended)
Refresh Rate : 60Hz (Recommended)

Laptop Screen ...
Resolution : 1920 x 1080 (Recommended)
Refresh Rate : 60.01Hz (Recommended)

I have a feeling the fix you offered here to @Matej Mrenica might fix my problem as well ... as i see in your comment 9 you've spoken of refresh rates of 60000 and 60001 ...

Yes, that looks like the same underlying problem, no further testing is needed. We don't support such old versions and especially not forks of them though.

The things you can do is to ask the Deepin maintainers to update their fork of KWin to include an equivalent of https://invent.kde.org/plasma/kwin/-/commit/9f9ad7036518f0d0df984a3eda28e2b4c57607d3, or switch to a distro that offers an upstream version of KWin 5.24 or newer.

(In reply to Zamundaaa from comment #22)
> Yes, that looks like the same underlying problem, no further testing is
> needed. We don't support such old versions and especially not forks of them
> though.
> 
> The things you can do is to ask the Deepin maintainers to update their fork
> of KWin to include an equivalent of
> https://invent.kde.org/plasma/kwin/-/commit/
> 9f9ad7036518f0d0df984a3eda28e2b4c57607d3, or switch to a distro that offers
> an upstream version of KWin 5.24 or newer.

oh ... thanks for the confirmation.

unfortunately they are proceeding at snails pace ... the only hope is that they may have picked up the latest kwin version in their next years release. 

@Zamundaaa ... i was hoping to ask a favor ... 
the following is the version package whose fork my distro may have relied on.
https://invent.kde.org/plasma/kwin/-/archive/v5.15.5/kwin-v5.15.5.zip

I see that the folder and file structure is way different to the latest version, which is expected given the above is 3 years old.

If its possible i was hoping you could point me too the file that has the equivalent of the refreshRate line implementation ...

props->refreshRate = mode["refresh"].toDouble() * 1000;

I was hoping, as its only one word 'round ()' to be modified ... i though i could initiate the pull request on my distro forked repo.

I have been trying since the last couple of days with regard to 'src' folder and a file named 'drm_backend.cpp' but with no luck, of course its understandable the code would have undergone considerable change in structure and implementation in the last 3 years.

hoping for a helping hand ...

thanks for identifying and confirming the posssible issue.

apologies for trying to be self reliant ... I am the only one facing this issues it seems ... so trying to help myself ...


could these be the possible file that needs the corrected code ?

kwin-v5.15.5/plugins/platforms/drm/drm_output.cpp

relevant parts ???

**************

namespace {
quint64 refreshRateForMode(_drmModeModeInfo *m)
{
    // Calculate higher precision (mHz) refresh rate
    // logic based on Weston, see compositor-drm.c
    quint64 refreshRate = (m->clock * 1000000LL / m->htotal + m->vtotal / 2) / m->vtotal;
    if (m->flags & DRM_MODE_FLAG_INTERLACE) {
        refreshRate *= 2;
    }
    if (m->flags & DRM_MODE_FLAG_DBLSCAN) {
        refreshRate /= 2;
    }
    if (m->vscan > 1) {
        refreshRate /= m->vscan;
    }
    return refreshRate;
}
}


**************


  // read in mode information
    QVector<KWayland::Server::OutputDeviceInterface::Mode> modes;
    for (int i = 0; i < connector->count_modes; ++i) {
        // TODO: in AMS here we could read and store for later every mode's blob_id
        // would simplify isCurrentMode(..) and presentAtomically(..) in case of mode set
        auto *m = &connector->modes[i];
        KWayland::Server::OutputDeviceInterface::ModeFlags deviceflags;
        if (isCurrentMode(m)) {
            deviceflags |= KWayland::Server::OutputDeviceInterface::ModeFlag::Current;
        }
        if (m->type & DRM_MODE_TYPE_PREFERRED) {
            deviceflags |= KWayland::Server::OutputDeviceInterface::ModeFlag::Preferred;
        }

        KWayland::Server::OutputDeviceInterface::Mode mode;
        mode.id = i;
        mode.size = QSize(m->hdisplay, m->vdisplay);
        mode.flags = deviceflags;
        mode.refreshRate = refreshRateForMode(m);
        modes << mode;
    }


**************

void DrmOutput::setWaylandMode()
{
    AbstractOutput::setWaylandMode(QSize(m_mode.hdisplay, m_mode.vdisplay),
                                   refreshRateForMode(&m_mode));
}

**************

oh ok ... seems like the exact same file ... "drm_backend.cpp" ... the implementation however seems quite different to the one you have in the latest version.


***************
 //2.init wayland output device
    QString name = "VGA-1";
    QString model = "VGA-1000-unknown";
    QString manufacturer = "UOS";
    QByteArray uuid = "ffffffffff";
    QVector<KWayland::Server::OutputDeviceInterface::Mode> modes;
    KWayland::Server::OutputDeviceInterface::Mode dmode;
    dmode.id = 0;
    dmode.size = QSize(mode.hdisplay, mode.vdisplay);
    dmode.flags = KWayland::Server::OutputDeviceInterface::ModeFlag::Current;
    dmode.refreshRate = mode.vrefresh * 1000LL;
    modes << dmode;
    output->initWaylandOutputDevice(name, model, manufacturer, uuid, modes);
**************

I know at the risk of sounding foolish ... will this do the trick ?

your MR ... for the latest version

-  props->refreshRate = mode["refresh"].toDouble() * 1000;
+  props->refreshRate = round(mode["refresh"].toDouble() * 1000);

will the following do the trick to the 3 yr old version ?

-  dmode.refreshRate = mode.vrefresh * 1000LL;
+  dmode.refreshRate = round(mode.vrefresh * 1000LL);

"A better fix would be to use integers instead of floating point numbers"

i don't see toDouble(floating point number) implementation in the above line of code as seen in the implementation of the latest versions ... I am guessing it may need some more tinkering with the code in files preceding "drm_backends.cpp"

hmmmm ...

I am guessing my fix lies in the following 3 files ... understanding code and what it does is way above my paygrade ...

https://github.com/linuxdeepin/deepin-kwin/blob/master/plugins/platforms/drm/screens_drm.cpp
https://github.com/linuxdeepin/deepin-kwin/blob/master/plugins/platforms/drm/drm_backend.cpp
https://github.com/linuxdeepin/deepin-kwin/blob/master/plugins/platforms/drm/drm_output.cpp



@Zamundaaa ... nothing important ... i don't see your MR (https://invent.kde.org/plasma/kwin/-/commit/9f9ad7036518f0d0df984a3eda28e2b4c57607d3) synced into the kwin repo on github (https://github.com/KDE/kwin/blob/master/src/backends/drm/drm_backend.cpp) ...

@Zamundaaa ... firstly apologies for what may seem spamming your thread for additional help ... it only goes to show my desperation ... apologies for the same.

Please find the commit from my distro team ...

https://github.com/linuxdeepin/deepin-kwin/pull/82/commits/8530e2cbf5641b6e56cfb04a202ae95237f3dbc8

they have made this changes as per your commit ... https://invent.kde.org/plasma/kwin/-/commit/9f9ad7036518f0d0df984a3eda28e2b4c57607d3

I have a foolish(may be) question ... after your commit is patched by the affected user, will the monitor frequency show 60Hz or will it continue to show 60.01Hz ... ? ... reason i ask is my display properties is still showing 60.01Hz after patching my machine with the distro's patch ...

@Zamundaaa ... any suggestions on the following ?

https://github.com/linuxdeepin/developer-center/issues/3480#issuecomment-1323123155

I have no knowledge of the deepin fork codebase, and no time to dig into it, sorry