From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 05/13] drm/i915: fix gen2-gen3 backlight set Date: Wed, 13 Nov 2013 10:04:18 +0100 Message-ID: <20131113090418.GA7251@phenom.ffwll.local> References: <5da7c92cb5634c7b7349065fe82f46c0564a6715.1383920621.git.jani.nikula@intel.com> <1384293603.2462.39.camel@ideak-mobl> <87habgso3p.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f49.google.com (mail-ee0-f49.google.com [74.125.83.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 523B8FAF2A for ; Wed, 13 Nov 2013 01:03:51 -0800 (PST) Received: by mail-ee0-f49.google.com with SMTP id b15so60077eek.8 for ; Wed, 13 Nov 2013 01:03:50 -0800 (PST) Content-Disposition: inline In-Reply-To: <87habgso3p.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Nov 13, 2013 at 10:27:38AM +0200, Jani Nikula wrote: > On Wed, 13 Nov 2013, Imre Deak wrote: > > On Fri, 2013-11-08 at 16:48 +0200, Jani Nikula wrote: > >> Signed-off-by: Jani Nikula > >> --- > >> drivers/gpu/drm/i915/intel_panel.c | 10 +++++++--- > >> 1 file changed, 7 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > >> index a821949..e82b2dd 100644 > >> --- a/drivers/gpu/drm/i915/intel_panel.c > >> +++ b/drivers/gpu/drm/i915/intel_panel.c > >> @@ -555,7 +555,7 @@ static void i9xx_set_backlight(struct intel_connector *connector, u32 level) > >> { > >> struct drm_device *dev = connector->base.dev; > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> - u32 tmp; > >> + u32 tmp, mask; > >> > >> if (is_backlight_combination_mode(dev)) { > >> u32 max = intel_panel_get_max_backlight(connector); > >> @@ -570,10 +570,14 @@ static void i9xx_set_backlight(struct intel_connector *connector, u32 level) > >> pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc); > >> } > >> > >> - if (INTEL_INFO(dev)->gen < 4) > >> + if (IS_GEN4(dev)) { > >> + mask = BACKLIGHT_DUTY_CYCLE_MASK; > >> + } else { > >> level <<= 1; > >> + mask = BACKLIGHT_DUTY_CYCLE_MASK_PNV; > >> + } > > > > According to the gen2/3 bspec I have, the correct mask is > > BACKLIGHT_DUTY_CYCLE_MASK_PNV only in case of IS_PINEVIEW(dev), for > > everything else it's BACKLIGHT_DUTY_CYCLE_MASK. > > What you say is correct, but we've treated all gen2/3 similar to PNV > since > > commit ca88479c1c3b7b1a9f94320745f5331e1de77f80 > Author: Keith Packard > Date: Fri Nov 18 11:09:24 2011 -0800 > > drm/i915: Treat pre-gen4 backlight duty cycle value consistently > > i.e. we only use the high 15 bits for all gen2/3. For non-PNV this just > means the lowest bit is always zero. For PNV the lowest bit has a > different meaning in both the PWM freq and duty cycle fields. > > I don't want to take any chances by changing this behaviour. I realize > there's zero comments about this in the code; would you like me to add > some? Yeah, I think some comment here would be good. Or maybe a follow-up patch to differentiate between pnv and everything else on gen2/3. -Daniel > > > BR, > Jani. > > > > > > --Imre > > > >> > >> - tmp = I915_READ(BLC_PWM_CTL) & ~BACKLIGHT_DUTY_CYCLE_MASK; > >> + tmp = I915_READ(BLC_PWM_CTL) & ~mask; > >> I915_WRITE(BLC_PWM_CTL, tmp | level); > >> } > >> > > > > > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch