* [PATCH] drm/i915: minor corner case fix to respect user's backlight setting
@ 2017-01-27 12:57 Harry Pan
2017-01-27 14:10 ` Jani Nikula
2017-01-30 11:54 ` ✓ Fi.CI.BAT: success for " Patchwork
0 siblings, 2 replies; 3+ messages in thread
From: Harry Pan @ 2017-01-27 12:57 UTC (permalink / raw)
To: LKML; +Cc: airlied, intel-gfx, Harry Pan, dri-devel, daniel.vetter, gs0622
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
Fixes: 13f3fbe827d0 ("fix inconsistent brightness after resume")
Signed-off-by: Harry Pan <harry.pan@intel.com>
---
drivers/gpu/drm/i915/intel_panel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 08ab6d7..e882139 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -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 =
--
2.6.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] drm/i915: minor corner case fix to respect user's backlight setting
2017-01-27 12:57 [PATCH] drm/i915: minor corner case fix to respect user's backlight setting Harry Pan
@ 2017-01-27 14:10 ` Jani Nikula
2017-01-30 11:54 ` ✓ Fi.CI.BAT: success for " Patchwork
1 sibling, 0 replies; 3+ messages in thread
From: Jani Nikula @ 2017-01-27 14:10 UTC (permalink / raw)
To: LKML; +Cc: intel-gfx, dri-devel, Harry Pan, daniel.vetter, gs0622
On Fri, 27 Jan 2017, Harry Pan <harry.pan@intel.com> wrote:
> 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
>
> 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.
> Signed-off-by: Harry Pan <harry.pan@intel.com> ---
>drivers/gpu/drm/i915/intel_panel.c | 2 +- 1 file changed, 1
>insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 08ab6d7..e882139 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -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.
> if (panel->backlight.device)
> panel->backlight.device->props.brightness =
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread* ✓ Fi.CI.BAT: success for drm/i915: minor corner case fix to respect user's backlight setting
2017-01-27 12:57 [PATCH] drm/i915: minor corner case fix to respect user's backlight setting Harry Pan
2017-01-27 14:10 ` Jani Nikula
@ 2017-01-30 11:54 ` Patchwork
1 sibling, 0 replies; 3+ messages in thread
From: Patchwork @ 2017-01-30 11:54 UTC (permalink / raw)
To: Harry Pan; +Cc: intel-gfx
== 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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-30 11:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-27 12:57 [PATCH] drm/i915: minor corner case fix to respect user's backlight setting Harry Pan
2017-01-27 14:10 ` Jani Nikula
2017-01-30 11:54 ` ✓ Fi.CI.BAT: success for " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox