Bug 481793 - ddcutil constantly locked by powerdevil
Summary: ddcutil constantly locked by powerdevil
Status: RESOLVED FIXED
Alias: None
Product: Powerdevil
Classification: Plasma
Component: general (show other bugs)
Version: 5.93.0
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Plasma Bugs List
URL:
Keywords: qt6
Depends on:
Blocks:
 
Reported: 2024-02-25 00:29 UTC by d-h
Modified: 2024-04-13 17:33 UTC (History)
7 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description d-h 2024-02-25 00:29:14 UTC
SUMMARY
Running ddcutil version 2.1.3, ddcutil detect and other commands always fail when Powerdevil is running.

Running ddcutil detect yields the following error messages:
(i2c_open_bus                  )           busno=9, flock() returned: EAGAIN(-11): Resource temporarily unavailable
/dev/i2c-9 locked.  Retrying...
(i2c_open_bus                  )           busno=9, flock() returned: EAGAIN(-11): Resource temporarily unavailable
(i2c_open_bus                  )           busno=9, flock() returned: EAGAIN(-11): Resource temporarily unavailable
(i2c_open_bus                  )           busno=9, flock() returned: EAGAIN(-11): Resource temporarily unavailable
(i2c_open_bus                  )           busno=9, flock() returned: EAGAIN(-11): Resource temporarily unavailable
(i2c_open_bus                  )           busno=9, flock() returned: EAGAIN(-11): Resource temporarily unavailable
(i2c_open_bus                  )           busno=9, flock() returned: EAGAIN(-11): Resource temporarily unavailable
Max wait exceeded for /dev/i2c-9
Programs holding /dev/i2c-9 open:
   lsof: WARNING: can't stat() btrfs file system /var/lib/docker/btrfs
         Output information may be incomplete.
   COMMAND   PID  USER FD   TYPE DEVICE SIZE/OFF NODE NAME
   ddcutil 13315 david 3u   CHR   89,9      0t0  744 /dev/i2c-9
   sh      13322 david 3u   CHR   89,9      0t0  744 /dev/i2c-9
   lsof: WARNING: can't stat() btrfs file system /var/lib/docker/btrfs
         Output information may be incomplete.
   COMMAND   PID  USER FD   TYPE DEVICE SIZE/OFF NODE NAME
   ddcutil 13315 david 3u   CHR   89,9      0t0  744 /dev/i2c-9
   sh      13325 david 3u   CHR   89,9      0t0  744 /dev/i2c-9
Processes locking /dev/i2c-9 (inode 744): 
   11285
      11285
   egrep: warning: egrep is obsolescent; using grep -E
   Name:        org_kde_powerde
   State:       S (sleeping)
   Pid: 11285
   egrep: warning: egrep is obsolescent; using grep -E
   Name:        org_kde_powerde
   State:       S (sleeping)
   Pid: 11285

(i2c_open_bus                  )           Cross instance locking failed

The log indicates that /dev/i2c-* files are locked by powerdevil/libddcutil. I can reliably reproduce this even if I haven't done a interaction involving external monitors since starting the plasma session.
A workaround is to run ddcutil with the --disable-cross-instance-locks flag, as this seems to disable all checking for lock files. However, this obviously breaks the otherwise helpful feature of locking the currently used files.  

STEPS TO REPRODUCE
1. Make sure Powerdevil is running
2. Run for example ddcutil detect in a terminal

OBSERVED RESULT
ddcutil fails to communicate with monitors because locks are present.

EXPECTED RESULT
ddcutil shouldn't reliably fail because dev/i2c files are locked. 

Operating System: Arch Linux 
KDE Plasma Version: 5.93.0
KDE Frameworks Version: 5.249.0
Qt Version: 6.7.0
Kernel Version: 6.7.5-arch1-1 (64-bit)
Graphics Platform: Wayland
Processors: 12 × AMD Ryzen 5 5600 6-Core Processor
Memory: 31.3 GiB of RAM
Graphics Processor: AMD Radeon RX 6750 XT
Comment 1 Jakob Petsovits 2024-02-27 18:23:18 UTC
Can confirm. Plasma's use of ddcutil hasn't really changed much in 6.0 vs. 5.27, I would presume that this is due to changes in (lib)ddcutil itself to avoid two different apps getting in each others' way. For Plasma 6.1, we're adding the ability to keep track of monitors being added or removed, i.e. hotplugging, so the need to keep ddcutil running within PowerDevil will only increase.

I'm not sure if ddcutil is considering plans to allow multiple programs to access it simultaneously. From the Plasma point of view, I don't see an easy way of resolving this conflict.

What you can do in Plasma 6.0 is to set an environment variable POWERDEVIL_NO_DDCUTIL=1 which will disable the use of ddcutil within powerdevil. On a system with systemd, you can add a snippet to the plasma-powerdevil service:

$ systemctl edit --user plasma-powerdevil.service
(opens an editor, modify as follows:)

### Editing /home/kpetso/.config/systemd/user/plasma-powerdevil.service.d/override.conf
### Anything between here and the comment below will become the contents of the drop-in file

[Service]
Environment="POWERDEVIL_NO_DDCUTIL=1"

### Edits below this comment will be discarded

and then restart PowerDevil with `systemctl restart --user plasma-powerdevil.service`. Obviously that also means you won't be able to configure brightness with it that way. But you'll be able to use the ddcutil CLI instead. Sorry that this is the best I can do at this time.
Comment 2 Saul Fautley 2024-03-09 15:40:35 UTC
This comment on a Reddit post suggests that the issue is due to PowerDevil creating a display handle for each /dev/i2c device at startup and never closing it. They also suggest proper use of the API is, for each DDC transaction, to open a display, create a display handle, perform the transaction, and then close it.

https://www.reddit.com/r/kde/comments/1aitqng/comment/kp5pikr/
Comment 3 Jakob Petsovits 2024-03-09 16:41:39 UTC
(In reply to Saul Fautley from comment #2)
> They also suggest proper use of the API is, for each DDC transaction,
> to open a display, create a display handle, perform the transaction,
> and then close it.

Confirming. I just saw an example of this in https://github.com/digitaltrails/ddcutil-service which closes the display handle the way you describe, and was going to change Plasma to work in a similar way. Not implemented yet but indeed possible!
Comment 4 Bug Janitor Service 2024-03-11 00:10:47 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/333
Comment 5 Jakob Petsovits 2024-03-16 23:46:43 UTC
Git commit db4a6c79bc5f58dccc43805502c58145ef341ea1 by Jakob Petsovits.
Committed on 16/03/2024 at 23:26.
Pushed by jpetso into branch 'master'.

daemon: Avoid constantly locking ddcutil display handles

Since libddcutil implemented functionality for per-monitor
device lock files, other programs that are also using libddcutil
(such as the ddcutil CLI itself) were blocked from performing
monitor commands because PowerDevil kept all its handles open.

The better way of interacting with libddcutil as a long-running
program is to store a DDCA_Display_Ref for each monitor in question,
use it to get a temporary display handle, and close it again.

Now we can peacefully coexist with other libddcutil programs,
and users can e.g. query their monitor properties again.

As a slight downside, this means it's possible for another program
to set display brightness independently without PowerDevil noticing.
That seems like a smaller evil though, and can't be fixed by simply
using libddcutil itself as it cannot signal any VCP changes.
Common infrastructure such as digitaltrails/ddcutil-service or a
future kernel DRM interface for DDC/CI could help to mitigate this.

M  +16   -22   daemon/controllers/ddcutilbrightness.cpp
M  +79   -14   daemon/controllers/ddcutildisplay.cpp
M  +4    -2    daemon/controllers/ddcutildisplay.h

https://invent.kde.org/plasma/powerdevil/-/commit/db4a6c79bc5f58dccc43805502c58145ef341ea1
Comment 6 Jakob Petsovits 2024-03-16 23:49:41 UTC
Marking fixed in 6.1.0 for now. I'll have a look how difficult/risky the backport to 6.0.3 would be.
Comment 7 Jakob Petsovits 2024-04-13 17:33:58 UTC
Git commit f996906dffa0c20d87609b1d8f1dc8aabe361888 by Jakob Petsovits.
Committed on 13/04/2024 at 17:31.
Pushed by jpetso into branch 'Plasma/6.0'.

daemon: Avoid constantly locking ddcutil display handles

Since libddcutil implemented functionality for per-monitor
device lock files, other programs that are also using libddcutil
(such as the ddcutil CLI itself) were blocked from performing
monitor commands because PowerDevil kept all its handles open.

The better way of interacting with libddcutil as a long-running
program is to store a DDCA_Display_Ref for each monitor in question,
use it to get a temporary display handle, and close it again.

Now we can peacefully coexist with other libddcutil programs,
and users can e.g. query their monitor properties again.

As a slight downside, this means it's possible for another program
to set display brightness independently without PowerDevil noticing.
That seems like a smaller evil though, and can't be fixed by simply
using libddcutil itself as it cannot signal any VCP changes.
Common infrastructure such as digitaltrails/ddcutil-service or a
future kernel DRM interface for DDC/CI could help to mitigate this.
FIXED-IN: 6.0.4

(cherry picked from commit db4a6c79bc5f58dccc43805502c58145ef341ea1)

(generateDisplayId() changes extracted from commit a6b9cf5f)

M  +20   -24   daemon/backends/upower/ddcutilbrightness.cpp
M  +1    -1    daemon/backends/upower/ddcutilbrightness.h
M  +70   -14   daemon/backends/upower/ddcutildisplay.cpp
M  +4    -2    daemon/backends/upower/ddcutildisplay.h

https://invent.kde.org/plasma/powerdevil/-/commit/f996906dffa0c20d87609b1d8f1dc8aabe361888