Re: [PATCH] drm/i915: don't warn if backlight unexpectedly enabled

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Aug 21, 2014 at 07:12:59AM +0000, Scot Doyle wrote:
> 
> On Tue, 19 Aug 2014, Daniel Vetter wrote:
> >On Tue, Aug 19, 2014 at 4:07 AM, Scot Doyle <lkml14@scotdoyle.com> wrote:
> >>BIOS or firmware can modify hardware state during suspend/resume,
> >>for example on the Toshiba CB35 or Lenovo T400, so log a debug message
> >>instead of a warning if the backlight is unexpectedly enabled.
> >>
> >>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80930
> >>Cc: Jani Nikula <jani.nikula@intel.com>
> >>Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> >
> >This should get cleaned up in the modeset state sanitization we do
> >upon resume, so without someone digging into this bug a bit and coming
> >up with an explanation for why that fails I'm reluctant to merge this.
> >-Daniel
> 
> When we enter intel_modeset_setup_hw_state during resume
> - BLC_PWM_CPU_CTL2 == BLM_PWM_ENABLE
> - the physical backlight is off

Hm, this is actually interesting - we have some other evidence that the
best way to shut off the backlight is actually to just set the pwm duty
cycle to 0. Can you please check that this is the case for your system?

Maybe we just need to extend the check to look for !PWM_ENABLE ||
duty_cycle == 0.

In general if we hit upon a WARN it's not a good idea to just shut it up,
but to dig a bit deeper and figure out why exactly something doesn't work
as we expected it to work.

Thanks, Daniel

> - readout_hw_state says crtcs, encoders and connectors are disabled
> 
> The sanitizations done before reaching the force_restore block are
> - drm_vblank_off for each crtc
> - sarea_priv->pipeA_w=0
> - sarea_priv->pipeA_h=0
> - ilk_wm_get_hw_state(dev)
> 
> Then the warning is logged inside the force_restore block
> [11.651495] [<ffffffff8106fc5c>] warn_slowpath_fmt+0x5c/0x80
> [11.651508] [<ffffffffa0576d0a>] pch_enable_backlight+0x1ba/0x200
> [11.651518] [<ffffffffa0577474>] intel_panel_enable_backlight+0xa4/0x100
> [11.651529] [<ffffffffa0568514>] intel_edp_backlight_on+0x54/0x140
> [11.651540] [<ffffffffa0560c23>] intel_enable_ddi+0xb3/0x100
> [11.651552] [<ffffffffa054b869>] haswell_crtc_enable+0x559/0xe80
> [11.651564] [<ffffffffa0545ea4>] __intel_set_mode+0xf94/0x1c00
> [11.651575] [<ffffffffa055062b>] intel_modeset_setup_hw_state+0x55b/0xd80
> [11.651609] [<ffffffffa04eb3a9>] __i915_drm_thaw+0x159/0x1d0
> [11.651617] [<ffffffffa04ebcd8>] i915_resume+0x28/0x50
> 
> I see no code in the execution path of intel_modeset_setup_hw_state to
> either note the state of BLC_PWM_CPU_CTL2 or reset it, apart from the
> pch_enable_backlight function. Which would be one explanation :-)

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch