Re: [RFC] drm/kms: control display brightness through drm_connector properties
Hi, On 4/8/22 17:11, Alex Deucher wrote: > On Fri, Apr 8, 2022 at 10:56 AM Hans de Goede wrote: >> >> Hi, >> >> On 4/8/22 16:08, Alex Deucher wrote: >>> On Fri, Apr 8, 2022 at 4:07 AM Daniel Vetter wrote: On Thu, Apr 07, 2022 at 05:05:52PM -0400, Alex Deucher wrote: > On Thu, Apr 7, 2022 at 1:43 PM Hans de Goede wrote: >> >> Hi Simon, >> >> On 4/7/22 18:51, Simon Ser wrote: >>> Very nice plan! Big +1 for the overall approach. >> >> Thanks. >> >>> On Thursday, April 7th, 2022 at 17:38, Hans de Goede >>> wrote: >>> The drm_connector brightness properties === bl_brightness: rw 0-int32_max property controlling the brightness setting of the connected display. The actual maximum of this will be less then int32_max and is given in bl_brightness_max. >>> >>> Do we need to split this up into two props for sw/hw state? The privacy >>> screen >>> stuff needed this, but you're pretty familiar with that. :) >> >> Luckily that won't be necessary, since the privacy-screen is a security >> feature the firmware/embedded-controller may refuse our requests >> (may temporarily lock-out changes) and/or may make changes without >> us requesting them itself. Neither is really the case with the >> brightness setting of displays. >> bl_brightness_max: ro 0-int32_max property giving the actual maximum of the display's brightness setting. This will report 0 when brightness control is not available (yet). >>> >>> I don't think we actually need that one. Integer KMS props all have a >>> range which can be fetched via drmModeGetProperty. The max can be >>> exposed via this range. Example with the existing alpha prop: >>> >>> "alpha": range [0, UINT16_MAX] = 65535 >> >> Right, I already knew that, which is why I explicitly added a range >> to the props already. The problem is that the range must be set >> before registering the connector and when the backlight driver >> only shows up (much) later during boot then we don't know the >> range when registering the connector. I guess we could "patch-up" >> the range later. But AFAIK that would be a bit of abuse of the >> property API as the range is intended to never change, not >> even after hotplug uevents. At least atm there is no infra >> in the kernel to change the range later. >> >> Which is why I added an explicit bl_brightness_max property >> of which the value gives the actual effective maximum of the >> brightness. Uh ... I'm not a huge fan tbh. The thing is, if we allow hotplugging brightness control later on then we just perpetuate the nonsense we have right now, forever. Imo we should support two kinds of drivers: - drivers which are non-crap, and make sure their backlight driver is loaded before they register the drm_device (or at least the drm_connector). For those we want the drm_connector->backlight pointer to bit static over the lifetime of the connector, and then we can also set up the brightness range correctly. - funny drivers which implement the glorious fallback dance which libbacklight implements currently in userspace. Imo for these drivers we should have a libbacklight_heuristics_backlight, which normalizes or whatever, and is also ways there. And then internally handles the fallback mess to the "right" backlight driver. We might have some gaps on acpi systems to make sure the drm driver can wait for the backlight driver to show up, but that's about it. Hotplugging random pieces later on is really not how drivers work nowadays with deferred probe and component framework and all that. >> I did consider using the range for this and updating it >> on the fly I think nothing is really preventing us from >> doing so, but it very much feels like abusing the generic >> properties API. >> bl_brightness_0_is_min_brightness: ro, boolean When this is set to true then it is safe to set brightness to 0 without worrying that this completely turns the backlight off causing the screen to become unreadable. When this is false setting brightness to 0 may turn the backlight off, but this is not guaranteed. This will e.g. be true when directly driving a PWM and the video-BIOS has provided a minimum (non 0) duty-cycle below which the driver will never go. >>> >>> Hm. It's quite unfortunate that it's impossible to have strong >>> guarantees >>> here. >>> >>> Is there any way we can avoid this prop? >> >> Not really, the problem is that we really don't know if 0 is off >> or min-brightness. In the given exampl
Re: [RFC] drm/kms: control display brightness through drm_connector properties
Hi, On 4/7/22 20:58, Carsten Haitzler wrote: > On Thu, 7 Apr 2022 17:38:59 +0200 Hans de Goede said: > > Below you covered our usual /sys/class/backlight device friends... what about > DDC monitor controls? These function similarly but just remotely control a > screen via I2C and also suffer from the same problems of "need root" and "have > to do some fun in mapping them to a given screen". Right, supporting this definitely is part of the plan, this is why my original email had the following footnote: 1) The need to be able to map the backlight device to a specific display has become clear once more with the recent proposal to add DDCDI support: https://lore.kernel.org/lkml/20220403230850.2986-1-yusisameri...@gmail.com/ :) > Otherwise I welcome this de-uglification of the backlight device and putting > it > together with the drm device that controls that monitor. Thx. > Just to make life more fun ... DDC does much more than backlight controls. It > can control essentially anything that is in the OSD for your monitor > (contrast, > brightness, backlight, sharpness, color temperatures, input to display (DP vs > HDMI vs DVI etc.), an for extra fun points can even tel you the current > rotation state of your monitor. All of these do make sense to live as drm > connector properties too. Perhaps not a first iteration but something to > consider in this design. One thing which I do want to take into account is to make sure that userspace can still do DDC/CI for all the other features. I know there is demand for adding brightness control over DDC/CI. I'm not aware of any concrete use-cases for the other DDC/CI settings. Also DDC/CI can include some pretty crazy stuff like setting up picture in picture using 2 different inputs of the monitor, which is very vendor specific. So all in all I think that we should just punt most of the DDC/CI stuff to userspace. With that said I agree that it would be good to think about possibly other some of the other settings in case some use-case does show up for those. The biggest problem with doing this is coming up with some prefix to namespace things. I've gone with bl_brightness to avoid a conflict with the existing TV specific properties which have plain "brightness" put if we want to e.g. also add DDC/CI contrast as a property later then it might be good to come up with another more generic prefix which can be shared between laptop-panel-brightness, DDC/CI-brightness and DDC/CI-contrast ... ? So any suggestions for a better prefix? Regards, Hans > >> As discussed already several times in the past: >> https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/ >> >> https://lore.kernel.org/all/4b17ba08-39f3-57dd-5aad-d37d844b0...@linux.intel.com/ >> >> The current userspace API for brightness control offered by >> /sys/class/backlight devices has various issues, the biggest 2 being: >> >> 1. There is no way to map the backlight device to a specific >>display-output / panel (1) >> 2. Controlling the brightness requires root-rights requiring >>desktop-environments to use suid-root helpers for this. >> >> As already discussed on various conference's hallway tracks >> and as has been proposed on the dri-devel list once before (2), >> it seems that there is consensus that the best way to to solve these >> 2 issues is to add support for controlling a video-output's brightness >> through properties on the drm_connector. >> >> This RFC outlines my plan to try and actually implement this, >> which has 3 phases: >> >> >> Phase 1: Stop registering multiple /sys/class/backlight devs for a single >> display >> = >> >> On x86 there can be multiple firmware + direct-hw-access methods >> for controlling the backlight and in some cases the kernel registers >> multiple backlight-devices for a single internal laptop LCD panel: >> >> a) i915 and nouveau unconditionally register their "native" backlight dev >>even on devices where /sys/class/backlight/acpi_video0 must be used >>to control the backlight, relying on userspace to prefer the "firmware" >>acpi_video0 device over "native" devices. >> b) amdgpu and nouveau rely on the acpi_video driver initializing before >>them, which currently causes /sys/class/backlight/acpi_video0 to usually >>show up and then they register their own native backlight driver after >>which the drivers/acpi/video_detect.c code unregisters the acpi_video0 >>device. This means that userspace briefly sees 2 devices and the >>disappearing of acpi_video0 after a brief time confuses the systemd >>backlight level save/restore code, see e.g.: >>https://bbs.archlinux.org/viewtopic.php?id=269920 >> >> I already have a pretty detailed plan to tackle this, which I will >> post in a separate RFC email. I plan to start working on this right >> away, as it will be good to have this fixed regardless. >> >> >> Phase 2: Add drm_connec
Re: [RFC] drm/kms: control display brightness through drm_connector properties
Hi Simon, On 4/8/22 10:22, Simon Ser wrote: > Hi Hans, > > Thanks for your details replies! > > On Thursday, April 7th, 2022 at 19:43, Hans de Goede > wrote: > >>> On Thursday, April 7th, 2022 at 17:38, Hans de Goede >>> wrote: >>> The drm_connector brightness properties === bl_brightness: rw 0-int32_max property controlling the brightness setting of the connected display. The actual maximum of this will be less then int32_max and is given in bl_brightness_max. >>> >>> Do we need to split this up into two props for sw/hw state? The privacy >>> screen >>> stuff needed this, but you're pretty familiar with that. :) >> >> Luckily that won't be necessary, since the privacy-screen is a security >> feature the firmware/embedded-controller may refuse our requests >> (may temporarily lock-out changes) and/or may make changes without >> us requesting them itself. Neither is really the case with the >> brightness setting of displays. > > Cool, makes sense to me! > bl_brightness_max: ro 0-int32_max property giving the actual maximum of the display's brightness setting. This will report 0 when brightness control is not available (yet). >>> >>> I don't think we actually need that one. Integer KMS props all have a >>> range which can be fetched via drmModeGetProperty. The max can be >>> exposed via this range. Example with the existing alpha prop: >>> >>> "alpha": range [0, UINT16_MAX] = 65535 >> >> Right, I already knew that, which is why I explicitly added a range >> to the props already. The problem is that the range must be set >> before registering the connector and when the backlight driver >> only shows up (much) later during boot then we don't know the >> range when registering the connector. I guess we could "patch-up" >> the range later. But AFAIK that would be a bit of abuse of the >> property API as the range is intended to never change, not >> even after hotplug uevents. At least atm there is no infra >> in the kernel to change the range later. >> >> Which is why I added an explicit bl_brightness_max property >> of which the value gives the actual effective maximum of the >> brightness. >> >> I did consider using the range for this and updating it >> on the fly I think nothing is really preventing us from >> doing so, but it very much feels like abusing the generic >> properties API. > > Since this is new uAPI there's no concern about backwards compat here. So it's > pretty much a matter of how we want the uAPI to look like. I was suggesting > using a range because it's self-describing, but maybe it's an abuse. > > Daniel Vetter, what do you think? If a property's range is going to be updated > on the fly, should we go for it, or should we use a separate prop to describe > the max value? Daniel, as explained in my replies to you, I do believe that dynamically updating the range is unavoidable. Especially once we also add support for setting a monitor's brightness over DDC/CI. Since external monitors (with/without DDC/CI support) can come and go and since properties cannot be added/removed after connector registration, we need a way to let userspace know if brightness control is actually available or not and what the range is. We can use a max value of 0 for not available and other values for the actual range, which I believe is a sane API. But the question from Simon then still remains, do we update the range of the property on the fly, followed by a connector hotplug uevent; or do we use a separate brightness_max property for this? Note that as Rasterman indicated that with DDC/CI support we could also offer other properties (for which I see no reason atm) and if we do say also add a contrast property over DDC/CI then if we go the separate brightness_max route that would mean adding 2 props for each setting which we want to support. Regards, Hans
Re: [RFC] drm/kms: control display brightness through drm_connector properties
On Mon, 11 Apr 2022 12:27:37 +0200 Hans de Goede said:
> Hi,
>
> On 4/7/22 20:58, Carsten Haitzler wrote:
> > On Thu, 7 Apr 2022 17:38:59 +0200 Hans de Goede said:
> >
> > Below you covered our usual /sys/class/backlight device friends... what
> > about DDC monitor controls? These function similarly but just remotely
> > control a screen via I2C and also suffer from the same problems of "need
> > root" and "have to do some fun in mapping them to a given screen".
>
> Right, supporting this definitely is part of the plan, this is why my original
> email had the following footnote:
Yay!
> 1) The need to be able to map the backlight device to a specific display
> has become clear once more with the recent proposal to add DDCDI support:
> https://lore.kernel.org/lkml/20220403230850.2986-1-yusisameri...@gmail.com/
Oh gee - I missed that. my bad!
> :)
>
> > Otherwise I welcome this de-uglification of the backlight device and
> > putting it together with the drm device that controls that monitor.
>
> Thx.
Having to deal with the backlight device madness is a big pain (have already
done it - DDC included) and properly exposing these things attached to the
proper KMS device is absolutely the right thing. Admittedly this punts the job
of matching a backlight device to the right video output in KMS to the kernel
so at least it gets solved in one place rather than it being re-invented again
and again per wm/desktop/compositor.
> > Just to make life more fun ... DDC does much more than backlight controls.
> > It can control essentially anything that is in the OSD for your monitor
> > (contrast, brightness, backlight, sharpness, color temperatures, input to
> > display (DP vs HDMI vs DVI etc.), an for extra fun points can even tel you
> > the current rotation state of your monitor. All of these do make sense to
> > live as drm connector properties too. Perhaps not a first iteration but
> > something to consider in this design.
>
> One thing which I do want to take into account is to make sure that userspace
> can still do DDC/CI for all the other features. I know there is demand for
> adding brightness control over DDC/CI. I'm not aware of any concrete use-cases
> for the other DDC/CI settings. Also DDC/CI can include some pretty crazy
> stuff like setting up picture in picture using 2 different inputs of the
> monitor, which is very vendor specific. So all in all I think that we should
> just punt most of the DDC/CI stuff to userspace.
Having spent some time with DDC you're right - it can have some interesting
properties, but a wide number seem to be highly common between monitors and
make total sense to regularly use if available. Backlight/brightness is just
the immediate focus here.
> With that said I agree that it would be good to think about possibly other
> some of the other settings in case some use-case does show up for those.
>
> The biggest problem with doing this is coming up with some prefix to
> namespace things. I've gone with bl_brightness to avoid a conflict
> with the existing TV specific properties which have plain "brightness"
> put if we want to e.g. also add DDC/CI contrast as a property later
> then it might be good to come up with another more generic prefix
> which can be shared between laptop-panel-brightness, DDC/CI-brightness
> and DDC/CI-contrast ... ?
>
> So any suggestions for a better prefix?
Well here is my take. Have DDC properties separate from a build-in backlight
device. Userspace code will have to essentially do something like:
if (built_in_backlight_exists(output)) // built in backlight device
set_backlight_brightness(output, val);
else if (ddc_prop_exists(output, 0x10)) // 0x10 is ddc brightness/backlight
set_ddc_int_val(output, 0x10, val);
else // fallback for ye olde setuid tooling
{ ...
}
DDC properties are quite simple in essence so just exposing the set so you can
read/write them (and check if they exist at all) would do the right thing - tie
DDC to the output visa KMS, (you still could use I2C directly if you like and
go behind KMS's back) but it'd then punt the policy decision of which
properties are common/sane to userspace without adding a possibly "endless" set
of "let's now adopt/abstract this DDC property" discussions. Wayland
compositors can adopts the properties they see as most useful for their uses.
Xorg could expose them via XRR output properties. So my take at least is to give
DDC it's own property namespace/set that allows an arbitrary set of numbered
properties and leave it pretty raw.
> Regards,
>
> Hans
>
>
>
>
>
> >
> >> As discussed already several times in the past:
> >> https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/
> >>
> >> https://lore.kernel.org/all/4b17ba08-39f3-57dd-5aad-d37d844b0...@linux.intel.com/
> >>
> >> The current userspace API for brightness control offered by
> >> /sys/class/backlight devices has various issues, the biggest 2 being:
> >>
> >> 1. There is no way to map the backlight dev
Re: [RFC] drm/kms: control display brightness through drm_connector properties
On Mon, 11 Apr 2022 12:18:30 +0200 Hans de Goede wrote: > Hi, > > On 4/8/22 17:11, Alex Deucher wrote: > > On Fri, Apr 8, 2022 at 10:56 AM Hans de Goede wrote: > >> ... > > So set it to a level we can guarantee can call it 0. If we have the > > flag we are just punting on the problem in my opinion. > > Right this an impossible problem to solve so the intent is indeed > to punt this to userspace, which IMHO is the best thing we can do > here. The idea behind the "bl_brightness_0_is_min_brightness: > ro, boolean" property is to provide a hint to userspace to help > userspace deal with this (and if userspace ends up using e.g. > systemd's hwdb for this to avoid unnecessary entries in hwdb). > > > The kernel > > driver would seem to have a better idea what is a valid minimum than > > some arbitrary userspace application. > > If the kernel driver knows the valid minimum then it can make 0 > actually be that valid minimum as you suggest and it can set the > hint flag to let userspace know this. OTOH there are many cases > where the kernel's guess is just as bad as userspace's guess and > there are too many laptops where this is the case to quirk > ourselves out of this situation. > > > Plus then if we need a > > workaround for what is the minimum valid brightness, we can fix it one > > place rather than letting every application try and fix it. > > I wish we could solve this in the kernel, but at least on > laptops with Intel integrated gfx many vendors don't bother > to put a non 0 value in the minimum duty-cycle field of the > VBT, so there really is no good way to solve this. > > If the userspace folks ever want to do a database for this, > I would expect them to do something with hwdb + udev which > can then be shared between the different desktop-environments. Hi Hans, assuming that it is impossible to reach a reasonable user experience by having a quirk database in the kernel in order to offer a consistent definition of bl_brightness=0, then should you not be recommending a userspace hwdb solution with full steam, rather than adding a hint in the kernel that might be just enough to have no-one ever bother investing in a proper solution? Re-reading your "bl_brightness_0_is_min_brightness" definition, it seems to be specified as exposing a certain condition in the system. When it is true, you imply that userspace can safely use value 0 as min brightness, but that is assuming the hint is correct. How likely is the hint incorrect? If the hint can be incorrect, does this hint actually give anything to userspace, or would userspace still choose to be safer than sorry and ignore the hint by assuming the worst? Is this situation much different to the quirk database libinput needs to work beautifully out of the box? Should desktop environments offer a couple more "advanced configuration" knobs for the lowest safe brightness value and the value-to-perceived brightness mapping to calibrate the familiar brightness slider? E.g. something like "click this button as soon as you see it on the display" for finding the lowest usable brightness, with defaults coming from a database. If the situation is as grim as you say, I would propose to drop "bl_brightness_0_is_min_brightness" (and "bl_brightness_control_method"), and document the dangers of using too low brightness values. Maybe also start looking for a project that would be appropriate for hosting such a database, just to point people to cooperate in a single place rather than each DE coming up with their own. Thanks, pq pgpOJCUBq_6pb.pgp Description: OpenPGP digital signature
Re: [RFC] drm/kms: control display brightness through drm_connector properties
Hi Pekka, On 4/11/22 13:34, Pekka Paalanen wrote: > On Mon, 11 Apr 2022 12:18:30 +0200 > Hans de Goede wrote: > >> Hi, >> >> On 4/8/22 17:11, Alex Deucher wrote: >>> On Fri, Apr 8, 2022 at 10:56 AM Hans de Goede wrote: > > ... > >>> So set it to a level we can guarantee can call it 0. If we have the >>> flag we are just punting on the problem in my opinion. >> >> Right this an impossible problem to solve so the intent is indeed >> to punt this to userspace, which IMHO is the best thing we can do >> here. The idea behind the "bl_brightness_0_is_min_brightness: >> ro, boolean" property is to provide a hint to userspace to help >> userspace deal with this (and if userspace ends up using e.g. >> systemd's hwdb for this to avoid unnecessary entries in hwdb). >> >>> The kernel >>> driver would seem to have a better idea what is a valid minimum than >>> some arbitrary userspace application. >> >> If the kernel driver knows the valid minimum then it can make 0 >> actually be that valid minimum as you suggest and it can set the >> hint flag to let userspace know this. OTOH there are many cases >> where the kernel's guess is just as bad as userspace's guess and >> there are too many laptops where this is the case to quirk >> ourselves out of this situation. >> >>> Plus then if we need a >>> workaround for what is the minimum valid brightness, we can fix it one >>> place rather than letting every application try and fix it. >> >> I wish we could solve this in the kernel, but at least on >> laptops with Intel integrated gfx many vendors don't bother >> to put a non 0 value in the minimum duty-cycle field of the >> VBT, so there really is no good way to solve this. >> >> If the userspace folks ever want to do a database for this, >> I would expect them to do something with hwdb + udev which >> can then be shared between the different desktop-environments. > > Hi Hans, > > assuming that it is impossible to reach a reasonable user experience by > having a quirk database in the kernel in order to offer a consistent > definition of bl_brightness=0, then should you not be recommending a > userspace hwdb solution with full steam, rather than adding a hint in > the kernel that might be just enough to have no-one ever bother > investing in a proper solution? The problem is we already lack the manpower for a quirk database, and even if we ever get the manpower then it would still be good to avoid the work necessary to add models to the database where the kernel already knows how things work, see below. As for no-one ever bothering coming up with a full-steam hwdb solution for the cases where the kernel has no idea what bl_brightness=0 means, yes that is likely, but that already is the status quo, the hint will at least allow using the full brightness range on devices where the kernel knows (with certainty) that this is correct. > Re-reading your "bl_brightness_0_is_min_brightness" definition, it > seems to be specified as exposing a certain condition in the system. > When it is true, you imply that userspace can safely use value 0 as min > brightness, but that is assuming the hint is correct. How likely > is the hint incorrect? It should never be incorrect, there are cases when the kernel knows reliably that bl_brightness=0 means minimum brightness (and NOT backlight off). > If the hint can be incorrect, does this hint > actually give anything to userspace, or would userspace still choose to > be safer than sorry and ignore the hint by assuming the worst? If the hint is incorrect then that would be a kernel bug and that should be fixed in the kernel. The whole idea behind the hind is that userspace can absolutely trust it to be correct when set to true (false basically means that the kernel does not know of 0=off or not). > Is this situation much different to the quirk database libinput needs > to work beautifully out of the box? libinput's quirk database is relatively pretty small and a lot of effort is done to fix things in generic ways where possible, to avoid growing it. As a general rule quirks should only be used to handle exceptions to general rules, the problem here is that bl_brightness=0 being backlight off is not a true exception it happens quite often. Which is also why I believe that a hwdb for this is not necessarily a great idea, because maintaining it will be a lot of work. > Should desktop environments offer a couple more "advanced > configuration" knobs for the lowest safe brightness value and the > value-to-perceived brightness mapping to calibrate the familiar > brightness slider? E.g. something like "click this button as soon as > you see it on the display" for finding the lowest usable brightness, > with defaults coming from a database. Maybe, but that would defeat all the attempts done to make Linux on the desktop just work. > If the situation is as grim as you say, I would propose to drop > "bl_brightness_0_is_min_brightness" (and > "bl_brightness_control_method"), an
Re: [RFC] drm/kms: control display brightness through drm_connector properties
On 11 Apr 2022, at 13:50, Hans de Goede wrote: > The problem is we already lack the manpower for a quirk database, > and even if we ever get the manpower then it would still be good > to avoid the work necessary to add models to the database where > the kernel already knows how things work, see below. I wonder how Windows developers solve this problem, and do they at all? Best, Mikhail.
Re: [RFC] drm/kms: control display brightness through drm_connector properties
On Mon, Apr 11, 2022 at 6:18 AM Hans de Goede wrote: > > Hi, > > On 4/8/22 17:11, Alex Deucher wrote: > > On Fri, Apr 8, 2022 at 10:56 AM Hans de Goede wrote: > >> > >> Hi, > >> > >> On 4/8/22 16:08, Alex Deucher wrote: > >>> On Fri, Apr 8, 2022 at 4:07 AM Daniel Vetter wrote: > > On Thu, Apr 07, 2022 at 05:05:52PM -0400, Alex Deucher wrote: > > On Thu, Apr 7, 2022 at 1:43 PM Hans de Goede > > wrote: > >> > >> Hi Simon, > >> > >> On 4/7/22 18:51, Simon Ser wrote: > >>> Very nice plan! Big +1 for the overall approach. > >> > >> Thanks. > >> > >>> On Thursday, April 7th, 2022 at 17:38, Hans de Goede > >>> wrote: > >>> > The drm_connector brightness properties > === > > bl_brightness: rw 0-int32_max property controlling the brightness > setting > of the connected display. The actual maximum of this will be less > then > int32_max and is given in bl_brightness_max. > >>> > >>> Do we need to split this up into two props for sw/hw state? The > >>> privacy screen > >>> stuff needed this, but you're pretty familiar with that. :) > >> > >> Luckily that won't be necessary, since the privacy-screen is a security > >> feature the firmware/embedded-controller may refuse our requests > >> (may temporarily lock-out changes) and/or may make changes without > >> us requesting them itself. Neither is really the case with the > >> brightness setting of displays. > >> > bl_brightness_max: ro 0-int32_max property giving the actual maximum > of the display's brightness setting. This will report 0 when > brightness > control is not available (yet). > >>> > >>> I don't think we actually need that one. Integer KMS props all have a > >>> range which can be fetched via drmModeGetProperty. The max can be > >>> exposed via this range. Example with the existing alpha prop: > >>> > >>> "alpha": range [0, UINT16_MAX] = 65535 > >> > >> Right, I already knew that, which is why I explicitly added a range > >> to the props already. The problem is that the range must be set > >> before registering the connector and when the backlight driver > >> only shows up (much) later during boot then we don't know the > >> range when registering the connector. I guess we could "patch-up" > >> the range later. But AFAIK that would be a bit of abuse of the > >> property API as the range is intended to never change, not > >> even after hotplug uevents. At least atm there is no infra > >> in the kernel to change the range later. > >> > >> Which is why I added an explicit bl_brightness_max property > >> of which the value gives the actual effective maximum of the > >> brightness. > > Uh ... I'm not a huge fan tbh. The thing is, if we allow hotplugging > brightness control later on then we just perpetuate the nonsense we have > right now, forever. > > Imo we should support two kinds of drivers: > > - drivers which are non-crap, and make sure their backlight driver is > loaded before they register the drm_device (or at least the > drm_connector). For those we want the drm_connector->backlight pointer > to bit static over the lifetime of the connector, and then we can also > set up the brightness range correctly. > > - funny drivers which implement the glorious fallback dance which > libbacklight implements currently in userspace. Imo for these drivers > we > should have a libbacklight_heuristics_backlight, which normalizes or > whatever, and is also ways there. And then internally handles the > fallback mess to the "right" backlight driver. > > We might have some gaps on acpi systems to make sure the drm driver can > wait for the backlight driver to show up, but that's about it. > > Hotplugging random pieces later on is really not how drivers work > nowadays > with deferred probe and component framework and all that. > > >> I did consider using the range for this and updating it > >> on the fly I think nothing is really preventing us from > >> doing so, but it very much feels like abusing the generic > >> properties API. > >> > bl_brightness_0_is_min_brightness: ro, boolean > When this is set to true then it is safe to set brightness to 0 > without worrying that this completely turns the backlight off causing > the screen to become unreadable. When this is false setting > brightness > to 0 may turn the backlight off, but this is not guaranteed. > This will e.g. be true when directly driving a PWM and the video-BIOS > has provided a minimum (non 0) duty-cycle below which the driver
