ksummit.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: ksummit@lists.linux.dev,
	ksummit-discuss@lists.linuxfoundation.org,
	Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [TECH TOPIC] New userspace API for display-panel brightness control
Date: Mon, 20 Jun 2022 10:07:44 +0200	[thread overview]
Message-ID: <6a252f40-9a84-1b6a-12a0-eb0a9eeec6c0@redhat.com> (raw)
In-Reply-To: <efde4273-ae38-c050-f3ed-177e175e0007@redhat.com>

Hi All,

On 6/16/22 22:04, Linus Walleij wrote:
> On Thu, Jun 16, 2022 at 11:33 AM Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> The current userspace API for brightness control offered by
>> /sys/class/backlight devices has various problems:
>>
>> 1. There is no way to map the backlight device to a specific
>> display-output / panel
> 
> For userspace, using just sysfs you mean?

Yes,

> But that makes it sound like userspace needs to understand
> things like backlight-to-panel topology etc.

The linked RFC/proposal for the new API makes brightness a property
on the drm connector object, making the backlight-to-panel topology
mapping a kernel problem.

One of the feature-requests behind this API is the ability to
control the beightness of external monitors over DDC/DI this
will mean (for laptops) multiple brightness controls (one
per supported display) at which point userspace indeed needs
to be aware of the brightness-control <-> panel mapping.

Making this a property on the drm connector object makes this
mapping very explicit.

> If you add the presence of ambient light sensors to this mix
> things get even messier.

Mapping ambient light-sensors to dipslays is admittedly still
an unsolved problem, even with my proposal. But I'm not aware
of any external monitors with the capability to report ambient
light-sensors back to the connected PC (or other display source).

So for ambient light sensors for now userspace can keep assuming
these belong to the device's internal panel, like it currently
is also doing for the /sys/class/backlight devices.

For external monitors with ambient light sensors, if
anything I would expect the entire auto-brightness "stack"
to be inside the external monitor, with the possibility
to turn it on/off and set some parameters (like desired
perceived brightness) through DDC/DI . In which case
the auto setting + parameters can be made parameters on
the drm-connector object just like brightness.

And I guess we could make the ambient-light-level a drm-connector
property too. This will require some kernel glue and is a bit
outside of the scope of the current proposal. But it would
make sense to do things this way and I think this would
be a good/clean userspace API for the ambient light level
reporting which automatically solves the display mapping
problem.

> I would rather make the analogy to the thermal subsystem:
> 
> - Handles multiple thermal sensors
> - Handle linearization of measurements
> - Some accumulate and work to monitor a thermal zone
> - Handle multiple thermal zones
> - Also has cooling devices (such as CPU frequency and fans)
> - Policies are applied in the kernel to handle thermal sensors
>   and cooling devices and control them in an orchestrated
>   manner
> - Userspace can sit back and enjoy the show, but it works
>   out-of-the box. No magic thermal daemon.
> 
> Examples:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> 
> Wouldn't backlight rather:
> 
> - Handle multiple backlight devices.
> - Handle linearization of backlight intensity
> - Some accumulate and work as a composite backlight
> - Handle multiple composite backlights such as screens
> - Also handle ambient light sensors
> - Policies are applied in the kernel to handle backlight
>   and ambient light sensors together
> - Userspace can sit back and enjoy the show but it works
>   out-of-the box, no magic backlight daemon
> 
> I understand userspace will want to force backlight to user
> preferences, older people need more backlight etc.
> 
> But isn't it more compelling to handle that as a composite
> backlight device than to expose several of them to
> userspace? I imagine one big knob per screen
> 1-100 for userspace, a bool for on/off and a bool to select
> augmentation from ambient light sensors or not, the rest
> the kernel can figure out.
> 
> My point is that this is not just a userspace API, it is
> a policy extension of the backlight subsystem.
> 
> Maybe this is in line with what you're suggesting.
> I guess I just needed to mention ambient light sensors
> here.
> 
> My personal annoyance is to see several diverging
> userspace implementations of policy for using ambient
> light sensors with backlight. It is already annoying,
> Android has something etc.
> 
> I understand that this drives a truck through the old mantra
> to keep policy in userspace, but so does thermal already,
> so I'd just ask myself what makes most sense.

I'm actually a firm believer in keeping policy in
userspace, especially for something like panel brightness
control.

There not only is an ambient-light sensor to take into
account (with multiple possible algorithms to deal with
+ various knobs to tweak the algorithm) but also e.g. dimming
the brightness when the machine is idle (no user input for X
amount of time), when the battery goes below a certain treshold
(battery saver mode) and when enabling the new-fangled builtin
electronic privacy-screens.

And more use-cases which impact the brightness control might
popup in the future but just the above list is IMHO complicated
enough to leave this to userspace.

Thermal is different because letting things overheat is really
really bad, so the kernel really must get involved here. But
in general I do believe keeping policy in userspace is best
(when possible).

E.g. we also don't have any sound volume-control policy
in the kernel and IMHO brightness control is more like
sound volume-control then like thermalzones.

Regards,

Hans



  reply	other threads:[~2022-06-20  8:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-20  7:41 [TECH TOPIC] New userspace API for display-panel brightness control Hans de Goede
2022-06-20  8:07 ` Hans de Goede [this message]
2022-06-28 13:53 ` Jani Nikula
2022-06-29 19:26   ` Hans de Goede
  -- strict thread matches above, loose matches on Subject: below --
2022-06-16  9:32 Hans de Goede
2022-06-16 20:04 ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6a252f40-9a84-1b6a-12a0-eb0a9eeec6c0@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=ksummit-discuss@lists.linuxfoundation.org \
    --cc=ksummit@lists.linux.dev \
    --cc=linus.walleij@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).