Bug 482713 - DDC-based Screen brightness control randomly unavailable in some sessions
Summary: DDC-based Screen brightness control randomly unavailable in some sessions
Status: CONFIRMED
Alias: None
Product: Powerdevil
Classification: Plasma
Component: general (show other bugs)
Version: 6.0.0
Platform: Fedora RPMs Linux
: HI normal
Target Milestone: ---
Assignee: Plasma Bugs List
URL:
Keywords:
: 487959 489563 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-03-07 13:25 UTC by rjawiygvozd
Modified: 2024-11-14 17:12 UTC (History)
14 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
powerdevil.log (22.14 KB, text/x-log)
2024-07-02 18:58 UTC, rjawiygvozd
Details
Log after connection with HDMI-HDMI and brightness slider was shown (6.52 KB, text/x-log)
2024-09-12 08:46 UTC, matterhorn103
Details
Log after connection with DP-HDMI and brightness slider was missing (5.61 KB, text/x-log)
2024-09-12 08:47 UTC, matterhorn103
Details

Note You need to log in before you can comment on or make changes to this bug.
Description rjawiygvozd 2024-03-07 13:25:28 UTC
SUMMARY
I've noticed that plasma showed a brightness icon in notifications even though I'm using a desktop, not a laptop. Turned out it actually works and when I change brightness it's actually being changed in my monitor settings which is great. The problem is it only works sometimes. For example, after booting the PC it may be unavailable, but after logging out back in it's available again, but it's not guaranteed and is seemingly random. When I say it's not available I mean at least two things:
1. "Brightness and color" dialog that opens when you click on brightness icon in the panel doesn't have a brightness slider at all, it only has night light settings.
2. Keyboard shortcuts I've set for controlling screen brightness don't work. But when brightness slider is visible they do work.

STEPS TO REPRODUCE
1. Reboot or logout and in and try to change brightness every time

OBSERVED RESULT
Sometimes you can control screen brightness and sometimes you can't

EXPECTED RESULT
Always being able to control screen brightness

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: Fedora 40 (Kinoite 40.20240306.n.0) with Unofficial LTS kernel 6.6.19-200.fc40.x86_64 (kwizart/kernel-longterm-6.6 copr)
KDE Plasma Version: 6.0.0
KDE Frameworks Version: 6.0.0 
Qt Version: 6.6.2

ADDITIONAL
GPU: AMD Radeon RX 6600
Display: AOC 2590G4
Comment 1 Nate Graham 2024-03-07 22:15:10 UTC
Bugs in DDC support, I suppose.
Comment 2 Jakob Petsovits 2024-03-07 22:43:46 UTC
We don't really have control over the list of monitors that ddcutil tells us about. It's unclear to me why your monitor would sometimes be detected and sometimes not. Maybe ddcutil would always detect it after a while, but it doesn't wait long enough before reporting back the list of displays to Plasma's power management service. Not sure.

Chances are, once we react better to changes in detected monitors, this might get fixed in practice. For Plasma 6.1, we've already merged a patch that recognizes added or removed DDC monitors. More work is needed to hook this up to the Brightness and Color applet so the slider gets added and removed on the fly, without having to log in again to restart Plasma.
Comment 3 rjawiygvozd 2024-03-08 05:03:04 UTC
In the meantime now that I know DDC is a thing I've learned there is something called "ddcci-driver-linux" which provides a kernel module that apparently exposes DDC brightness control as typical brightness control so it just works everywhere
Comment 4 rjawiygvozd 2024-06-03 13:03:49 UTC
This problem doesn't seem to be present on Plasma 6.0.5 that comes with Manjaro
Comment 5 rjawiygvozd 2024-06-05 04:17:11 UTC
(In reply to rjawiygvozd from comment #4)
> This problem doesn't seem to be present on Plasma 6.0.5 that comes with
> Manjaro

Actually no, it still exists
Comment 6 zvova7890 2024-06-25 17:58:51 UTC
I don't think it is my case, but with 6.1 I have issues when the monitor goes to sleep (DPMS). Probably, when it wakes up, Powerdevil tries to work with HDMI/DP i2c too early, and the monitor sometimes isn't yet ready to respond.

I have fixed(seem, still testing) it with this patch:
diff --git a/daemon/controllers/ddcutildisplay.cpp b/daemon/controllers/ddcutildisplay.cpp
index 1e1a274..09a5c93 100644
--- a/daemon/controllers/ddcutildisplay.cpp
+++ b/daemon/controllers/ddcutildisplay.cpp
@@ -49,7 +49,15 @@ DDCutilDisplay::DDCutilDisplay(DDCA_Display_Ref displayRef)
     // backed by libddcutil (like the ddcutil CLI itself) from functioning.
 
     DDCA_Display_Handle displayHandle = nullptr;
-    if (status = ddca_open_display2(m_displayRef, true, &displayHandle); status != DDCRC_OK) {
+    status = ~status;
+    for (int i = 0; i < 2 && status != DDCRC_OK; ++i) {
+        if (status = ddca_open_display2(m_displayRef, true, &displayHandle); status != DDCRC_OK) {
+            QThread::msleep(200);
+            continue;
+        }
+    }
+
+    if (status != DDCRC_OK) {
         qCWarning(POWERDEVIL) << "[DDCutilDisplay]: ddca_open_display2" << status;
         return;
     }

Just a quick fix, but it can potentially confirm my theory. So, maybe it is required to add some mechanism to handle this situations.
Comment 7 Nate Graham 2024-06-26 20:17:17 UTC
Nice, would you like to submit that upstream so the developers can take a look. Generally patches in Bugzilla tickets get overlooked.
Comment 8 zvova7890 2024-06-26 21:41:59 UTC
Yes, sure. When I am certain it is working, I will make an MR. Today, I encountered the issue again with this patch, so the place I am fixing is probably wrong. Now, I have enabled more logs and am testing another fix.
Comment 9 Jakob Petsovits 2024-06-26 21:55:59 UTC
(In reply to zvova7890 from comment #6)
> I don't think it is my case, but with 6.1 I have issues when the monitor
> goes to sleep (DPMS). Probably, when it wakes up, Powerdevil tries to work
> with HDMI/DP i2c too early, and the monitor sometimes isn't yet ready to
> respond.

Thanks for running this experiment. Might be related to Bug 476540, which is also seemingly related to DPMS. (My theory is that we should disable any brightness commands while DPMS has the monitor turned off, and then apply the last requested brightness after the monitor comes back.)

Could you check what the value of `status` is when ddca_open_display2() fails? ddcutil_status_codes.h doesn't have a "wait and try again" status code like EAGAIN, but I wonder if perhaps DDCRC_DPMS_ASLEEP (value -3030) serves a similar purpose in this case.
Comment 10 zvova7890 2024-06-27 14:33:38 UTC
I'm getting -3029 error.

Created MR: https://invent.kde.org/plasma/powerdevil/-/merge_requests/393
Comment 11 Zamundaaa 2024-06-28 13:28:01 UTC
Git commit 584cfdf0256bc7034be0b3fadf94f7b486597aa6 by Xaver Hugl.
Committed on 28/06/2024 at 13:17.
Pushed by zamundaaa into branch 'master'.

core: reload actions on brightness controller changes

Whether or not the brightness control action is supported depends on the currently
connected list of screens, which is dynamic and can't just be checked on startup.
To fix this, reload actions when devices get added or removed

M  +24   -27   daemon/powerdevilcore.cpp
M  +2    -2    daemon/powerdevilcore.h

https://invent.kde.org/plasma/powerdevil/-/commit/584cfdf0256bc7034be0b3fadf94f7b486597aa6
Comment 12 Bug Janitor Service 2024-06-28 13:29:22 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/395
Comment 13 Zamundaaa 2024-06-28 15:04:52 UTC
Git commit d91bc62fa6ff93ac62ded148fee3722deab41442 by Xaver Hugl.
Committed on 28/06/2024 at 15:00.
Pushed by zamundaaa into branch 'Plasma/6.1'.

core: reload actions on brightness controller changes

Whether or not the brightness control action is supported depends on the currently
connected list of screens, which is dynamic and can't just be checked on startup.
To fix this, reload actions when devices get added or removed

(cherry picked from commit 584cfdf0256bc7034be0b3fadf94f7b486597aa6)

Co-authored-by: Xaver Hugl <xaver.hugl@gmail.com>

M  +4    -0    daemon/controllers/screenbrightnesscontroller.cpp
M  +1    -0    daemon/controllers/screenbrightnesscontroller.h
M  +23   -27   daemon/powerdevilcore.cpp
M  +2    -2    daemon/powerdevilcore.h

https://invent.kde.org/plasma/powerdevil/-/commit/d91bc62fa6ff93ac62ded148fee3722deab41442
Comment 14 Jakob Petsovits 2024-07-02 07:55:04 UTC
Git commit 220b6b9531b21bb3d2cc790c6a3718af3df7aa7b by Volodymyr Zolotopupov.
Committed on 02/07/2024 at 08:35 CET.
Pushed by jpetso into branch 'master'.

ddcutildisplay: give some time before changing brightness after the monitor resumes

M  +4    -1    daemon/controllers/ddcutildisplay.cpp

https://invent.kde.org/plasma/powerdevil/-/commit/220b6b9531b21bb3d2cc790c6a3718af3df7aa7b

And also cherry-picked into Plasma/6.1 (will be released with 6.1.2):
https://invent.kde.org/plasma/powerdevil/-/commit/0c084dcb444173273b60934af7bdb321d39dbf13
Comment 15 zvova7890 2024-07-02 12:40:29 UTC
rjawiygvozd@gmail.com,

Could you please put this into /etc/environment and catch the situation when you have unavailable brightness controls?
QT_LOGGING_RULES="*powerdevil*.debug=true"

Then save the logs: journalctl -b -t org_kde_powerdevil > /tmp/powerdevil.log
... and attach here. 

Just wan't to understand what is going on.
Comment 16 Nate Graham 2024-07-02 18:47:58 UTC
*** Bug 489563 has been marked as a duplicate of this bug. ***
Comment 17 rjawiygvozd 2024-07-02 18:58:35 UTC
Created attachment 171302 [details]
powerdevil.log
Comment 18 rjawiygvozd 2024-07-02 19:00:21 UTC
(In reply to rjawiygvozd from comment #17)
> Created attachment 171302 [details]
> powerdevil.log

I had to restart the session multiple times before I replicated it but the last one has the problem.
This is Plasma 6.1.1
Comment 19 rjawiygvozd 2024-07-02 19:05:15 UTC
(In reply to rjawiygvozd from comment #18)
> (In reply to rjawiygvozd from comment #17)
> > Created attachment 171302 [details]
> > powerdevil.log
> 
> I had to restart the session multiple times before I replicated it but the
> last one has the problem.
> This is Plasma 6.1.1

Also I don't know if this is relevant but this monitor actually has a horrible startup time and whenever it goes from bios graphics mode or whatever it's called to a proper screen resolution it also spends at least a few seconds in the black screen, to the point that pc boots way before I can actually see anything
Comment 20 rjawiygvozd 2024-07-02 19:07:11 UTC
and this transition is also happening when it goes from sddm to plasma
Comment 21 zvova7890 2024-07-02 20:23:59 UTC
It is probably due to a race condition between KWin mode setting up and Powerdevil DDC/CI work. DDC detection should probably also be delayed.
Comment 22 Nate Graham 2024-07-27 12:58:53 UTC
*** Bug 487959 has been marked as a duplicate of this bug. ***
Comment 23 matterhorn103 2024-09-12 08:46:46 UTC
Created attachment 173580 [details]
Log after connection with HDMI-HDMI and brightness slider was shown
Comment 24 matterhorn103 2024-09-12 08:47:58 UTC
Created attachment 173581 [details]
Log after connection with DP-HDMI and brightness slider was missing
Comment 25 matterhorn103 2024-09-12 08:49:17 UTC
I've also had similar odd behaviour so thought I'd record it here. For more than three years using KDE on Tumbleweed I had my monitor (HP M24f 1080p) plugged into my PC via the bundled HDMI cable, and was blissfully unaware that changing the brightness of a monitor from software was even possible. Suddenly – and I forget when exactly this was, but earlier this year – I noticed that a brightness slider had appeared next to the Night Control toggle. And it worked! I assumed that a Plasma update had added either the functionality or the compatibility with my monitor.

I don't remember it being only sporadically available, though I didn't use it all that much.

A short while ago, maybe a month or so but no idea really, I noticed that the option was gone. And it stayed gone, so I assumed it was a regression. I didn't think to report it at the time.

Now I got to wondering why it didn't work any more, did some googling, found this bug report. I yesterday switched to using a DP to HDMI cable rather than the old HDMI cable, and over the course of several reboots and logout/login cycles it doesn't seem to ever work with this cable.

Weirdly, when I now switch back to the old cable, or another HDMI-HDMI cable, the brightness control seems to consistently appear. Which would imply that maybe the new DP-HDMI cable can't carry the DCC signal, something that would be entirely unsurprising – but the thing is, it is only now when I'm fiddling around with it and swapping back and forth that the brightness control appears with the HDMI-HDMI cable, whereas at least for the past few weeks I am absolutely 100% certain that it wasn't. But now I can't reproduce that.

Another point is that if I swap from one cable to another during a session (i.e. the monitor is disconnected then reconnected via a different cable and port on the GPU) there are no new entries printed to the log, and the brightness slider either remains hidden or remains shown, but if I boot with HDMI-HDMI, get shown the slider, then switch to DP-HDMI, the slider is still shown but no longer has any effect (indeed, if I try to change the brightness the widget gets stuck and freezes).

Should it be relevant, the cable in question is this one: https://www.amazon.de/UGREEN-DisplayPort-Uni-Direktional-Multi-Screen-Kompatibel/dp/B07V3RYBVY . It's advertised specifically as being one-way only, but at least some information must flow from the monitor to the PC as it still picks up the model name for instance.

I've attached the log output of both the successful and failed cases. If I ever replicate the brightness slider failing to show with the HDMI-HDMI connection then I'll post a log of that, too.
Comment 26 Jakob Petsovits 2024-10-14 23:03:44 UTC
Git commit 44e6922ae9f06a3e2fa3e7640be7eb32591cd579 by Jakob Petsovits.
Committed on 14/10/2024 at 22:36.
Pushed by jpetso into branch 'master'.

daemon: Retry failed DDC/CI reads and writes repeatedly

DDC/CI communication with monitors can be unreliable,
especially when the monitor is still in the process of
fully waking up. The moment that we receive a connection
or DPMS awake event may not be the moment that brightness
commands start working.

Prior to this commit, `DDCutilDisplay` protected itself against
this case with a drastic measure: if a read or write command
failed, the object would claim brightness controls as
unsupported. This error condition meant that we won't end up
with inconsistent state, but it also makes the monitor's
brightness slider go away (or fall back to software
brightness controls if KWin wants those instead).

This commit still uses the same failure mode, but will try
harder before we give up on the monitor altogether.
Both initialization (reading the initial brightness value)
and `setBrightness()` will now retry a few times if the
first attempt didn't work out. Subsequent retries will be
spaced out further, until we finally give up on that monitor.

To avoid exposing uninitialized `DDCutilDisplay` objects,
a list of pending displays now holds these objects while
they're waiting for another initialization attempt.

`setBrightness()` will only be performed once initialization
has succeeded, so the two operations are mutually exclusive.
We can reuse DDCutilDisplay's existing brightness delay timer
for both kinds of retry operations.
Related: bug 493329

M  +23   -11   daemon/controllers/ddcutildetector.cpp
M  +69   -15   daemon/controllers/ddcutildisplay.cpp
M  +8    -2    daemon/controllers/ddcutildisplay.h

https://invent.kde.org/plasma/powerdevil/-/commit/44e6922ae9f06a3e2fa3e7640be7eb32591cd579
Comment 27 Jakob Petsovits 2024-10-14 23:07:25 UTC
Git commit 48453745b1cfb0fce66e16ec9ef8caf961e79677 by Jakob Petsovits.
Committed on 14/10/2024 at 23:04.
Pushed by jpetso into branch 'Plasma/6.2'.

daemon: Retry failed DDC/CI reads and writes repeatedly

DDC/CI communication with monitors can be unreliable,
especially when the monitor is still in the process of
fully waking up. The moment that we receive a connection
or DPMS awake event may not be the moment that brightness
commands start working.

Prior to this commit, `DDCutilDisplay` protected itself against
this case with a drastic measure: if a read or write command
failed, the object would claim brightness controls as
unsupported. This error condition meant that we won't end up
with inconsistent state, but it also makes the monitor's
brightness slider go away (or fall back to software
brightness controls if KWin wants those instead).

This commit still uses the same failure mode, but will try
harder before we give up on the monitor altogether.
Both initialization (reading the initial brightness value)
and `setBrightness()` will now retry a few times if the
first attempt didn't work out. Subsequent retries will be
spaced out further, until we finally give up on that monitor.

To avoid exposing uninitialized `DDCutilDisplay` objects,
a list of pending displays now holds these objects while
they're waiting for another initialization attempt.

`setBrightness()` will only be performed once initialization
has succeeded, so the two operations are mutually exclusive.
We can reuse DDCutilDisplay's existing brightness delay timer
for both kinds of retry operations.
Related: bug 493329


(cherry picked from commit 44e6922ae9f06a3e2fa3e7640be7eb32591cd579)

Co-authored-by: Jakob Petsovits <jpetso@petsovits.com>

M  +23   -11   daemon/controllers/ddcutildetector.cpp
M  +69   -15   daemon/controllers/ddcutildisplay.cpp
M  +8    -2    daemon/controllers/ddcutildisplay.h

https://invent.kde.org/plasma/powerdevil/-/commit/48453745b1cfb0fce66e16ec9ef8caf961e79677
Comment 28 Jakob Petsovits 2024-10-14 23:59:14 UTC
Hey guys, the commit above is introducing repeated retries for PowerDevil's DDC/CI support. This applies to both initialization and setting brightness. Plasma 6.2.1 will have the change, please test if this fixes any issues for you and leave a comment in case something still isn't working.

Thanks!
Comment 29 Julien Delquié 2024-10-15 06:02:22 UTC
Patch applied. I will test it.
Comment 30 Julien Delquié 2024-10-16 17:01:07 UTC
(In reply to Julien Delquié from comment #29)
> Patch applied. I will test it.

Didn’t see anything strange, but patch doesn’t apply anymore against powerdevil v6.2.1.
Comment 31 Jakob Petsovits 2024-10-16 17:03:46 UTC
(In reply to Julien Delquié from comment #30)
> Didn’t see anything strange, but patch doesn’t apply anymore against
> powerdevil v6.2.1.

PowerDevil in 6.2.1 already includes it (and a minor fix-up that also always uses repeated tries after monitor wake-up).
Comment 32 Julien Delquié 2024-10-16 17:05:24 UTC
(In reply to Jakob Petsovits from comment #31)
> (In reply to Julien Delquié from comment #30)
> > Didn’t see anything strange, but patch doesn’t apply anymore against
> > powerdevil v6.2.1.
> 
> PowerDevil in 6.2.1 already includes it (and a minor fix-up that also always
> uses repeated tries after monitor wake-up).

Oh I remember you already told this in a previous comment, sorry!
Comment 33 d3vilguard 2024-10-17 10:37:43 UTC
This seems very similar to my findings  - https://bugs.kde.org/show_bug.cgi?id=494233
Comment 34 Marcus Johansson 2024-11-14 17:12:36 UTC
Some kind of regression has been implemented.
This bug was fixed for me, but it came back again a week or so ago.