Re: ddcutil in Plasma Powerdevil

[Sanford Rockowitz]

You might want to look at the launchpad packaging 
(https://launchpad.net/~rockowitz/+archive/ubuntu/ddcutil), which 
includes the library version and development package.  It is at the 
current release level (0.9.7).


If the cmake error is not obvious, let me know what it is.  The locus of 
the problem may jump out at me.


Sanford

On 9/30/19 9:07 AM, Jonathan Riddell wrote:
I put ddcutils package in Neon unstable to be able to test it, using 
Sanford's packaging from Debian but with the libraries turned on.  
However powerdevil doesn't compile with DDCUtils turned on, it fails 
on a cmake error.


Jonathan


On Fri, 27 Sep 2019 at 07:14, Dorian Vogel > wrote:


Hey Jonathan,
Unfortunately I did not test it for a long time (last time was
around 5.15), and don't really have the possibility now because
I'm away from home.
*If I remember correctly,* back then I had a few instances when
the startup of plasma (during the splashscreen) was quite longer
due to retries of the DDC communication (of course this will not
happen if the user does not have ddcutil itself installed). As I
understand it, this is due to the architecture of Powerdevil,
which assumes the monitors are a fixed configuration as far as
brightness is concerned: backends are queried at startup and never
refreshed. This is why enumeration has to happen during startup of
plasma, and one would have to restart powerdevil when connecting a
new DDC capable monitor

Thinking back I wonder if a plasmoid would have been a better way,
enabling to have the enumeration done after startup is finished,
and making it easier to have a "refresh" button, and would still
allow to have both an integrated screen (laptop) and external (for
now the backends in powerdevil are exclusive).

Considering this is my first contribution and I am not sure of the
quality, I think some testing from more experienced devs would be
required. I haven't heard much so far.

Cheers,
*Dorian*

On Thu, Sep 26, 2019 at 11:13 AM Jonathan Riddell mailto:j...@jriddell.org>> wrote:

The DDCUtil features of Powerdevil in KDE Plasma are currently
disabled by default
"DDCUtil library support is disabled by default as recomended
by authors, add -DHAVE_DDCUTIL=On to enable"
This is based on a conversation from 2017 where it was decided
it was unstable.

I see work has continued on DDCUtil and on support for it in
Powerdevil.  Is it ready to be set to on by default?

It is too late for Plasma 5.17 due for release in 2 weeks but
we can tell distros the message is old as appropriate.

Jonathan



-- 
VOGEL Dorian

Re: ddcutil .deb

[Sanford Rockowitz]

Jonathan,

Much as I'd like to see some external pressure for ddcutil's inclusion 
in distros (it's been a slow process), I think it's premature to be 
including the library version in distros. Probably better off for now 
for PowerDevil to use its own private copy.  I'm happy to work with 
you/Dorian to improve the PowerDevil code.  It will help identify 
changes that the libddcutil API needs in order to be more usable.  One 
particular are of concern I have is support for multi-threaded clients.  
As I noted, DDC/CI operations are slow because of protocol mandated 
sleeps (typically 50 ms per operation).  GUIs may not want to wait for 
operations to complete.


Regards,
Sanford

On 09/04/2017 08:07 AM, Jonathan Riddell wrote:

Thanks for your comments Sanford.  The code isn't mine it's done by
Dorian for KDE Plasma's Powerdevil which gets a beta release next week
(I'm the release dude).

Dorian please review ddcutil Sanford's comments below for changes that could be 
made.

Sanford given the API/ABI stability should we be recommending distros
package up libddcutil and use this code or would that cause more
problems and we should mark it as in-development for this release?

Jonathan


- Forwarded message from Sanford Rockowitz <rockow...@minsoft.com> -

Date: Fri, 1 Sep 2017 10:52:29 -0400
From: Sanford Rockowitz <rockow...@minsoft.com>
To: Jonathan Riddell <j...@jriddell.org>
Subject: Re: ddcutil .deb

Jonathan,

I've taken a look at your powerdevil code at 
https://cgit.kde.org/powerdevil.git/tree/daemon/backends/upower/ddcutilbrightness.cpp.
It can be simplified to improve performance and reliability.

1)  The DDCA_Display_Identifier -> DDCA_Display_Ref lookup exists to
allow for choosing a monitor by its characteristics, when you don't
already have a list of monitors to select from, e.g. on the ddcutil
command line.   Struct DDCA_Display_Info, returned (as an array) by
ddca_display_info_list(), contains field dref, which is a valid
DDCA_Display_Ref you can use.   See function
display_selection_using_ddca_get_displays() in sample file
demo_display_selection.c.

The documentation could be clearer here.

2) There's no need to look up the VCP value for the Brightness field.
Per the MCCS spec, it's always x10.

3) Rather than reading and parsing the capabilities string to
determine if a monitor supports VCP feature code x10, it's both faster
and more reliable just to try reading the value of code x10.

Reading the capabilities string is a very expensive operation,
entailing multiple I2C write/read exchanges, as a single read can
return at most 32 bytes.  Every write/read exchange contains sleeps
mandated by the DDC/CI protocol.  (In fact, ddcutil spends 90% of it's
elapsed time sleeping.)Since the I2C protocol is unreliable, a
write/read exchange may have to be retried, and even worse a whole
sequence of write/read exchanges may have be to restarted.   Most
monitors are relatively clean, but some, such Dell P2412h monitors
that I have tested, have so many I2C failures that read capabilities
sometimes fails even after retries.

Moreover, the capabilities string is not necessarily accurate. See,
for example, the description of the HP LP2480zx monitor. on page
http://www.ddcutil.com/monitor_notes/.  The string does not include
VCP feature code x10, even though the monitor supports it.

BTW, while monitors vary enormously in the VCP feature codes they
support, I've yet to see a monitor that doesn't support feature x10.

4) You may want to control/capture messages issued from the library.
See functions ddca_set_fout(), ddca_set_ferr(),
ddca_set_output_level(), etc.

5) In certain very exceptional situations (i.e. things that should
never happen, but hey, it's conceivable) the library may abort.
Instead of aborting, the library can return to a location in the
caller registered using function ddca_register_jmp_buf().  See
function handle_library_abort() in sample file demo_global_settings.c.
I have to admit that I'm not entirely comfortable with making a
longjmp visible to the library client, but I'm trying to avoid forcing
the library user to check for truly exceptional failures on each API
call, while still avoiding library aborts that cause the client to
fail.   Let me know if you find this design useful/usable.

Regards,
Sanford