Merge branch 'bdw-fixes' into backlight-rework
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 14 Nov 2013 14:17:41 +0000 (15:17 +0100)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Fri, 15 Nov 2013 08:44:29 +0000 (09:44 +0100)
Merge the bdw changes into the backlight rework branch so that we can
adapt the new code for bdw, too.  This is a bit a mess, but doing this
another way would have delayed the merging of the backlight
refactoring. Mea culpa.

As discussed with Jani on irc only do bdw-specific callbacks for the
set/get methods and bake in the only other special-case into the pch
enable function.

Conflicts:
drivers/gpu/drm/i915/intel_panel.c

v2: Don't enable the PWM too early for bdw (Jani).

v3: Create new bdw_ functions for setup and enable - the rules change
sufficiently imo with the switch from controlling the pwm from the cpu
to controlling it completel from the pch to warrant this.

v4: Rip out unused pipe variable in bdw_enable_backlight (0-day
builder).

Tested-by: Ben Widawsky <ben@bwidawsk.net> (on bdw)
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/i915_dma.c
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/i915_suspend.c
drivers/gpu/drm/i915/i915_ums.c
drivers/gpu/drm/i915/intel_display.c
drivers/gpu/drm/i915/intel_drv.h
drivers/gpu/drm/i915/intel_opregion.c
drivers/gpu/drm/i915/intel_panel.c

index 0cab2d045135b66d0462c88d390e43eff3df7b20..9a2a17507df4f4d9fede650143364cd71c5b2557 100644 (file)
@@ -1486,7 +1486,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
        spin_lock_init(&dev_priv->irq_lock);
        spin_lock_init(&dev_priv->gpu_error.lock);
-       spin_lock_init(&dev_priv->backlight.lock);
+       spin_lock_init(&dev_priv->backlight_lock);
        spin_lock_init(&dev_priv->uncore.lock);
        spin_lock_init(&dev_priv->mm.object_stat_lock);
        mutex_init(&dev_priv->dpio_lock);
index 51951ef7e71df865a13d2f6a885b4518a5ba59c4..2a83e14abea602bf406e12ae7125feaa238163ad 100644 (file)
@@ -351,6 +351,7 @@ struct drm_i915_error_state {
        enum intel_ring_hangcheck_action hangcheck_action[I915_NUM_RINGS];
 };
 
+struct intel_connector;
 struct intel_crtc_config;
 struct intel_crtc;
 struct intel_limit;
@@ -413,6 +414,13 @@ struct drm_i915_display_funcs {
        /* render clock increase/decrease */
        /* display clock increase/decrease */
        /* pll clock increase/decrease */
+
+       int (*setup_backlight)(struct intel_connector *connector);
+       uint32_t (*get_backlight)(struct intel_connector *connector);
+       void (*set_backlight)(struct intel_connector *connector,
+                             uint32_t level);
+       void (*disable_backlight)(struct intel_connector *connector);
+       void (*enable_backlight)(struct intel_connector *connector);
 };
 
 struct intel_uncore_funcs {
@@ -708,7 +716,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;
@@ -761,8 +768,6 @@ struct i915_suspend_saved_registers {
        u32 saveBLC_PWM_CTL;
        u32 saveBLC_PWM_CTL2;
        u32 saveBLC_HIST_CTL_B;
-       u32 saveBLC_PWM_CTL_B;
-       u32 saveBLC_PWM_CTL2_B;
        u32 saveBLC_CPU_PWM_CTL;
        u32 saveBLC_CPU_PWM_CTL2;
        u32 saveFPB0;
@@ -1368,13 +1373,8 @@ typedef struct drm_i915_private {
        struct intel_overlay *overlay;
        unsigned int sprite_scaling_enabled;
 
-       /* backlight */
-       struct {
-               int level;
-               bool enabled;
-               spinlock_t lock; /* bl registers and the above bl fields */
-               struct backlight_device *device;
-       } backlight;
+       /* backlight registers and fields in struct intel_panel */
+       spinlock_t backlight_lock;
 
        /* LVDS info */
        bool no_aux_handshake;
index 98790c7cccb1ab0902662e4cc541de9966f2058c..6b8fef7fb3bb94f3aa442c867452637e9dd0919f 100644 (file)
@@ -192,7 +192,6 @@ static void i915_restore_vga(struct drm_device *dev)
 static void i915_save_display(struct drm_device *dev)
 {
        struct drm_i915_private *dev_priv = dev->dev_private;
-       unsigned long flags;
 
        /* Display arbitration control */
        if (INTEL_INFO(dev)->gen <= 4)
@@ -203,46 +202,27 @@ static void i915_save_display(struct drm_device *dev)
        if (!drm_core_check_feature(dev, DRIVER_MODESET))
                i915_save_display_reg(dev);
 
-       spin_lock_irqsave(&dev_priv->backlight.lock, flags);
-
        /* LVDS state */
        if (HAS_PCH_SPLIT(dev)) {
                dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
-               dev_priv->regfile.saveBLC_PWM_CTL = I915_READ(BLC_PWM_PCH_CTL1);
-               dev_priv->regfile.saveBLC_PWM_CTL2 = I915_READ(BLC_PWM_PCH_CTL2);
-               dev_priv->regfile.saveBLC_CPU_PWM_CTL = I915_READ(BLC_PWM_CPU_CTL);
-               dev_priv->regfile.saveBLC_CPU_PWM_CTL2 = I915_READ(BLC_PWM_CPU_CTL2);
                if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
                        dev_priv->regfile.saveLVDS = I915_READ(PCH_LVDS);
        } else if (IS_VALLEYVIEW(dev)) {
                dev_priv->regfile.savePP_CONTROL = I915_READ(PP_CONTROL);
                dev_priv->regfile.savePFIT_PGM_RATIOS = I915_READ(PFIT_PGM_RATIOS);
 
-               dev_priv->regfile.saveBLC_PWM_CTL =
-                       I915_READ(VLV_BLC_PWM_CTL(PIPE_A));
                dev_priv->regfile.saveBLC_HIST_CTL =
                        I915_READ(VLV_BLC_HIST_CTL(PIPE_A));
-               dev_priv->regfile.saveBLC_PWM_CTL2 =
-                       I915_READ(VLV_BLC_PWM_CTL2(PIPE_A));
-               dev_priv->regfile.saveBLC_PWM_CTL_B =
-                       I915_READ(VLV_BLC_PWM_CTL(PIPE_B));
                dev_priv->regfile.saveBLC_HIST_CTL_B =
                        I915_READ(VLV_BLC_HIST_CTL(PIPE_B));
-               dev_priv->regfile.saveBLC_PWM_CTL2_B =
-                       I915_READ(VLV_BLC_PWM_CTL2(PIPE_B));
        } else {
                dev_priv->regfile.savePP_CONTROL = I915_READ(PP_CONTROL);
                dev_priv->regfile.savePFIT_PGM_RATIOS = I915_READ(PFIT_PGM_RATIOS);
-               dev_priv->regfile.saveBLC_PWM_CTL = I915_READ(BLC_PWM_CTL);
                dev_priv->regfile.saveBLC_HIST_CTL = I915_READ(BLC_HIST_CTL);
-               if (INTEL_INFO(dev)->gen >= 4)
-                       dev_priv->regfile.saveBLC_PWM_CTL2 = I915_READ(BLC_PWM_CTL2);
                if (IS_MOBILE(dev) && !IS_I830(dev))
                        dev_priv->regfile.saveLVDS = I915_READ(LVDS);
        }
 
-       spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
-
        if (!IS_I830(dev) && !IS_845G(dev) && !HAS_PCH_SPLIT(dev))
                dev_priv->regfile.savePFIT_CONTROL = I915_READ(PFIT_CONTROL);
 
@@ -278,7 +258,6 @@ static void i915_restore_display(struct drm_device *dev)
 {
        struct drm_i915_private *dev_priv = dev->dev_private;
        u32 mask = 0xffffffff;
-       unsigned long flags;
 
        /* Display arbitration */
        if (INTEL_INFO(dev)->gen <= 4)
@@ -287,12 +266,6 @@ static void i915_restore_display(struct drm_device *dev)
        if (!drm_core_check_feature(dev, DRIVER_MODESET))
                i915_restore_display_reg(dev);
 
-       spin_lock_irqsave(&dev_priv->backlight.lock, flags);
-
-       /* LVDS state */
-       if (INTEL_INFO(dev)->gen >= 4 && !HAS_PCH_SPLIT(dev))
-               I915_WRITE(BLC_PWM_CTL2, dev_priv->regfile.saveBLC_PWM_CTL2);
-
        if (drm_core_check_feature(dev, DRIVER_MODESET))
                mask = ~LVDS_PORT_EN;
 
@@ -305,13 +278,6 @@ static void i915_restore_display(struct drm_device *dev)
                I915_WRITE(PFIT_CONTROL, dev_priv->regfile.savePFIT_CONTROL);
 
        if (HAS_PCH_SPLIT(dev)) {
-               I915_WRITE(BLC_PWM_PCH_CTL1, dev_priv->regfile.saveBLC_PWM_CTL);
-               I915_WRITE(BLC_PWM_PCH_CTL2, dev_priv->regfile.saveBLC_PWM_CTL2);
-               /* NOTE: BLC_PWM_CPU_CTL must be written after BLC_PWM_CPU_CTL2;
-                * otherwise we get blank eDP screen after S3 on some machines
-                */
-               I915_WRITE(BLC_PWM_CPU_CTL2, dev_priv->regfile.saveBLC_CPU_PWM_CTL2);
-               I915_WRITE(BLC_PWM_CPU_CTL, dev_priv->regfile.saveBLC_CPU_PWM_CTL);
                I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
                I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
                I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
@@ -319,21 +285,12 @@ static void i915_restore_display(struct drm_device *dev)
                I915_WRITE(RSTDBYCTL,
                           dev_priv->regfile.saveMCHBAR_RENDER_STANDBY);
        } else if (IS_VALLEYVIEW(dev)) {
-               I915_WRITE(VLV_BLC_PWM_CTL(PIPE_A),
-                          dev_priv->regfile.saveBLC_PWM_CTL);
                I915_WRITE(VLV_BLC_HIST_CTL(PIPE_A),
                           dev_priv->regfile.saveBLC_HIST_CTL);
-               I915_WRITE(VLV_BLC_PWM_CTL2(PIPE_A),
-                          dev_priv->regfile.saveBLC_PWM_CTL2);
-               I915_WRITE(VLV_BLC_PWM_CTL(PIPE_B),
-                          dev_priv->regfile.saveBLC_PWM_CTL);
                I915_WRITE(VLV_BLC_HIST_CTL(PIPE_B),
                           dev_priv->regfile.saveBLC_HIST_CTL);
-               I915_WRITE(VLV_BLC_PWM_CTL2(PIPE_B),
-                          dev_priv->regfile.saveBLC_PWM_CTL2);
        } else {
                I915_WRITE(PFIT_PGM_RATIOS, dev_priv->regfile.savePFIT_PGM_RATIOS);
-               I915_WRITE(BLC_PWM_CTL, dev_priv->regfile.saveBLC_PWM_CTL);
                I915_WRITE(BLC_HIST_CTL, dev_priv->regfile.saveBLC_HIST_CTL);
                I915_WRITE(PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
                I915_WRITE(PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
@@ -341,8 +298,6 @@ static void i915_restore_display(struct drm_device *dev)
                I915_WRITE(PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
        }
 
-       spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
-
        /* only restore FBC info on the platform that supports FBC*/
        intel_disable_fbc(dev);
        if (I915_HAS_FBC(dev)) {
index 967da4772c449a2be05d16bce648fff9bd19a63e..caa18e855815eaf04a39ecb6eedcc572cf4d6290 100644 (file)
@@ -270,6 +270,18 @@ void i915_save_display_reg(struct drm_device *dev)
        }
        /* FIXME: regfile.save TV & SDVO state */
 
+       /* Backlight */
+       if (HAS_PCH_SPLIT(dev)) {
+               dev_priv->regfile.saveBLC_PWM_CTL = I915_READ(BLC_PWM_PCH_CTL1);
+               dev_priv->regfile.saveBLC_PWM_CTL2 = I915_READ(BLC_PWM_PCH_CTL2);
+               dev_priv->regfile.saveBLC_CPU_PWM_CTL = I915_READ(BLC_PWM_CPU_CTL);
+               dev_priv->regfile.saveBLC_CPU_PWM_CTL2 = I915_READ(BLC_PWM_CPU_CTL2);
+       } else {
+               dev_priv->regfile.saveBLC_PWM_CTL = I915_READ(BLC_PWM_CTL);
+               if (INTEL_INFO(dev)->gen >= 4)
+                       dev_priv->regfile.saveBLC_PWM_CTL2 = I915_READ(BLC_PWM_CTL2);
+       }
+
        return;
 }
 
@@ -280,6 +292,21 @@ void i915_restore_display_reg(struct drm_device *dev)
        int dpll_b_reg, fpb0_reg, fpb1_reg;
        int i;
 
+       /* Backlight */
+       if (HAS_PCH_SPLIT(dev)) {
+               I915_WRITE(BLC_PWM_PCH_CTL1, dev_priv->regfile.saveBLC_PWM_CTL);
+               I915_WRITE(BLC_PWM_PCH_CTL2, dev_priv->regfile.saveBLC_PWM_CTL2);
+               /* NOTE: BLC_PWM_CPU_CTL must be written after BLC_PWM_CPU_CTL2;
+                * otherwise we get blank eDP screen after S3 on some machines
+                */
+               I915_WRITE(BLC_PWM_CPU_CTL2, dev_priv->regfile.saveBLC_CPU_PWM_CTL2);
+               I915_WRITE(BLC_PWM_CPU_CTL, dev_priv->regfile.saveBLC_CPU_PWM_CTL);
+       } else {
+               if (INTEL_INFO(dev)->gen >= 4)
+                       I915_WRITE(BLC_PWM_CTL2, dev_priv->regfile.saveBLC_PWM_CTL2);
+               I915_WRITE(BLC_PWM_CTL, dev_priv->regfile.saveBLC_PWM_CTL);
+       }
+
        /* Display port ratios (must be done before clock is set) */
        if (SUPPORTS_INTEGRATED_DP(dev)) {
                I915_WRITE(_PIPEA_DATA_M_G4X, dev_priv->regfile.savePIPEA_GMCH_DATA_M);
index 3cddd508d1105a5d0fcdc15fa6ea54e93bd046cc..b9f763c637ec1a85c2eeff3a2d9048cbb494c371 100644 (file)
@@ -10418,6 +10418,8 @@ static void intel_init_display(struct drm_device *dev)
                dev_priv->display.queue_flip = intel_gen7_queue_flip;
                break;
        }
+
+       intel_panel_init_backlight_funcs(dev);
 }
 
 /*
@@ -10454,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;
@@ -10524,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)
@@ -11070,12 +11056,11 @@ void intel_modeset_cleanup(struct drm_device *dev)
        /* flush any delayed tasks or pending work */
        flush_scheduled_work();
 
-       /* destroy backlight, if any, before the connectors */
-       intel_panel_destroy_backlight(dev);
-
-       /* destroy the sysfs files before encoders/connectors */
-       list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+       /* destroy the backlight and sysfs files before encoders/connectors */
+       list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+               intel_panel_destroy_backlight(connector);
                drm_sysfs_connector_remove(connector);
+       }
 
        drm_mode_config_cleanup(dev);
 
index 1e49aa8f5377395c8803d5a55765199bd38af037..4fbd073810ef33d3d0a3a0bad6425a9f3b9ff3c4 100644 (file)
@@ -156,6 +156,17 @@ struct intel_encoder {
 struct intel_panel {
        struct drm_display_mode *fixed_mode;
        int fitting_mode;
+
+       /* backlight */
+       struct {
+               bool present;
+               u32 level;
+               u32 max;
+               bool enabled;
+               bool combination_mode;  /* gen 2/4 only */
+               bool active_low_pwm;
+               struct backlight_device *device;
+       } backlight;
 };
 
 struct intel_connector {
@@ -808,7 +819,8 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
 int intel_panel_setup_backlight(struct drm_connector *connector);
 void intel_panel_enable_backlight(struct intel_connector *connector);
 void intel_panel_disable_backlight(struct intel_connector *connector);
-void intel_panel_destroy_backlight(struct drm_device *dev);
+void intel_panel_destroy_backlight(struct drm_connector *connector);
+void intel_panel_init_backlight_funcs(struct drm_device *dev);
 enum drm_connector_status intel_panel_detect(struct drm_device *dev);
 
 
index 1b2f41c3f19122c731356aff91f3b490959bde60..6506df26ac9e2e72ae1a2c10a5965ce0fd177bf4 100644 (file)
@@ -396,13 +396,10 @@ int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
        struct drm_i915_private *dev_priv = dev->dev_private;
-       struct drm_encoder *encoder;
        struct drm_connector *connector;
-       struct intel_connector *intel_connector = NULL;
-       struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[0];
+       struct intel_connector *intel_connector;
+       struct intel_panel *panel;
        struct opregion_asle __iomem *asle = dev_priv->opregion.asle;
-       u32 ret = 0;
-       bool found = false;
 
        DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
 
@@ -414,38 +411,24 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
                return ASLC_BACKLIGHT_FAILED;
 
        mutex_lock(&dev->mode_config.mutex);
+
        /*
-        * Could match the OpRegion connector here instead, but we'd also need
-        * to verify the connector could handle a backlight call.
+        * Update backlight on all connectors that support backlight (usually
+        * only one).
         */
-       list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
-               if (encoder->crtc == crtc) {
-                       found = true;
-                       break;
-               }
-
-       if (!found) {
-               ret = ASLC_BACKLIGHT_FAILED;
-               goto out;
-       }
-
-       list_for_each_entry(connector, &dev->mode_config.connector_list, head)
-               if (connector->encoder == encoder)
-                       intel_connector = to_intel_connector(connector);
-
-       if (!intel_connector) {
-               ret = ASLC_BACKLIGHT_FAILED;
-               goto out;
-       }
-
        DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
-       intel_panel_set_backlight(intel_connector, bclp, 255);
+       list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+               intel_connector = to_intel_connector(connector);
+               panel = &intel_connector->panel;
+               if (panel->backlight.present)
+                       intel_panel_set_backlight(intel_connector, bclp, 255);
+       }
        iowrite32(DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID, &asle->cblv);
 
-out:
        mutex_unlock(&dev->mode_config.mutex);
 
-       return ret;
+
+       return 0;
 }
 
 static u32 asle_set_als_illum(struct drm_device *dev, u32 alsi)
index e6f782d1c6696d94fe4d4a80cf7f4d6ee6b5c7d7..e480cf41c5365f5551eaff1a04efd458b7e37d12 100644 (file)
@@ -325,214 +325,170 @@ out:
        pipe_config->gmch_pfit.lvds_border_bits = border;
 }
 
-static int is_backlight_combination_mode(struct drm_device *dev)
+static int i915_panel_invert_brightness;
+MODULE_PARM_DESC(invert_brightness, "Invert backlight brightness "
+       "(-1 force normal, 0 machine defaults, 1 force inversion), please "
+       "report PCI device ID, subsystem vendor and subsystem device ID "
+       "to dri-devel@lists.freedesktop.org, if your machine needs it. "
+       "It will then be included in an upcoming module version.");
+module_param_named(invert_brightness, i915_panel_invert_brightness, int, 0600);
+static u32 intel_panel_compute_brightness(struct intel_connector *connector,
+                                         u32 val)
 {
+       struct drm_device *dev = connector->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
 
-       if (IS_GEN4(dev))
-               return I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE;
+       WARN_ON(panel->backlight.max == 0);
 
-       if (IS_GEN2(dev))
-               return I915_READ(BLC_PWM_CTL) & BLM_LEGACY_MODE;
+       if (i915_panel_invert_brightness < 0)
+               return val;
 
-       return 0;
+       if (i915_panel_invert_brightness > 0 ||
+           dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS) {
+               return panel->backlight.max - val;
+       }
+
+       return val;
 }
 
-/* XXX: query mode clock or hardware clock and program max PWM appropriately
- * when it's 0.
- */
-static u32 i915_read_blc_pwm_ctl(struct drm_device *dev, enum pipe pipe)
+static u32 bdw_get_backlight(struct intel_connector *connector)
 {
+       struct drm_device *dev = connector->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
-       u32 val;
 
-       WARN_ON_SMP(!spin_is_locked(&dev_priv->backlight.lock));
-
-       /* Restore the CTL value if it lost, e.g. GPU reset */
-
-       if (HAS_PCH_SPLIT(dev_priv->dev)) {
-               val = I915_READ(BLC_PWM_PCH_CTL2);
-               if (dev_priv->regfile.saveBLC_PWM_CTL2 == 0) {
-                       dev_priv->regfile.saveBLC_PWM_CTL2 = val;
-               } else if (val == 0) {
-                       val = dev_priv->regfile.saveBLC_PWM_CTL2;
-                       I915_WRITE(BLC_PWM_PCH_CTL2, val);
-               }
-       } else if (IS_VALLEYVIEW(dev)) {
-               val = I915_READ(VLV_BLC_PWM_CTL(pipe));
-               if (dev_priv->regfile.saveBLC_PWM_CTL == 0) {
-                       dev_priv->regfile.saveBLC_PWM_CTL = val;
-                       dev_priv->regfile.saveBLC_PWM_CTL2 =
-                               I915_READ(VLV_BLC_PWM_CTL2(pipe));
-               } else if (val == 0) {
-                       val = dev_priv->regfile.saveBLC_PWM_CTL;
-                       I915_WRITE(VLV_BLC_PWM_CTL(pipe), val);
-                       I915_WRITE(VLV_BLC_PWM_CTL2(pipe),
-                                  dev_priv->regfile.saveBLC_PWM_CTL2);
-               }
+       return I915_READ(BLC_PWM_PCH_CTL2) & BACKLIGHT_DUTY_CYCLE_MASK;
+}
 
-               if (!val)
-                       val = 0x0f42ffff;
-       } else {
-               val = I915_READ(BLC_PWM_CTL);
-               if (dev_priv->regfile.saveBLC_PWM_CTL == 0) {
-                       dev_priv->regfile.saveBLC_PWM_CTL = val;
-                       if (INTEL_INFO(dev)->gen >= 4)
-                               dev_priv->regfile.saveBLC_PWM_CTL2 =
-                                       I915_READ(BLC_PWM_CTL2);
-               } else if (val == 0) {
-                       val = dev_priv->regfile.saveBLC_PWM_CTL;
-                       I915_WRITE(BLC_PWM_CTL, val);
-                       if (INTEL_INFO(dev)->gen >= 4)
-                               I915_WRITE(BLC_PWM_CTL2,
-                                          dev_priv->regfile.saveBLC_PWM_CTL2);
-               }
-       }
+static u32 pch_get_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
 
-       return val;
+       return I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
 }
 
-static u32 intel_panel_get_max_backlight(struct drm_device *dev,
-                                        enum pipe pipe)
+static u32 i9xx_get_backlight(struct intel_connector *connector)
 {
-       u32 max;
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
+       u32 val;
 
-       max = i915_read_blc_pwm_ctl(dev, pipe);
+       val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
+       if (INTEL_INFO(dev)->gen < 4)
+               val >>= 1;
 
-       if (HAS_PCH_SPLIT(dev)) {
-               max >>= 16;
-       } else {
-               if (INTEL_INFO(dev)->gen < 4)
-                       max >>= 17;
-               else
-                       max >>= 16;
+       if (panel->backlight.combination_mode) {
+               u8 lbpc;
 
-               if (is_backlight_combination_mode(dev))
-                       max *= 0xff;
+               pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc);
+               val *= lbpc;
        }
 
-       DRM_DEBUG_DRIVER("max backlight PWM = %d\n", max);
-
-       return max;
+       return val;
 }
 
-static int i915_panel_invert_brightness;
-MODULE_PARM_DESC(invert_brightness, "Invert backlight brightness "
-       "(-1 force normal, 0 machine defaults, 1 force inversion), please "
-       "report PCI device ID, subsystem vendor and subsystem device ID "
-       "to dri-devel@lists.freedesktop.org, if your machine needs it. "
-       "It will then be included in an upcoming module version.");
-module_param_named(invert_brightness, i915_panel_invert_brightness, int, 0600);
-static u32 intel_panel_compute_brightness(struct drm_device *dev,
-                                         enum pipe pipe, u32 val)
+static u32 _vlv_get_backlight(struct drm_device *dev, enum pipe pipe)
 {
        struct drm_i915_private *dev_priv = dev->dev_private;
 
-       if (i915_panel_invert_brightness < 0)
-               return val;
+       return I915_READ(VLV_BLC_PWM_CTL(pipe)) & BACKLIGHT_DUTY_CYCLE_MASK;
+}
 
-       if (i915_panel_invert_brightness > 0 ||
-           dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS) {
-               u32 max = intel_panel_get_max_backlight(dev, pipe);
-               if (max)
-                       return max - val;
-       }
+static u32 vlv_get_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       enum pipe pipe = intel_get_pipe_from_connector(connector);
 
-       return val;
+       return _vlv_get_backlight(dev, pipe);
 }
 
-static u32 intel_panel_get_backlight(struct drm_device *dev,
-                                    enum pipe pipe)
+static u32 intel_panel_get_backlight(struct intel_connector *connector)
 {
+       struct drm_device *dev = connector->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
        u32 val;
        unsigned long flags;
-       int reg;
-
-       spin_lock_irqsave(&dev_priv->backlight.lock, flags);
 
-       if (IS_BROADWELL(dev)) {
-               val = I915_READ(BLC_PWM_PCH_CTL2) & BACKLIGHT_DUTY_CYCLE_MASK;
-       } else if (HAS_PCH_SPLIT(dev)) {
-               val = I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
-       } else {
-               if (IS_VALLEYVIEW(dev))
-                       reg = VLV_BLC_PWM_CTL(pipe);
-               else
-                       reg = BLC_PWM_CTL;
+       spin_lock_irqsave(&dev_priv->backlight_lock, flags);
 
-               val = I915_READ(reg) & BACKLIGHT_DUTY_CYCLE_MASK;
-               if (INTEL_INFO(dev)->gen < 4)
-                       val >>= 1;
+       val = dev_priv->display.get_backlight(connector);
+       val = intel_panel_compute_brightness(connector, val);
 
-               if (is_backlight_combination_mode(dev)) {
-                       u8 lbpc;
-
-                       pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc);
-                       val *= lbpc;
-               }
-       }
-
-       val = intel_panel_compute_brightness(dev, pipe, val);
-
-       spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
+       spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
 
        DRM_DEBUG_DRIVER("get backlight PWM = %d\n", val);
        return val;
 }
 
-static void intel_bdw_panel_set_backlight(struct drm_device *dev, u32 level)
+static void bdw_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 val = I915_READ(BLC_PWM_PCH_CTL2) & ~BACKLIGHT_DUTY_CYCLE_MASK;
        I915_WRITE(BLC_PWM_PCH_CTL2, val | level);
 }
 
-static void intel_pch_panel_set_backlight(struct drm_device *dev, u32 level)
+static void pch_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 val = I915_READ(BLC_PWM_CPU_CTL) & ~BACKLIGHT_DUTY_CYCLE_MASK;
-       I915_WRITE(BLC_PWM_CPU_CTL, val | level);
+       u32 tmp;
+
+       tmp = I915_READ(BLC_PWM_CPU_CTL) & ~BACKLIGHT_DUTY_CYCLE_MASK;
+       I915_WRITE(BLC_PWM_CPU_CTL, tmp | level);
 }
 
-static void intel_panel_actually_set_backlight(struct drm_device *dev,
-                                              enum pipe pipe, u32 level)
+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;
-       int reg;
-
-       DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level);
-       level = intel_panel_compute_brightness(dev, pipe, level);
+       struct intel_panel *panel = &connector->panel;
+       u32 tmp, mask;
 
-       if (IS_BROADWELL(dev))
-               return intel_bdw_panel_set_backlight(dev, level);
-       else if (HAS_PCH_SPLIT(dev))
-               return intel_pch_panel_set_backlight(dev, level);
+       WARN_ON(panel->backlight.max == 0);
 
-       if (is_backlight_combination_mode(dev)) {
-               u32 max = intel_panel_get_max_backlight(dev, pipe);
+       if (panel->backlight.combination_mode) {
                u8 lbpc;
 
-               /* we're screwed, but keep behaviour backwards compatible */
-               if (!max)
-                       max = 1;
-
-               lbpc = level * 0xfe / max + 1;
+               lbpc = level * 0xfe / panel->backlight.max + 1;
                level /= lbpc;
                pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc);
        }
 
-       if (IS_VALLEYVIEW(dev))
-               reg = VLV_BLC_PWM_CTL(pipe);
-       else
-               reg = BLC_PWM_CTL;
-
-       tmp = I915_READ(reg);
-       if (INTEL_INFO(dev)->gen < 4)
+       if (IS_GEN4(dev)) {
+               mask = BACKLIGHT_DUTY_CYCLE_MASK;
+       } else {
                level <<= 1;
-       tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
-       I915_WRITE(reg, tmp | level);
+               mask = BACKLIGHT_DUTY_CYCLE_MASK_PNV;
+       }
+
+       tmp = I915_READ(BLC_PWM_CTL) & ~mask;
+       I915_WRITE(BLC_PWM_CTL, tmp | level);
+}
+
+static void vlv_set_backlight(struct intel_connector *connector, u32 level)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       enum pipe pipe = intel_get_pipe_from_connector(connector);
+       u32 tmp;
+
+       tmp = I915_READ(VLV_BLC_PWM_CTL(pipe)) & ~BACKLIGHT_DUTY_CYCLE_MASK;
+       I915_WRITE(VLV_BLC_PWM_CTL(pipe), tmp | level);
+}
+
+static void
+intel_panel_actually_set_backlight(struct intel_connector *connector, u32 level)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+
+       DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level);
+
+       level = intel_panel_compute_brightness(connector, level);
+       dev_priv->display.set_backlight(connector, level);
 }
 
 /* set backlight brightness to level in range [0..max] */
@@ -541,6 +497,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
 {
        struct drm_device *dev = connector->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
        enum pipe pipe = intel_get_pipe_from_connector(connector);
        u32 freq;
        unsigned long flags;
@@ -548,34 +505,77 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
        if (pipe == INVALID_PIPE)
                return;
 
-       spin_lock_irqsave(&dev_priv->backlight.lock, flags);
+       spin_lock_irqsave(&dev_priv->backlight_lock, flags);
 
-       freq = intel_panel_get_max_backlight(dev, pipe);
-       if (!freq) {
-               /* we are screwed, bail out */
-               goto out;
-       }
+       WARN_ON(panel->backlight.max == 0);
 
-       /* scale to hardware, but be careful to not overflow */
+       /* scale to hardware max, but be careful to not overflow */
+       freq = panel->backlight.max;
        if (freq < max)
                level = level * freq / max;
        else
                level = freq / max * level;
 
-       dev_priv->backlight.level = level;
-       if (dev_priv->backlight.device)
-               dev_priv->backlight.device->props.brightness = level;
+       panel->backlight.level = level;
+       if (panel->backlight.device)
+               panel->backlight.device->props.brightness = level;
 
-       if (dev_priv->backlight.enabled)
-               intel_panel_actually_set_backlight(dev, pipe, level);
-out:
-       spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
+       if (panel->backlight.enabled)
+               intel_panel_actually_set_backlight(connector, level);
+
+       spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
+}
+
+static void pch_disable_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       u32 tmp;
+
+       intel_panel_actually_set_backlight(connector, 0);
+
+       tmp = I915_READ(BLC_PWM_CPU_CTL2);
+       I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
+
+       tmp = I915_READ(BLC_PWM_PCH_CTL1);
+       I915_WRITE(BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
+}
+
+static void i9xx_disable_backlight(struct intel_connector *connector)
+{
+       intel_panel_actually_set_backlight(connector, 0);
+}
+
+static void i965_disable_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       u32 tmp;
+
+       intel_panel_actually_set_backlight(connector, 0);
+
+       tmp = I915_READ(BLC_PWM_CTL2);
+       I915_WRITE(BLC_PWM_CTL2, tmp & ~BLM_PWM_ENABLE);
+}
+
+static void vlv_disable_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       enum pipe pipe = intel_get_pipe_from_connector(connector);
+       u32 tmp;
+
+       intel_panel_actually_set_backlight(connector, 0);
+
+       tmp = I915_READ(VLV_BLC_PWM_CTL2(pipe));
+       I915_WRITE(VLV_BLC_PWM_CTL2(pipe), tmp & ~BLM_PWM_ENABLE);
 }
 
 void intel_panel_disable_backlight(struct intel_connector *connector)
 {
        struct drm_device *dev = connector->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
        enum pipe pipe = intel_get_pipe_from_connector(connector);
        unsigned long flags;
 
@@ -593,150 +593,215 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
                return;
        }
 
-       spin_lock_irqsave(&dev_priv->backlight.lock, flags);
+       spin_lock_irqsave(&dev_priv->backlight_lock, flags);
 
-       dev_priv->backlight.enabled = false;
-       intel_panel_actually_set_backlight(dev, pipe, 0);
+       panel->backlight.enabled = false;
+       dev_priv->display.disable_backlight(connector);
 
-       if (INTEL_INFO(dev)->gen >= 4) {
-               uint32_t reg, tmp;
+       spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
+}
 
-               if (HAS_PCH_SPLIT(dev))
-                       reg = BLC_PWM_CPU_CTL2;
-               else if (IS_VALLEYVIEW(dev))
-                       reg = VLV_BLC_PWM_CTL2(pipe);
-               else
-                       reg = BLC_PWM_CTL2;
+static void bdw_enable_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
+       u32 pch_ctl1, pch_ctl2;
+
+       pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
+       if (pch_ctl1 & BLM_PCH_PWM_ENABLE) {
+               DRM_DEBUG_KMS("pch backlight already enabled\n");
+               pch_ctl1 &= ~BLM_PCH_PWM_ENABLE;
+               I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1);
+       }
 
-               I915_WRITE(reg, I915_READ(reg) & ~BLM_PWM_ENABLE);
+       pch_ctl2 = panel->backlight.max << 16;
+       I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2);
 
-               if (HAS_PCH_SPLIT(dev)) {
-                       tmp = I915_READ(BLC_PWM_PCH_CTL1);
-                       tmp &= ~BLM_PCH_PWM_ENABLE;
-                       I915_WRITE(BLC_PWM_PCH_CTL1, tmp);
-               }
-       }
+       pch_ctl1 = 0;
+       if (panel->backlight.active_low_pwm)
+               pch_ctl1 |= BLM_PCH_POLARITY;
 
-       spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
+       /* BDW always uses the pch pwm controls. */
+       pch_ctl1 |= BLM_PCH_OVERRIDE_ENABLE;
+
+       I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1);
+       POSTING_READ(BLC_PWM_PCH_CTL1);
+       I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
+
+       /* This won't stick until the above enable. */
+       intel_panel_actually_set_backlight(connector, panel->backlight.level);
 }
 
-void intel_panel_enable_backlight(struct intel_connector *connector)
+static void pch_enable_backlight(struct intel_connector *connector)
 {
        struct drm_device *dev = connector->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
        enum pipe pipe = intel_get_pipe_from_connector(connector);
        enum transcoder cpu_transcoder =
                intel_pipe_to_cpu_transcoder(dev_priv, pipe);
-       unsigned long flags;
+       u32 cpu_ctl2, pch_ctl1, pch_ctl2;
 
-       if (pipe == INVALID_PIPE)
-               return;
+       cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
+       if (cpu_ctl2 & BLM_PWM_ENABLE) {
+               WARN(1, "cpu backlight already enabled\n");
+               cpu_ctl2 &= ~BLM_PWM_ENABLE;
+               I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2);
+       }
 
-       DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
+       pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
+       if (pch_ctl1 & BLM_PCH_PWM_ENABLE) {
+               DRM_DEBUG_KMS("pch backlight already enabled\n");
+               pch_ctl1 &= ~BLM_PCH_PWM_ENABLE;
+               I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1);
+       }
 
-       spin_lock_irqsave(&dev_priv->backlight.lock, flags);
+       if (cpu_transcoder == TRANSCODER_EDP)
+               cpu_ctl2 = BLM_TRANSCODER_EDP;
+       else
+               cpu_ctl2 = BLM_PIPE(cpu_transcoder);
+       I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2);
+       POSTING_READ(BLC_PWM_CPU_CTL2);
+       I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2 | BLM_PWM_ENABLE);
+
+       /* This won't stick until the above enable. */
+       intel_panel_actually_set_backlight(connector, panel->backlight.level);
+
+       pch_ctl2 = panel->backlight.max << 16;
+       I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2);
+
+       pch_ctl1 = 0;
+       if (panel->backlight.active_low_pwm)
+               pch_ctl1 |= BLM_PCH_POLARITY;
+
+       I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1);
+       POSTING_READ(BLC_PWM_PCH_CTL1);
+       I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
+}
+
+static void i9xx_enable_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
+       u32 ctl, freq;
 
-       if (dev_priv->backlight.level == 0) {
-               dev_priv->backlight.level = intel_panel_get_max_backlight(dev,
-                                                                         pipe);
-               if (dev_priv->backlight.device)
-                       dev_priv->backlight.device->props.brightness =
-                               dev_priv->backlight.level;
+       ctl = I915_READ(BLC_PWM_CTL);
+       if (ctl & BACKLIGHT_DUTY_CYCLE_MASK_PNV) {
+               WARN(1, "backlight already enabled\n");
+               I915_WRITE(BLC_PWM_CTL, 0);
        }
 
-       if (INTEL_INFO(dev)->gen >= 4) {
-               uint32_t reg, tmp;
+       freq = panel->backlight.max;
+       if (panel->backlight.combination_mode)
+               freq /= 0xff;
 
-               if (HAS_PCH_SPLIT(dev))
-                       reg = BLC_PWM_CPU_CTL2;
-               else if (IS_VALLEYVIEW(dev))
-                       reg = VLV_BLC_PWM_CTL2(pipe);
-               else
-                       reg = BLC_PWM_CTL2;
+       ctl = freq << 17;
+       if (IS_GEN2(dev) && panel->backlight.combination_mode)
+               ctl |= BLM_LEGACY_MODE;
+       if (IS_PINEVIEW(dev) && panel->backlight.active_low_pwm)
+               ctl |= BLM_POLARITY_PNV;
 
-               tmp = I915_READ(reg);
+       I915_WRITE(BLC_PWM_CTL, ctl);
+       POSTING_READ(BLC_PWM_CTL);
 
-               /* Note that this can also get called through dpms changes. And
-                * we don't track the backlight dpms state, hence check whether
-                * we have to do anything first. */
-               if (tmp & BLM_PWM_ENABLE)
-                       goto set_level;
+       /* XXX: combine this into above write? */
+       intel_panel_actually_set_backlight(connector, panel->backlight.level);
+}
 
-               if (INTEL_INFO(dev)->num_pipes == 3)
-                       tmp &= ~BLM_PIPE_SELECT_IVB;
-               else
-                       tmp &= ~BLM_PIPE_SELECT;
+static void i965_enable_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
+       enum pipe pipe = intel_get_pipe_from_connector(connector);
+       u32 ctl, ctl2, freq;
 
-               if (cpu_transcoder == TRANSCODER_EDP)
-                       tmp |= BLM_TRANSCODER_EDP;
-               else
-                       tmp |= BLM_PIPE(cpu_transcoder);
-               tmp &= ~BLM_PWM_ENABLE;
-
-               I915_WRITE(reg, tmp);
-               POSTING_READ(reg);
-               I915_WRITE(reg, tmp | BLM_PWM_ENABLE);
-
-               if (IS_BROADWELL(dev)) {
-                       /*
-                        * Broadwell requires PCH override to drive the PCH
-                        * backlight pin. The above will configure the CPU
-                        * backlight pin, which we don't plan to use.
-                        */
-                       tmp = I915_READ(BLC_PWM_PCH_CTL1);
-                       tmp |= BLM_PCH_OVERRIDE_ENABLE | BLM_PCH_PWM_ENABLE;
-                       I915_WRITE(BLC_PWM_PCH_CTL1, tmp);
-               } else if (HAS_PCH_SPLIT(dev) &&
-                   !(dev_priv->quirks & QUIRK_NO_PCH_PWM_ENABLE)) {
-                       tmp = I915_READ(BLC_PWM_PCH_CTL1);
-                       tmp |= BLM_PCH_PWM_ENABLE;
-                       tmp &= ~BLM_PCH_OVERRIDE_ENABLE;
-                       I915_WRITE(BLC_PWM_PCH_CTL1, tmp);
-               }
+       ctl2 = I915_READ(BLC_PWM_CTL2);
+       if (ctl2 & BLM_PWM_ENABLE) {
+               WARN(1, "backlight already enabled\n");
+               ctl2 &= ~BLM_PWM_ENABLE;
+               I915_WRITE(BLC_PWM_CTL2, ctl2);
        }
 
-set_level:
-       /* Call below after setting BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1.
-        * BLC_PWM_CPU_CTL may be cleared to zero automatically when these
-        * registers are set.
-        */
-       dev_priv->backlight.enabled = true;
-       intel_panel_actually_set_backlight(dev, pipe,
-                                          dev_priv->backlight.level);
+       freq = panel->backlight.max;
+       if (panel->backlight.combination_mode)
+               freq /= 0xff;
 
-       spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
+       ctl = freq << 16;
+       I915_WRITE(BLC_PWM_CTL, ctl);
+
+       /* XXX: combine this into above write? */
+       intel_panel_actually_set_backlight(connector, panel->backlight.level);
+
+       ctl2 = BLM_PIPE(pipe);
+       if (panel->backlight.combination_mode)
+               ctl2 |= BLM_COMBINATION_MODE;
+       if (panel->backlight.active_low_pwm)
+               ctl2 |= BLM_POLARITY_I965;
+       I915_WRITE(BLC_PWM_CTL2, ctl2);
+       POSTING_READ(BLC_PWM_CTL2);
+       I915_WRITE(BLC_PWM_CTL2, ctl2 | BLM_PWM_ENABLE);
 }
 
-/* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */
-static void intel_panel_init_backlight_regs(struct drm_device *dev)
+static void vlv_enable_backlight(struct intel_connector *connector)
 {
+       struct drm_device *dev = connector->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
+       enum pipe pipe = intel_get_pipe_from_connector(connector);
+       u32 ctl, ctl2;
 
-       if (IS_VALLEYVIEW(dev)) {
-               enum pipe pipe;
+       ctl2 = I915_READ(VLV_BLC_PWM_CTL2(pipe));
+       if (ctl2 & BLM_PWM_ENABLE) {
+               WARN(1, "backlight already enabled\n");
+               ctl2 &= ~BLM_PWM_ENABLE;
+               I915_WRITE(VLV_BLC_PWM_CTL2(pipe), ctl2);
+       }
 
-               for_each_pipe(pipe) {
-                       u32 cur_val = I915_READ(VLV_BLC_PWM_CTL(pipe));
+       ctl = panel->backlight.max << 16;
+       I915_WRITE(VLV_BLC_PWM_CTL(pipe), ctl);
 
-                       /* Skip if the modulation freq is already set */
-                       if (cur_val & ~BACKLIGHT_DUTY_CYCLE_MASK)
-                               continue;
+       /* XXX: combine this into above write? */
+       intel_panel_actually_set_backlight(connector, panel->backlight.level);
 
-                       cur_val &= BACKLIGHT_DUTY_CYCLE_MASK;
-                       I915_WRITE(VLV_BLC_PWM_CTL(pipe), (0xf42 << 16) |
-                                  cur_val);
-               }
-       }
+       ctl2 = 0;
+       if (panel->backlight.active_low_pwm)
+               ctl2 |= BLM_POLARITY_I965;
+       I915_WRITE(VLV_BLC_PWM_CTL2(pipe), ctl2);
+       POSTING_READ(VLV_BLC_PWM_CTL2(pipe));
+       I915_WRITE(VLV_BLC_PWM_CTL2(pipe), ctl2 | BLM_PWM_ENABLE);
 }
 
-static void intel_panel_init_backlight(struct drm_device *dev)
+void intel_panel_enable_backlight(struct intel_connector *connector)
 {
+       struct drm_device *dev = connector->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
+       enum pipe pipe = intel_get_pipe_from_connector(connector);
+       unsigned long flags;
 
-       intel_panel_init_backlight_regs(dev);
+       if (pipe == INVALID_PIPE)
+               return;
+
+       DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
+
+       spin_lock_irqsave(&dev_priv->backlight_lock, flags);
+
+       WARN_ON(panel->backlight.max == 0);
+
+       if (panel->backlight.level == 0) {
+               panel->backlight.level = panel->backlight.max;
+               if (panel->backlight.device)
+                       panel->backlight.device->props.brightness =
+                               panel->backlight.level;
+       }
 
-       dev_priv->backlight.level = intel_panel_get_backlight(dev, 0);
-       dev_priv->backlight.enabled = dev_priv->backlight.level != 0;
+       dev_priv->display.enable_backlight(connector);
+       panel->backlight.enabled = true;
+
+       spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
 }
 
 enum drm_connector_status
@@ -762,7 +827,7 @@ intel_panel_detect(struct drm_device *dev)
 }
 
 #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
-static int intel_panel_update_status(struct backlight_device *bd)
+static int intel_backlight_device_update_status(struct backlight_device *bd)
 {
        struct intel_connector *connector = bl_get_data(bd);
        struct drm_device *dev = connector->base.dev;
@@ -776,85 +841,306 @@ static int intel_panel_update_status(struct backlight_device *bd)
        return 0;
 }
 
-static int intel_panel_get_brightness(struct backlight_device *bd)
+static int intel_backlight_device_get_brightness(struct backlight_device *bd)
 {
        struct intel_connector *connector = bl_get_data(bd);
        struct drm_device *dev = connector->base.dev;
-       enum pipe pipe;
+       int ret;
 
        mutex_lock(&dev->mode_config.mutex);
-       pipe = intel_get_pipe_from_connector(connector);
+       ret = intel_panel_get_backlight(connector);
        mutex_unlock(&dev->mode_config.mutex);
-       if (pipe == INVALID_PIPE)
-               return 0;
 
-       return intel_panel_get_backlight(connector->base.dev, pipe);
+       return ret;
 }
 
-static const struct backlight_ops intel_panel_bl_ops = {
-       .update_status = intel_panel_update_status,
-       .get_brightness = intel_panel_get_brightness,
+static const struct backlight_ops intel_backlight_device_ops = {
+       .update_status = intel_backlight_device_update_status,
+       .get_brightness = intel_backlight_device_get_brightness,
 };
 
-int intel_panel_setup_backlight(struct drm_connector *connector)
+static int intel_backlight_device_register(struct intel_connector *connector)
 {
-       struct drm_device *dev = connector->dev;
-       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
        struct backlight_properties props;
-       unsigned long flags;
-
-       intel_panel_init_backlight(dev);
 
-       if (WARN_ON(dev_priv->backlight.device))
+       if (WARN_ON(panel->backlight.device))
                return -ENODEV;
 
+       BUG_ON(panel->backlight.max == 0);
+
        memset(&props, 0, sizeof(props));
        props.type = BACKLIGHT_RAW;
-       props.brightness = dev_priv->backlight.level;
-
-       spin_lock_irqsave(&dev_priv->backlight.lock, flags);
-       props.max_brightness = intel_panel_get_max_backlight(dev, 0);
-       spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
+       props.brightness = panel->backlight.level;
+       props.max_brightness = panel->backlight.max;
 
-       if (props.max_brightness == 0) {
-               DRM_DEBUG_DRIVER("Failed to get maximum backlight value\n");
-               return -ENODEV;
-       }
-       dev_priv->backlight.device =
+       /*
+        * Note: using the same name independent of the connector prevents
+        * registration of multiple backlight devices in the driver.
+        */
+       panel->backlight.device =
                backlight_device_register("intel_backlight",
-                                         connector->kdev,
-                                         to_intel_connector(connector),
-                                         &intel_panel_bl_ops, &props);
+                                         connector->base.kdev,
+                                         connector,
+                                         &intel_backlight_device_ops, &props);
 
-       if (IS_ERR(dev_priv->backlight.device)) {
+       if (IS_ERR(panel->backlight.device)) {
                DRM_ERROR("Failed to register backlight: %ld\n",
-                         PTR_ERR(dev_priv->backlight.device));
-               dev_priv->backlight.device = NULL;
+                         PTR_ERR(panel->backlight.device));
+               panel->backlight.device = NULL;
                return -ENODEV;
        }
        return 0;
 }
 
-void intel_panel_destroy_backlight(struct drm_device *dev)
+static void intel_backlight_device_unregister(struct intel_connector *connector)
+{
+       struct intel_panel *panel = &connector->panel;
+
+       if (panel->backlight.device) {
+               backlight_device_unregister(panel->backlight.device);
+               panel->backlight.device = NULL;
+       }
+}
+#else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
+static int intel_backlight_device_register(struct intel_connector *connector)
+{
+       return 0;
+}
+static void intel_backlight_device_unregister(struct intel_connector *connector)
+{
+}
+#endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */
+
+/*
+ * Note: The setup hooks can't assume pipe is set!
+ *
+ * XXX: Query mode clock or hardware clock and program PWM modulation frequency
+ * appropriately when it's 0. Use VBT and/or sane defaults.
+ */
+static int bdw_setup_backlight(struct intel_connector *connector)
 {
+       struct drm_device *dev = connector->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
-       if (dev_priv->backlight.device) {
-               backlight_device_unregister(dev_priv->backlight.device);
-               dev_priv->backlight.device = NULL;
+       struct intel_panel *panel = &connector->panel;
+       u32 pch_ctl1, pch_ctl2, val;
+
+       pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
+       panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
+
+       pch_ctl2 = I915_READ(BLC_PWM_PCH_CTL2);
+       panel->backlight.max = pch_ctl2 >> 16;
+       if (!panel->backlight.max)
+               return -ENODEV;
+
+       val = bdw_get_backlight(connector);
+       panel->backlight.level = intel_panel_compute_brightness(connector, val);
+
+       panel->backlight.enabled = (pch_ctl1 & BLM_PCH_PWM_ENABLE) &&
+               panel->backlight.level != 0;
+
+       return 0;
+}
+
+static int pch_setup_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
+       u32 cpu_ctl2, pch_ctl1, pch_ctl2, val;
+
+       pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
+       panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
+
+       pch_ctl2 = I915_READ(BLC_PWM_PCH_CTL2);
+       panel->backlight.max = pch_ctl2 >> 16;
+       if (!panel->backlight.max)
+               return -ENODEV;
+
+       val = pch_get_backlight(connector);
+       panel->backlight.level = intel_panel_compute_brightness(connector, val);
+
+       cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
+       panel->backlight.enabled = (cpu_ctl2 & BLM_PWM_ENABLE) &&
+               (pch_ctl1 & BLM_PCH_PWM_ENABLE) && panel->backlight.level != 0;
+
+       return 0;
+}
+
+static int i9xx_setup_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
+       u32 ctl, val;
+
+       ctl = I915_READ(BLC_PWM_CTL);
+
+       if (IS_GEN2(dev))
+               panel->backlight.combination_mode = ctl & BLM_LEGACY_MODE;
+
+       if (IS_PINEVIEW(dev))
+               panel->backlight.active_low_pwm = ctl & BLM_POLARITY_PNV;
+
+       panel->backlight.max = ctl >> 17;
+       if (panel->backlight.combination_mode)
+               panel->backlight.max *= 0xff;
+
+       if (!panel->backlight.max)
+               return -ENODEV;
+
+       val = i9xx_get_backlight(connector);
+       panel->backlight.level = intel_panel_compute_brightness(connector, val);
+
+       panel->backlight.enabled = panel->backlight.level != 0;
+
+       return 0;
+}
+
+static int i965_setup_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
+       u32 ctl, ctl2, val;
+
+       ctl2 = I915_READ(BLC_PWM_CTL2);
+       panel->backlight.combination_mode = ctl2 & BLM_COMBINATION_MODE;
+       panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965;
+
+       ctl = I915_READ(BLC_PWM_CTL);
+       panel->backlight.max = ctl >> 16;
+       if (panel->backlight.combination_mode)
+               panel->backlight.max *= 0xff;
+
+       if (!panel->backlight.max)
+               return -ENODEV;
+
+       val = i9xx_get_backlight(connector);
+       panel->backlight.level = intel_panel_compute_brightness(connector, val);
+
+       panel->backlight.enabled = (ctl2 & BLM_PWM_ENABLE) &&
+               panel->backlight.level != 0;
+
+       return 0;
+}
+
+static int vlv_setup_backlight(struct intel_connector *connector)
+{
+       struct drm_device *dev = connector->base.dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_panel *panel = &connector->panel;
+       enum pipe pipe;
+       u32 ctl, ctl2, val;
+
+       for_each_pipe(pipe) {
+               u32 cur_val = I915_READ(VLV_BLC_PWM_CTL(pipe));
+
+               /* Skip if the modulation freq is already set */
+               if (cur_val & ~BACKLIGHT_DUTY_CYCLE_MASK)
+                       continue;
+
+               cur_val &= BACKLIGHT_DUTY_CYCLE_MASK;
+               I915_WRITE(VLV_BLC_PWM_CTL(pipe), (0xf42 << 16) |
+                          cur_val);
        }
+
+       ctl2 = I915_READ(VLV_BLC_PWM_CTL2(PIPE_A));
+       panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965;
+
+       ctl = I915_READ(VLV_BLC_PWM_CTL(PIPE_A));
+       panel->backlight.max = ctl >> 16;
+       if (!panel->backlight.max)
+               return -ENODEV;
+
+       val = _vlv_get_backlight(dev, PIPE_A);
+       panel->backlight.level = intel_panel_compute_brightness(connector, val);
+
+       panel->backlight.enabled = (ctl2 & BLM_PWM_ENABLE) &&
+               panel->backlight.level != 0;
+
+       return 0;
 }
-#else
+
 int intel_panel_setup_backlight(struct drm_connector *connector)
 {
-       intel_panel_init_backlight(connector->dev);
+       struct drm_device *dev = connector->dev;
+       struct drm_i915_private *dev_priv = dev->dev_private;
+       struct intel_connector *intel_connector = to_intel_connector(connector);
+       struct intel_panel *panel = &intel_connector->panel;
+       unsigned long flags;
+       int ret;
+
+       /* set level and max in panel struct */
+       spin_lock_irqsave(&dev_priv->backlight_lock, flags);
+       ret = dev_priv->display.setup_backlight(intel_connector);
+       spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
+
+       if (ret) {
+               DRM_DEBUG_KMS("failed to setup backlight for connector %s\n",
+                             drm_get_connector_name(connector));
+               return ret;
+       }
+
+       intel_backlight_device_register(intel_connector);
+
+       panel->backlight.present = true;
+
+       DRM_DEBUG_KMS("backlight initialized, %s, brightness %u/%u, "
+                     "sysfs interface %sregistered\n",
+                     panel->backlight.enabled ? "enabled" : "disabled",
+                     panel->backlight.level, panel->backlight.max,
+                     panel->backlight.device ? "" : "not ");
+
        return 0;
 }
 
-void intel_panel_destroy_backlight(struct drm_device *dev)
+void intel_panel_destroy_backlight(struct drm_connector *connector)
 {
-       return;
+       struct intel_connector *intel_connector = to_intel_connector(connector);
+       struct intel_panel *panel = &intel_connector->panel;
+
+       panel->backlight.present = false;
+       intel_backlight_device_unregister(intel_connector);
+}
+
+/* Set up chip specific backlight functions */
+void intel_panel_init_backlight_funcs(struct drm_device *dev)
+{
+       struct drm_i915_private *dev_priv = dev->dev_private;
+
+       if (IS_BROADWELL(dev)) {
+               dev_priv->display.setup_backlight = bdw_setup_backlight;
+               dev_priv->display.enable_backlight = bdw_enable_backlight;
+               dev_priv->display.disable_backlight = pch_disable_backlight;
+               dev_priv->display.set_backlight = bdw_set_backlight;
+               dev_priv->display.get_backlight = bdw_get_backlight;
+       } else if (HAS_PCH_SPLIT(dev)) {
+               dev_priv->display.setup_backlight = pch_setup_backlight;
+               dev_priv->display.enable_backlight = pch_enable_backlight;
+               dev_priv->display.disable_backlight = pch_disable_backlight;
+               dev_priv->display.set_backlight = pch_set_backlight;
+               dev_priv->display.get_backlight = pch_get_backlight;
+       } else if (IS_VALLEYVIEW(dev)) {
+               dev_priv->display.setup_backlight = vlv_setup_backlight;
+               dev_priv->display.enable_backlight = vlv_enable_backlight;
+               dev_priv->display.disable_backlight = vlv_disable_backlight;
+               dev_priv->display.set_backlight = vlv_set_backlight;
+               dev_priv->display.get_backlight = vlv_get_backlight;
+       } else if (IS_GEN4(dev)) {
+               dev_priv->display.setup_backlight = i965_setup_backlight;
+               dev_priv->display.enable_backlight = i965_enable_backlight;
+               dev_priv->display.disable_backlight = i965_disable_backlight;
+               dev_priv->display.set_backlight = i9xx_set_backlight;
+               dev_priv->display.get_backlight = i9xx_get_backlight;
+       } else {
+               dev_priv->display.setup_backlight = i9xx_setup_backlight;
+               dev_priv->display.enable_backlight = i9xx_enable_backlight;
+               dev_priv->display.disable_backlight = i9xx_disable_backlight;
+               dev_priv->display.set_backlight = i9xx_set_backlight;
+               dev_priv->display.get_backlight = i9xx_get_backlight;
+       }
 }
-#endif
 
 int intel_panel_init(struct intel_panel *panel,
                     struct drm_display_mode *fixed_mode)