Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
Hi, On 10/3/22 12:32, Pekka Paalanen wrote: > On Mon, 3 Oct 2022 11:44:56 +0200 > Hans de Goede wrote: > >> One example mentioned here is that laptops with Intel integrated >> graphics may have some "perceived brightness" mapping table >> in their Video BIOS Tables (VBT) it would be interesting to use >> this and to export the curve coming from that to userspace as >> extra information (including allow userspace to write it). >> Since in this case (unlike many other cases) at least this >> curve is done in the kernel I guess we can then also add a boolean >> property to disable the curve (making it linear) in case that >> is helpful for HDR scenarios. > > Hi Hans, > > what if you would export that curve to userspace and not apply it in > the kernel, aside from the min-luminance=0 offset? > > I'm not sure if that was your intention, I didn't see it clearly said. > Of course this can be only about curves that are supposed to be applied > by the OS and not applied in firmware or hardware. Call it "software > curve"? > > Let userspace apply that curve if it happens to exist. Then, if we get > some other definition replacing that curve on some hardware, the kernel > could just expose the other thing as a new thing, and the old curve API > would not be interfering. Userspace that does not understand the new > thing (and the old curve property is not populated) would still be able > to control the brightness, just not in as pleasant way. > > It would also make using a custom curve a completely userspace thing with > no need for the kernel to support overwriting it. Userspace comes in many forms, which is why my preference for "software curve" support would be for it to be applied by the kernel by default (if present in the VBT) but then with a "software curve enable" flag to allow advanced userspace to disable it and then apply a curve of userspace's own choosing itself. This allows a simple implementation for desktop-environments which are unlikely to implement all this themselves, giving everyone the benefit of the nicer behavior of the VBT provided curve while allowing advanced desktop environments (likely mainly KDE + GNOME/mutter) to do something more advanced. Either way, this is all future extensions. First lets get the basic brightness control with just brightness + brightness-max attributes in place. Regards, Hans
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Mon, 3 Oct 2022 11:44:56 +0200 Hans de Goede wrote: > One example mentioned here is that laptops with Intel integrated > graphics may have some "perceived brightness" mapping table > in their Video BIOS Tables (VBT) it would be interesting to use > this and to export the curve coming from that to userspace as > extra information (including allow userspace to write it). > Since in this case (unlike many other cases) at least this > curve is done in the kernel I guess we can then also add a boolean > property to disable the curve (making it linear) in case that > is helpful for HDR scenarios. Hi Hans, what if you would export that curve to userspace and not apply it in the kernel, aside from the min-luminance=0 offset? I'm not sure if that was your intention, I didn't see it clearly said. Of course this can be only about curves that are supposed to be applied by the OS and not applied in firmware or hardware. Call it "software curve"? Let userspace apply that curve if it happens to exist. Then, if we get some other definition replacing that curve on some hardware, the kernel could just expose the other thing as a new thing, and the old curve API would not be interfering. Userspace that does not understand the new thing (and the old curve property is not populated) would still be able to control the brightness, just not in as pleasant way. It would also make using a custom curve a completely userspace thing with no need for the kernel to support overwriting it. Thanks, pq pgp69AHOPFZPI.pgp Description: OpenPGP digital signature
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Mon, 3 Oct 2022 12:29:01 +0300 Ville Syrjälä wrote: > On Mon, Oct 03, 2022 at 11:37:50AM +0300, Pekka Paalanen wrote: > > On Fri, 30 Sep 2022 18:17:39 +0200 > > Sebastian Wick wrote: > > > > > On Fri, Sep 30, 2022 at 5:27 PM Pekka Paalanen > > > wrote: > > > > > > > > On Fri, 30 Sep 2022 17:44:17 +0300 > > > > Ville Syrjälä wrote: > > > > > > > > > On Fri, Sep 30, 2022 at 04:20:29PM +0200, Sebastian Wick wrote: > > > > > > On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen > > > > > > wrote: > > > > > > > > > > > > > > On Thu, 29 Sep 2022 20:06:50 +0200 > > > > > > > Sebastian Wick wrote: > > > > > > > > > > > > > > > If it is supposed to be a non-linear luminance curve, which one > > > > > > > > is it? > > > > > > > > It would be much clearer if user space can control linear > > > > > > > > luminance > > > > > > > > and use whatever definition of perceived brightness it wants. > > > > > > > > The > > > > > > > > obvious downside of it is that it requires bits to encode > > > > > > > > changes that > > > > > > > > users can't perceive. What about backlights which only have a > > > > > > > > few > > > > > > > > predefined luminance levels? How would user space differentiate > > > > > > > > between the continuous and discrete backlight? What about > > > > > > > > self-emitting displays? They usually increase the dynamic range > > > > > > > > when > > > > > > > > they increase in brightness because the black level doesn't > > > > > > > > rise. They > > > > > > > > also probably employ some tonemapping to adjust for that. What > > > > > > > > about > > > > > > > > the range of the backlight? What about the absolute luminance > > > > > > > > of the > > > > > > > > backlight? I want to know about that all in user space. > > > > > > > > > > > > > > > > I understand that most of the time the kernel doesn't have > > > > > > > > answers to > > > > > > > > those questions right now but the API should account for all of > > > > > > > > that. ... > > I suppose the firmware may expose some tables that may allow mapping > > raw hardware values into something more pleasant to use. Like something > > where each step is more or less a visible change. That does not have to > > imply anything about linearity in any space, they may just be "good > > values" for e.g. keyboard-based changing of backlight levels with no > > mathematical or physical basis. > > > > Ville, what kind of tables do you know about? What do they actually > > tell? > > We just get a set of points (up to 20 originally, and I think up to > 32 in later spec revisions) that map input brightness (in %) to > output duty cycle (0-0xff in old spec, 0-0x in new spec). > And I *think* we're supposed to just linearly inteprolate between > the entries, though the spec doesn't really state that in super > clear terms. > > There is some mention in the spec about this being more or less > designed for Windows Vista, so a look through eg. > https://learn.microsoft.com/en-us/windows-hardware/drivers/display/supporting-brightness-controls-on-integrated-display-panels > might be a good idea here. That's a nice link. Quote: "Brightness levels are represented as single-byte values in the range from zero to 100 where zero is off and 100 is the maximum brightness that a laptop computer supports. Every laptop computer must report a maximum brightness level of 100; however, a laptop computer is not required to support a level of zero. The only requirement for values from zero to 100 is that larger values must represent higher brightness levels. The increment between levels is not required to be uniform, and a laptop computer can support any number of distinct values up to the maximum of 101 levels. You must decide how to map hardware levels to the range of brightness level values. However, a call to the display miniport driver's DxgkDdiGetPossibleBrightness function should not report more brightness level values than the hardware supports." Sounds like "good values" to me, and that each value must be distinguishable from any another (at least electrically). Thanks, pq pgpwsFeSNM432.pgp Description: OpenPGP digital signature
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
Hi, On 9/30/22 18:17, Sebastian Wick wrote: > On Fri, Sep 30, 2022 at 5:27 PM Pekka Paalanen wrote: >> >> On Fri, 30 Sep 2022 17:44:17 +0300 >> Ville Syrjälä wrote: >> >>> On Fri, Sep 30, 2022 at 04:20:29PM +0200, Sebastian Wick wrote: On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen wrote: > > On Thu, 29 Sep 2022 20:06:50 +0200 > Sebastian Wick wrote: > >> If it is supposed to be a non-linear luminance curve, which one is it? >> It would be much clearer if user space can control linear luminance >> and use whatever definition of perceived brightness it wants. The >> obvious downside of it is that it requires bits to encode changes that >> users can't perceive. What about backlights which only have a few >> predefined luminance levels? How would user space differentiate >> between the continuous and discrete backlight? What about >> self-emitting displays? They usually increase the dynamic range when >> they increase in brightness because the black level doesn't rise. They >> also probably employ some tonemapping to adjust for that. What about >> the range of the backlight? What about the absolute luminance of the >> backlight? I want to know about that all in user space. >> >> I understand that most of the time the kernel doesn't have answers to >> those questions right now but the API should account for all of that. > > Hi, > > if the API accounts for all that, and the kernel doesn't know, then how > can the API not lie? If the API sometimes lies, how could we ever trust > it at all? Make it possible for the API to say "I don't know". I'd much rather have an API tell me explicitly what it does and doesn't know instead of having to guess what data I can actually rely on. For example if the kernel knows the luminance is linear on one display and doesn't know anything about the other display and it exposes them both in the same way I can not possibly write any code which relies on exact control over the luminance for either display. > > Personally I have the feeling that if we can even get to the level of > "each step in the value is a more or less perceivable change", that > would be good enough. Think of UI, e.g. hotkeys to change brightness. > You'd expect almost every press to change it a bit. The nice thing is that you can have that even if you have no further information about the brightness control and it might be good enough for some use cases but it isn't for others. > If an end user wants defined and controlled luminance, I'd suggest they > need to profile (physically measure) the response of the display at > hand. This is no different from color profiling displays, but you need > a measurement device that produces absolute measurements if absolute > control is what they want. If that's the kind of user experience you're after, good for you. I certainly want things to work out of the box which makes this just a big no-go. >>> >>> I think if we have the information to make the default behaviour >>> better then we should do that. Ie. if the firmaware gives us a >>> table to remap the values for a more linear response we should >>> make use of that by default. >> >> But that's only like 20% of what Sebastian is asking for. >> >> What's "linear"? Radiometric or perceptual? >> >> Radiometric linear control would make a terrible UX, so if the control >> is radiometric, userspace needs to remap it. That might be a good >> thing, but it's also complicated, because the relationship between >> brightness and luminance is somewhere between a power curve and >> exponential curve. You need to make sure that the userspace remapping >> works for different backlights with different luminance ranges. That's >> not obvious to me. >> >>> We can of course provide a way for the user to plug in their own >>> actually measured data later. But IMO that doesn't even have to >>> happen in the initial implementation. Just need to avoid painting >>> ourselves totally in the corner in a way that would prevent later >>> additions like that. >> >> For userspace delivering its own curve, you need to define the units. >> Absolute or relative? Radiometric or perceptual? Otherwise the >> resulting control is not portable between window systems. >> >>> I just hate the current limbo where we're somehow too afraid to >>> change the current behaviour to do the remapping by default. >>> I see no upsides in the current behaviour of just blindly >>> exposing the raw hardware register values more or less. They >>> mean absolutely nothing to any user. >> >> I never argued like that. >> >> I'm saying that what looks realistic to me is somewhere *between* >> status quo and what Sebastian is asking for. Whatever you mean by "linear >> remapping" is probably a realistic goal, because you know you have
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Mon, Oct 03, 2022 at 11:37:50AM +0300, Pekka Paalanen wrote: > On Fri, 30 Sep 2022 18:17:39 +0200 > Sebastian Wick wrote: > > > On Fri, Sep 30, 2022 at 5:27 PM Pekka Paalanen wrote: > > > > > > On Fri, 30 Sep 2022 17:44:17 +0300 > > > Ville Syrjälä wrote: > > > > > > > On Fri, Sep 30, 2022 at 04:20:29PM +0200, Sebastian Wick wrote: > > > > > On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen > > > > > wrote: > > > > > > > > > > > > On Thu, 29 Sep 2022 20:06:50 +0200 > > > > > > Sebastian Wick wrote: > > > > > > > > > > > > > If it is supposed to be a non-linear luminance curve, which one > > > > > > > is it? > > > > > > > It would be much clearer if user space can control linear > > > > > > > luminance > > > > > > > and use whatever definition of perceived brightness it wants. The > > > > > > > obvious downside of it is that it requires bits to encode changes > > > > > > > that > > > > > > > users can't perceive. What about backlights which only have a few > > > > > > > predefined luminance levels? How would user space differentiate > > > > > > > between the continuous and discrete backlight? What about > > > > > > > self-emitting displays? They usually increase the dynamic range > > > > > > > when > > > > > > > they increase in brightness because the black level doesn't rise. > > > > > > > They > > > > > > > also probably employ some tonemapping to adjust for that. What > > > > > > > about > > > > > > > the range of the backlight? What about the absolute luminance of > > > > > > > the > > > > > > > backlight? I want to know about that all in user space. > > > > > > > > > > > > > > I understand that most of the time the kernel doesn't have > > > > > > > answers to > > > > > > > those questions right now but the API should account for all of > > > > > > > that. > > ... > > > > I'm saying that what looks realistic to me is somewhere *between* > > > status quo and what Sebastian is asking for. Whatever you mean by "linear > > > remapping" is probably a realistic goal, because you know you have some > > > hardware/firmware delivering that information already. > > > > > > OTOH, designing UAPI for information that exists only in our dreams > > > is... well. > > > > I also didn't say that we should design an UAPI for what doesn't > > exist, just that we should design the UAPI we actually need in a way > > that when we get more information we can properly expose that. So if > > the UAPI exposes anything other than the luminance (e.g. "each step is > > a perceptual step in brightness", "linear brightness", ..) we have to > > put some human perception model into the kernel which is ridiculous > > and it makes it impossible to expose luminance to user space if the > > kernel has that information. > > You don't need a human perception model in the kernel. You also cannot > have one, because I would expect most or all backlight and their > metadata to not define luminance at all. But that is just a guess. > > I suppose the firmware may expose some tables that may allow mapping > raw hardware values into something more pleasant to use. Like something > where each step is more or less a visible change. That does not have to > imply anything about linearity in any space, they may just be "good > values" for e.g. keyboard-based changing of backlight levels with no > mathematical or physical basis. > > Ville, what kind of tables do you know about? What do they actually > tell? We just get a set of points (up to 20 originally, and I think up to 32 in later spec revisions) that map input brightness (in %) to output duty cycle (0-0xff in old spec, 0-0x in new spec). And I *think* we're supposed to just linearly inteprolate between the entries, though the spec doesn't really state that in super clear terms. There is some mention in the spec about this being more or less designed for Windows Vista, so a look through eg. https://learn.microsoft.com/en-us/windows-hardware/drivers/display/supporting-brightness-controls-on-integrated-display-panels might be a good idea here. -- Ville Syrjälä Intel
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
Hi, On 9/28/22 12:57, Ville Syrjälä wrote: > On Wed, Sep 28, 2022 at 01:04:01PM +0300, Jani Nikula wrote: >> On Fri, 09 Sep 2022, Hans de Goede wrote: >>> Hi all, >>> >>> Here is v2 of my "drm/kms: control display brightness through drm_connector >>> properties" RFC: >>> >>> Changes from version 1: >>> - Drop bl_brightness_0_is_min_brightness from list of new connector >>> properties. >>> - Clearly define that 0 is always min-brightness when setting the brightness >>> through the connector properties. >>> - Drop bl_brightness_control_method from list of new connector >>> properties. >>> - Phase 1 of the plan has been completed >>> >>> 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: >>> >>> 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. >>> 3. The meaning of 0 is not clearly defined, it can be either off, >>>or minimum brightness at which the display is still readable >>>(in a low light environment) >>> 4. It's not possible to change both the gamma and the brightness in the >>>same KMS atomic commit. You'd want to be able to reduce brightness to >>>conserve power, and counter the effects of that by changing gamma to >>>reach a visually similar image. And you'd want to have the changes take >>>effect at the same time instead of reducing brightness at some frame and >>>change gamma at some other frame. This is pretty much impossible to do >>>via the sysfs interface. >>> >>> 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 plan to fix this was posted here: >>> https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ >>> And a pull-req actually implementing this plan has been send out this week: >>> https://lore.kernel.org/dri-devel/261afe3d-7790-e945-adf6-a2c96c9b1...@redhat.com/ >>> >>> >>> Phase 2: Add drm_connector properties mirroring the matching backlight >>> device >>> = >>> >>> The plan is to add a drm_connector helper function, which optionally takes >>> a pointer to the backlight device for the GPU's native backlight device, >>> which will then mirror the backlight settings from the backlight device >>> in a set of read/write brightness* properties on the connector. >>> >>> This function can then be called by GPU drivers for the drm_connector for >>> the internal panel and it will then take care of everything. When there >>> is no native GPU backlight device, or when it should not be used then >>> (on x86) the helper will use the acpi_video_get_backlight_type() to >>> determine which backlight-device should be used instead and it will find >>> + mirror that one. >>> >>> >>> Phase 3: Deprecate /sys/class/backlight uAPI >>> >>> >>> Once most userspace has moved over to using the new drm_connector >>> brightness props, a Kconfig option can be added to stop exporting >>> the backlight-devices under /sys/class/backlight. The plan is to >>> just disable the sysfs interface and keep the existing backlight-device >>> internal kernel abstraction as is, since some abstraction for (non GPU >>> native) backlight devices will be necessary regardless. >>> >>> It is unsure if we will ever be able to do this. For example people using >>> non fully integrated desktop environments like e.g. sway often use custom >>> scripts binded to hotkeys to get functionality like the brightness >>> up/down keyboard hotkeys changing the brightness. This typically involves >>> e.g. the xbacklight utility. >>> >>> Even if the xbacklight utility is ported to use kms with the new connector >>> object brightness properties then this still will not work because >>> changing the properties will require drm-master rights and e.g. sway will >>>
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
Hi, On 9/28/22 12:04, Jani Nikula wrote: > On Fri, 09 Sep 2022, Hans de Goede wrote: >> Hi all, >> >> Here is v2 of my "drm/kms: control display brightness through drm_connector >> properties" RFC: >> >> Changes from version 1: >> - Drop bl_brightness_0_is_min_brightness from list of new connector >> properties. >> - Clearly define that 0 is always min-brightness when setting the brightness >> through the connector properties. >> - Drop bl_brightness_control_method from list of new connector >> properties. >> - Phase 1 of the plan has been completed >> >> 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: >> >> 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. >> 3. The meaning of 0 is not clearly defined, it can be either off, >>or minimum brightness at which the display is still readable >>(in a low light environment) >> 4. It's not possible to change both the gamma and the brightness in the >>same KMS atomic commit. You'd want to be able to reduce brightness to >>conserve power, and counter the effects of that by changing gamma to >>reach a visually similar image. And you'd want to have the changes take >>effect at the same time instead of reducing brightness at some frame and >>change gamma at some other frame. This is pretty much impossible to do >>via the sysfs interface. >> >> 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 plan to fix this was posted here: >> https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ >> And a pull-req actually implementing this plan has been send out this week: >> https://lore.kernel.org/dri-devel/261afe3d-7790-e945-adf6-a2c96c9b1...@redhat.com/ >> >> >> Phase 2: Add drm_connector properties mirroring the matching backlight device >> = >> >> The plan is to add a drm_connector helper function, which optionally takes >> a pointer to the backlight device for the GPU's native backlight device, >> which will then mirror the backlight settings from the backlight device >> in a set of read/write brightness* properties on the connector. >> >> This function can then be called by GPU drivers for the drm_connector for >> the internal panel and it will then take care of everything. When there >> is no native GPU backlight device, or when it should not be used then >> (on x86) the helper will use the acpi_video_get_backlight_type() to >> determine which backlight-device should be used instead and it will find >> + mirror that one. >> >> >> Phase 3: Deprecate /sys/class/backlight uAPI >> >> >> Once most userspace has moved over to using the new drm_connector >> brightness props, a Kconfig option can be added to stop exporting >> the backlight-devices under /sys/class/backlight. The plan is to >> just disable the sysfs interface and keep the existing backlight-device >> internal kernel abstraction as is, since some abstraction for (non GPU >> native) backlight devices will be necessary regardless. >> >> It is unsure if we will ever be able to do this. For example people using >> non fully integrated desktop environments like e.g. sway often use custom >> scripts binded to hotkeys to get functionality like the brightness >> up/down keyboard hotkeys changing the brightness. This typically involves >> e.g. the xbacklight utility. >> >> Even if the xbacklight utility is ported to use kms with the new connector >> object brightness properties then this still will not work because >> changing the properties will require drm-master rights and e.g. sway will >> already hold those. >> >> >> The drm_connector brightness properties >> === >> >> The new uAPI for this consists of 2 properties: >>
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Fri, 30 Sep 2022 18:17:39 +0200 Sebastian Wick wrote: > On Fri, Sep 30, 2022 at 5:27 PM Pekka Paalanen wrote: > > > > On Fri, 30 Sep 2022 17:44:17 +0300 > > Ville Syrjälä wrote: > > > > > On Fri, Sep 30, 2022 at 04:20:29PM +0200, Sebastian Wick wrote: > > > > On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen > > > > wrote: > > > > > > > > > > On Thu, 29 Sep 2022 20:06:50 +0200 > > > > > Sebastian Wick wrote: > > > > > > > > > > > If it is supposed to be a non-linear luminance curve, which one is > > > > > > it? > > > > > > It would be much clearer if user space can control linear luminance > > > > > > and use whatever definition of perceived brightness it wants. The > > > > > > obvious downside of it is that it requires bits to encode changes > > > > > > that > > > > > > users can't perceive. What about backlights which only have a few > > > > > > predefined luminance levels? How would user space differentiate > > > > > > between the continuous and discrete backlight? What about > > > > > > self-emitting displays? They usually increase the dynamic range when > > > > > > they increase in brightness because the black level doesn't rise. > > > > > > They > > > > > > also probably employ some tonemapping to adjust for that. What about > > > > > > the range of the backlight? What about the absolute luminance of the > > > > > > backlight? I want to know about that all in user space. > > > > > > > > > > > > I understand that most of the time the kernel doesn't have answers > > > > > > to > > > > > > those questions right now but the API should account for all of > > > > > > that. ... > > I'm saying that what looks realistic to me is somewhere *between* > > status quo and what Sebastian is asking for. Whatever you mean by "linear > > remapping" is probably a realistic goal, because you know you have some > > hardware/firmware delivering that information already. > > > > OTOH, designing UAPI for information that exists only in our dreams > > is... well. > > I also didn't say that we should design an UAPI for what doesn't > exist, just that we should design the UAPI we actually need in a way > that when we get more information we can properly expose that. So if > the UAPI exposes anything other than the luminance (e.g. "each step is > a perceptual step in brightness", "linear brightness", ..) we have to > put some human perception model into the kernel which is ridiculous > and it makes it impossible to expose luminance to user space if the > kernel has that information. You don't need a human perception model in the kernel. You also cannot have one, because I would expect most or all backlight and their metadata to not define luminance at all. But that is just a guess. I suppose the firmware may expose some tables that may allow mapping raw hardware values into something more pleasant to use. Like something where each step is more or less a visible change. That does not have to imply anything about linearity in any space, they may just be "good values" for e.g. keyboard-based changing of backlight levels with no mathematical or physical basis. Ville, what kind of tables do you know about? What do they actually tell? Let's say we have these first properties defined as "reasonable steps for manual backlight control": one integer for the value, another integer for the max. If we later see that we can actually get more precise or high-level information, we can add new optional properties, e.g. a blob with table that maps the integers into some better defined quantity. Then we know what the values mean, but the steps may be quite coarse, much coarser than what the raw control value allows. That's the next problem: if we want as fine control as hardware is capable, how do you expose that? Should the answer be, that the exposed integer is actually a raw value for hardware, merely offsetted so that zero maps to minimum but visible backlight level, and then add another property from the start with a table for the "good values" for keyboard control? And the "good values" would be literally just that, no implication of linearity of anything. Thanks, pq pgpFr5nNsdCX0.pgp Description: OpenPGP digital signature
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Fri, Sep 30, 2022 at 5:27 PM Pekka Paalanen wrote: > > On Fri, 30 Sep 2022 17:44:17 +0300 > Ville Syrjälä wrote: > > > On Fri, Sep 30, 2022 at 04:20:29PM +0200, Sebastian Wick wrote: > > > On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen > > > wrote: > > > > > > > > On Thu, 29 Sep 2022 20:06:50 +0200 > > > > Sebastian Wick wrote: > > > > > > > > > If it is supposed to be a non-linear luminance curve, which one is it? > > > > > It would be much clearer if user space can control linear luminance > > > > > and use whatever definition of perceived brightness it wants. The > > > > > obvious downside of it is that it requires bits to encode changes that > > > > > users can't perceive. What about backlights which only have a few > > > > > predefined luminance levels? How would user space differentiate > > > > > between the continuous and discrete backlight? What about > > > > > self-emitting displays? They usually increase the dynamic range when > > > > > they increase in brightness because the black level doesn't rise. They > > > > > also probably employ some tonemapping to adjust for that. What about > > > > > the range of the backlight? What about the absolute luminance of the > > > > > backlight? I want to know about that all in user space. > > > > > > > > > > I understand that most of the time the kernel doesn't have answers to > > > > > those questions right now but the API should account for all of that. > > > > > > > > Hi, > > > > > > > > if the API accounts for all that, and the kernel doesn't know, then how > > > > can the API not lie? If the API sometimes lies, how could we ever trust > > > > it at all? > > > > > > Make it possible for the API to say "I don't know". I'd much rather > > > have an API tell me explicitly what it does and doesn't know instead > > > of having to guess what data I can actually rely on. > > > > > > For example if the kernel knows the luminance is linear on one display > > > and doesn't know anything about the other display and it exposes them > > > both in the same way I can not possibly write any code which relies on > > > exact control over the luminance for either display. > > > > > > > > > > > Personally I have the feeling that if we can even get to the level of > > > > "each step in the value is a more or less perceivable change", that > > > > would be good enough. Think of UI, e.g. hotkeys to change brightness. > > > > You'd expect almost every press to change it a bit. > > > > > > The nice thing is that you can have that even if you have no further > > > information about the brightness control and it might be good enough > > > for some use cases but it isn't for others. > > > > > > > If an end user wants defined and controlled luminance, I'd suggest they > > > > need to profile (physically measure) the response of the display at > > > > hand. This is no different from color profiling displays, but you need > > > > a measurement device that produces absolute measurements if absolute > > > > control is what they want. > > > > > > If that's the kind of user experience you're after, good for you. I > > > certainly want things to work out of the box which makes this just a > > > big no-go. > > > > I think if we have the information to make the default behaviour > > better then we should do that. Ie. if the firmaware gives us a > > table to remap the values for a more linear response we should > > make use of that by default. > > But that's only like 20% of what Sebastian is asking for. > > What's "linear"? Radiometric or perceptual? > > Radiometric linear control would make a terrible UX, so if the control > is radiometric, userspace needs to remap it. That might be a good > thing, but it's also complicated, because the relationship between > brightness and luminance is somewhere between a power curve and > exponential curve. You need to make sure that the userspace remapping > works for different backlights with different luminance ranges. That's > not obvious to me. > > > We can of course provide a way for the user to plug in their own > > actually measured data later. But IMO that doesn't even have to > > happen in the initial implementation. Just need to avoid painting > > ourselves totally in the corner in a way that would prevent later > > additions like that. > > For userspace delivering its own curve, you need to define the units. > Absolute or relative? Radiometric or perceptual? Otherwise the > resulting control is not portable between window systems. > > > I just hate the current limbo where we're somehow too afraid to > > change the current behaviour to do the remapping by default. > > I see no upsides in the current behaviour of just blindly > > exposing the raw hardware register values more or less. They > > mean absolutely nothing to any user. > > I never argued like that. > > I'm saying that what looks realistic to me is somewhere *between* > status quo and what Sebastian is asking for. Whatever you mean by "linear > remapping" is probably a
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Fri, 30 Sep 2022 17:44:17 +0300 Ville Syrjälä wrote: > On Fri, Sep 30, 2022 at 04:20:29PM +0200, Sebastian Wick wrote: > > On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen wrote: > > > > > > > > On Thu, 29 Sep 2022 20:06:50 +0200 > > > Sebastian Wick wrote: > > > > > > > If it is supposed to be a non-linear luminance curve, which one is it? > > > > It would be much clearer if user space can control linear luminance > > > > and use whatever definition of perceived brightness it wants. The > > > > obvious downside of it is that it requires bits to encode changes that > > > > users can't perceive. What about backlights which only have a few > > > > predefined luminance levels? How would user space differentiate > > > > between the continuous and discrete backlight? What about > > > > self-emitting displays? They usually increase the dynamic range when > > > > they increase in brightness because the black level doesn't rise. They > > > > also probably employ some tonemapping to adjust for that. What about > > > > the range of the backlight? What about the absolute luminance of the > > > > backlight? I want to know about that all in user space. > > > > > > > > I understand that most of the time the kernel doesn't have answers to > > > > those questions right now but the API should account for all of that. > > > > > > Hi, > > > > > > if the API accounts for all that, and the kernel doesn't know, then how > > > can the API not lie? If the API sometimes lies, how could we ever trust > > > it at all? > > > > Make it possible for the API to say "I don't know". I'd much rather > > have an API tell me explicitly what it does and doesn't know instead > > of having to guess what data I can actually rely on. > > > > For example if the kernel knows the luminance is linear on one display > > and doesn't know anything about the other display and it exposes them > > both in the same way I can not possibly write any code which relies on > > exact control over the luminance for either display. > > > > > > > > Personally I have the feeling that if we can even get to the level of > > > "each step in the value is a more or less perceivable change", that > > > would be good enough. Think of UI, e.g. hotkeys to change brightness. > > > You'd expect almost every press to change it a bit. > > > > The nice thing is that you can have that even if you have no further > > information about the brightness control and it might be good enough > > for some use cases but it isn't for others. > > > > > If an end user wants defined and controlled luminance, I'd suggest they > > > need to profile (physically measure) the response of the display at > > > hand. This is no different from color profiling displays, but you need > > > a measurement device that produces absolute measurements if absolute > > > control is what they want. > > > > If that's the kind of user experience you're after, good for you. I > > certainly want things to work out of the box which makes this just a > > big no-go. > > I think if we have the information to make the default behaviour > better then we should do that. Ie. if the firmaware gives us a > table to remap the values for a more linear response we should > make use of that by default. But that's only like 20% of what Sebastian is asking for. What's "linear"? Radiometric or perceptual? Radiometric linear control would make a terrible UX, so if the control is radiometric, userspace needs to remap it. That might be a good thing, but it's also complicated, because the relationship between brightness and luminance is somewhere between a power curve and exponential curve. You need to make sure that the userspace remapping works for different backlights with different luminance ranges. That's not obvious to me. > We can of course provide a way for the user to plug in their own > actually measured data later. But IMO that doesn't even have to > happen in the initial implementation. Just need to avoid painting > ourselves totally in the corner in a way that would prevent later > additions like that. For userspace delivering its own curve, you need to define the units. Absolute or relative? Radiometric or perceptual? Otherwise the resulting control is not portable between window systems. > I just hate the current limbo where we're somehow too afraid to > change the current behaviour to do the remapping by default. > I see no upsides in the current behaviour of just blindly > exposing the raw hardware register values more or less. They > mean absolutely nothing to any user. I never argued like that. I'm saying that what looks realistic to me is somewhere *between* status quo and what Sebastian is asking for. Whatever you mean by "linear remapping" is probably a realistic goal, because you know you have some hardware/firmware delivering that information already. OTOH, designing UAPI for information that exists only in our dreams is... well. Thanks, pq pgpM4fdr9toCF.pgp Description:
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Friday, September 30th, 2022 at 16:44, Ville Syrjälä wrote: > On Fri, Sep 30, 2022 at 04:20:29PM +0200, Sebastian Wick wrote: > > > On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen ppaala...@gmail.com wrote: > > > > > On Thu, 29 Sep 2022 20:06:50 +0200 > > > Sebastian Wick sebastian.w...@redhat.com wrote: > > > > > > > If it is supposed to be a non-linear luminance curve, which one is it? > > > > It would be much clearer if user space can control linear luminance > > > > and use whatever definition of perceived brightness it wants. The > > > > obvious downside of it is that it requires bits to encode changes that > > > > users can't perceive. What about backlights which only have a few > > > > predefined luminance levels? How would user space differentiate > > > > between the continuous and discrete backlight? What about > > > > self-emitting displays? They usually increase the dynamic range when > > > > they increase in brightness because the black level doesn't rise. They > > > > also probably employ some tonemapping to adjust for that. What about > > > > the range of the backlight? What about the absolute luminance of the > > > > backlight? I want to know about that all in user space. > > > > > > > > I understand that most of the time the kernel doesn't have answers to > > > > those questions right now but the API should account for all of that. > > > > > > Hi, > > > > > > if the API accounts for all that, and the kernel doesn't know, then how > > > can the API not lie? If the API sometimes lies, how could we ever trust > > > it at all? > > > > Make it possible for the API to say "I don't know". I'd much rather > > have an API tell me explicitly what it does and doesn't know instead > > of having to guess what data I can actually rely on. > > > > For example if the kernel knows the luminance is linear on one display > > and doesn't know anything about the other display and it exposes them > > both in the same way I can not possibly write any code which relies on > > exact control over the luminance for either display. > > > > > Personally I have the feeling that if we can even get to the level of > > > "each step in the value is a more or less perceivable change", that > > > would be good enough. Think of UI, e.g. hotkeys to change brightness. > > > You'd expect almost every press to change it a bit. > > > > The nice thing is that you can have that even if you have no further > > information about the brightness control and it might be good enough > > for some use cases but it isn't for others. > > > > > If an end user wants defined and controlled luminance, I'd suggest they > > > need to profile (physically measure) the response of the display at > > > hand. This is no different from color profiling displays, but you need > > > a measurement device that produces absolute measurements if absolute > > > control is what they want. > > > > If that's the kind of user experience you're after, good for you. I > > certainly want things to work out of the box which makes this just a > > big no-go. > > > I think if we have the information to make the default behaviour > better then we should do that. Ie. if the firmaware gives us a > table to remap the values for a more linear response we should > make use of that by default. > > We can of course provide a way for the user to plug in their own > actually measured data later. But IMO that doesn't even have to > happen in the initial implementation. Just need to avoid painting > ourselves totally in the corner in a way that would prevent later > additions like that. Yes. Please don't try to solve all of the problems at once. There have been many tries to add brightness to KMS, and having *something* would be a lot better than having nothing. If we try to have the perfect complete API from the start then we'll never get anything done.
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Fri, Sep 30, 2022 at 04:20:29PM +0200, Sebastian Wick wrote: > On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen wrote: > > > > On Thu, 29 Sep 2022 20:06:50 +0200 > > Sebastian Wick wrote: > > > > > If it is supposed to be a non-linear luminance curve, which one is it? > > > It would be much clearer if user space can control linear luminance > > > and use whatever definition of perceived brightness it wants. The > > > obvious downside of it is that it requires bits to encode changes that > > > users can't perceive. What about backlights which only have a few > > > predefined luminance levels? How would user space differentiate > > > between the continuous and discrete backlight? What about > > > self-emitting displays? They usually increase the dynamic range when > > > they increase in brightness because the black level doesn't rise. They > > > also probably employ some tonemapping to adjust for that. What about > > > the range of the backlight? What about the absolute luminance of the > > > backlight? I want to know about that all in user space. > > > > > > I understand that most of the time the kernel doesn't have answers to > > > those questions right now but the API should account for all of that. > > > > Hi, > > > > if the API accounts for all that, and the kernel doesn't know, then how > > can the API not lie? If the API sometimes lies, how could we ever trust > > it at all? > > Make it possible for the API to say "I don't know". I'd much rather > have an API tell me explicitly what it does and doesn't know instead > of having to guess what data I can actually rely on. > > For example if the kernel knows the luminance is linear on one display > and doesn't know anything about the other display and it exposes them > both in the same way I can not possibly write any code which relies on > exact control over the luminance for either display. > > > > > Personally I have the feeling that if we can even get to the level of > > "each step in the value is a more or less perceivable change", that > > would be good enough. Think of UI, e.g. hotkeys to change brightness. > > You'd expect almost every press to change it a bit. > > The nice thing is that you can have that even if you have no further > information about the brightness control and it might be good enough > for some use cases but it isn't for others. > > > If an end user wants defined and controlled luminance, I'd suggest they > > need to profile (physically measure) the response of the display at > > hand. This is no different from color profiling displays, but you need > > a measurement device that produces absolute measurements if absolute > > control is what they want. > > If that's the kind of user experience you're after, good for you. I > certainly want things to work out of the box which makes this just a > big no-go. I think if we have the information to make the default behaviour better then we should do that. Ie. if the firmaware gives us a table to remap the values for a more linear response we should make use of that by default. We can of course provide a way for the user to plug in their own actually measured data later. But IMO that doesn't even have to happen in the initial implementation. Just need to avoid painting ourselves totally in the corner in a way that would prevent later additions like that. I just hate the current limbo where we're somehow too afraid to change the current behaviour to do the remapping by default. I see no upsides in the current behaviour of just blindly exposing the raw hardware register values more or less. They mean absolutely nothing to any user. -- Ville Syrjälä Intel
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Fri, 30 Sep 2022, Sebastian Wick wrote: > On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen wrote: >> >> On Thu, 29 Sep 2022 20:06:50 +0200 >> Sebastian Wick wrote: >> >> > If it is supposed to be a non-linear luminance curve, which one is it? >> > It would be much clearer if user space can control linear luminance >> > and use whatever definition of perceived brightness it wants. The >> > obvious downside of it is that it requires bits to encode changes that >> > users can't perceive. What about backlights which only have a few >> > predefined luminance levels? How would user space differentiate >> > between the continuous and discrete backlight? What about >> > self-emitting displays? They usually increase the dynamic range when >> > they increase in brightness because the black level doesn't rise. They >> > also probably employ some tonemapping to adjust for that. What about >> > the range of the backlight? What about the absolute luminance of the >> > backlight? I want to know about that all in user space. >> > >> > I understand that most of the time the kernel doesn't have answers to >> > those questions right now but the API should account for all of that. >> >> Hi, >> >> if the API accounts for all that, and the kernel doesn't know, then how >> can the API not lie? If the API sometimes lies, how could we ever trust >> it at all? > > Make it possible for the API to say "I don't know". I'd much rather > have an API tell me explicitly what it does and doesn't know instead > of having to guess what data I can actually rely on. > > For example if the kernel knows the luminance is linear on one display > and doesn't know anything about the other display and it exposes them > both in the same way I can not possibly write any code which relies on > exact control over the luminance for either display. My idea has been all along that you'd have an API for defining points on a curve, a kind of mapping, and the kernel would linear intrapolate between the set points. If the kernel knows how to pre-fill the information, it would do so. Otherwise, it would just be linear. The userspace could adjust in either case. For i915, in some cases we'd be able to pre-fill the curve, and have a better brightness adjustment experience. Ville has done something like this for himself, but IIUC has not polished it into an interface. BR, Jani. > >> >> Personally I have the feeling that if we can even get to the level of >> "each step in the value is a more or less perceivable change", that >> would be good enough. Think of UI, e.g. hotkeys to change brightness. >> You'd expect almost every press to change it a bit. > > The nice thing is that you can have that even if you have no further > information about the brightness control and it might be good enough > for some use cases but it isn't for others. > >> If an end user wants defined and controlled luminance, I'd suggest they >> need to profile (physically measure) the response of the display at >> hand. This is no different from color profiling displays, but you need >> a measurement device that produces absolute measurements if absolute >> control is what they want. > > If that's the kind of user experience you're after, good for you. I > certainly want things to work out of the box which makes this just a > big no-go. > >> >> If there ever becomes an industry standard and conformance test >> definitions for luminance levels and backlight control, then things >> could be different. But until that, I believe trying to make one in the >> kernel is futile, because I have got the impression that there is >> practically no consistency between different displays in general. > > I'm aware that this is the current situation but it's one that must > change and we should at least try to create an API which still works > when we get more and better data. > >> >> Besides, I would expect some backlights to wear over time, grow dimmer >> for the same input value. Without a physical active feedback loop >> (measurements), it just won't work. >> >> If this is mostly for laptop displays, would end users even care? >> >> >> Thanks, >> pq >> >> > On Wed, Sep 28, 2022 at 1:14 PM Ville Syrjälä >> > wrote: >> > > >> > > On Wed, Sep 28, 2022 at 01:57:18PM +0300, Ville Syrjälä wrote: >> > > > On Wed, Sep 28, 2022 at 01:04:01PM +0300, Jani Nikula wrote: >> > > > > On Fri, 09 Sep 2022, Hans de Goede wrote: >> > > > > > Hi all, >> > > > > > >> > > > > > Here is v2 of my "drm/kms: control display brightness through >> > > > > > drm_connector properties" RFC: >> >> ... >> >> > > > > > Unlike the /sys/class/backlight/foo/brightness this brightness >> > > > > > property >> > > > > > has a clear definition for the value 0. The kernel must ensure >> > > > > > that 0 >> > > > > > means minimum brightness (so 0 should _never_ turn the backlight >> > > > > > off). >> > > > > > If necessary the kernel must enforce a minimum value by adding >> > > > > > an offset to the value seen in the property
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen wrote: > > On Thu, 29 Sep 2022 20:06:50 +0200 > Sebastian Wick wrote: > > > If it is supposed to be a non-linear luminance curve, which one is it? > > It would be much clearer if user space can control linear luminance > > and use whatever definition of perceived brightness it wants. The > > obvious downside of it is that it requires bits to encode changes that > > users can't perceive. What about backlights which only have a few > > predefined luminance levels? How would user space differentiate > > between the continuous and discrete backlight? What about > > self-emitting displays? They usually increase the dynamic range when > > they increase in brightness because the black level doesn't rise. They > > also probably employ some tonemapping to adjust for that. What about > > the range of the backlight? What about the absolute luminance of the > > backlight? I want to know about that all in user space. > > > > I understand that most of the time the kernel doesn't have answers to > > those questions right now but the API should account for all of that. > > Hi, > > if the API accounts for all that, and the kernel doesn't know, then how > can the API not lie? If the API sometimes lies, how could we ever trust > it at all? Make it possible for the API to say "I don't know". I'd much rather have an API tell me explicitly what it does and doesn't know instead of having to guess what data I can actually rely on. For example if the kernel knows the luminance is linear on one display and doesn't know anything about the other display and it exposes them both in the same way I can not possibly write any code which relies on exact control over the luminance for either display. > > Personally I have the feeling that if we can even get to the level of > "each step in the value is a more or less perceivable change", that > would be good enough. Think of UI, e.g. hotkeys to change brightness. > You'd expect almost every press to change it a bit. The nice thing is that you can have that even if you have no further information about the brightness control and it might be good enough for some use cases but it isn't for others. > If an end user wants defined and controlled luminance, I'd suggest they > need to profile (physically measure) the response of the display at > hand. This is no different from color profiling displays, but you need > a measurement device that produces absolute measurements if absolute > control is what they want. If that's the kind of user experience you're after, good for you. I certainly want things to work out of the box which makes this just a big no-go. > > If there ever becomes an industry standard and conformance test > definitions for luminance levels and backlight control, then things > could be different. But until that, I believe trying to make one in the > kernel is futile, because I have got the impression that there is > practically no consistency between different displays in general. I'm aware that this is the current situation but it's one that must change and we should at least try to create an API which still works when we get more and better data. > > Besides, I would expect some backlights to wear over time, grow dimmer > for the same input value. Without a physical active feedback loop > (measurements), it just won't work. > > If this is mostly for laptop displays, would end users even care? > > > Thanks, > pq > > > On Wed, Sep 28, 2022 at 1:14 PM Ville Syrjälä > > wrote: > > > > > > On Wed, Sep 28, 2022 at 01:57:18PM +0300, Ville Syrjälä wrote: > > > > On Wed, Sep 28, 2022 at 01:04:01PM +0300, Jani Nikula wrote: > > > > > On Fri, 09 Sep 2022, Hans de Goede wrote: > > > > > > Hi all, > > > > > > > > > > > > Here is v2 of my "drm/kms: control display brightness through > > > > > > drm_connector properties" RFC: > > ... > > > > > > > Unlike the /sys/class/backlight/foo/brightness this brightness > > > > > > property > > > > > > has a clear definition for the value 0. The kernel must ensure that > > > > > > 0 > > > > > > means minimum brightness (so 0 should _never_ turn the backlight > > > > > > off). > > > > > > If necessary the kernel must enforce a minimum value by adding > > > > > > an offset to the value seen in the property to ensure this behavior. > > > > > > > > > > > > For example if necessary the driver must clamp 0-255 to 10-255, > > > > > > which then > > > > > > becomes 0-245 on the brightness property, adding 10 internally to > > > > > > writes > > > > > > done to the brightness property. This adding of an extra offset when > > > > > > necessary must only be done on the brightness property, > > > > > > the /sys/class/backlight interface should be left unchanged to not > > > > > > break > > > > > > userspace which may rely on 0 = off on some systems. > > > > > > > > > > > > Note amdgpu already does something like this even for > > > > > > /sys/class/backlight, > > > > > > see the use of
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Thu, 29 Sep 2022 20:06:50 +0200 Sebastian Wick wrote: > If it is supposed to be a non-linear luminance curve, which one is it? > It would be much clearer if user space can control linear luminance > and use whatever definition of perceived brightness it wants. The > obvious downside of it is that it requires bits to encode changes that > users can't perceive. What about backlights which only have a few > predefined luminance levels? How would user space differentiate > between the continuous and discrete backlight? What about > self-emitting displays? They usually increase the dynamic range when > they increase in brightness because the black level doesn't rise. They > also probably employ some tonemapping to adjust for that. What about > the range of the backlight? What about the absolute luminance of the > backlight? I want to know about that all in user space. > > I understand that most of the time the kernel doesn't have answers to > those questions right now but the API should account for all of that. Hi, if the API accounts for all that, and the kernel doesn't know, then how can the API not lie? If the API sometimes lies, how could we ever trust it at all? Personally I have the feeling that if we can even get to the level of "each step in the value is a more or less perceivable change", that would be good enough. Think of UI, e.g. hotkeys to change brightness. You'd expect almost every press to change it a bit. If an end user wants defined and controlled luminance, I'd suggest they need to profile (physically measure) the response of the display at hand. This is no different from color profiling displays, but you need a measurement device that produces absolute measurements if absolute control is what they want. If there ever becomes an industry standard and conformance test definitions for luminance levels and backlight control, then things could be different. But until that, I believe trying to make one in the kernel is futile, because I have got the impression that there is practically no consistency between different displays in general. Besides, I would expect some backlights to wear over time, grow dimmer for the same input value. Without a physical active feedback loop (measurements), it just won't work. If this is mostly for laptop displays, would end users even care? Thanks, pq > On Wed, Sep 28, 2022 at 1:14 PM Ville Syrjälä > wrote: > > > > On Wed, Sep 28, 2022 at 01:57:18PM +0300, Ville Syrjälä wrote: > > > On Wed, Sep 28, 2022 at 01:04:01PM +0300, Jani Nikula wrote: > > > > On Fri, 09 Sep 2022, Hans de Goede wrote: > > > > > Hi all, > > > > > > > > > > Here is v2 of my "drm/kms: control display brightness through > > > > > drm_connector properties" RFC: ... > > > > > Unlike the /sys/class/backlight/foo/brightness this brightness > > > > > property > > > > > has a clear definition for the value 0. The kernel must ensure that 0 > > > > > means minimum brightness (so 0 should _never_ turn the backlight off). > > > > > If necessary the kernel must enforce a minimum value by adding > > > > > an offset to the value seen in the property to ensure this behavior. > > > > > > > > > > For example if necessary the driver must clamp 0-255 to 10-255, which > > > > > then > > > > > becomes 0-245 on the brightness property, adding 10 internally to > > > > > writes > > > > > done to the brightness property. This adding of an extra offset when > > > > > necessary must only be done on the brightness property, > > > > > the /sys/class/backlight interface should be left unchanged to not > > > > > break > > > > > userspace which may rely on 0 = off on some systems. > > > > > > > > > > Note amdgpu already does something like this even for > > > > > /sys/class/backlight, > > > > > see the use of AMDGPU_DM_DEFAULT_MIN_BACKLIGHT in amdgpu. > > > > > > > > > > Also whenever possible the kernel must ensure that the brightness > > > > > range > > > > > is in perceived brightness, but this cannot always be guaranteed. > > > > > > > > Do you mean every step should be a visible change? > > > > > > Hmm. I guess due to this. I'd prefer the opposite tbh so I could > > > just put in my opregion BCLM patch. It's annoying to have to > > > carry it locally just to have reasonable backlight behaviour > > > > After second though I guess I'm actually agreeing with Hans here. > > The current situation is where small change in the value near one > > end of the range does basically nothing, while a small change at > > the other of the range causes a massive brightness change. That > > is no good. > > > > -- > > Ville Syrjälä > > Intel > > > pgptE1MP8P5pr.pgp Description: OpenPGP digital signature
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
If it is supposed to be a non-linear luminance curve, which one is it? It would be much clearer if user space can control linear luminance and use whatever definition of perceived brightness it wants. The obvious downside of it is that it requires bits to encode changes that users can't perceive. What about backlights which only have a few predefined luminance levels? How would user space differentiate between the continuous and discrete backlight? What about self-emitting displays? They usually increase the dynamic range when they increase in brightness because the black level doesn't rise. They also probably employ some tonemapping to adjust for that. What about the range of the backlight? What about the absolute luminance of the backlight? I want to know about that all in user space. I understand that most of the time the kernel doesn't have answers to those questions right now but the API should account for all of that. On Wed, Sep 28, 2022 at 1:14 PM Ville Syrjälä wrote: > > On Wed, Sep 28, 2022 at 01:57:18PM +0300, Ville Syrjälä wrote: > > On Wed, Sep 28, 2022 at 01:04:01PM +0300, Jani Nikula wrote: > > > On Fri, 09 Sep 2022, Hans de Goede wrote: > > > > Hi all, > > > > > > > > Here is v2 of my "drm/kms: control display brightness through > > > > drm_connector properties" RFC: > > > > > > > > Changes from version 1: > > > > - Drop bl_brightness_0_is_min_brightness from list of new connector > > > > properties. > > > > - Clearly define that 0 is always min-brightness when setting the > > > > brightness > > > > through the connector properties. > > > > - Drop bl_brightness_control_method from list of new connector > > > > properties. > > > > - Phase 1 of the plan has been completed > > > > > > > > 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: > > > > > > > > 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. > > > > 3. The meaning of 0 is not clearly defined, it can be either off, > > > >or minimum brightness at which the display is still readable > > > >(in a low light environment) > > > > 4. It's not possible to change both the gamma and the brightness in the > > > >same KMS atomic commit. You'd want to be able to reduce brightness to > > > >conserve power, and counter the effects of that by changing gamma to > > > >reach a visually similar image. And you'd want to have the changes > > > > take > > > >effect at the same time instead of reducing brightness at some frame > > > > and > > > >change gamma at some other frame. This is pretty much impossible to > > > > do > > > >via the sysfs interface. > > > > > > > > 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 plan to fix this was posted here: > > > > https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ > > > > And a pull-req actually implementing this plan has been send out this > > > > week: > > > > https://lore.kernel.org/dri-devel/261afe3d-7790-e945-adf6-a2c96c9b1...@redhat.com/ > > > > > > > > > > > > Phase 2: Add drm_connector properties mirroring the matching backlight > > > > device > > > > = > > > > > > > > The plan is to add a drm_connector helper function, which optionally > > > > takes > > > > a pointer to the backlight device for the GPU's native backlight device, > > > > which will then mirror the backlight settings from the backlight device > > > > in a set of read/write brightness* properties on the connector. > > > > > > > > This function can then be called by GPU drivers for the drm_connector > > > > for > > > > the internal panel and it will then take care of
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Wed, Sep 28, 2022 at 01:57:18PM +0300, Ville Syrjälä wrote: > On Wed, Sep 28, 2022 at 01:04:01PM +0300, Jani Nikula wrote: > > On Fri, 09 Sep 2022, Hans de Goede wrote: > > > Hi all, > > > > > > Here is v2 of my "drm/kms: control display brightness through > > > drm_connector properties" RFC: > > > > > > Changes from version 1: > > > - Drop bl_brightness_0_is_min_brightness from list of new connector > > > properties. > > > - Clearly define that 0 is always min-brightness when setting the > > > brightness > > > through the connector properties. > > > - Drop bl_brightness_control_method from list of new connector > > > properties. > > > - Phase 1 of the plan has been completed > > > > > > 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: > > > > > > 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. > > > 3. The meaning of 0 is not clearly defined, it can be either off, > > >or minimum brightness at which the display is still readable > > >(in a low light environment) > > > 4. It's not possible to change both the gamma and the brightness in the > > >same KMS atomic commit. You'd want to be able to reduce brightness to > > >conserve power, and counter the effects of that by changing gamma to > > >reach a visually similar image. And you'd want to have the changes take > > >effect at the same time instead of reducing brightness at some frame > > > and > > >change gamma at some other frame. This is pretty much impossible to do > > >via the sysfs interface. > > > > > > 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 plan to fix this was posted here: > > > https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ > > > And a pull-req actually implementing this plan has been send out this > > > week: > > > https://lore.kernel.org/dri-devel/261afe3d-7790-e945-adf6-a2c96c9b1...@redhat.com/ > > > > > > > > > Phase 2: Add drm_connector properties mirroring the matching backlight > > > device > > > = > > > > > > The plan is to add a drm_connector helper function, which optionally takes > > > a pointer to the backlight device for the GPU's native backlight device, > > > which will then mirror the backlight settings from the backlight device > > > in a set of read/write brightness* properties on the connector. > > > > > > This function can then be called by GPU drivers for the drm_connector for > > > the internal panel and it will then take care of everything. When there > > > is no native GPU backlight device, or when it should not be used then > > > (on x86) the helper will use the acpi_video_get_backlight_type() to > > > determine which backlight-device should be used instead and it will find > > > + mirror that one. > > > > > > > > > Phase 3: Deprecate /sys/class/backlight uAPI > > > > > > > > > Once most userspace has moved over to using the new drm_connector > > > brightness props, a Kconfig option can be added to stop exporting > > > the backlight-devices under /sys/class/backlight. The plan is to > > > just disable the sysfs interface and keep the existing backlight-device > > > internal kernel abstraction as is, since some abstraction for (non GPU > > > native) backlight devices will be necessary regardless. > > > > > > It is unsure if we will ever be able to do this. For example people using > > > non fully integrated desktop environments like e.g. sway often use custom > > > scripts binded to hotkeys to get functionality like the brightness > > > up/down keyboard hotkeys changing the brightness. This typically involves > > > e.g. the xbacklight utility. > > > > >
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Wed, Sep 28, 2022 at 01:04:01PM +0300, Jani Nikula wrote: > On Fri, 09 Sep 2022, Hans de Goede wrote: > > Hi all, > > > > Here is v2 of my "drm/kms: control display brightness through drm_connector > > properties" RFC: > > > > Changes from version 1: > > - Drop bl_brightness_0_is_min_brightness from list of new connector > > properties. > > - Clearly define that 0 is always min-brightness when setting the brightness > > through the connector properties. > > - Drop bl_brightness_control_method from list of new connector > > properties. > > - Phase 1 of the plan has been completed > > > > 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: > > > > 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. > > 3. The meaning of 0 is not clearly defined, it can be either off, > >or minimum brightness at which the display is still readable > >(in a low light environment) > > 4. It's not possible to change both the gamma and the brightness in the > >same KMS atomic commit. You'd want to be able to reduce brightness to > >conserve power, and counter the effects of that by changing gamma to > >reach a visually similar image. And you'd want to have the changes take > >effect at the same time instead of reducing brightness at some frame and > >change gamma at some other frame. This is pretty much impossible to do > >via the sysfs interface. > > > > 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 plan to fix this was posted here: > > https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ > > And a pull-req actually implementing this plan has been send out this week: > > https://lore.kernel.org/dri-devel/261afe3d-7790-e945-adf6-a2c96c9b1...@redhat.com/ > > > > > > Phase 2: Add drm_connector properties mirroring the matching backlight > > device > > = > > > > The plan is to add a drm_connector helper function, which optionally takes > > a pointer to the backlight device for the GPU's native backlight device, > > which will then mirror the backlight settings from the backlight device > > in a set of read/write brightness* properties on the connector. > > > > This function can then be called by GPU drivers for the drm_connector for > > the internal panel and it will then take care of everything. When there > > is no native GPU backlight device, or when it should not be used then > > (on x86) the helper will use the acpi_video_get_backlight_type() to > > determine which backlight-device should be used instead and it will find > > + mirror that one. > > > > > > Phase 3: Deprecate /sys/class/backlight uAPI > > > > > > Once most userspace has moved over to using the new drm_connector > > brightness props, a Kconfig option can be added to stop exporting > > the backlight-devices under /sys/class/backlight. The plan is to > > just disable the sysfs interface and keep the existing backlight-device > > internal kernel abstraction as is, since some abstraction for (non GPU > > native) backlight devices will be necessary regardless. > > > > It is unsure if we will ever be able to do this. For example people using > > non fully integrated desktop environments like e.g. sway often use custom > > scripts binded to hotkeys to get functionality like the brightness > > up/down keyboard hotkeys changing the brightness. This typically involves > > e.g. the xbacklight utility. > > > > Even if the xbacklight utility is ported to use kms with the new connector > > object brightness properties then this still will not work because > > changing the properties will require drm-master rights and e.g. sway will > > already hold those. > > > > > > The
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Fri, 09 Sep 2022, Hans de Goede wrote: > Hi all, > > Here is v2 of my "drm/kms: control display brightness through drm_connector > properties" RFC: > > Changes from version 1: > - Drop bl_brightness_0_is_min_brightness from list of new connector > properties. > - Clearly define that 0 is always min-brightness when setting the brightness > through the connector properties. > - Drop bl_brightness_control_method from list of new connector > properties. > - Phase 1 of the plan has been completed > > 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: > > 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. > 3. The meaning of 0 is not clearly defined, it can be either off, >or minimum brightness at which the display is still readable >(in a low light environment) > 4. It's not possible to change both the gamma and the brightness in the >same KMS atomic commit. You'd want to be able to reduce brightness to >conserve power, and counter the effects of that by changing gamma to >reach a visually similar image. And you'd want to have the changes take >effect at the same time instead of reducing brightness at some frame and >change gamma at some other frame. This is pretty much impossible to do >via the sysfs interface. > > 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 plan to fix this was posted here: > https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ > And a pull-req actually implementing this plan has been send out this week: > https://lore.kernel.org/dri-devel/261afe3d-7790-e945-adf6-a2c96c9b1...@redhat.com/ > > > Phase 2: Add drm_connector properties mirroring the matching backlight device > = > > The plan is to add a drm_connector helper function, which optionally takes > a pointer to the backlight device for the GPU's native backlight device, > which will then mirror the backlight settings from the backlight device > in a set of read/write brightness* properties on the connector. > > This function can then be called by GPU drivers for the drm_connector for > the internal panel and it will then take care of everything. When there > is no native GPU backlight device, or when it should not be used then > (on x86) the helper will use the acpi_video_get_backlight_type() to > determine which backlight-device should be used instead and it will find > + mirror that one. > > > Phase 3: Deprecate /sys/class/backlight uAPI > > > Once most userspace has moved over to using the new drm_connector > brightness props, a Kconfig option can be added to stop exporting > the backlight-devices under /sys/class/backlight. The plan is to > just disable the sysfs interface and keep the existing backlight-device > internal kernel abstraction as is, since some abstraction for (non GPU > native) backlight devices will be necessary regardless. > > It is unsure if we will ever be able to do this. For example people using > non fully integrated desktop environments like e.g. sway often use custom > scripts binded to hotkeys to get functionality like the brightness > up/down keyboard hotkeys changing the brightness. This typically involves > e.g. the xbacklight utility. > > Even if the xbacklight utility is ported to use kms with the new connector > object brightness properties then this still will not work because > changing the properties will require drm-master rights and e.g. sway will > already hold those. > > > The drm_connector brightness properties > === > > The new uAPI for this consists of 2 properties: > > 1. "display brightness": rw 0-int32_max property controlling the brightness > setting > of the connected display. The actual maximum of this
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
Hi, On 9/9/22 15:39, Simon Ser wrote: > On Friday, September 9th, 2022 at 12:12, Hans de Goede > wrote: > >> Phase 3: Deprecate /sys/class/backlight uAPI >> >> >> Once most userspace has moved over to using the new drm_connector >> brightness props, a Kconfig option can be added to stop exporting >> the backlight-devices under /sys/class/backlight. The plan is to >> just disable the sysfs interface and keep the existing backlight-device >> internal kernel abstraction as is, since some abstraction for (non GPU >> native) backlight devices will be necessary regardless. >> >> It is unsure if we will ever be able to do this. For example people using >> non fully integrated desktop environments like e.g. sway often use custom >> scripts binded to hotkeys to get functionality like the brightness >> up/down keyboard hotkeys changing the brightness. This typically involves >> e.g. the xbacklight utility. >> >> Even if the xbacklight utility is ported to use kms with the new connector >> object brightness properties then this still will not work because >> changing the properties will require drm-master rights and e.g. sway will >> already hold those. > > I replied to this here in another thread [1]. > > tl;dr I think it would be fine even for Sway-like compositors. Ok, that is good to know. > (Also note the utilities used right now are not xbacklight, but > brightnessctl/light/brillo/etc [2]) Ah I thought that xbacklight got patched at one point to support the sysfs API, but I see now that instead alternative utilities have popped up. Regards, Hans > > [1]: > https://lore.kernel.org/dri-devel/bZJU9OkYWFyaLHVa4XUE4d5iBTPFXBRyPe1wMd_ztKh5VBMu-EDNGoUDpvwtFn_u9-JMvN8QmIZVS3pzMZM_hZTkTCA9gOBnCGXc5HFmsnc=@emersion.fr/ > [2]: https://github.com/swaywm/sway/wiki#xbacklight > >> The drm_connector brightness properties >> === >> >> The new uAPI for this consists of 2 properties: >> >> 1. "display 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 "display brightness max". >> >> Unlike the /sys/class/backlight/foo/brightness this brightness property >> has a clear definition for the value 0. The kernel must ensure that 0 >> means minimum brightness (so 0 should never turn the backlight off). >> If necessary the kernel must enforce a minimum value by adding >> an offset to the value seen in the property to ensure this behavior. >> >> For example if necessary the driver must clamp 0-255 to 10-255, which then >> becomes 0-245 on the brightness property, adding 10 internally to writes >> done to the brightness property. This adding of an extra offset when >> necessary must only be done on the brightness property, >> the /sys/class/backlight interface should be left unchanged to not break >> userspace which may rely on 0 = off on some systems. >> >> Note amdgpu already does something like this even for /sys/class/backlight, >> see the use of AMDGPU_DM_DEFAULT_MIN_BACKLIGHT in amdgpu. >> >> Also whenever possible the kernel must ensure that the brightness range >> is in perceived brightness, but this cannot always be guaranteed. >> >> >> 2. "display 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. >> >> The value of "display brightness max" may change at runtime, either by >> a legacy drivers/platform/x86 backlight driver loading after the drm >> driver has loaded; or when plugging in a monitor which allows brightness >> control over DDC/CI. In both these cases the max value will change from 0 >> to the actual max value, indicating backlight control has become available >> on this connector. > > The kernel will need to ensure that a hotplug uevent is sent to > user-space at this point. Otherwise user-space has no way to figure out > that the prop has changed. > > Overall this all looks very solid to me! > > Simon >
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Friday, September 9th, 2022 at 15:53, Pekka Paalanen wrote: > On Fri, 09 Sep 2022 13:39:37 + > Simon Ser cont...@emersion.fr wrote: > > > On Friday, September 9th, 2022 at 12:12, Hans de Goede hdego...@redhat.com > > wrote: > > > > > Phase 3: Deprecate /sys/class/backlight uAPI > > > > > > > > > Once most userspace has moved over to using the new drm_connector > > > brightness props, a Kconfig option can be added to stop exporting > > > the backlight-devices under /sys/class/backlight. The plan is to > > > just disable the sysfs interface and keep the existing backlight-device > > > internal kernel abstraction as is, since some abstraction for (non GPU > > > native) backlight devices will be necessary regardless. > > > > > > It is unsure if we will ever be able to do this. For example people using > > > non fully integrated desktop environments like e.g. sway often use custom > > > scripts binded to hotkeys to get functionality like the brightness > > > up/down keyboard hotkeys changing the brightness. This typically involves > > > e.g. the xbacklight utility. > > > > > > Even if the xbacklight utility is ported to use kms with the new connector > > > object brightness properties then this still will not work because > > > changing the properties will require drm-master rights and e.g. sway will > > > already hold those. > > > > I replied to this here in another thread 1. > > > > tl;dr I think it would be fine even for Sway-like compositors. > > Furthermore, if other compositors are like Weston in their KMS state > handling, and do not track which property has already been programmed > to KMS and which hasn't, and instead just smash all KMS properties > every update anyway (it's also great for debugging, you always have the > full state in flight), anything changed via sysfs will be immediately > reverted. > > Therefore I think there is a high probability that the external or > sysfs controls just naturally stop working anyway, even if the kernel > does not remove them first. Ah yes, that's a good point. And this wouldn't be a kernel regression, it would be user-space like Sway or Weston taking the decision to use the new uAPI in a way which breaks the sysfs interface. (User-space could also take the decision to _not_ break the sysfs interface, by implementing a simple "dirty" flag.)
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Fri, 09 Sep 2022 13:39:37 + Simon Ser wrote: > On Friday, September 9th, 2022 at 12:12, Hans de Goede > wrote: > > > Phase 3: Deprecate /sys/class/backlight uAPI > > > > > > Once most userspace has moved over to using the new drm_connector > > brightness props, a Kconfig option can be added to stop exporting > > the backlight-devices under /sys/class/backlight. The plan is to > > just disable the sysfs interface and keep the existing backlight-device > > internal kernel abstraction as is, since some abstraction for (non GPU > > native) backlight devices will be necessary regardless. > > > > It is unsure if we will ever be able to do this. For example people using > > non fully integrated desktop environments like e.g. sway often use custom > > scripts binded to hotkeys to get functionality like the brightness > > up/down keyboard hotkeys changing the brightness. This typically involves > > e.g. the xbacklight utility. > > > > Even if the xbacklight utility is ported to use kms with the new connector > > object brightness properties then this still will not work because > > changing the properties will require drm-master rights and e.g. sway will > > already hold those. > > I replied to this here in another thread [1]. > > tl;dr I think it would be fine even for Sway-like compositors. Furthermore, if other compositors are like Weston in their KMS state handling, and do not track which property has already been programmed to KMS and which hasn't, and instead just smash all KMS properties every update anyway (it's also great for debugging, you always have the full state in flight), anything changed via sysfs will be immediately reverted. Therefore I think there is a high probability that the external or sysfs controls just naturally stop working anyway, even if the kernel does not remove them first. Thanks, pq > (Also note the utilities used right now are not xbacklight, but > brightnessctl/light/brillo/etc [2]) > > [1]: > https://lore.kernel.org/dri-devel/bZJU9OkYWFyaLHVa4XUE4d5iBTPFXBRyPe1wMd_ztKh5VBMu-EDNGoUDpvwtFn_u9-JMvN8QmIZVS3pzMZM_hZTkTCA9gOBnCGXc5HFmsnc=@emersion.fr/ > [2]: https://github.com/swaywm/sway/wiki#xbacklight > pgpWmDQJXU6Wl.pgp Description: OpenPGP digital signature
Re: [RFC v2] drm/kms: control display brightness through drm_connector properties
On Friday, September 9th, 2022 at 12:12, Hans de Goede wrote: > Phase 3: Deprecate /sys/class/backlight uAPI > > > Once most userspace has moved over to using the new drm_connector > brightness props, a Kconfig option can be added to stop exporting > the backlight-devices under /sys/class/backlight. The plan is to > just disable the sysfs interface and keep the existing backlight-device > internal kernel abstraction as is, since some abstraction for (non GPU > native) backlight devices will be necessary regardless. > > It is unsure if we will ever be able to do this. For example people using > non fully integrated desktop environments like e.g. sway often use custom > scripts binded to hotkeys to get functionality like the brightness > up/down keyboard hotkeys changing the brightness. This typically involves > e.g. the xbacklight utility. > > Even if the xbacklight utility is ported to use kms with the new connector > object brightness properties then this still will not work because > changing the properties will require drm-master rights and e.g. sway will > already hold those. I replied to this here in another thread [1]. tl;dr I think it would be fine even for Sway-like compositors. (Also note the utilities used right now are not xbacklight, but brightnessctl/light/brillo/etc [2]) [1]: https://lore.kernel.org/dri-devel/bZJU9OkYWFyaLHVa4XUE4d5iBTPFXBRyPe1wMd_ztKh5VBMu-EDNGoUDpvwtFn_u9-JMvN8QmIZVS3pzMZM_hZTkTCA9gOBnCGXc5HFmsnc=@emersion.fr/ [2]: https://github.com/swaywm/sway/wiki#xbacklight > The drm_connector brightness properties > === > > The new uAPI for this consists of 2 properties: > > 1. "display 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 "display brightness max". > > Unlike the /sys/class/backlight/foo/brightness this brightness property > has a clear definition for the value 0. The kernel must ensure that 0 > means minimum brightness (so 0 should never turn the backlight off). > If necessary the kernel must enforce a minimum value by adding > an offset to the value seen in the property to ensure this behavior. > > For example if necessary the driver must clamp 0-255 to 10-255, which then > becomes 0-245 on the brightness property, adding 10 internally to writes > done to the brightness property. This adding of an extra offset when > necessary must only be done on the brightness property, > the /sys/class/backlight interface should be left unchanged to not break > userspace which may rely on 0 = off on some systems. > > Note amdgpu already does something like this even for /sys/class/backlight, > see the use of AMDGPU_DM_DEFAULT_MIN_BACKLIGHT in amdgpu. > > Also whenever possible the kernel must ensure that the brightness range > is in perceived brightness, but this cannot always be guaranteed. > > > 2. "display 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. > > The value of "display brightness max" may change at runtime, either by > a legacy drivers/platform/x86 backlight driver loading after the drm > driver has loaded; or when plugging in a monitor which allows brightness > control over DDC/CI. In both these cases the max value will change from 0 > to the actual max value, indicating backlight control has become available > on this connector. The kernel will need to ensure that a hotplug uevent is sent to user-space at this point. Otherwise user-space has no way to figure out that the prop has changed. Overall this all looks very solid to me! Simon
[RFC v2] drm/kms: control display brightness through drm_connector properties
Hi all, Here is v2 of my "drm/kms: control display brightness through drm_connector properties" RFC: Changes from version 1: - Drop bl_brightness_0_is_min_brightness from list of new connector properties. - Clearly define that 0 is always min-brightness when setting the brightness through the connector properties. - Drop bl_brightness_control_method from list of new connector properties. - Phase 1 of the plan has been completed 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: 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. 3. The meaning of 0 is not clearly defined, it can be either off, or minimum brightness at which the display is still readable (in a low light environment) 4. It's not possible to change both the gamma and the brightness in the same KMS atomic commit. You'd want to be able to reduce brightness to conserve power, and counter the effects of that by changing gamma to reach a visually similar image. And you'd want to have the changes take effect at the same time instead of reducing brightness at some frame and change gamma at some other frame. This is pretty much impossible to do via the sysfs interface. 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 plan to fix this was posted here: https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/ And a pull-req actually implementing this plan has been send out this week: https://lore.kernel.org/dri-devel/261afe3d-7790-e945-adf6-a2c96c9b1...@redhat.com/ Phase 2: Add drm_connector properties mirroring the matching backlight device = The plan is to add a drm_connector helper function, which optionally takes a pointer to the backlight device for the GPU's native backlight device, which will then mirror the backlight settings from the backlight device in a set of read/write brightness* properties on the connector. This function can then be called by GPU drivers for the drm_connector for the internal panel and it will then take care of everything. When there is no native GPU backlight device, or when it should not be used then (on x86) the helper will use the acpi_video_get_backlight_type() to determine which backlight-device should be used instead and it will find + mirror that one. Phase 3: Deprecate /sys/class/backlight uAPI Once most userspace has moved over to using the new drm_connector brightness props, a Kconfig option can be added to stop exporting the backlight-devices under /sys/class/backlight. The plan is to just disable the sysfs interface and keep the existing backlight-device internal kernel abstraction as is, since some abstraction for (non GPU native) backlight devices will be necessary regardless. It is unsure if we will ever be able to do this. For example people using non fully integrated desktop environments like e.g. sway often use custom scripts binded to hotkeys to get functionality like the brightness up/down keyboard hotkeys changing the brightness. This typically involves e.g. the xbacklight utility. Even if the xbacklight utility is ported to use kms with the new connector object brightness properties then this still will not work because changing the properties will require drm-master rights and e.g. sway will already hold those. The drm_connector brightness properties === The new uAPI for this consists of 2 properties: 1. "display 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 "display brightness max". Unlike the /sys/class/backlight/foo/brightness this brightness property has a clear definition for the value 0. The kernel must ensure that 0 means minimum
