drm/i915: handle backlight through chip specific functions

The backlight code has grown rather hairy, not least because the
hardware registers and bits have repeatedly been shuffled around. And
this isn't expected to get any easier with new hardware. Make things
easier for our (read: my) poor brains, and split the code up into chip
specific functions.

There should be no functional changes.
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

@@ -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,14 @@ 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_max_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 {

drivers/gpu/drm/i915/intel_display.c

@@ -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);
 }
 
 /*

drivers/gpu/drm/i915/intel_drv.h

@@ -161,6 +161,7 @@ struct intel_panel {
 	struct {
 		bool present;
 		u32 level;
+		u32 max;
 		bool enabled;
 		struct backlight_device *device;
 	} backlight;
@@ -817,6 +818,7 @@ 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_connector *connector);
+void intel_panel_init_backlight_funcs(struct drm_device *dev);
 enum drm_connector_status intel_panel_detect(struct drm_device *dev);
 
 

drivers/gpu/drm/i915/intel_panel.c

@@ -338,19 +338,12 @@ static int is_backlight_combination_mode(struct drm_device *dev)
 	return 0;
 }
 
-/* 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 pch_get_max_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;
@@ -358,58 +351,103 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev, enum pipe pipe)
 		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));
+
+	val >>= 16;
+
+	return val;
+}
+
+static u32 i9xx_get_max_backlight(struct intel_connector *connector)
+{
+	struct drm_device *dev = connector->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val;
+
+	val = I915_READ(BLC_PWM_CTL);
 	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);
+		I915_WRITE(BLC_PWM_CTL, val);
 	}
 
-		if (!val)
-			val = 0x0f42ffff;
-	} else {
+	val >>= 17;
+
+	if (is_backlight_combination_mode(dev))
+		val *= 0xff;
+
+	return val;
+}
+
+static u32 i965_get_max_backlight(struct intel_connector *connector)
+{
+	struct drm_device *dev = connector->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val;
+
 	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);
+		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);
+		I915_WRITE(BLC_PWM_CTL2, dev_priv->regfile.saveBLC_PWM_CTL2);
 	}
+
+	val >>= 16;
+
+	if (is_backlight_combination_mode(dev))
+		val *= 0xff;
+
+	return val;
+}
+
+static u32 _vlv_get_max_backlight(struct drm_device *dev, enum pipe pipe)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val;
+
+	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);
 	}
 
+	if (!val)
+		val = 0x0f42ffff;
+
+	val >>= 16;
+
 	return val;
 }
 
-static u32 intel_panel_get_max_backlight(struct drm_device *dev,
-					 enum pipe pipe)
+static u32 vlv_get_max_backlight(struct intel_connector *connector)
 {
-	u32 max;
+	struct drm_device *dev = connector->base.dev;
+	enum pipe pipe = intel_get_pipe_from_connector(connector);
 
-	max = i915_read_blc_pwm_ctl(dev, pipe);
+	return _vlv_get_max_backlight(dev, pipe);
+}
 
-	if (HAS_PCH_SPLIT(dev)) {
-		max >>= 16;
-	} else {
-		if (INTEL_INFO(dev)->gen < 4)
-			max >>= 17;
-		else
-			max >>= 16;
+/* XXX: query mode clock or hardware clock and program max PWM appropriately
+ * when it's 0.
+ */
+static u32 intel_panel_get_max_backlight(struct intel_connector *connector)
+{
+	struct drm_device *dev = connector->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 max;
 
-		if (is_backlight_combination_mode(dev))
-			max *= 0xff;
-	}
+	WARN_ON_SMP(!spin_is_locked(&dev_priv->backlight_lock));
+
+	max = dev_priv->display.get_max_backlight(connector);
 
 	DRM_DEBUG_DRIVER("max backlight PWM = %d\n", max);
 
@@ -423,9 +461,10 @@ MODULE_PARM_DESC(invert_brightness, "Invert backlight brightness "
 	"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 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;
 
 	if (i915_panel_invert_brightness < 0)
@@ -433,7 +472,7 @@ static u32 intel_panel_compute_brightness(struct drm_device *dev,
 
 	if (i915_panel_invert_brightness > 0 ||
 	    dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS) {
-		u32 max = intel_panel_get_max_backlight(dev, pipe);
+		u32 max = intel_panel_get_max_backlight(connector);
 		if (max)
 			return max - val;
 	}
@@ -441,25 +480,21 @@ static u32 intel_panel_compute_brightness(struct drm_device *dev,
 	return val;
 }
 
-static u32 intel_panel_get_backlight(struct drm_device *dev,
-				     enum pipe pipe)
+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;
-	u32 val;
-	unsigned long flags;
-	int reg;
 
-	spin_lock_irqsave(&dev_priv->backlight_lock, flags);
+	return I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
+}
 
-	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;
+static u32 i9xx_get_backlight(struct intel_connector *connector)
+{
+	struct drm_device *dev = connector->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val;
 
-		val = I915_READ(reg) & BACKLIGHT_DUTY_CYCLE_MASK;
+	val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
 	if (INTEL_INFO(dev)->gen < 4)
 		val >>= 1;
 
@@ -469,9 +504,36 @@ static u32 intel_panel_get_backlight(struct drm_device *dev,
 		pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc);
 		val *= lbpc;
 	}
-	}
 
-	val = intel_panel_compute_brightness(dev, pipe, val);
+	return val;
+}
+
+static u32 _vlv_get_backlight(struct drm_device *dev, enum pipe pipe)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return I915_READ(VLV_BLC_PWM_CTL(pipe)) & BACKLIGHT_DUTY_CYCLE_MASK;
+}
+
+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 _vlv_get_backlight(dev, 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;
+
+	spin_lock_irqsave(&dev_priv->backlight_lock, flags);
+
+	val = dev_priv->display.get_backlight(connector);
+	val = intel_panel_compute_brightness(connector, val);
 
 	spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
 
@@ -479,28 +541,24 @@ static u32 intel_panel_get_backlight(struct drm_device *dev,
 	return val;
 }
 
-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);
-
-	if (HAS_PCH_SPLIT(dev))
-		return intel_pch_panel_set_backlight(dev, level);
 
 	if (is_backlight_combination_mode(dev)) {
-		u32 max = intel_panel_get_max_backlight(dev, pipe);
+		u32 max = intel_panel_get_max_backlight(connector);
 		u8 lbpc;
 
 		/* we're screwed, but keep behaviour backwards compatible */
@@ -512,16 +570,34 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev,
 		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)
 		level <<= 1;
-	tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
-	I915_WRITE(reg, tmp | level);
+
+	tmp = I915_READ(BLC_PWM_CTL) & ~BACKLIGHT_DUTY_CYCLE_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] */
@@ -540,7 +616,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
 
 	spin_lock_irqsave(&dev_priv->backlight_lock, flags);
 
-	freq = intel_panel_get_max_backlight(dev, pipe);
+	freq = intel_panel_get_max_backlight(connector);
 	if (!freq) {
 		/* we are screwed, bail out */
 		goto out;
@@ -557,11 +633,45 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
 		panel->backlight.device->props.brightness = level;
 
 	if (panel->backlight.enabled)
-		intel_panel_actually_set_backlight(dev, pipe, level);
+		intel_panel_actually_set_backlight(connector, level);
 out:
 	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;
+
+	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 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;
+
+	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;
+
+	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;
@@ -587,72 +697,30 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
 	spin_lock_irqsave(&dev_priv->backlight_lock, flags);
 
 	panel->backlight.enabled = false;
-	intel_panel_actually_set_backlight(dev, pipe, 0);
+	intel_panel_actually_set_backlight(connector, 0);
 
-	if (INTEL_INFO(dev)->gen >= 4) {
-		uint32_t reg, tmp;
-
-		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;
-
-		I915_WRITE(reg, I915_READ(reg) & ~BLM_PWM_ENABLE);
-
-		if (HAS_PCH_SPLIT(dev)) {
-			tmp = I915_READ(BLC_PWM_PCH_CTL1);
-			tmp &= ~BLM_PCH_PWM_ENABLE;
-			I915_WRITE(BLC_PWM_PCH_CTL1, tmp);
-		}
-	}
+	if (dev_priv->display.disable_backlight)
+		dev_priv->display.disable_backlight(connector);
 
 	spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
 }
 
-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;
-
-	if (pipe == INVALID_PIPE)
-		return;
-
-	DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
-
-	spin_lock_irqsave(&dev_priv->backlight_lock, flags);
-
-	if (panel->backlight.level == 0) {
-		panel->backlight.level = intel_panel_get_max_backlight(dev,
-								       pipe);
-		if (panel->backlight.device)
-			panel->backlight.device->props.brightness =
-				panel->backlight.level;
-	}
-
-	if (INTEL_INFO(dev)->gen >= 4) {
-		uint32_t reg, tmp;
-
-		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;
+	u32 tmp;
 
-		tmp = I915_READ(reg);
+	tmp = I915_READ(BLC_PWM_CPU_CTL2);
 
 	/* 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;
+		return;
 
 	if (INTEL_INFO(dev)->num_pipes == 3)
 		tmp &= ~BLM_PIPE_SELECT_IVB;
@@ -665,51 +733,99 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
 		tmp |= BLM_PIPE(cpu_transcoder);
 	tmp &= ~BLM_PWM_ENABLE;
 
-		I915_WRITE(reg, tmp);
-		POSTING_READ(reg);
-		I915_WRITE(reg, tmp | BLM_PWM_ENABLE);
+	I915_WRITE(BLC_PWM_CPU_CTL2, tmp);
+	POSTING_READ(BLC_PWM_CPU_CTL2);
+	I915_WRITE(BLC_PWM_CPU_CTL2, tmp | BLM_PWM_ENABLE);
 
-		if (HAS_PCH_SPLIT(dev) &&
-		    !(dev_priv->quirks & QUIRK_NO_PCH_PWM_ENABLE)) {
+	if (!(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);
 	}
-	}
+}
 
-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.
-	 */
-	panel->backlight.enabled = true;
-	intel_panel_actually_set_backlight(dev, pipe,
-					   panel->backlight.level);
+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;
+	enum pipe pipe = intel_get_pipe_from_connector(connector);
+	u32 tmp;
 
-	spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
+	tmp = I915_READ(BLC_PWM_CTL2);
+
+	/* 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)
+		return;
+
+	tmp &= ~BLM_PIPE_SELECT;
+	tmp |= BLM_PIPE(pipe);
+	tmp &= ~BLM_PWM_ENABLE;
+
+	I915_WRITE(BLC_PWM_CTL2, tmp);
+	POSTING_READ(BLC_PWM_CTL2);
+	I915_WRITE(BLC_PWM_CTL2, tmp | 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;
+	enum pipe pipe = intel_get_pipe_from_connector(connector);
+	u32 tmp;
 
-	if (IS_VALLEYVIEW(dev)) {
-		enum pipe pipe;
+	tmp = I915_READ(VLV_BLC_PWM_CTL2(pipe));
 
-		for_each_pipe(pipe) {
-			u32 cur_val = I915_READ(VLV_BLC_PWM_CTL(pipe));
+	/* 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)
+		return;
 
-			/* Skip if the modulation freq is already set */
-			if (cur_val & ~BACKLIGHT_DUTY_CYCLE_MASK)
-				continue;
+	tmp &= ~BLM_PIPE_SELECT;
+	tmp |= BLM_PIPE(pipe);
+	tmp &= ~BLM_PWM_ENABLE;
 
-			cur_val &= BACKLIGHT_DUTY_CYCLE_MASK;
-			I915_WRITE(VLV_BLC_PWM_CTL(pipe), (0xf42 << 16) |
-				   cur_val);
-		}
+	I915_WRITE(VLV_BLC_PWM_CTL2(pipe), tmp);
+	POSTING_READ(VLV_BLC_PWM_CTL2(pipe));
+	I915_WRITE(VLV_BLC_PWM_CTL2(pipe), tmp | BLM_PWM_ENABLE);
+}
+
+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;
+
+	if (pipe == INVALID_PIPE)
+		return;
+
+	DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
+
+	spin_lock_irqsave(&dev_priv->backlight_lock, flags);
+
+	if (panel->backlight.level == 0) {
+		panel->backlight.level = intel_panel_get_max_backlight(connector);
+		if (panel->backlight.device)
+			panel->backlight.device->props.brightness =
+				panel->backlight.level;
 	}
+
+	if (dev_priv->display.enable_backlight)
+		dev_priv->display.enable_backlight(connector);
+
+	/* 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.
+	 */
+	panel->backlight.enabled = true;
+	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+
+	spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
 }
 
 enum drm_connector_status
@@ -753,15 +869,13 @@ 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_backlight_device_ops = {
@@ -771,27 +885,18 @@ static const struct backlight_ops intel_backlight_device_ops = {
 
 static int intel_backlight_device_register(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;
 	struct backlight_properties props;
-	unsigned long flags;
 
 	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 = panel->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);
-
-	if (props.max_brightness == 0) {
-		DRM_DEBUG_DRIVER("Failed to get maximum backlight value\n");
-		return -ENODEV;
-	}
+	props.max_brightness = panel->backlight.max;
 
 	/*
 	 * Note: using the same name independent of the connector prevents
@@ -831,15 +936,102 @@ 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! */
+static int pch_setup_backlight(struct intel_connector *connector)
+{
+	struct intel_panel *panel = &connector->panel;
+	u32 val;
+
+	panel->backlight.max = pch_get_max_backlight(connector);
+	if (!panel->backlight.max)
+		return -ENODEV;
+
+	val = pch_get_backlight(connector);
+	panel->backlight.level = intel_panel_compute_brightness(connector, val);
+
+	return 0;
+}
+
+static int i9xx_setup_backlight(struct intel_connector *connector)
+{
+	struct intel_panel *panel = &connector->panel;
+	u32 val;
+
+	panel->backlight.max = i9xx_get_max_backlight(connector);
+	if (!panel->backlight.max)
+		return -ENODEV;
+
+	val = i9xx_get_backlight(connector);
+	panel->backlight.level = intel_panel_compute_brightness(connector, val);
+
+	return 0;
+}
+
+static int i965_setup_backlight(struct intel_connector *connector)
+{
+	struct intel_panel *panel = &connector->panel;
+	u32 val;
+
+	panel->backlight.max = i965_get_max_backlight(connector);
+	if (!panel->backlight.max)
+		return -ENODEV;
+
+	val = i9xx_get_backlight(connector);
+	panel->backlight.level = intel_panel_compute_brightness(connector, val);
+
+	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 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);
+	}
+
+	panel->backlight.max = _vlv_get_max_backlight(dev, PIPE_A);
+	if (!panel->backlight.max)
+		return -ENODEV;
+
+	val = _vlv_get_backlight(dev, PIPE_A);
+	panel->backlight.level = intel_panel_compute_brightness(connector, val);
+
+	return 0;
+}
+
 int intel_panel_setup_backlight(struct drm_connector *connector)
 {
 	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);
 
-	intel_panel_init_backlight_regs(dev);
+	if (ret) {
+		DRM_DEBUG_KMS("failed to setup backlight for connector %s\n",
+			      drm_get_connector_name(connector));
+		return ret;
+	}
 
-	panel->backlight.level = intel_panel_get_backlight(dev, 0);
 	panel->backlight.enabled = panel->backlight.level != 0;
 
 	intel_backlight_device_register(intel_connector);
@@ -858,6 +1050,40 @@ void intel_panel_destroy_backlight(struct drm_connector *connector)
 	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 (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;
+		dev_priv->display.get_max_backlight = pch_get_max_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;
+		dev_priv->display.get_max_backlight = vlv_get_max_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;
+		dev_priv->display.get_max_backlight = i965_get_max_backlight;
+	} else {
+		dev_priv->display.setup_backlight = i9xx_setup_backlight;
+		dev_priv->display.set_backlight = i9xx_set_backlight;
+		dev_priv->display.get_backlight = i9xx_get_backlight;
+		dev_priv->display.get_max_backlight = i9xx_get_max_backlight;
+	}
+}
+
 int intel_panel_init(struct intel_panel *panel,
 		     struct drm_display_mode *fixed_mode)
 {