[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: D6011: let's continue in debug code instead of returning from XRandRConfig::applyKScreenConfig
From: Christoph Lutz <noreply () phabricator ! kde ! org>
Date: 2017-06-01 7:00:21
Message-ID: 20170601065950.121410.881F316C41479A28 () phabricator ! kde ! org
[Download RAW message or body]
clutz added a comment.
Thanks David for reviewing that. Oh, I already tried to find the underlying bug \
that causes the inconsistency. The debugging led me from libkscreen to xrandr to \
libxrandr (XRRGetOutputInfo) to the kernel. Here I noticed the following things:
1. outputInfo = XRRGetOutputInfo(outputId) returns some outputInfo that dosn't \
contain the mode it should contain (concrete the ID 77). It was simply missing. 2. \
crtcInfo = XRRGetCrtcInfo(outputInfo->crtc) But this crtcInfo-Objekt does reference \
the mode 77: crtcInfo->mode is 77 3. XRRGetScreenResourcesCurrent() also thinks that \
mode 77 should be present 4. Because mode 77 was not in outputInfo, \
XRandROutput::updateModes could not re-add this mode to the map m_modes and \
XRandROutput::currentMode() had to return NULL. Nothing wrong in updateModes!
Because libxrandr in 1) also delegates the above mentioned requests directly to the \
CRTC-driver from the kernel, I came to the conclusion that the inconsistency behind \
this situation comes from the kernel. It was also hard to reproduce on side of the \
kernel since multiple requests returned different results (kind of sporadic).
As mentioned above, I could sporadically reproduce the inconsistency with kernel \
3.13.0 and as I tried a newer kernel the issue seemed to be vanished. This is the \
point where I decided to not dig deeper into such an old kernel version.
So in conclusion for me the underlying bug seems to be a kernel bug and not a \
libkscreen-bug. Maybe in the meanwhile nobody will ever notice the above situation \
again. BUT in my concrete situation libkscreen seems to be able to recover things \
from this inconsistency and "continuing" instead of "returning" simply improves the \
behaviour of the complete system. So for me this is a clear improvement and I see no \
reason to not apply it. Do you?
REPOSITORY
R110 KScreen Library
REVISION DETAIL
https://phabricator.kde.org/D6011
To: clutz
Cc: davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, \
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
[Attachment #3 (unknown)]
<table><tr><td style="">clutz added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: \
right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: \
#F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: \
inline-block; border: 1px solid rgba(71,87,120,.2);" \
href="https://phabricator.kde.org/D6011" rel="noreferrer">View \
Revision</a></tr></table><br /><div><div><p>Thanks David for reviewing that. Oh, I \
already tried to find the underlying bug that causes the inconsistency. The debugging \
led me from libkscreen to xrandr to libxrandr (XRRGetOutputInfo) to the kernel. Here \
I noticed the following things:</p>
<ol class="remarkup-list">
<li class="remarkup-list-item">outputInfo = XRRGetOutputInfo(outputId) returns some \
outputInfo that dosn't contain the mode it should contain (concrete the ID 77). \
It was simply missing.</li> <li class="remarkup-list-item">crtcInfo = \
XRRGetCrtcInfo(outputInfo->crtc) But this crtcInfo-Objekt does reference the mode \
77: crtcInfo->mode is 77</li> <li \
class="remarkup-list-item">XRRGetScreenResourcesCurrent() also thinks that mode 77 \
should be present</li> <li class="remarkup-list-item">Because mode 77 was not in \
outputInfo, XRandROutput::updateModes could not re-add this mode to the map m_modes \
and XRandROutput::currentMode() had to return NULL. Nothing wrong in \
updateModes!</li> </ol>
<p>Because libxrandr in 1) also delegates the above mentioned requests directly to \
the CRTC-driver from the kernel, I came to the conclusion that the inconsistency \
behind this situation comes from the kernel. It was also hard to reproduce on side of \
the kernel since multiple requests returned different results (kind of sporadic).</p>
<p>As mentioned above, I could sporadically reproduce the inconsistency with kernel \
3.13.0 and as I tried a newer kernel the issue seemed to be vanished. This is the \
point where I decided to not dig deeper into such an old kernel version.</p>
<p>So in conclusion for me the underlying bug seems to be a kernel bug and not a \
libkscreen-bug. Maybe in the meanwhile nobody will ever notice the above situation \
again. BUT in my concrete situation libkscreen seems to be able to recover things \
from this inconsistency and "continuing" instead of "returning" \
simply improves the behaviour of the complete system. So for me this is a clear \
improvement and I see no reason to not apply it. Do you?</p></div></div><br \
/><div><strong>REPOSITORY</strong><div><div>R110 KScreen Library</div></div></div><br \
/><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D6011" \
rel="noreferrer">https://phabricator.kde.org/D6011</a></div></div><br \
/><div><strong>To: </strong>clutz<br /><strong>Cc: </strong>davidedmundson, \
plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, \
abetts, sebas, apol, mart, lukas<br /></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic