From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH 05/13] drm/i915: fix gen2-gen3 backlight set Date: Wed, 13 Nov 2013 10:27:38 +0200 Message-ID: <87habgso3p.fsf@intel.com> References: <5da7c92cb5634c7b7349065fe82f46c0564a6715.1383920621.git.jani.nikula@intel.com> <1384293603.2462.39.camel@ideak-mobl> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id F040EFAF3F for ; Wed, 13 Nov 2013 00:30:35 -0800 (PST) In-Reply-To: <1384293603.2462.39.camel@ideak-mobl> 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: imre.deak@intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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? 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