Created attachment 152911 [details] dmesg After upgrading from 3.16 to 3.17 I can't adjust brightness in KDE using Fn-* hotkeys. Only intel_backlight is present in /sys/class/backlight. And adjusting value here works. I've bisected it and got followed commit: commit 6dda730e55f412a6dfb181cae6784822ba463847 Author: Jani Nikula <jani.nikula@intel.com> Date: Tue Jun 24 18:27:40 2014 +0300 drm/i915: respect the VBT minimum backlight brightness Historically we've exposed the full backlight PWM duty cycle range to the userspace, in the name of "mechanism, not policy". However, it turns out there are both panels and board designs where there is a minimum duty cycle that is required for proper operation. The minimum duty cycle is available in the VBT. The backlight class sysfs interface does not make any promises to the userspace about the physical meaning of the range 0..max_brightness. Specifically there is no guarantee that 0 means off; indeed for acpi_backlight 0 usually is not off, but the minimum acceptable value. Respect the minimum backlight, and expose the range acceptable to the hardware as 0..max_brightness to the userspace via the backlight class device; 0 means the minimum acceptable enabled value. To switch off the backlight, the user must disable the encoder. As a side effect, make the backlight class device max brightness and physical PWM modulation frequency (i.e. max duty cycle) independent. This allows a follow-up patch to virtualize the max value exposed to the userspace. Signed-off-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> [danvet: s/BUG_ON/WARN_ON/] Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Reverting it fixes issue for me. 00:02.0 VGA compatible controller: Intel Corporation 4th Gen Core Processor Integrated Graphics Controller (rev 06)
Probably duplicate of #84381 but I'm not sure.
Please attach /sys/kernel/debug/dri/0/i915_opregion and the dmesg with drm.debug=14 module parameter set.
Created attachment 152921 [details] dmesg with drm.debug
Created attachment 152931 [details] dmesg with drm.debug (commit reverted)
Created attachment 152941 [details] i915_opregion
Same problem here, Lenovo Thinkpad X1. Worked fine up to 3.16.4, but since 3.17.0 the Fn-backlight controls no longer work. The controls do register in xev however. I made same dmesg and i915 opregions files, too, and I can attach them if needed.
Forgot to say I'm also using KDE (in Fedora 20, but with vanilla kernels) and that the backlight slider from the battery widget does indeed control the backlight as before. It's only the keys that don't seem to control it anymore. And to be a little more precise about xev output I should add that while both 3.16 and 3.17 both register the XF86MonBrightness{Up,Down} presses and releases only the 3.16 has the RRNotify events there for the screen backlight.
I really have a hard time understanding why a little bit of scaling would affect the event handling in userspace.
Please try this patch: diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 18784470a760..443b62356e55 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1103,8 +1103,7 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector) WARN_ON(panel->backlight.max == 0); /* vbt value is a coefficient in range [0..255] */ - return scale(dev_priv->vbt.backlight.min_brightness, 0, 255, - 0, panel->backlight.max); + return 0; } static int bdw_setup_backlight(struct intel_connector *connector)
Applied that to vanilla 3.17.1 and indeed the old behaviour is restored, i.e. the Fn-<brightness> keys work again as expected.
/me scratches head. Please try commit fff95501f2095fa8a9dbd7e177bbf642d241b2fe Author: U. Artie Eoff <ullysses.a.eoff@intel.com> Date: Mon Sep 29 15:49:32 2014 -0700 drm/i915: intel_backlight scale() math WA http://cgit.freedesktop.org/drm-intel/commit/?id=fff95501f2095fa8a9dbd7e177bbf642d241b2fe on top of vanilla 3.17.something.
I mean, cherry pick that one commit, don't merge.
Applied that to 3.17.1 and it works, too, _but_ a little differently wrt. 0% kernel FN-<brightness> 3.1x.y vanilla works, 0% is very dark 3.16.4 vanilla works, 0% is black 3.17.1 vanilla doesn't work 3.17.1 reverted 6dda730 works, 0% is black 3.17.1 applied #9 patch works, 0% is black 3.17.1 applied fff95501 works, 0% is very dark I can't remember when you guys introduced the 0% is black, but it was some weeks/months ago. I can check up on that if you like, but I reckon you already know when that happened. Although the totally black 0% it initially struck me as odd I learned to actually like that better than the "very very dark" 0%. I suppose that's a matter of opinion and at least I now know where to mess around if I indeed want a black 0%. As a side note, I think the userland could benefit from non-linear scaling. Changes in upper end of the spectrum seem a lot less to a human eye than in the darker end of the spectrum. I rarely find any benefit in going from 100% to 80%, but often find myself not finding quite enough resolution around 10% to 20% or there abouts. We added quadratic scaling to resolve similar shortcomings in audio control in our gaming machines. But that is certainly userland stuff, and I'm not even sure if there's already some mechanisms already in place.
Thanks for testing. My only guess is that the userspace (could I be more vague?) expects actual_brightness to always match brightness in /sys/class/backlight/intel_backlight when its changed. Can you check whether the backported commit makes a difference here? acpi_backlight usually did not go all the way to black. intel_backlight used to do that. However that broke some setups (by physically pulling other lanes down on the eDP connector), so we had to start respecting the minimum. But we're bringing 0 = black back in 3.18 with a different mechanism... There's disagreement whether the non-linear scaling should be done in userspace or kernel. Userspace could be more clever about it, but kernel does have some extra information to use through the VBT for this purpose.
Anyway, this is fixed in upstream drm-intel. We could close, but I'd like to keep this open until the commit has hit Linus' tree *and* we've done a stable backport request to the stable team. Thanks.
Sounds good. But which backport are you referring to? I did cherry-pick the fff9501 from comment #11 to a vanilla 3.17.1 and it worked the way I mentioned, i.e. userspace (kde powerdevil?) was happy, fn-keys worked and 0% was very dark but not black. Or have these changes been staged to earlier stables, too? And you want to see if that fff9501 also works with 3.16.6 or so? I'll be sure to monitor this bug here and of course the stable 3.17.y serias and can't wait for the 3.18.
(In reply to Tommi Kyntola from comment #16) > But which backport are you referring to? Backporting the fix to official v3.17.y stable releases. But we can't do that until the commit hits Linus' upstream tree, and that's expected to happen in v3.18-rc2. Since the regressing commit was introduced in 3.17, there's no use backporting to kernels before that.
Oops, I referenced the wrong backport reference. I was trying to refer to this question in comment #14: > Can you check whether the backported commit makes a difference here?
(In reply to Tommi Kyntola from comment #18) > Oops, I referenced the wrong backport reference. I was trying to refer to > this question in comment #14: > > > Can you check whether the backported commit makes a difference here? Ah, I meant the same commit from comment #11 that fixes the problem. Does actual_brightness behave differently wrt brightness with vs. without that commit?
The brightness and actual_brightness in sysfs match and behave identically (go linearily between 0 and 4437 along with the slider in about 10% steps) with both patches from comment #9 and comment #11. The unpatched 3.17.1 behaves differently though. From the boot the brightness and actual_brightness are off by one with the brightness being one less. Every time I press brightness up or down from the Fn-keys they both drop by 2. I suppose, the kde powerdevil is not happy with that and fails in what ever manoeuvre it happens to try.
And by "from the boot" I'm not talking about init=/bin/sh or anything like that, but rather when the system is fully up yet without having touched the brightness controls, which most likely means that the kde powerdevil has already made some adjustments. When I booted (the unpatched 3.17.1) to a single user mode without X the values were both at the max 4437 and a manual setting to 3993 (i.e. $(( 4437 * 9 / 10 ))) resulted in brightness setting of 3993 and actual_brightness 3992. I didn't try this with the patched kernels as I have no reason to expect the numbers wouldn't match in them because the do that already when up and running and kde powerdevil is happy with both of them.
(In reply to Tommi Kyntola from comment #20) > The unpatched 3.17.1 behaves differently though. From the boot the > brightness and actual_brightness are off by one with the brightness being > one less. Every time I press brightness up or down from the Fn-keys they > both drop by 2. I suppose, the kde powerdevil is not happy with that and > fails in what ever manoeuvre it happens to try. If that really is the reason for the breakage, the userspace is making assumptions it really shouldn't be making.
What are the semantics behind the 'actual_brightness' then? Are there cases where it can indeed differ from the set value in 'brightness' in some reasonable way outside of rounding errors? (it'd be nice to know if anyone is thinking about informing the kde people about all this) Why is it necessary to even try do the inverse for the scaling, because with integers that simply is not a bijection unless the ranges are of equal size? The commit in comment #14 works only by chance here, too, because with the minimum set the ranges aren't of equal size and indeed in many spots the brightness and actual_brightness again show different values, which would break kde powerdevil. The commit in comment #11 is of course different and retains the identical ranges and make the scaling and identity operation and thus leave no room for rounding errors of any kind. Why not store the user value and return that for the reverse when asked for? I tried that with quadratic scaling and it works like a charm all along the range quite nicely (*), in fact, I'm likely to keep that patch in my own laptop. I can clean it up a notch and sign-off, if anyone's interested. (*) Well, I did add the inverse quadratic scaling there, too, but I only went for it if the previous values that passed through scale could not be used, i.e. only if there was a cache miss in a sense. Turns out, there's only one miss and that's during a boot which is non-ciritical and could probably be avoided, too.
(In reply to Tommi Kyntola from comment #23) > What are the semantics behind the 'actual_brightness' then? http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/stable/sysfs-class-backlight > Are there cases where it can indeed differ from the set value in > 'brightness' in some reasonable way outside of rounding errors? actual_brightness is queried from the hardware. The brightness in the hardware may be represented with something completely different from a linear range from 0..max_brightness. Also, the BIOS might (and has been known to) change the value behind our back. > (it'd be nice to know if anyone is thinking about informing the kde people > about all this) > > Why is it necessary to even try do the inverse for the scaling, because with > integers that simply is not a bijection unless the ranges are of equal size? Because the interface expects a range 0..max_brightness. > The commit in comment #14 works only by chance here, too, because with the > minimum set the ranges aren't of equal size and indeed in many spots the > brightness and actual_brightness again show different values, which would > break kde powerdevil. > > The commit in comment #11 is of course different and retains the identical > ranges and make the scaling and identity operation and thus leave no room > for rounding errors of any kind. > > Why not store the user value and return that for the reverse when asked for? > I tried that with quadratic scaling and it works like a charm all along the > range quite nicely (*), in fact, I'm likely to keep that patch in my own > laptop. I can clean it up a notch and sign-off, if anyone's interested. > > (*) Well, I did add the inverse quadratic scaling there, too, but I only > went for it if the previous values that passed through scale could not be > used, i.e. only if there was a cache miss in a sense. Turns out, there's > only one miss and that's during a boot which is non-ciritical and could > probably be avoided, too. If the userspace wants the value it stored, it can use brightness...
A mystery, at a glance, powerdevil does not use actual_brightness.
> The brightness in the hardware may be represented with something completely > different from a linear range from 0..max_brightness. But actual_brightness is transferred back to user values and it's this scale that I'm questioning. And I understand why the scale from user to hw is needed, but it was the inverse that I was questioning, but this next point makes it clear: > Also, the BIOS might (and has been known to) change the value behind our > back. Ok, in that case indeed the most sensible thing to do, would be for userspace to not demand accurate actual_brightness wrt to brightness. Although, a lot confusion could be avoided by caching the user input and spewing that out as actual_brightness if the values match, which is what I'm doing right now. (I cache the scale_quad input and output and then for actual_brightness in scale_quad_inv I check if what the hardware is returing that said scale output then it's safe and correct to return the earlier cached scale input. And if they don't match (as is the case during the boot and would be in abrupt BIOS tweaks to brightness) then compute the inverse quad, which obviously cannot be made one-to-one. The fact that I'm using quad scale is irrelevant here, because same problems are currently there with the 57..4437 hardware scale and the 0..4437 user scale. Another way to by-pass these problems for now would be do the minimum brightness by a user max value reduction guaranteeing equal ranges and one-to-one mappings across the whole domain. > Because the interface expects a range 0..max_brightness. Oh, I get that, I was merely questioning if the hardware value ever changes on its own, because if not, then indeed caching the setting from user_to_hw would be sufficient. > A mystery, at a glance, powerdevil does not use actual_brightness. Someone sure does as I can see quite a number of calls to scale (I have some printk's in there) whenever I touch the Fn-brightness buttons. And as i have the split it into scale_quad and scale_quad_inv for user_to_hw and hw_to_user, respectively, I can pretty much tell that someone sets 'brightness' and then checks up on 'actual_brightness' to match that setting. And if they fail in a way that actual_brightness doesn't match what was just set to brightness then further calls to Fn-brightness fail.
Hi, I have applied patch: http://cgit.freedesktop.org/drm-intel/commit/?h=drm-intel-fixes&id=673e7bbdb3920b62cfc6c710bea626b0a9b0f43a on kernel-3.17.2 and nothing changed. :-( Back to 3.16 and keys works again. Hardware: Thinkpad T420S
Andrzej, I suspect you have a different bug.
(In reply to Jani Nikula from comment #28) > Andrzej, I suspect you have a different bug. I've tried also this one: https://bugzilla.kernel.org/show_bug.cgi?id=84381 Not helps.
I've just updated to 3.17.3 stable kernel and it pretty looks like my issue is resolved now. So I think that it's ok to close it.
(In reply to Dmitry Nezhevenko from comment #30) > I've just updated to 3.17.3 stable kernel and it pretty looks like my issue > is resolved now. So I think that it's ok to close it. Thanks for reporting this issue and the update, closing this issue. Everyone else who still has issue on 3.17.3: Please file a new bug, thanks.