DeviceKit-power and backlight brightness

[Richard Hughes]

2009/7/3 David Zeuthen <david at fubar.dk>:
> It's completely wrong to do this as this, for example, will break
> multi-monitor setups. You should know very well that it's outside the
> scope of DeviceKit-power to do this - instead, I believe, you want to
> fix X or whatever display server you are using.

Right, and that's what I've done for the last year or so. I've watched
X go from having XBACKLIGHT support added into the server, and then
into the intel driver. And it worked for a few weeks. And then it
broke.

And then someone fixed it. And then KMS came along and it broke. And
then someone fixed it. And now it's been broken for a few months. And
that's just intel. The other drivers don't work either.

And that's just me, who's comfortable compiling out of tree drm; for
the distro user this just isn't ready yet.

Ohh, and multi-monitor brightness support. That means doing the i2c
probing in a separate thread (ha!) in the xserver as it's so slow. And
we also need to fix all the i2c routines to use the high-speed
variant. ajax wasn't exactly thrilled with the concept last time I
asked.

>> * Does the brightness functionality belong in DeviceKit-power or
>> something else (DeviceKit-backlight?)
>
> It belongs in the display server, e.g. currently X (and for example
> wayland in some version of the future). We've been through this
> discussion already when I released the first version of DeviceKit-power,
> I don't know why you chose to ignore that large thread.

Because I'm tired of making excuses for X. Also a lot of embedded
hardware won't be running X, or if they are, then it'll be with a
shitty (possibly non-free) driver that doesn't have XBACKLIGHT. The
kernel exports it as a device for a reason.

> Having two separate pieces of software (X and devkit-power) pretend that
> they control backlight and present different interfaces to, perhaps,
> multiple clients, is just a recipe for disaster.

Right, I agree. That's why g-p-m first tries using xrandr, and if that
fails (or isn't present) then it falls back to poking values into the
backlight device. But it always tries to use xrandr first if it's
present.

> No, you want X to support that. Or wayland. Or whatever daemon you are
> using. You don't want some separate daemon like devkit-power to
> interfere - it has no business doing this. If you add this to
> devkit-power the next thing people are going to add to devkit-power is
> to control backlight via other mechanisms (such as monitors with an
> auxiliary USB connection).

No, the backlight device in DKp is tied to a physical device, the
kernel backlight device. If it's anything more complex than a single
laptop panel the only way it makes sense is using Xorg. In fact, in
handwavy style, if we assume that X is always running, and that the
drivers support xbacklight just fine then we can remove the backlight
classes from the kernel completely.

> It's a slippery slope and I, for one, don't like it.

Right, and the reason I've not added this months ago is because I like
the X interface, I really do, but it just doesn't work, and isn't
supported by more than a few driver versions. I can't release software
in a few months time, knowing I've broken the vast majority of all the
laptops out there. Closing bugs NOTGNOME doesn't seem right, when I
know the hapless users have no hope of fixing XBACKLIGHT, or it being
fixed within a few months.

> Listen you most probably mean well by adding this feature. You want to
> make as many laptops as possible "Just Work"(tm). But that doesn't mean
> we should add hacks because X has some deficiencies.. The way to fix
> that is, surprise surprise, to fix X - not add hacks.

Right, and fixing X is more than just a few hours work. I'll make you
a promise: If XBACKLIGHT works for me on my T61 (intel, KMS), my Dell
(nouveau, non-KMS) and my notebook (intel, non-KMS) for the release of
F12, I'll remove that backlight code. And for now, maybe I just need
to add DKP_I_KNOW_IM_MEANT_TO_USE_X compiler defines to make it clear.

Richard.