3.18.0-rc3: i915: eDP connected Display stays blank

Commit Message

Jani Nikula Nov. 6, 2014, 12:53 p.m. UTC

On Thu, 06 Nov 2014, Arnd Hannemann <[e-mail]> wrote:
> Hi,
>
> thanks for your quick response.
>
> Am 06.11.2014 um 10:39 schrieb Jani Nikula:
>> On Thu, 06 Nov 2014, Arnd Hannemann <[e-mail]> wrote:
>>> Hi,
>>>
>>> I have a Thinkpad T440s (Haswell) connected to two additional Monitors
>>> via a Docking Station (MST).
>>>
>>> During Bootup all three displays work, even when X is started.
>>> However, if the laptop display is turned off once (either because of
>>> power saving, or via xrandr), it fails to "come back".
>>> That is if I try to re-enable it the Display stays blank.
>>> I believe this used to work in 3.17.
>>>
>>> Here is the xrandr Ouput of the edp, when its enabled (but staying blank):
>>> Screen 0: minimum 8 x 8, current 3840 x 1200, maximum 32767 x 32767
>>> eDP1 connected 1920x1080+0+0 (normal left inverted right x axis y axis) 309mm x 175mm
>>>    1920x1080      60.0*+   59.9
>>>
>>> here is the debug output, while trying to enable it:
>>>
>> 
>> ...
>> 
>>> [  416.538848] [drm:intel_edp_backlight_power] panel power control backlight disable
>>>
>>>
>>> I'm happy to provide further input.
>> 
>> What does cat /sys/class/backlight/intel_backlight/bl_power say? What if
>
> root@kallisto:~# cat /sys/class/backlight/intel_backlight/bl_power
> 1
>
>> you echo 0 there?
>
> :-) Works my display comes back, when I echo 0 there.
>
> Is user-space doing something wrong here?

If the userspace wishes to switch off backlight, then it's doing nothing
wrong at all! ;)

Here's the story as I know it.

Once upon a time someone added the bl_power attribute to the sysfs class
backlight interface. Even though the name implies a boolean backlight
power, the values are in fact FB_BLANK_* from fb.h, and power on is
FB_BLANK_UNBLANK, or 0. All the other values are various levels of
blanking which make little sense to backlight, and thus any non-zero
values mean power off. [1]

Until recently, intel_backlight of drm/i915 did not support bl_power at
all. We ignored the attribute altogether. However changing bl_power from
its default 0 did cause a backlight update hook to be called. In some
edge cases doing this fixed some backlight issues by reprogramming the
backlight intensity, and probably lead to the false assumption that
bl_power needed to be set to 1 to enable power.

Now that we've enabled support for bl_power attribute (on eDP at least),
the previously harmless, or sometimes even helpful, bl_power=1 actually
does what it means. That is, switch off the backlight.

Please try this patch (untested) to find out the culprit.



BR,
Jani.




[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/stable/sysfs-class-backlight
("Read the documentation and you'll get it right" does not score well in
Rusty's API design manifesto)

Comments

Arnd Hannemann Nov. 7, 2014, 9:27 a.m. UTC | #1

Hi,

Am 06.11.2014 um 13:53 schrieb Jani Nikula:

>> root@kallisto:~# cat /sys/class/backlight/intel_backlight/bl_power
>> 1
>>
>>> you echo 0 there?
>>
>> :-) Works my display comes back, when I echo 0 there.
>>
>> Is user-space doing something wrong here?
> 
> If the userspace wishes to switch off backlight, then it's doing nothing
> wrong at all! ;)
> 
> Here's the story as I know it.
> 
> Once upon a time someone added the bl_power attribute to the sysfs class
> backlight interface. Even though the name implies a boolean backlight
> power, the values are in fact FB_BLANK_* from fb.h, and power on is
> FB_BLANK_UNBLANK, or 0. All the other values are various levels of
> blanking which make little sense to backlight, and thus any non-zero
> values mean power off. [1]
> 
> Until recently, intel_backlight of drm/i915 did not support bl_power at
> all. We ignored the attribute altogether. However changing bl_power from
> its default 0 did cause a backlight update hook to be called. In some
> edge cases doing this fixed some backlight issues by reprogramming the
> backlight intensity, and probably lead to the false assumption that
> bl_power needed to be set to 1 to enable power.
> 
> Now that we've enabled support for bl_power attribute (on eDP at least),
> the previously harmless, or sometimes even helpful, bl_power=1 actually
> does what it means. That is, switch off the backlight.
> 

Thanks for your elaborated answer.

> Please try this patch (untested) to find out the culprit.

Thanks, its the intel xorg driver:
[  255.777798] bl_power 1 by Xorg

I seems it was already corrected upstream, by Chris Wilson two days ago:
http://cgit.freedesktop.org/xorg/driver/xf86-video-intel/commit/?id=7ecc778691c452285f754743a93a46fa1d3da52f


Best regards
Arnd

Patch

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index bddc8b17a4d8..8cf5c6cdeef5 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -143,6 +143,7 @@  static ssize_t bl_power_store(struct device *dev, struct device_attribute *attr,
 	rc = -ENXIO;
 	mutex_lock(&bd->ops_lock);
 	if (bd->ops) {
+		printk(KERN_INFO "bl_power %lu by %s\n", power, current->comm);
 		pr_debug("set power to %lu\n", power);
 		if (bd->props.power != power) {
 			bd->props.power = power;