From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: don't warn if backlight unexpectedly enabled Date: Wed, 27 Aug 2014 14:01:06 +0300 Message-ID: <87d2bmjix9.fsf@intel.com> References: <20140826104537.GO15520@phenom.ffwll.local> <20140826173325.GF15520@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 9684189D43 for ; Wed, 27 Aug 2014 04:01:09 -0700 (PDT) In-Reply-To: <20140826173325.GF15520@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter , Scot Doyle Cc: intel-gfx List-Id: intel-gfx@lists.freedesktop.org On Tue, 26 Aug 2014, Daniel Vetter 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 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