From mboxrd@z Thu Jan 1 00:00:00 1970 From: Carsten Emde Subject: Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4 Date: Mon, 23 Apr 2012 15:15:02 +0200 Message-ID: <4F9555D6.2020708@osadl.org> References: <1335173535-14811-1-git-send-email-daniel.vetter@ffwll.ch> <4F9542EF.3010208@osadl.org> <20120423123257.GE4935@phenom.ffwll.local> <20120423123619.GF4935@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from toro.web-alm.net (toro.web-alm.net [62.245.132.31]) by gabe.freedesktop.org (Postfix) with ESMTP id D68559E76E for ; Mon, 23 Apr 2012 06:22:11 -0700 (PDT) In-Reply-To: <20120423123619.GF4935@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: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On 04/23/2012 02:36 PM, Daniel Vetter wrote: > On Mon, Apr 23, 2012 at 02:32:57PM +0200, Daniel Vetter wrote: >> On Mon, Apr 23, 2012 at 01:54:23PM +0200, Carsten Emde wrote: >>> On 04/23/2012 11:32 AM, Daniel Vetter wrote: >>>> There's a bit in the docs for gen4 only that says whether the >>>> backlight control is inverted. And both the quirk we have and >>>> all bugs only concern i965gm and gm45 (and mostly Acer) afaics. >>>> >>>> So lets drop the quirk and use the bit instead. >>>> >>>> Also clean up the BLC register definitions a bit by correctly >>>> grouping the CTL and CTL2 definitions together. >>>> [..] >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >>>> index 4c844c6..5d215f0 100644 >>>> --- a/drivers/gpu/drm/i915/intel_display.c >>>> +++ b/drivers/gpu/drm/i915/intel_display.c >>>> @@ -6482,9 +6482,6 @@ static struct intel_quirk intel_quirks[] = { >>>> >>>> /* Sony Vaio Y cannot use SSC on LVDS */ >>>> { 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable }, >>>> - >>>> - /* Acer Aspire 5734Z must invert backlight brightness */ >>>> - { 0x2a42, 0x1025, 0x0459, quirk_invert_brightness }, >>> Made a first test with these lines removed and the new mechanism >>> added. Unfortunately, the screen is now dark again. Works after >>> re-adding the quirk. >>> >>>> + /* gen4 has a polarity bit */ >>>> + if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965)) >>>> + return intel_panel_get_max_backlight(dev) - val; >>>> + >>> Apparently, these conditions are not met on the Acer Aspire 5734Z. >>> Will add some debug output and evaluate further. For the time being, >>> please leave the quirk in place. >> >> Hm, I've tried this and when I set this bit, panel brightness is inverted >> on my gm45. Can you install intel-gpu-tools and read out the 2 backlight >> control registers with >> >> # intel_reg_read 0x61254 >> # intel_reg_read 0x61250 > > I've forgotten to add: Also please grab these register values when booting > with nomodeset, so that we can compare the values the bios sets with > whatever i915 sets. Without nomodeset: # intel_reg_read 0x61254 0x61254 : 0xB4A0B4A # intel_reg_read 0x61250 0x61250 : 0xE0000000 With nomodeset: # intel_reg_read 0x61254 0x61254 : 0xB4A0B4A # intel_reg_read 0x61250 0x61250 : 0xE0000000 Thus, (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965) is never set. But bit 29 would work, so we could add another conditional such as: #define BLM_POLARITY_PCH_I965 (1 << 29) /* PCH only */ if (PCH) try (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_PCH_I965) -Carsten.