From: Jani Nikula <jani.nikula@linux.intel.com>
To: gareth.yu@intel.com, intel-gfx@lists.freedesktop.org
Cc: Gareth Yu <gareth.yu@intel.com>,
Ville Syrjala <ville.syrjala@linux.intel.com>
Subject: Re: [PATCH] drm/i915/display: Fixed a screen flickering when turning on display from off
Date: Tue, 26 Mar 2024 15:11:33 +0200 [thread overview]
Message-ID: <87plvhgmt6.fsf@intel.com> (raw)
In-Reply-To: <20240321045311.2124111-1-gareth.yu@intel.com>
On Thu, 21 Mar 2024, gareth.yu@intel.com wrote:
> From: Gareth Yu <gareth.yu@intel.com>
>
> Turn on the panel from zero brightness of the last state, the panel was
> set a maximum PWM in the flow. Once the panel initialization is completed,
> the backlight is restored to xero brightness. There is a flckering
> generated. This flicker happens in "Screen dimming and power off" of
> Google's design and resume from sleep. The sample of DMESG is below.
>
> (suspend)
> [53949.248875] i915 0000:00:02.0: [drm:intel_edp_backlight_off]
> [53949.452046] i915 0000:00:02.0: [drm:intel_backlight_set_pwm_level] set backlight PWM = 0
>
> (wakeup)
> [53986.067356] i915 0000:00:02.0: [drm:intel_edp_backlight_on]
> [53986.067367] i915 0000:00:02.0: [drm:intel_backlight_enable] pipe A
> [53986.067476] i915 0000:00:02.0: [drm:intel_backlight_set_pwm_level] set backlight PWM = 96000
> [53986.119766] backlight intel_backlight: PM: calling backlight_resume+0x0/0x7a @ 4916, parent: card0-eDP-1
> [53986.119781] backlight intel_backlight: PM: backlight_resume+0x0/0x7a returned 0 after 0 usecs
> [53986.220068] [drm:intel_backlight_device_update_status] updating intel_backlight, brightness=26321/96000
> [53986.220086] i915 0000:00:02.0: [drm:intel_panel_actually_set_backlight] set backlight level = 27961
>
> Set the brightness to the minimum value when the new brightness is less
> or equal to the minimum value to mitigate this flickering.
Thanks for the patch. I've pushed this to drm-intel-next, but took the
liberty to rewrite the commit message:
commit 92714006eb4d10ddfaa0eca41c81e6b483e02f93
Author: Gareth Yu <gareth.yu@intel.com>
Date: Thu Mar 21 12:53:11 2024 +0800
drm/i915/backlight: Do not bump min brightness to max on enable
Historically the expectation was to set brightness to max on enable, if
it was zero. However, the policy should be to preserve brightness across
disable/enable, for example the userspace might want to dim the
brightness before disable (e.g. on suspend) and gradually bring it back
up after enable (e.g. on resume). If the brightness gets bumped to max
at enable, this causes flicker as the userspace expects the brightness
to have been preserved across disable/enable.
For example:
(suspend)
[53949.248875] i915 0000:00:02.0: [drm:intel_edp_backlight_off]
[53949.452046] i915 0000:00:02.0: [drm:intel_backlight_set_pwm_level] set backlight PWM = 0
(wakeup)
[53986.067356] i915 0000:00:02.0: [drm:intel_edp_backlight_on]
[53986.067367] i915 0000:00:02.0: [drm:intel_backlight_enable] pipe A
[53986.067476] i915 0000:00:02.0: [drm:intel_backlight_set_pwm_level] set backlight PWM = 96000
[53986.119766] backlight intel_backlight: PM: calling backlight_resume+0x0/0x7a @ 4916, parent: card0-eDP-1
[53986.119781] backlight intel_backlight: PM: backlight_resume+0x0/0x7a returned 0 after 0 usecs
[53986.220068] [drm:intel_backlight_device_update_status] updating intel_backlight, brightness=26321/96000
[53986.220086] i915 0000:00:02.0: [drm:intel_panel_actually_set_backlight] set backlight level = 27961
Reduce the check to respecting the minimum brightness, and avoid bumping
min brightness to max on enable.
Note: It's possible there's still userspace out there that relies on the
old behaviour. It would be unfortunate, but there's really only one way
to find out.
Fingers crossed, let's hope nothing regresses.
BR,
Jani.
>
> Cc : Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
> Cc : Matt Roper <matthew.d.roper@intel.com>
> Cc : Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Gareth Yu <gareth.yu@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_backlight.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
> index 3f3cd944a1c5..23fd011b6bfb 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -761,8 +761,8 @@ static void __intel_backlight_enable(const struct intel_crtc_state *crtc_state,
>
> WARN_ON(panel->backlight.max == 0);
>
> - if (panel->backlight.level <= panel->backlight.min) {
> - panel->backlight.level = panel->backlight.max;
> + if (panel->backlight.level < panel->backlight.min) {
> + panel->backlight.level = panel->backlight.min;
> if (panel->backlight.device)
> panel->backlight.device->props.brightness =
> scale_hw_to_user(connector,
--
Jani Nikula, Intel
next prev parent reply other threads:[~2024-03-26 13:11 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-06 3:13 [PATCH] drm/i915/display: Fixed a screen flickering when turning on display from off gareth.yu
2024-03-06 8:32 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2024-03-06 8:48 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-03-06 10:19 ` [PATCH] " Jani Nikula
2024-03-15 11:02 ` Ville Syrjälä
2024-03-18 2:20 ` Yu, Gareth
2024-03-18 7:58 ` Dolan Liu
2024-03-18 13:45 ` Jani Nikula
2024-03-19 2:04 ` Yu, Gareth
2024-03-12 3:17 ` gareth.yu
2024-03-12 4:14 ` ✗ Fi.CI.BAT: failure for drm/i915/display: Fixed a screen flickering when turning on display from off (rev2) Patchwork
2024-03-14 4:20 ` ✓ Fi.CI.BAT: success for drm/i915/display: Fixed a screen flickering when turning on display from off (rev3) Patchwork
2024-03-14 14:47 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-03-15 10:01 ` [PATCH] drm/i915/display: Fixed a screen flickering when turning on display from off gareth.yu
2024-03-15 12:34 ` ✓ Fi.CI.BAT: success for drm/i915/display: Fixed a screen flickering when turning on display from off (rev4) Patchwork
2024-03-16 14:19 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-03-18 2:15 ` [PATCH] drm/i915/display: Fixed a screen flickering when turning on display from off gareth.yu
2024-03-18 2:50 ` ✓ Fi.CI.BAT: success for drm/i915/display: Fixed a screen flickering when turning on display from off (rev5) Patchwork
2024-03-18 4:37 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-03-20 16:27 ` ✓ Fi.CI.IGT: success " Patchwork
2024-03-20 16:56 ` Patchwork
2024-03-21 4:53 ` [PATCH] drm/i915/display: Fixed a screen flickering when turning on display from off gareth.yu
2024-03-26 13:11 ` Jani Nikula [this message]
2024-03-21 5:30 ` ✓ Fi.CI.BAT: success for drm/i915/display: Fixed a screen flickering when turning on display from off (rev6) Patchwork
2024-03-22 1:24 ` ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87plvhgmt6.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=gareth.yu@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.