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

From: Jani Nikula <jani.nikula@intel.com>
To: Daniel Vetter <daniel@ffwll.ch>, Scot Doyle <lkml14@scotdoyle.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: don't warn if backlight unexpectedly enabled
Date: Wed, 27 Aug 2014 14:01:06 +0300	[thread overview]
Message-ID: <87d2bmjix9.fsf@intel.com> (raw)
In-Reply-To: <20140826173325.GF15520@phenom.ffwll.local>

On Tue, 26 Aug 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Aug 26, 2014 at 04:15:53PM +0000, Scot Doyle wrote:
>> On Tue, 26 Aug 2014, Daniel Vetter wrote:
>> > On Thu, Aug 21, 2014 at 07:12:59AM +0000, Scot Doyle wrote:
>> >> 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?
>> 
>> /sys/class/backlight/intel_backlight/brightness
>> 0 -> backlight not visible
>> 1 -> backlight visible
>> 937 -> max backlight
>> 
>> Setting /sys/class/backlight/intel_backlight/brightness to 0 updates 
>> BLC_PWM_CPU_CTL, but BLC_PWM_CPU_CTL2 remains 0xe0000000.
>> 
>> 
>> > Maybe we just need to extend the check to look for !PWM_ENABLE ||
>> > duty_cycle == 0.
>> 
>> The following measurements hold true no matter the duty cycle before 
>> suspend:
>> 
>> When entering hsw_enable_pc8 during suspend
>> - the physical backlight is off
>> - BLC_PWM_CPU_CTL == 0x3a900000 (BACKLIGHT_DUTY_CYCLE_MASK == ffff)
>> - BLC_PWM_CPU_CTL2 == 0x60000000 (BLM_PWM_ENABLE)
>> 
>> When exiting hsw_disable_pc8 during resume
>> - the physical backlight is off
>> - BLC_PWM_CPU_CTL == 0x200
>> - BLC_PWM_CPU_CTL2 == 0x80000000 (BLM_PWM_ENABLE | BLM_TRANSCODER_EDP)
>> 
>> When entering pch_enable_backlight during resume
>> - the physical backlight is off
>> - BLC_PWM_CPU_CTL == 0x200
>> - BLC_PWM_CPU_CTL2 == 0x80000000 (BLM_PWM_ENABLE)
>> 
>> When exiting pch_enable_backlight during resume
>> - the physical backlight is off
>> - BLC_PWM_CPU_CTL == duty cycle prior to suspend
>> - BLC_PWM_CPU_CTL2 == 0xe0000000 (BLM_PWM_ENABLE | BLM_TRANSCODER_EDP)
>> 
>> 
>> So the BIOS is setting BLC_PWM_CPU_CTL=0x200 and BLC_PWM_CPU_CTL2=0x80000000 ?
>
> Indeed the bios seems to just but gunk into that register. And if we add
> in all the knobs there's piles of them (you have semi-duplicated backlight
> registers on hsw on the PCH), so I guess it doesn't make sense to combine
> them all and warn if something goes awry, at least not in a -fixes patch.
> So Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> on your original
> patch.

Pushed to drm-intel-fixes, thanks for the patch and review.

BR,
Jani.

>
> Jani can decide whether he wants to save this WARN_ON (imo it's useful to
> have such sanity-checks) in -next by taking all the various bits and duty
> cycles into account. But maybe just on the latest platforms, that still
> should give is good coverage, but with a lot less fuss.
>
> Thanks for tracking this all down.
>
> Cheers, Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center