diff mbox series

docs: backlight: Clarify `actual_brightness`

Message ID 20250408192729.4091391-1-superm1@kernel.org (mailing list archive)
State New, archived
Headers show
Series docs: backlight: Clarify `actual_brightness` | expand

Commit Message

Mario Limonciello April 8, 2025, 7:27 p.m. UTC
From: Mario Limonciello <mario.limonciello@amd.com>

Currently userspace software systemd treats `brightness` and
`actual_brightness` identically due to a bug found in an out of tree
driver.

This however causes problems for in-tree drivers that use brightness
to report user requested `brightness` and `actual_brightness` to report
what the hardware actually has programmed.

Clarify the documentation to match the behavior described in commit
6ca017658b1f9 ("[PATCH] backlight: Backlight Class Improvements").

Cc: Lennart Poettering <lennart@poettering.net>
Cc: richard.purdie@linuxfoundation.org
Link: https://github.com/systemd/systemd/pull/36881
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 Documentation/ABI/stable/sysfs-class-backlight | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Lee Jones April 15, 2025, 3:53 p.m. UTC | #1
On Tue, 08 Apr 2025, Mario Limonciello wrote:

> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> Currently userspace software systemd treats `brightness` and
> `actual_brightness` identically due to a bug found in an out of tree
> driver.
> 
> This however causes problems for in-tree drivers that use brightness
> to report user requested `brightness` and `actual_brightness` to report
> what the hardware actually has programmed.
> 
> Clarify the documentation to match the behavior described in commit
> 6ca017658b1f9 ("[PATCH] backlight: Backlight Class Improvements").
> 
> Cc: Lennart Poettering <lennart@poettering.net>
> Cc: richard.purdie@linuxfoundation.org
> Link: https://github.com/systemd/systemd/pull/36881
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  Documentation/ABI/stable/sysfs-class-backlight | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/stable/sysfs-class-backlight b/Documentation/ABI/stable/sysfs-class-backlight
> index 6102d6bebdf9a..909cacde85041 100644
> --- a/Documentation/ABI/stable/sysfs-class-backlight
> +++ b/Documentation/ABI/stable/sysfs-class-backlight
> @@ -26,7 +26,9 @@ Date:		March 2006
>  KernelVersion:	2.6.17
>  Contact:	Richard Purdie <rpurdie@rpsys.net>
>  Description:
> -		Show the actual brightness by querying the hardware.
> +		Show the actual brightness by querying the hardware. This
> +		takes into account other factors besides the value programmed
> +		for 'brightness' and may not match the value in 'brightness'.

Which factors?  Under what circumstances may the values not match?

>  Users:		HAL
>  
>  What:		/sys/class/backlight/<backlight>/max_brightness
> -- 
> 2.43.0
>
Mario Limonciello April 15, 2025, 4:02 p.m. UTC | #2
On 4/15/2025 10:53 AM, Lee Jones wrote:
> On Tue, 08 Apr 2025, Mario Limonciello wrote:
> 
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> Currently userspace software systemd treats `brightness` and
>> `actual_brightness` identically due to a bug found in an out of tree
>> driver.
>>
>> This however causes problems for in-tree drivers that use brightness
>> to report user requested `brightness` and `actual_brightness` to report
>> what the hardware actually has programmed.
>>
>> Clarify the documentation to match the behavior described in commit
>> 6ca017658b1f9 ("[PATCH] backlight: Backlight Class Improvements").
>>
>> Cc: Lennart Poettering <lennart@poettering.net>
>> Cc: richard.purdie@linuxfoundation.org
>> Link: https://github.com/systemd/systemd/pull/36881
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   Documentation/ABI/stable/sysfs-class-backlight | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/ABI/stable/sysfs-class-backlight b/Documentation/ABI/stable/sysfs-class-backlight
>> index 6102d6bebdf9a..909cacde85041 100644
>> --- a/Documentation/ABI/stable/sysfs-class-backlight
>> +++ b/Documentation/ABI/stable/sysfs-class-backlight
>> @@ -26,7 +26,9 @@ Date:		March 2006
>>   KernelVersion:	2.6.17
>>   Contact:	Richard Purdie <rpurdie@rpsys.net>
>>   Description:
>> -		Show the actual brightness by querying the hardware.
>> +		Show the actual brightness by querying the hardware. This
>> +		takes into account other factors besides the value programmed
>> +		for 'brightness' and may not match the value in 'brightness'.
> 
> Which factors?  Under what circumstances may the values not match?

Is this question for your background or are you saying these should be 
enumerated here in the documentation?

Richard Purdie mentioned:

"The challenge back then was that things like framebuffer blanking would 
turn off the backlight but the single brightness attribute was 
implemented inconsistently. Some drivers would report no backlight when 
blanking was active, some would report the value stored in brightness. 
This had multiple issues"

For AMD the two cases that can cause this on amdgpu are Custom 
brightness curves and Adaptive Backlight Management (ABM).


> 
>>   Users:		HAL
>>   
>>   What:		/sys/class/backlight/<backlight>/max_brightness
>> -- 
>> 2.43.0
>>
>
Lee Jones April 15, 2025, 4:31 p.m. UTC | #3
On Tue, 15 Apr 2025, Mario Limonciello wrote:

> On 4/15/2025 10:53 AM, Lee Jones wrote:
> > On Tue, 08 Apr 2025, Mario Limonciello wrote:
> > 
> > > From: Mario Limonciello <mario.limonciello@amd.com>
> > > 
> > > Currently userspace software systemd treats `brightness` and
> > > `actual_brightness` identically due to a bug found in an out of tree
> > > driver.
> > > 
> > > This however causes problems for in-tree drivers that use brightness
> > > to report user requested `brightness` and `actual_brightness` to report
> > > what the hardware actually has programmed.
> > > 
> > > Clarify the documentation to match the behavior described in commit
> > > 6ca017658b1f9 ("[PATCH] backlight: Backlight Class Improvements").
> > > 
> > > Cc: Lennart Poettering <lennart@poettering.net>
> > > Cc: richard.purdie@linuxfoundation.org
> > > Link: https://github.com/systemd/systemd/pull/36881
> > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > ---
> > >   Documentation/ABI/stable/sysfs-class-backlight | 4 +++-
> > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/ABI/stable/sysfs-class-backlight b/Documentation/ABI/stable/sysfs-class-backlight
> > > index 6102d6bebdf9a..909cacde85041 100644
> > > --- a/Documentation/ABI/stable/sysfs-class-backlight
> > > +++ b/Documentation/ABI/stable/sysfs-class-backlight
> > > @@ -26,7 +26,9 @@ Date:		March 2006
> > >   KernelVersion:	2.6.17
> > >   Contact:	Richard Purdie <rpurdie@rpsys.net>
> > >   Description:
> > > -		Show the actual brightness by querying the hardware.
> > > +		Show the actual brightness by querying the hardware. This
> > > +		takes into account other factors besides the value programmed
> > > +		for 'brightness' and may not match the value in 'brightness'.
> > 
> > Which factors?  Under what circumstances may the values not match?
> 
> Is this question for your background or are you saying these should be
> enumerated here in the documentation?

Both.

> Richard Purdie mentioned:
> 
> "The challenge back then was that things like framebuffer blanking would
> turn off the backlight but the single brightness attribute was implemented
> inconsistently. Some drivers would report no backlight when blanking was
> active, some would report the value stored in brightness. This had multiple
> issues"
> 
> For AMD the two cases that can cause this on amdgpu are Custom brightness
> curves and Adaptive Backlight Management (ABM).

Okay, that was for me.  Please update the documentation. =:-)
Jani Nikula April 16, 2025, 8:39 a.m. UTC | #4
On Tue, 15 Apr 2025, Lee Jones <lee@kernel.org> wrote:
> On Tue, 08 Apr 2025, Mario Limonciello wrote:
>
>> From: Mario Limonciello <mario.limonciello@amd.com>
>> 
>> Currently userspace software systemd treats `brightness` and
>> `actual_brightness` identically due to a bug found in an out of tree
>> driver.
>> 
>> This however causes problems for in-tree drivers that use brightness
>> to report user requested `brightness` and `actual_brightness` to report
>> what the hardware actually has programmed.
>> 
>> Clarify the documentation to match the behavior described in commit
>> 6ca017658b1f9 ("[PATCH] backlight: Backlight Class Improvements").
>> 
>> Cc: Lennart Poettering <lennart@poettering.net>
>> Cc: richard.purdie@linuxfoundation.org
>> Link: https://github.com/systemd/systemd/pull/36881
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>  Documentation/ABI/stable/sysfs-class-backlight | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/ABI/stable/sysfs-class-backlight b/Documentation/ABI/stable/sysfs-class-backlight
>> index 6102d6bebdf9a..909cacde85041 100644
>> --- a/Documentation/ABI/stable/sysfs-class-backlight
>> +++ b/Documentation/ABI/stable/sysfs-class-backlight
>> @@ -26,7 +26,9 @@ Date:		March 2006
>>  KernelVersion:	2.6.17
>>  Contact:	Richard Purdie <rpurdie@rpsys.net>
>>  Description:
>> -		Show the actual brightness by querying the hardware.
>> +		Show the actual brightness by querying the hardware. This
>> +		takes into account other factors besides the value programmed
>> +		for 'brightness' and may not match the value in 'brightness'.
>
> Which factors?  Under what circumstances may the values not match?

At least for i915, the brightness range in sysfs isn't guaranteed to be
the range in hardware. For example, you might want sysfs to start from
0, but the hardware has a minimum that is non-zero. Or you might just
want to have a fixed range, say, 0-100 in sysfs. Any scaling may
introduce rounding errors in the round-trip.

BR,
Jani.
diff mbox series

Patch

diff --git a/Documentation/ABI/stable/sysfs-class-backlight b/Documentation/ABI/stable/sysfs-class-backlight
index 6102d6bebdf9a..909cacde85041 100644
--- a/Documentation/ABI/stable/sysfs-class-backlight
+++ b/Documentation/ABI/stable/sysfs-class-backlight
@@ -26,7 +26,9 @@  Date:		March 2006
 KernelVersion:	2.6.17
 Contact:	Richard Purdie <rpurdie@rpsys.net>
 Description:
-		Show the actual brightness by querying the hardware.
+		Show the actual brightness by querying the hardware. This
+		takes into account other factors besides the value programmed
+		for 'brightness' and may not match the value in 'brightness'.
 Users:		HAL
 
 What:		/sys/class/backlight/<backlight>/max_brightness