drm/i915: remove QUIRK_NO_PCH_PWM_ENABLE
authorJani Nikula <jani.nikula@intel.com>
Thu, 14 Nov 2013 10:14:29 +0000 (12:14 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 14 Nov 2013 11:22:09 +0000 (12:22 +0100)
The quirk was added as what I'd say was a stopgap measure in

commit e85843bec6c2ea7c10ec61238396891cc2b753a9
Author: Kamal Mostafa <kamal@canonical.com>
Date:   Fri Jul 19 15:02:01 2013 -0700

    drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight

without really digging into what was going on.

Also, as mentioned in the related bug [1], having the quirk regressed
some of the machines it was supposed to fix to begin with, and there
were patches posted to disable the quirk on such machines [2]!

The fact is, we do need the BLM_PCH_PWM_ENABLE bit set to have
backlight. With the quirk, we've relied on BIOS to have set it, and our
save/restore code to retain it. With the full backlight setup at enable,
we have no place for things that rely on previous state.

With the per platform hooks, we've also made a change in the PCH
platform enable order: setting the backlight duty cycle between CPU and
PCH PWM enable. Some experimenting and

commit 770c12312ad617172b1a65b911d3e6564fc5aca8
Author: Takashi Iwai <tiwai@suse.de>
Date:   Sat Aug 11 08:56:42 2012 +0200

    drm/i915: Fix blank panel at reopening lid

indicate that we can't set the backlight before enabling CPU PWM; the
value just won't stick. But AFAICT we should do it before enabling the
PCH PWM.

Finally, any fallout we should fix properly, preferrably without quirks,
and absolutely without quirks that rely on existing state. With the per
platform hooks have much more flexibility to adjust the sequence as
required by platforms.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=47941
[2] http://lkml.kernel.org/r/1378229848-29113-1-git-send-email-kamal@canonical.com

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/intel_display.c
drivers/gpu/drm/i915/intel_panel.c

index c243b8e954a06750de48cd6215abf82a67336e4f..e726ab985100ef6b6f67c2ca905922a2d8506a24 100644 (file)
@@ -717,7 +717,6 @@ enum intel_sbi_destination {
 #define QUIRK_PIPEA_FORCE (1<<0)
 #define QUIRK_LVDS_SSC_DISABLE (1<<1)
 #define QUIRK_INVERT_BRIGHTNESS (1<<2)
-#define QUIRK_NO_PCH_PWM_ENABLE (1<<3)
 
 struct intel_fbdev;
 struct intel_fbc_work;
index 25ef080d1bc2fbc182d4aca779551d53ce992341..b9f763c637ec1a85c2eeff3a2d9048cbb494c371 100644 (file)
@@ -10456,17 +10456,6 @@ static void quirk_invert_brightness(struct drm_device *dev)
        DRM_INFO("applying inverted panel brightness quirk\n");
 }
 
-/*
- * Some machines (Dell XPS13) suffer broken backlight controls if
- * BLM_PCH_PWM_ENABLE is set.
- */
-static void quirk_no_pcm_pwm_enable(struct drm_device *dev)
-{
-       struct drm_i915_private *dev_priv = dev->dev_private;
-       dev_priv->quirks |= QUIRK_NO_PCH_PWM_ENABLE;
-       DRM_INFO("applying no-PCH_PWM_ENABLE quirk\n");
-}
-
 struct intel_quirk {
        int device;
        int subsystem_vendor;
@@ -10526,11 +10515,6 @@ static struct intel_quirk intel_quirks[] = {
         * seem to use inverted backlight PWM.
         */
        { 0x2a42, 0x1025, PCI_ANY_ID, quirk_invert_brightness },
-
-       /* Dell XPS13 HD Sandy Bridge */
-       { 0x0116, 0x1028, 0x052e, quirk_no_pcm_pwm_enable },
-       /* Dell XPS13 HD and XPS13 FHD Ivy Bridge */
-       { 0x0166, 0x1028, 0x058b, quirk_no_pcm_pwm_enable },
 };
 
 static void intel_init_quirks(struct drm_device *dev)
index 0986472d425424ab996529b543d480cd5a61d047..da088e33dd19cd7e366708fc42091d43317b5365 100644 (file)
@@ -749,10 +749,6 @@ static void pch_enable_backlight(struct intel_connector *connector)
        pch_ctl2 = panel->backlight.max << 16;
        I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2);
 
-       /* XXX: transitional */
-       if (dev_priv->quirks & QUIRK_NO_PCH_PWM_ENABLE)
-               return;
-
        pch_ctl1 = 0;
        if (panel->backlight.active_low_pwm)
                pch_ctl1 |= BLM_PCH_POLARITY;