From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Indan Zupancic" Subject: Re: [PATCH 2/4] drm/i915: remove combination mode for backlight control, again Date: Thu, 30 Aug 2012 11:29:11 +0200 Message-ID: <3b608bee1d5a0c45f909f1f6eebcd410.squirrel@webmail.greenhost.nl> References: <918c526028f34df880d0eb5db6bd3b862092743e.1346136448.git.jani.nikula@intel.com> <20120828145557.GD5125@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from smarthost1.greenhost.nl (smarthost1.greenhost.nl [195.190.28.78]) by gabe.freedesktop.org (Postfix) with ESMTP id 3BC879EC15 for ; Thu, 30 Aug 2012 02:29:13 -0700 (PDT) In-Reply-To: <20120828145557.GD5125@phenom.ffwll.local> 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: Daniel Vetter Cc: Jani Nikula , Takashi Iwai , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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. Greetings, Indan