Prevent powerdevil from calling DPMS extension calls when DPMS extension isn't present
ClosedPublic

Authored by alnikiforov on Feb 10 2020, 10:28 AM.

Details

Summary

When DPMS extension isn't available, powerdevil unexpectedly exits due to a call to unavailable DPMS function and Qt error handling for such errors.

Following is a full log of powerdevil process when DPMS extension isn't present:

$ /usr/libexec/kf5/org_kde_powerdevil
QDBusArgument: read from a write-only object
QDBusArgument: read from a write-only object
QDBusArgument: read from a write-only object
powerdevil: No outputs have backlight property
powerdevil: Xrandr not supported, trying ddc, helper
powerdevil: [DDCutilBrightness]  0 display(s) were detected
powerdevil: org.kde5.powerdevil.backlighthelper.brightness failed
powerdevil: DPMS extension not available
The X11 connection broke: Unsupported extension used (code 2)
XIO:  fatal IO error 2 (No such file or directory) on X server ":0" 
      after 408 requests (408 known processed) with 0 events remaining.

This was detected while running KDE in a VM (without x2go). For some reason 'xdpyinfo -queryExtensions' didn't report DPMS extension being present.

Only change in 'PowerDevilDPMSAction::onProfileUnload' was required to fix issue for me, but changes in other functions could be needed as well, thus I implemented it. Two calls to m_helper, 'm_helper->startFade()' and 'm_helper->stopFade()', had corresponding conditions left intact since it looks like it doesn't call DPMS-related functions.

Diff Detail

Repository
R122 Powerdevil
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
alnikiforov created this revision.Feb 10 2020, 10:28 AM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 10 2020, 10:28 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
alnikiforov requested review of this revision.Feb 10 2020, 10:28 AM
broulik accepted this revision.Feb 10 2020, 10:41 AM
broulik added a subscriber: broulik.

Makes sense, this can go into 5.18 branch.
Do I need to land this for you?

This revision is now accepted and ready to land.Feb 10 2020, 10:41 AM
broulik requested changes to this revision.Feb 10 2020, 10:43 AM

There seem to be a bunch of additional m_helper.isNull() that could be ported to be consistent?

daemon/actions/dpms/powerdevildpmsaction.cpp
196

There's a superfluous parenthesis.

This revision now requires changes to proceed.Feb 10 2020, 10:43 AM
alnikiforov added inline comments.Feb 10 2020, 10:46 AM
daemon/actions/dpms/powerdevildpmsaction.cpp
196

Right. Thanks for finding it. I've just recently made additional changes in all functions except for initial change in 'PowerDevilDPMSAction::onProfileUnload()' and didn't recheck it. I'll recheck it and post fixed diff.

Fixed superfluous parenthesis in one of conditions.

What about the !m_helper.isNull() checks in onWakeupFromIdle and onIdleTimeout?

alnikiforov marked an inline comment as done.Feb 10 2020, 10:59 AM

There seem to be a bunch of additional m_helper.isNull() that could be ported to be consistent?

What about the !m_helper.isNull() checks in onWakeupFromIdle and onIdleTimeout?

Only two remaining ones are calls to m_helper->startFade() and m_helper->stopFade() functions. Since it looks like it doesn't invoke any DPMS related function calls, I didn't decide yet to update it there too. If you think these conditions should be updated too, I'll add it to this change.

If you think these conditions should be updated too, I'll add it to this change.

I think we should be consistent here, so please do add them. I'll land this afterwards. Thanks a lot!

Makes sense, this can go into 5.18 branch.
Do I need to land this for you?

Yes, please land it when you think it's ready to be merged. AFAIK, I have no push permissions.

Updated remaining conditions

This revision was not accepted when it landed; it landed in state Needs Review.Feb 10 2020, 11:27 AM
This revision was automatically updated to reflect the committed changes.