DeviceKit-power and backlight brightness

[Kay Sievers]

On Fri, Jul 3, 2009 at 16:01, Richard Hughes<hughsient at gmail.com> wrote:
> 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.

I don't think you should _ever_ need unfinished or broken things
around you as an excuse to implement things in your software. Just
push it to the guys in charge, it's not your fault when it's not
working. Or what's next you want to add and "fix" here? :)

It might fit into what the current g-p-m applet offers, but that
should not matter at all. We are doing new pieces here instead of HAL,
because exactly it was wrong to do it _this_ way.

The backlight setting is definitely the domain of the display server,
just like the actual resolution or the monitor enable/disable switch
in a multi-monitor setup.

Please keep DK-power in its own domain which is probably nothing more
than: batteries, USVs, power supplies, CPU policy settings (to some
extent), and parts of suspend handling.

Thanks,
Kay