Hi,
On 06/06/2014 04:51 PM, Chris Wilson wrote:
On Fri, Jun 06, 2014 at 04:37:36PM +0200, Hans de Goede wrote:
Hi,
On 06/05/2014 10:24 PM, Chris Wilson wrote:
On Thu, Jun 05, 2014 at 09:08:33PM +0200, Hans de Goede wrote:
Note that it is read after the framebuffer has been resized and a new mode
has been set on "pipe 0 using LVDS1", could this perhaps cause the 0 to be
read when using actual_brightness ?
Indeed, that is likely the explanation, and shows the fallacy in the
current approach. And also explains why acpi_backlight works with the
current code, but that the kernel interfering with intel_backlight does
not.
Also I've just had a user who has been testing this patch come back to
me it does help, but he still has a suspend/resume issue. It seems that
some X app / gnome-component is doing the following:
1) DPMS off
2) Read backlight xrandr property -> this will now return 0
3) Set backlight xrandr property value to the value just read, aka 0
4) DPMS on -> "restores" backlight to 0 because of the property set
I believe the best way to fix this will be to make
xxx_output_get_property("backlight") return backlight_active_level
when in DPMS off, rather then calling xxx_output_backlight_get.
I had the same thought when reviewing the code following your email. I
modified sna, but I think I want to restructure how backlight is saved
around modesets.
Ok, FWIW attached is a patch which I'm asking the user to test to confirm that
the above steps 1-4 are the problem of his suspend / resume problems.
It should be fixed upstream now.
About solving this differently / more completely why is the driver saving
the brightness at all? IMHO the driver / xrandr property should be the canonical
source of the brightness level once X is running, so we should only read it once
on init and then just always use backlight_active_level, this way the code becomes
simpler and we won't have any of this issues.
I really so no use-case where we want something to change the brightness underneath
us and then for the driver to correctly pick up this change.
ACPI buttons may modify the brightness directly and we need to propagate
those changes back to the RandR clients. See the current tree for how
that works.
commit c6cd10f536e099277cdc46643725a5a50ea8b525
Author: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Date: Thu Jun 5 22:43:37 2014 +0100
sna: Hook up a backlight udev monitor for external changes
Changes to the backlights are notified through uevents. Hooking up a
udev monitor to listen out for external changes to the backlight (e.g.
through ACPI function keys, or by the user writing to
/sys/class/backlight directly) is easier than enabling polling on the
backlight sysfs file using X's select() mechanism.
Since we listen to backlight changes, we have to be careful not toack the backli
confuse the side-effects of disabling connectors (which may cause either
ourselves or the kernel to turn off the backlight) with the user value.
Many thanks to Alexander Mezin for the suggestion to use udev for
tracking the notifications for external changes to the backlight.
Reported-by: Alexander Mezin <mezin.alexander@xxxxxxxxx>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79699
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Thanks, I fail to see how this addresses the original problem though,
current master still reads back the backlight from the kernel at DPMS
off, so the problem my original patch in this thread tries to fix
still exists AFAIK, we will still read back 0 on DPMS off in the
scenario my patch tries to address.
You've cleaned up the code in sna_output_dpms, but the logic is unchanged.
Possibly this even makes the problem bigger, because if the modeset
changes the brightness before the dpms off (as it seems to do) then an
udev event may get triggered and process before sna_output_dpms changes
the sna_output->dpms_mode and the
if (sna_output->dpms_mode != DPMSModeOn)
Guard in sna_backlight_uevent won't protect against storing the 0 brightness
value caused by the modeset to end up in active_level
Regards,
Hans
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
