From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 2/4] drm/i915: remove combination mode for backlight control, again Date: Wed, 14 Nov 2012 08:48:07 -0800 Message-ID: <20121114084807.330431fb@jbarnes-desktop> References: <918c526028f34df880d0eb5db6bd3b862092743e.1346136448.git.jani.nikula@intel.com> <20120828145557.GD5125@phenom.ffwll.local> <3b608bee1d5a0c45f909f1f6eebcd410.squirrel@webmail.greenhost.nl> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy8-pub.bluehost.com (oproxy8-pub.bluehost.com [69.89.22.20]) by gabe.freedesktop.org (Postfix) with SMTP id 1ED10A0874 for ; Wed, 14 Nov 2012 08:47:35 -0800 (PST) In-Reply-To: <3b608bee1d5a0c45f909f1f6eebcd410.squirrel@webmail.greenhost.nl> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Indan Zupancic Cc: Jani Nikula , Takashi Iwai , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 30 Aug 2012 11:29:11 +0200 "Indan Zupancic" wrote: > On Tue, August 28, 2012 16:55, Daniel Vetter wrote: > > On Tue, Aug 28, 2012 at 04:39:34PM +0200, Indan Zupancic wrote: > >> Some backlight problems on GEN4 can be solved by not fiddling with the > >> backlight. The current code sets the backlight to 0 to disable the panel > >> (last year anyway, maybe the code changed), but on gen >= 4 it can do the > >> same by clearing bit 31 in BLC_PWM_CLT2. That way the original backlight > >> value doesn't need to be saved anywhere. > > > > A while back I've improved the backlight code to properly switch the pipe > > that controls the pwm and also to properly toggle the enable bit for > > gen4+, see the new intel_panel_enable_backlight functions. Would it be > > correct in your opinion to simply ditch the call to > > intel_panel_actually_set_backlight for gen4+ unconditionally? Same for > > intel_panel_disable_backlight obviously? > > Yes, that seems the whole point of having the PWM disable bit. > > I would also ditch the backlight_enabled state tracking, as it's > unnecessary and incorrect because the enable/disable calls aren't > balanced. > > I'll update my kernel to the latest git code and try out how the > current code works and if not touching LBPC at all works for gen 2 > hardware. If it doesn't then LBPC needs to be saved/restored in > i915_suspend.c after all. > > It would be nice if backlight control worked with ASLE disabled, > that would get rid of all complexity, including combination mode. > Or alternatively, if disabling combination mode at boot works then > we can get rid of the complicated brightness setting code to cope > with it. Did we bottom out on this discussion? Reading through the thread I don't see any firm conclusions, and we definitely still have bugs open that may be resolved by this incorrect patchset... -- Jesse Barnes, Intel Open Source Technology Center