* [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
@ 2013-03-26 11:48 Danny Baumann
2013-03-26 11:48 ` [PATCH 1/1] " Danny Baumann
2013-03-26 17:02 ` [PATCH 0/1] " Matthew Garrett
0 siblings, 2 replies; 14+ messages in thread
From: Danny Baumann @ 2013-03-26 11:48 UTC (permalink / raw)
To: David Airlie; +Cc: intel-gfx, dri-devel, linux-kernel, Danny Baumann
This patch makes the behaviour of the intel_backlight backlight device
consistent to e.g. acpi_videoX: When writing the value 0, the set brightness
makes the panel content barely readable instead of turning the backlight off.
This matches the expectations of user space (e.g. kde-workspace or the Intel
X11 driver), which expects that it can use intel_backlight as a drop-in
replacement for acpi_videoX.
As BIOSes written for Windows 8 support seem to expect the display driver
taking care of the brightness control instead of ACPI methods (see [1]),
I would expect quite a number of people (like me ;) ) having the need of
using intel_backlight instead of acpi_videoX. A quick Google search
indicated that most people are confused by the current behaviour, so I think
it's a good idea to make the default behaviour somehow match acpi_videoX.
Regards,
Danny
[1] https://bugzilla.kernel.org/show_bug.cgi?id=55071
Danny Baumann (1):
drm/i915: Allow specifying a minimum brightness level for sysfs
control.
drivers/gpu/drm/i915/intel_panel.c | 48 ++++++++++++++++++++++++++++++++------
1 file changed, 41 insertions(+), 7 deletions(-)
--
1.8.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-26 11:48 [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control Danny Baumann
@ 2013-03-26 11:48 ` Danny Baumann
2013-03-26 15:13 ` Daniel Vetter
2013-03-26 17:02 ` [PATCH 0/1] " Matthew Garrett
1 sibling, 1 reply; 14+ messages in thread
From: Danny Baumann @ 2013-03-26 11:48 UTC (permalink / raw)
To: David Airlie; +Cc: intel-gfx, dri-devel, linux-kernel, Danny Baumann
When controlling backlight devices via sysfs interface, user space
generally assumes the minimum level (0) still providing a brightness
that is usable by the user (probably due to the most commonly used
backlight device, acpi_videoX, behaving exactly like that). This doesn't
match the current behaviour of the intel_backlight control, though, as
the value 0 means 'backlight off'.
In order to make intel_backlight consistent to other backlight devices,
introduce a module parameter that allows shifting the 0 level of the
intel_backlight device. It's expressed in percentages of the maximum
level. The default is 5, which provides a backlight level that is barely
readable. Setting it to 0 restores the old behaviour.
Signed-off-by: Danny Baumann <dannybaumann@web.de>
---
drivers/gpu/drm/i915/intel_panel.c | 48 ++++++++++++++++++++++++++++++++------
1 file changed, 41 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index bee8cb6..5bad49d 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -395,10 +395,35 @@ intel_panel_detect(struct drm_device *dev)
}
#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
+
+static int i915_panel_min_brightness_percent = 5;
+MODULE_PARM_DESC(min_sysfs_brightness, "Minimum brightness percentage settable "
+ "via sysfs. This adjusts the brightness level of the value '0' in the "
+ "intel_backlight sysfs backlight interface.");
+module_param_named(min_sysfs_brightness,
+ i915_panel_min_brightness_percent, int, 0400);
+
+static int intel_panel_min_brightness(struct drm_device *dev)
+{
+ int max;
+
+ if (i915_panel_min_brightness_percent <= 0)
+ return 0;
+
+ max = intel_panel_get_max_backlight(dev);
+ if (i915_panel_min_brightness_percent >= 100)
+ return max;
+
+ return max * i915_panel_min_brightness_percent / 100;
+}
+
static int intel_panel_update_status(struct backlight_device *bd)
{
struct drm_device *dev = bl_get_data(bd);
- intel_panel_set_backlight(dev, bd->props.brightness);
+ int brightness =
+ bd->props.brightness + intel_panel_min_brightness(dev);
+
+ intel_panel_set_backlight(dev, brightness);
return 0;
}
@@ -406,7 +431,10 @@ static int intel_panel_get_brightness(struct backlight_device *bd)
{
struct drm_device *dev = bl_get_data(bd);
struct drm_i915_private *dev_priv = dev->dev_private;
- return dev_priv->backlight_level;
+ int brightness =
+ dev_priv->backlight_level - intel_panel_min_brightness(dev);
+
+ return brightness;
}
static const struct backlight_ops intel_panel_bl_ops = {
@@ -419,16 +447,21 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
struct drm_device *dev = connector->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct backlight_properties props;
+ int max_brightness;
intel_panel_init_backlight(dev);
- memset(&props, 0, sizeof(props));
- props.type = BACKLIGHT_RAW;
- props.max_brightness = _intel_panel_get_max_backlight(dev);
- if (props.max_brightness == 0) {
+ max_brightness = _intel_panel_get_max_backlight(dev);
+ if (max_brightness == 0) {
DRM_DEBUG_DRIVER("Failed to get maximum backlight value\n");
return -ENODEV;
}
+
+ memset(&props, 0, sizeof(props));
+ props.type = BACKLIGHT_RAW;
+ props.max_brightness =
+ max_brightness - intel_panel_min_brightness(dev);
+
dev_priv->backlight =
backlight_device_register("intel_backlight",
&connector->kdev, dev,
@@ -440,7 +473,8 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
dev_priv->backlight = NULL;
return -ENODEV;
}
- dev_priv->backlight->props.brightness = intel_panel_get_backlight(dev);
+ dev_priv->backlight->props.brightness = intel_panel_get_backlight(dev)
+ - intel_panel_min_brightness(dev);
return 0;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-26 11:48 ` [PATCH 1/1] " Danny Baumann
@ 2013-03-26 15:13 ` Daniel Vetter
2013-03-26 15:20 ` Chris Wilson
2013-03-26 16:55 ` Danny Baumann
0 siblings, 2 replies; 14+ messages in thread
From: Daniel Vetter @ 2013-03-26 15:13 UTC (permalink / raw)
To: Danny Baumann; +Cc: David Airlie, intel-gfx, linux-kernel, dri-devel
On Tue, Mar 26, 2013 at 12:48:45PM +0100, Danny Baumann wrote:
> When controlling backlight devices via sysfs interface, user space
> generally assumes the minimum level (0) still providing a brightness
> that is usable by the user (probably due to the most commonly used
> backlight device, acpi_videoX, behaving exactly like that). This doesn't
> match the current behaviour of the intel_backlight control, though, as
> the value 0 means 'backlight off'.
>
> In order to make intel_backlight consistent to other backlight devices,
> introduce a module parameter that allows shifting the 0 level of the
> intel_backlight device. It's expressed in percentages of the maximum
> level. The default is 5, which provides a backlight level that is barely
> readable. Setting it to 0 restores the old behaviour.
>
> Signed-off-by: Danny Baumann <dannybaumann@web.de>
Thus far our assumption always was that the acpi backlight works better
than the intel native backlight. So everything only uses the intel
backlight if there's no other backlight driver by default.
So if I should merge this as a general solution for Windows 8 machines not
working properly, we first need to figure out what windows does on these
machines and either disable the acpi backlight or adapt it.
Adding more kernel options is not a viable solution for the backlight mess
imo.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_panel.c | 48 ++++++++++++++++++++++++++++++++------
> 1 file changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index bee8cb6..5bad49d 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -395,10 +395,35 @@ intel_panel_detect(struct drm_device *dev)
> }
>
> #ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
> +
> +static int i915_panel_min_brightness_percent = 5;
> +MODULE_PARM_DESC(min_sysfs_brightness, "Minimum brightness percentage settable "
> + "via sysfs. This adjusts the brightness level of the value '0' in the "
> + "intel_backlight sysfs backlight interface.");
> +module_param_named(min_sysfs_brightness,
> + i915_panel_min_brightness_percent, int, 0400);
> +
> +static int intel_panel_min_brightness(struct drm_device *dev)
> +{
> + int max;
> +
> + if (i915_panel_min_brightness_percent <= 0)
> + return 0;
> +
> + max = intel_panel_get_max_backlight(dev);
> + if (i915_panel_min_brightness_percent >= 100)
> + return max;
> +
> + return max * i915_panel_min_brightness_percent / 100;
> +}
> +
> static int intel_panel_update_status(struct backlight_device *bd)
> {
> struct drm_device *dev = bl_get_data(bd);
> - intel_panel_set_backlight(dev, bd->props.brightness);
> + int brightness =
> + bd->props.brightness + intel_panel_min_brightness(dev);
> +
> + intel_panel_set_backlight(dev, brightness);
> return 0;
> }
>
> @@ -406,7 +431,10 @@ static int intel_panel_get_brightness(struct backlight_device *bd)
> {
> struct drm_device *dev = bl_get_data(bd);
> struct drm_i915_private *dev_priv = dev->dev_private;
> - return dev_priv->backlight_level;
> + int brightness =
> + dev_priv->backlight_level - intel_panel_min_brightness(dev);
> +
> + return brightness;
> }
>
> static const struct backlight_ops intel_panel_bl_ops = {
> @@ -419,16 +447,21 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
> struct drm_device *dev = connector->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct backlight_properties props;
> + int max_brightness;
>
> intel_panel_init_backlight(dev);
>
> - memset(&props, 0, sizeof(props));
> - props.type = BACKLIGHT_RAW;
> - props.max_brightness = _intel_panel_get_max_backlight(dev);
> - if (props.max_brightness == 0) {
> + max_brightness = _intel_panel_get_max_backlight(dev);
> + if (max_brightness == 0) {
> DRM_DEBUG_DRIVER("Failed to get maximum backlight value\n");
> return -ENODEV;
> }
> +
> + memset(&props, 0, sizeof(props));
> + props.type = BACKLIGHT_RAW;
> + props.max_brightness =
> + max_brightness - intel_panel_min_brightness(dev);
> +
> dev_priv->backlight =
> backlight_device_register("intel_backlight",
> &connector->kdev, dev,
> @@ -440,7 +473,8 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
> dev_priv->backlight = NULL;
> return -ENODEV;
> }
> - dev_priv->backlight->props.brightness = intel_panel_get_backlight(dev);
> + dev_priv->backlight->props.brightness = intel_panel_get_backlight(dev)
> + - intel_panel_min_brightness(dev);
> return 0;
> }
>
> --
> 1.8.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-26 15:13 ` Daniel Vetter
@ 2013-03-26 15:20 ` Chris Wilson
2013-03-26 17:04 ` Danny Baumann
2013-03-26 16:55 ` Danny Baumann
1 sibling, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2013-03-26 15:20 UTC (permalink / raw)
To: Danny Baumann, David Airlie, intel-gfx, linux-kernel, dri-devel
On Tue, Mar 26, 2013 at 04:13:13PM +0100, Daniel Vetter wrote:
> On Tue, Mar 26, 2013 at 12:48:45PM +0100, Danny Baumann wrote:
> > When controlling backlight devices via sysfs interface, user space
> > generally assumes the minimum level (0) still providing a brightness
> > that is usable by the user (probably due to the most commonly used
> > backlight device, acpi_videoX, behaving exactly like that). This doesn't
> > match the current behaviour of the intel_backlight control, though, as
> > the value 0 means 'backlight off'.
> >
> > In order to make intel_backlight consistent to other backlight devices,
> > introduce a module parameter that allows shifting the 0 level of the
> > intel_backlight device. It's expressed in percentages of the maximum
> > level. The default is 5, which provides a backlight level that is barely
> > readable. Setting it to 0 restores the old behaviour.
> >
> > Signed-off-by: Danny Baumann <dannybaumann@web.de>
>
> Thus far our assumption always was that the acpi backlight works better
> than the intel native backlight. So everything only uses the intel
> backlight if there's no other backlight driver by default.
There are machines, such as the pnv netbook on my desk, on which
intel_backlight does nothing and the only control is through
acpi_backlight0. Generalising expected behaviour based on one firmware
implementation is fraught with danger.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-26 15:13 ` Daniel Vetter
2013-03-26 15:20 ` Chris Wilson
@ 2013-03-26 16:55 ` Danny Baumann
1 sibling, 0 replies; 14+ messages in thread
From: Danny Baumann @ 2013-03-26 16:55 UTC (permalink / raw)
To: David Airlie, intel-gfx, linux-kernel, dri-devel
Hi,
> Thus far our assumption always was that the acpi backlight works better
> than the intel native backlight. So everything only uses the intel
> backlight if there's no other backlight driver by default.
>
> So if I should merge this as a general solution for Windows 8 machines not
> working properly, we first need to figure out what windows does on these
> machines and either disable the acpi backlight or adapt it.
I fully agree to that. Making acpi_video control usable is preferable
over using intel_backlight. As I lack the detail knowledge about the
ACPI video stuff, I'd be great of one (or some) of you guys could look
at the mentioned bug report ([1]) and comment on what the problem might
be. I have added the basic needed information already and would be happy
to provide any needed debugging info.
> Adding more kernel options is not a viable solution for the backlight mess
> imo.
Actually I'm fine with hardcoding the percentage as well ;) I just
figured it might make sense to make it controllable for special-case
uses, while still making intel_backlight usable for the 'normal' use case.
Regards,
Danny
[1] https://bugzilla.kernel.org/show_bug.cgi?id=55071
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-26 11:48 [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control Danny Baumann
2013-03-26 11:48 ` [PATCH 1/1] " Danny Baumann
@ 2013-03-26 17:02 ` Matthew Garrett
2013-03-26 17:10 ` Danny Baumann
1 sibling, 1 reply; 14+ messages in thread
From: Matthew Garrett @ 2013-03-26 17:02 UTC (permalink / raw)
To: Danny Baumann; +Cc: David Airlie, intel-gfx, dri-devel, linux-kernel
On Tue, Mar 26, 2013 at 12:48:44PM +0100, Danny Baumann wrote:
> This patch makes the behaviour of the intel_backlight backlight device
> consistent to e.g. acpi_videoX: When writing the value 0, the set brightness
> makes the panel content barely readable instead of turning the backlight off.
> This matches the expectations of user space (e.g. kde-workspace or the Intel
> X11 driver), which expects that it can use intel_backlight as a drop-in
> replacement for acpi_videoX.
I'm not quite clear what you mean here. The behaviour of "0" isn't well
defined for the ACPI backlight driver - it's perfectly reasonable for it
to turn the backlight off entirely. Anything assuming that "0" is still
visible is broken.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-26 15:20 ` Chris Wilson
@ 2013-03-26 17:04 ` Danny Baumann
0 siblings, 0 replies; 14+ messages in thread
From: Danny Baumann @ 2013-03-26 17:04 UTC (permalink / raw)
To: Chris Wilson, David Airlie, intel-gfx, linux-kernel, dri-devel
Hi,
>> Thus far our assumption always was that the acpi backlight works better
>> than the intel native backlight. So everything only uses the intel
>> backlight if there's no other backlight driver by default.
>
> There are machines, such as the pnv netbook on my desk, on which
> intel_backlight does nothing and the only control is through
> acpi_backlight0.
That's fine - but on my machine, (at least currently) it's the other way
around. acpi_video[0|1] do nothing, while intel_backlight is the only
method that works. This sucks (please also see my reply to Daniel), but
it's a fact.
> Generalising expected behaviour based on one firmware
> implementation is fraught with danger.
I'm not sure what you mean here. I interpret the statement as 'user
space should treat acpi_videoX and intel_backlight differently'. Is this
interpretation correct?
If so, how is user space supposed to know how the respective backlight
device will behave, as this behaviour might change at any point in time
if there's no understanding in how it _should_ behave? What currently
happens on my machine is that KDE completely turns off the backlight
after the dim timeout, because it assumes that the value 0 will keep the
backlight at a readable level (which would be the case if it used
acpi_videoX because this behaviour is mandated by the spec there). I
first thought of sending a patch to KDE, but I couldn't figure out how
KDE is _supposed_ to correctly handle the situation, especially given
that the actual sysfs interface used is abstracted away by Xrandr (and
chosen by the Intel Xorg driver). With my patch, intel_backlight behaves
in a way that roughly matches the behaviour mandated by the ACPI spec.
Do you have a way in mind that allows fixing this in user space?
Thanks,
Danny
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-26 17:02 ` [PATCH 0/1] " Matthew Garrett
@ 2013-03-26 17:10 ` Danny Baumann
2013-03-26 17:21 ` Matthew Garrett
0 siblings, 1 reply; 14+ messages in thread
From: Danny Baumann @ 2013-03-26 17:10 UTC (permalink / raw)
To: Matthew Garrett; +Cc: David Airlie, intel-gfx, dri-devel, linux-kernel
Hi,
Am 26.03.2013 18:02, schrieb Matthew Garrett:
> On Tue, Mar 26, 2013 at 12:48:44PM +0100, Danny Baumann wrote:
>> This patch makes the behaviour of the intel_backlight backlight device
>> consistent to e.g. acpi_videoX: When writing the value 0, the set brightness
>> makes the panel content barely readable instead of turning the backlight off.
>> This matches the expectations of user space (e.g. kde-workspace or the Intel
>> X11 driver), which expects that it can use intel_backlight as a drop-in
>> replacement for acpi_videoX.
>
> I'm not quite clear what you mean here. The behaviour of "0" isn't well
> defined for the ACPI backlight driver - it's perfectly reasonable for it
> to turn the backlight off entirely. Anything assuming that "0" is still
> visible is broken.
Well, the ACPI spec says this (section B.5.2):
"
The OEM may define the number 0 as "Zero brightness" that can mean to
turn off the lighting (e.g. LCD panel backlight) in the device. This may
be useful in the case of an output device that can still be viewed using
only ambient light, for example, a transflective LCD.
"
My interpretation of this is that the value 0 is supposed to still be
visible. I'm pretty sure I saw a statement that 0 is supposed to mean
"barely visible" somewhere, but can't find it at the moment. I'll search
for the source of it.
Regards,
Danny
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-26 17:10 ` Danny Baumann
@ 2013-03-26 17:21 ` Matthew Garrett
2013-03-27 11:56 ` Danny Baumann
0 siblings, 1 reply; 14+ messages in thread
From: Matthew Garrett @ 2013-03-26 17:21 UTC (permalink / raw)
To: Danny Baumann; +Cc: David Airlie, intel-gfx, dri-devel, linux-kernel
On Tue, Mar 26, 2013 at 06:10:30PM +0100, Danny Baumann wrote:
> Am 26.03.2013 18:02, schrieb Matthew Garrett:
> >I'm not quite clear what you mean here. The behaviour of "0" isn't well
> >defined for the ACPI backlight driver - it's perfectly reasonable for it
> >to turn the backlight off entirely. Anything assuming that "0" is still
> >visible is broken.
>
> Well, the ACPI spec says this (section B.5.2):
>
> "
> The OEM may define the number 0 as "Zero brightness" that can mean
> to turn off the lighting (e.g. LCD panel backlight) in the device.
> This may be useful in the case of an output device that can still be
> viewed using only ambient light, for example, a transflective LCD.
> "
>
> My interpretation of this is that the value 0 is supposed to still
> be visible. I'm pretty sure I saw a statement that 0 is supposed to
> mean "barely visible" somewhere, but can't find it at the moment.
> I'll search for the source of it.
I think that's a stretch - "This may be useful" isn't normative
language, "The OEM may define" is. But even if we do assert it for the
ACPI backlight, it's not true for other interfaces - zero backlight
intensity is supposed to be screen off on Apple hardware, for instance.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-26 17:21 ` Matthew Garrett
@ 2013-03-27 11:56 ` Danny Baumann
2013-03-27 12:35 ` Alex Deucher
2013-03-27 15:10 ` Matthew Garrett
0 siblings, 2 replies; 14+ messages in thread
From: Danny Baumann @ 2013-03-27 11:56 UTC (permalink / raw)
To: Matthew Garrett; +Cc: David Airlie, intel-gfx, dri-devel, linux-kernel
Hi,
>> Well, the ACPI spec says this (section B.5.2):
>>
>> "
>> The OEM may define the number 0 as "Zero brightness" that can mean
>> to turn off the lighting (e.g. LCD panel backlight) in the device.
>> This may be useful in the case of an output device that can still be
>> viewed using only ambient light, for example, a transflective LCD.
>> "
>>
>> My interpretation of this is that the value 0 is supposed to still
>> be visible. I'm pretty sure I saw a statement that 0 is supposed to
>> mean "barely visible" somewhere, but can't find it at the moment.
>> I'll search for the source of it.
>
> I think that's a stretch - "This may be useful" isn't normative
> language, "The OEM may define" is. But even if we do assert it for the
> ACPI backlight, it's not true for other interfaces - zero backlight
> intensity is supposed to be screen off on Apple hardware, for instance.
OK, I see. And there is user space depending on that behaviour? And
again - how is user space supposed to know about the behavioral
differences? Is it something like 'if type is raw, don't expect anything'?
The reason for my question is that I want to determine what a) the
correct place to fix this and b) the correct fix is. As Xrandr abstracts
away the used backlight interface, I see no way for user space using
Xrandr (e.g. KDE) to meaningfully handle this.
Thanks,
Danny
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-27 11:56 ` Danny Baumann
@ 2013-03-27 12:35 ` Alex Deucher
2013-03-27 12:56 ` Danny Baumann
2013-03-27 15:10 ` Matthew Garrett
1 sibling, 1 reply; 14+ messages in thread
From: Alex Deucher @ 2013-03-27 12:35 UTC (permalink / raw)
To: Danny Baumann; +Cc: Matthew Garrett, intel-gfx, linux-kernel, dri-devel
On Wed, Mar 27, 2013 at 7:56 AM, Danny Baumann <dannybaumann@web.de> wrote:
> Hi,
>
>>> Well, the ACPI spec says this (section B.5.2):
>>>
>>> "
>>> The OEM may define the number 0 as "Zero brightness" that can mean
>>> to turn off the lighting (e.g. LCD panel backlight) in the device.
>>> This may be useful in the case of an output device that can still be
>>> viewed using only ambient light, for example, a transflective LCD.
>>> "
>>>
>>> My interpretation of this is that the value 0 is supposed to still
>>> be visible. I'm pretty sure I saw a statement that 0 is supposed to
>>> mean "barely visible" somewhere, but can't find it at the moment.
>>> I'll search for the source of it.
>>
>>
>> I think that's a stretch - "This may be useful" isn't normative
>> language, "The OEM may define" is. But even if we do assert it for the
>> ACPI backlight, it's not true for other interfaces - zero backlight
>> intensity is supposed to be screen off on Apple hardware, for instance.
>
>
> OK, I see. And there is user space depending on that behaviour? And again -
> how is user space supposed to know about the behavioral differences? Is it
> something like 'if type is raw, don't expect anything'?
> The reason for my question is that I want to determine what a) the correct
> place to fix this and b) the correct fix is. As Xrandr abstracts away the
> used backlight interface, I see no way for user space using Xrandr (e.g.
> KDE) to meaningfully handle this.
In practice does it really matter? As a user if you set the
brightness really low and you either can't see the screen or can
barely make it out does it matter if the screen is off or just really,
really dim? The 0 brightness setting is probably not practically
usable regardless of what it does.
Alex
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-27 12:35 ` Alex Deucher
@ 2013-03-27 12:56 ` Danny Baumann
2013-03-27 13:06 ` Alex Deucher
0 siblings, 1 reply; 14+ messages in thread
From: Danny Baumann @ 2013-03-27 12:56 UTC (permalink / raw)
To: Alex Deucher; +Cc: Matthew Garrett, intel-gfx, linux-kernel, dri-devel
Hi,
>>>> Well, the ACPI spec says this (section B.5.2):
>>>>
>>>> "
>>>> The OEM may define the number 0 as "Zero brightness" that can mean
>>>> to turn off the lighting (e.g. LCD panel backlight) in the device.
>>>> This may be useful in the case of an output device that can still be
>>>> viewed using only ambient light, for example, a transflective LCD.
>>>> "
>>>>
>>>> My interpretation of this is that the value 0 is supposed to still
>>>> be visible. I'm pretty sure I saw a statement that 0 is supposed to
>>>> mean "barely visible" somewhere, but can't find it at the moment.
>>>> I'll search for the source of it.
BTW, I found the source for that statement: [1], section
System.Client.BrightnessControls.SmoothBrightness. While formally it's
not part of the ACPI spec, I'm pretty sure most vendors (except Apple,
obviously) will follow it as if it were, if not more strictly.
>> OK, I see. And there is user space depending on that behaviour? And again -
>> how is user space supposed to know about the behavioral differences? Is it
>> something like 'if type is raw, don't expect anything'?
>> The reason for my question is that I want to determine what a) the correct
>> place to fix this and b) the correct fix is. As Xrandr abstracts away the
>> used backlight interface, I see no way for user space using Xrandr (e.g.
>> KDE) to meaningfully handle this.
>
> In practice does it really matter? As a user if you set the
> brightness really low and you either can't see the screen or can
> barely make it out does it matter if the screen is off or just really,
> really dim? The 0 brightness setting is probably not practically
> usable regardless of what it does.
That's right. I'm not intending to use the laptop with that low
brightness, though, I'd just like to distinguish between my laptop being
turned off / suspended or just its display being dimmed down by the
desktop environment to conserve power. In order to do the latter, KDE
sets brightness to 0 ([2]), which worked fine for me as long as
acpi_video was working on this laptop. This isn't the case at present,
which is why I'm using intel_backlight at the moment. As you may have
noticed, things aren't working as expected with it, which in turn is
what brought me over here ;) I'm fine with sending a patch to KDE if
that's the correct thing to do, but I'm not yet sure what the correct
thing to do is.
Thanks,
Danny
[1] http://msdn.microsoft.com/en-us/library/windows/hardware/jj128256.aspx
[2]
https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/master/entry/powerdevil/daemon/actions/bundled/dimdisplay.cpp#L53
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-27 12:56 ` Danny Baumann
@ 2013-03-27 13:06 ` Alex Deucher
0 siblings, 0 replies; 14+ messages in thread
From: Alex Deucher @ 2013-03-27 13:06 UTC (permalink / raw)
To: Danny Baumann; +Cc: Matthew Garrett, intel-gfx, linux-kernel, dri-devel
On Wed, Mar 27, 2013 at 8:56 AM, Danny Baumann <dannybaumann@web.de> wrote:
> Hi,
>
>>>>> Well, the ACPI spec says this (section B.5.2):
>>>>>
>>>>> "
>>>>> The OEM may define the number 0 as "Zero brightness" that can mean
>>>>> to turn off the lighting (e.g. LCD panel backlight) in the device.
>>>>> This may be useful in the case of an output device that can still be
>>>>> viewed using only ambient light, for example, a transflective LCD.
>>>>> "
>>>>>
>>>>> My interpretation of this is that the value 0 is supposed to still
>>>>> be visible. I'm pretty sure I saw a statement that 0 is supposed to
>>>>> mean "barely visible" somewhere, but can't find it at the moment.
>>>>> I'll search for the source of it.
>
>
> BTW, I found the source for that statement: [1], section
> System.Client.BrightnessControls.SmoothBrightness. While formally it's not
> part of the ACPI spec, I'm pretty sure most vendors (except Apple,
> obviously) will follow it as if it were, if not more strictly.
>
>>> OK, I see. And there is user space depending on that behaviour? And again
>>> -
>>> how is user space supposed to know about the behavioral differences? Is
>>> it
>>> something like 'if type is raw, don't expect anything'?
>>> The reason for my question is that I want to determine what a) the
>>> correct
>>> place to fix this and b) the correct fix is. As Xrandr abstracts away the
>>> used backlight interface, I see no way for user space using Xrandr (e.g.
>>> KDE) to meaningfully handle this.
>>
>>
>> In practice does it really matter? As a user if you set the
>> brightness really low and you either can't see the screen or can
>> barely make it out does it matter if the screen is off or just really,
>> really dim? The 0 brightness setting is probably not practically
>> usable regardless of what it does.
>
>
> That's right. I'm not intending to use the laptop with that low brightness,
> though, I'd just like to distinguish between my laptop being turned off /
> suspended or just its display being dimmed down by the desktop environment
> to conserve power. In order to do the latter, KDE sets brightness to 0
> ([2]), which worked fine for me as long as acpi_video was working on this
> laptop. This isn't the case at present, which is why I'm using
> intel_backlight at the moment. As you may have noticed, things aren't
> working as expected with it, which in turn is what brought me over here ;)
> I'm fine with sending a patch to KDE if that's the correct thing to do, but
> I'm not yet sure what the correct thing to do is.
FWIW, when I implemented native backlight support in the radeon
driver, I special cased level 0 as off since that was what a lot of
the other native backlight drivers did. I'm open to changing it if
there is a plan for some kind of consistency, but it seems pretty
random at the moment.
Alex
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control.
2013-03-27 11:56 ` Danny Baumann
2013-03-27 12:35 ` Alex Deucher
@ 2013-03-27 15:10 ` Matthew Garrett
1 sibling, 0 replies; 14+ messages in thread
From: Matthew Garrett @ 2013-03-27 15:10 UTC (permalink / raw)
To: Danny Baumann; +Cc: David Airlie, intel-gfx, dri-devel, linux-kernel
On Wed, Mar 27, 2013 at 12:56:37PM +0100, Danny Baumann wrote:
> OK, I see. And there is user space depending on that behaviour? And
> again - how is user space supposed to know about the behavioral
> differences? Is it something like 'if type is raw, don't expect
> anything'?
"Do not rely upon 0 being visible".
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-03-27 15:10 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-26 11:48 [PATCH 0/1] drm/i915: Allow specifying a minimum brightness level for sysfs control Danny Baumann
2013-03-26 11:48 ` [PATCH 1/1] " Danny Baumann
2013-03-26 15:13 ` Daniel Vetter
2013-03-26 15:20 ` Chris Wilson
2013-03-26 17:04 ` Danny Baumann
2013-03-26 16:55 ` Danny Baumann
2013-03-26 17:02 ` [PATCH 0/1] " Matthew Garrett
2013-03-26 17:10 ` Danny Baumann
2013-03-26 17:21 ` Matthew Garrett
2013-03-27 11:56 ` Danny Baumann
2013-03-27 12:35 ` Alex Deucher
2013-03-27 12:56 ` Danny Baumann
2013-03-27 13:06 ` Alex Deucher
2013-03-27 15:10 ` Matthew Garrett
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).