drm/i915: minor corner case fix to respect user's backlight setting
Subsystems (2)
PATCH 1/1 drm/i915: minor corner case fix to respect user's backlight setting
When enabling panel backlight, if the current backlight level setting matches the panel's minimal, it would apply default policy to override the current level by the panel's maximum until next request to update brightness, this leads unexpected user confusion with temporary full power backlight.
This odd could be reproduced as commands like these:
$ xbacklight -set 0 $ sudo sh -c 'echo mem > /sys/power/state' (resume)
To fix this, slightly tinker the backlight level comparison from 'less-and-equal-to' to 'less-than'.
Before: (dmesg | grep backlight # with drm.debug=0xe)
[ 82.249265] [drm:intel_backlight_device_update_status [i915]] updating intel_backlight, brightness=0/5273 [ 82.249282] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 207 [ 82.249306] [drm:intel_edp_backlight_power [i915]] panel power control backlight disable [ 92.066041] [drm:intel_edp_backlight_off [i915]] [ 92.270489] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 0 [ 94.080434] [drm:intel_edp_backlight_on.part.25 [i915]] [ 94.080476] [drm:intel_panel_enable_backlight [i915]] pipe A [ 94.080539] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 5273
After:
[ 72.874465] [drm:intel_backlight_device_update_status [i915]] updating intel_backlight, brightness=0/5273 [ 72.874499] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 207
[ 72.874540] [drm:intel_edp_backlight_power [i915]] panel power control backlight disable [ 86.807928] [drm:intel_edp_backlight_off [i915]]
[ 87.013227] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 0
[ 89.001829] [drm:intel_edp_backlight_on.part.25 [i915]]
[ 89.001859] [drm:intel_panel_enable_backlight [i915]] pipe A
[ 89.001926] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 207
--- drivers/gpu/drm/i915/intel_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/i915/intel_panel.c +1 -1
@@ -1104,7 +1104,7 @@ void intel_panel_enable_backlight(struct intel_connector *connector)WARN_ON (panel->backlight.max ==0 ); -if (panel->backlight.level <= panel->backlight.min) { +if (panel->backlight.level < panel->backlight.min) { panel->backlight.level = panel->backlight.max;if (panel->backlight.device) panel->backlight.device->props.brightness =
Signature · 6 lines
2.6.6 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula
maintainerQuoted 34 lines · Harry Pan
When enabling panel backlight, if the current backlight level setting matches the panel's minimal, it would apply default policy to override the current level by the panel's maximum until next request to update brightness, this leads unexpected user confusion with temporary full power backlight.
This odd could be reproduced as commands like these:
$ xbacklight -set 0 $ sudo sh -c 'echo mem > /sys/power/state' (resume)To fix this, slightly tinker the backlight level comparison from 'less-and-equal-to' to 'less-than'.
Before: (dmesg | grep backlight # with drm.debug=0xe)
[ 82.249265] [drm:intel_backlight_device_update_status [i915]] updating intel_backlight, brightness=0/5273 [ 82.249282] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 207 [ 82.249306] [drm:intel_edp_backlight_power [i915]] panel power control backlight disable [ 92.066041] [drm:intel_edp_backlight_off [i915]] [ 92.270489] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 0 [ 94.080434] [drm:intel_edp_backlight_on.part.25 [i915]] [ 94.080476] [drm:intel_panel_enable_backlight [i915]] pipe A [ 94.080539] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 5273After:
[ 72.874465] [drm:intel_backlight_device_update_status [i915]] updatingintel_backlight, brightness=0/5273 [ 72.874499] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 207
[ 72.874540] [drm:intel_edp_backlight_power [i915]] panel power control backlight disable [ 86.807928] [drm:intel_edp_backlight_off [i915]]
[ 87.013227] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 0
[ 89.001829] [drm:intel_edp_backlight_on.part.25 [i915]]
[ 89.001859] [drm:intel_panel_enable_backlight [i915]] pipe A
[ 89.001926] [drm:intel_panel_actually_set_backlight [i915]] set backlight PWM = 207
Quoted 1 line
Fixes 13f3fbe827d0 ("fix inconsistent brightness after resume")
That reference is not really true. We've had this policy of setting the backlight to max at enable if it was previously zero for eons. Yes, it's policy, not mechanism, but it's basically ABI.
For some reason the expectation is that the sequence:
1. set backlight to 0
2. dpms off
3. dpms on
does not lead to a black screen regardless of the user request to have 0 backlight. Your change breaks this.
Quoted 9 lines
Signed-off-by Harry Pan <[email protected]> ---
drivers/gpu/drm/i915/intel_panel.c| 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/i915/intel_panel.c +0
-0
drivers/gpu/drm/i915/intel_panel.c void intel_panel_enable_backlight(struct intel_connector *connector) · line 1108
Quoted 6 lines
drivers/gpu/drm/i915/intel_panel.c +1
-1@@ -1104,7 +1104,7 @@ void intel_panel_enable_backlight(struct intel_connector *connector)WARN_ON (panel->backlight.max ==0 ); -if (panel->backlight.level <= panel->backlight.min) { +if (panel->backlight.level < panel->backlight.min) { panel->backlight.level = panel->backlight.max;
If we changed this to follow your logic, the sensible thing to do would be to set the backlight to min, not max, in this case.
But the point is moot. I don't want to deal with the regressions that I predict the change will inevitably cause.
BR,
Jani.
Quoted 2 lines
drivers/gpu/drm/i915/intel_panel.c +0
-0if (panel->backlight.device) panel->backlight.device->props.brightness =
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Patchwork
CI✓ Fi.CI.BAT: success for drm/i915: minor corner case fix to respect user's backlight setting
In reply to Harry Pan== Series Details ==
== Series Details ==
Series: drm/i915: minor corner case fix to respect user's backlight setting
URL : https://patchwork.freedesktop.org/series/18685/
State : success
== Summary ==
Series 18685v1 drm/i915: minor corner case fix to respect user's backlight setting
https://patchwork.freedesktop.org/api/1.0/series/18685/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-b-frame-sequence:
dmesg-warn -> PASS (fi-snb-2520m)fi-bdw-5557u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:246 pass:207 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 fi-bxt-t5700 total:78 pass:65 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-j1900 total:246 pass:219 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:246 pass:227 dwarn:0 dfail:0 fail:0 skip:19 fi-ivb-3520m total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:246 pass:225 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:246 pass:223 dwarn:0 dfail:0 fail:2 skip:21 fi-skl-6260u total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hq total:246 pass:226 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:246 pass:221 dwarn:4 dfail:0 fail:0 skip:21 fi-skl-6770hq total:246 pass:233 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:246 pass:214 dwarn:0 dfail:0 fail:0 skip:32
dc47d8d6ed4ea669aeccad104fde843fe039b9a5 drm-tip: 2017y-01m-30d-09h-12m-47s UTC integration manifest 269b261f drm/i915: minor corner case fix to respect user's backlight setting
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3632/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx