Attachment 38059 Details for Bug 29716 – Refactor backlight controls

[patch] Refactor backlight controls

0001-drm-i915-Refactor-panel-backlight-controls.patch (text/plain), 11.41 KB, created by Chris Wilson on 2010-08-22 07:01:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2010-08-22 07:01:34 UTC

Size: 11.41 KB

patch

obsolete

>From 18213ff4fcfa440d96ca135d8bcd5c831f6af77f Mon Sep 17 00:00:00 2001
>From: Chris Wilson <chris@chris-wilson.co.uk>
>Date: Sun, 22 Aug 2010 13:18:16 +0100
>Subject: [PATCH] drm/i915: Refactor panel backlight controls
>
>There were two instances of code to control the panel backlight and
>neither handled the complete set of device variations.
>
>Fixes:
>
>  Bug 29716 - [GM965] Regression: Backlight resets to minimum when changing resolution
>  https://bugs.freedesktop.org/show_bug.cgi?id=29716
>
>And a bug on one of my PineView boxes which overflowed the backlight
>value.
>
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> drivers/gpu/drm/i915/i915_drv.h      |    2 +-
> drivers/gpu/drm/i915/i915_opregion.c |   59 ++-----------------------
> drivers/gpu/drm/i915/intel_drv.h     |    4 +-
> drivers/gpu/drm/i915/intel_lvds.c    |   56 ++---------------------
> drivers/gpu/drm/i915/intel_panel.c   |   81 ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_sdvo.c    |    2 -
> 6 files changed, 94 insertions(+), 110 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index bf41f70..e6c86f9 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -321,7 +321,7 @@ typedef struct drm_i915_private {
> 	struct intel_overlay *overlay;
> 
> 	/* LVDS info */
>-	int backlight_duty_cycle;  /* restore backlight to this value */
>+	int backlight_level;  /* restore backlight to this value */
> 	bool panel_wants_dither;
> 	struct drm_display_mode *panel_fixed_mode;
> 	struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
>diff --git a/drivers/gpu/drm/i915/i915_opregion.c b/drivers/gpu/drm/i915/i915_opregion.c
>index 53d90a2..29ffb3d 100644
>--- a/drivers/gpu/drm/i915/i915_opregion.c
>+++ b/drivers/gpu/drm/i915/i915_opregion.c
>@@ -31,9 +31,9 @@
> #include "drmP.h"
> #include "i915_drm.h"
> #include "i915_drv.h"
>+#include "intel_drv.h"
> 
> #define PCI_ASLE 0xe4
>-#define PCI_LBPC 0xf4
> #define PCI_ASLS 0xfc
> 
> #define OPREGION_SZ            (8*1024)
>@@ -147,36 +147,15 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
> {
> 	struct drm_i915_private *dev_priv = dev->dev_private;
> 	struct opregion_asle *asle = dev_priv->opregion.asle;
>-	u32 blc_pwm_ctl, blc_pwm_ctl2;
>-	u32 max_backlight, level, shift;
> 
> 	if (!(bclp & ASLE_BCLP_VALID))
> 		return ASLE_BACKLIGHT_FAILED;
> 
> 	bclp &= ASLE_BCLP_MSK;
>-	if (bclp < 0 || bclp > 255)
>+	if (bclp > 255)
> 		return ASLE_BACKLIGHT_FAILED;
> 
>-	blc_pwm_ctl = I915_READ(BLC_PWM_CTL);
>-	blc_pwm_ctl2 = I915_READ(BLC_PWM_CTL2);
>-
>-	if (IS_I965G(dev) && (blc_pwm_ctl2 & BLM_COMBINATION_MODE))
>-		pci_write_config_dword(dev->pdev, PCI_LBPC, bclp);
>-	else {
>-		if (IS_PINEVIEW(dev)) {
>-			blc_pwm_ctl &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
>-			max_backlight = (blc_pwm_ctl & BACKLIGHT_MODULATION_FREQ_MASK) >> 
>-					BACKLIGHT_MODULATION_FREQ_SHIFT;
>-			shift = BACKLIGHT_DUTY_CYCLE_SHIFT + 1;
>-		} else {
>-			blc_pwm_ctl &= ~BACKLIGHT_DUTY_CYCLE_MASK;
>-			max_backlight = ((blc_pwm_ctl & BACKLIGHT_MODULATION_FREQ_MASK) >> 
>-					BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
>-			shift = BACKLIGHT_DUTY_CYCLE_SHIFT;
>-		}
>-		level = (bclp * max_backlight) / 255;
>-		I915_WRITE(BLC_PWM_CTL, blc_pwm_ctl | (level << shift));
>-	}
>+	intel_panel_set_backlight(dev, bclp);
> 	asle->cblv = (bclp*0x64)/0xff | ASLE_CBLV_VALID;
> 
> 	return 0;
>@@ -243,36 +222,6 @@ void opregion_asle_intr(struct drm_device *dev)
> 	asle->aslc = asle_stat;
> }
> 
>-static u32 asle_set_backlight_ironlake(struct drm_device *dev, u32 bclp)
>-{
>-	struct drm_i915_private *dev_priv = dev->dev_private;
>-	struct opregion_asle *asle = dev_priv->opregion.asle;
>-	u32 cpu_pwm_ctl, pch_pwm_ctl2;
>-	u32 max_backlight, level;
>-
>-	if (!(bclp & ASLE_BCLP_VALID))
>-		return ASLE_BACKLIGHT_FAILED;
>-
>-	bclp &= ASLE_BCLP_MSK;
>-	if (bclp < 0 || bclp > 255)
>-		return ASLE_BACKLIGHT_FAILED;
>-
>-	cpu_pwm_ctl = I915_READ(BLC_PWM_CPU_CTL);
>-	pch_pwm_ctl2 = I915_READ(BLC_PWM_PCH_CTL2);
>-	/* get the max PWM frequency */
>-	max_backlight = (pch_pwm_ctl2 >> 16) & BACKLIGHT_DUTY_CYCLE_MASK;
>-	/* calculate the expected PMW frequency */
>-	level = (bclp * max_backlight) / 255;
>-	/* reserve the high 16 bits */
>-	cpu_pwm_ctl &= ~(BACKLIGHT_DUTY_CYCLE_MASK);
>-	/* write the updated PWM frequency */
>-	I915_WRITE(BLC_PWM_CPU_CTL, cpu_pwm_ctl | level);
>-
>-	asle->cblv = (bclp*0x64)/0xff | ASLE_CBLV_VALID;
>-
>-	return 0;
>-}
>-
> void ironlake_opregion_gse_intr(struct drm_device *dev)
> {
> 	struct drm_i915_private *dev_priv = dev->dev_private;
>@@ -296,7 +245,7 @@ void ironlake_opregion_gse_intr(struct drm_device *dev)
> 	}
> 
> 	if (asle_req & ASLE_SET_BACKLIGHT)
>-		asle_stat |= asle_set_backlight_ironlake(dev, asle->bclp);
>+		asle_stat |= asle_set_backlight(dev, asle->bclp);
> 
> 	if (asle_req & ASLE_SET_PFIT) {
> 		DRM_DEBUG_DRIVER("Pfit is not supported\n");
>diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>index 21d4c9c..f4a7f3d 100644
>--- a/drivers/gpu/drm/i915/intel_drv.h
>+++ b/drivers/gpu/drm/i915/intel_drv.h
>@@ -177,13 +177,15 @@ extern bool intel_pch_has_edp(struct drm_crtc *crtc);
> extern bool intel_dpd_is_edp(struct drm_device *dev);
> extern void intel_edp_link_config (struct intel_encoder *, int *, int *);
> 
>-
>+/* intel_panel.c */
> extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
> 				   struct drm_display_mode *adjusted_mode);
> extern void intel_pch_panel_fitting(struct drm_device *dev,
> 				    int fitting_mode,
> 				    struct drm_display_mode *mode,
> 				    struct drm_display_mode *adjusted_mode);
>+extern void intel_panel_set_backlight(struct drm_device *dev, u8 level);
>+extern u8 intel_panel_get_backlight(struct drm_device *dev);
> 
> extern int intel_panel_fitter_pipe (struct drm_device *dev);
> extern void intel_crtc_load_lut(struct drm_crtc *crtc);
>diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
>index 5d0582d..e0de075 100644
>--- a/drivers/gpu/drm/i915/intel_lvds.c
>+++ b/drivers/gpu/drm/i915/intel_lvds.c
>@@ -54,43 +54,6 @@ static struct intel_lvds *enc_to_intel_lvds(struct drm_encoder *encoder)
> }
> 
> /**
>- * Sets the backlight level.
>- *
>- * \param level backlight level, from 0 to intel_lvds_get_max_backlight().
>- */
>-static void intel_lvds_set_backlight(struct drm_device *dev, int level)
>-{
>-	struct drm_i915_private *dev_priv = dev->dev_private;
>-	u32 blc_pwm_ctl, reg;
>-
>-	if (HAS_PCH_SPLIT(dev))
>-		reg = BLC_PWM_CPU_CTL;
>-	else
>-		reg = BLC_PWM_CTL;
>-
>-	blc_pwm_ctl = I915_READ(reg) & ~BACKLIGHT_DUTY_CYCLE_MASK;
>-	I915_WRITE(reg, (blc_pwm_ctl |
>-				 (level << BACKLIGHT_DUTY_CYCLE_SHIFT)));
>-}
>-
>-/**
>- * Returns the maximum level of the backlight duty cycle field.
>- */
>-static u32 intel_lvds_get_max_backlight(struct drm_device *dev)
>-{
>-	struct drm_i915_private *dev_priv = dev->dev_private;
>-	u32 reg;
>-
>-	if (HAS_PCH_SPLIT(dev))
>-		reg = BLC_PWM_PCH_CTL2;
>-	else
>-		reg = BLC_PWM_CTL;
>-
>-	return ((I915_READ(reg) & BACKLIGHT_MODULATION_FREQ_MASK) >>
>-		BACKLIGHT_MODULATION_FREQ_SHIFT) * 2;
>-}
>-
>-/**
>  * Sets the power state for the panel.
>  */
> static void intel_lvds_set_power(struct drm_device *dev, bool on)
>@@ -121,9 +84,9 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on)
> 			DRM_ERROR("timed out waiting to enable LVDS pipe");
> #endif
> 
>-		intel_lvds_set_backlight(dev, dev_priv->backlight_duty_cycle);
>+		intel_panel_set_backlight(dev, dev_priv->backlight_level);
> 	} else {
>-		intel_lvds_set_backlight(dev, 0);
>+		intel_panel_set_backlight(dev, 0);
> 
> 		I915_WRITE(ctl_reg, I915_READ(ctl_reg) & ~POWER_TARGET_ON);
> 		if (wait_for((I915_READ(status_reg) & PP_ON) == 0, 1000, 0))
>@@ -389,16 +352,8 @@ static int intel_lvds_prepare(struct drm_encoder *encoder)
> {
> 	struct drm_device *dev = encoder->dev;
> 	struct drm_i915_private *dev_priv = dev->dev_private;
>-	u32 reg;
>-
>-	if (HAS_PCH_SPLIT(dev))
>-		reg = BLC_PWM_CPU_CTL;
>-	else
>-		reg = BLC_PWM_CTL;
> 
>-	dev_priv->saveBLC_PWM_CTL = I915_READ(reg);
>-	dev_priv->backlight_duty_cycle = (dev_priv->saveBLC_PWM_CTL &
>-				       BACKLIGHT_DUTY_CYCLE_MASK);
>+	dev_priv->backlight_level = intel_panel_get_backlight(dev);
> 
> 	intel_lvds_set_power(dev, false);
> 	return 0;
>@@ -409,9 +364,8 @@ static int intel_lvds_commit(struct drm_encoder *encoder)
> 	struct drm_device *dev = encoder->dev;
> 	struct drm_i915_private *dev_priv = dev->dev_private;
> 
>-	if (dev_priv->backlight_duty_cycle == 0)
>-		dev_priv->backlight_duty_cycle =
>-			intel_lvds_get_max_backlight(dev);
>+	if (dev_priv->backlight_level == 0)
>+		dev_priv->backlight_level = 255;
> 
> 	intel_lvds_set_power(dev, true);
> 	return 0;
>diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>index e7f5299..e30e2c0 100644
>--- a/drivers/gpu/drm/i915/intel_panel.c
>+++ b/drivers/gpu/drm/i915/intel_panel.c
>@@ -30,6 +30,8 @@
> 
> #include "intel_drv.h"
> 
>+#define PCI_LBPC 0xf4 /* for backlight on i965 */
>+
> void
> intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
> 		       struct drm_display_mode *adjusted_mode)
>@@ -109,3 +111,82 @@ done:
> 	dev_priv->pch_pf_pos = (x << 16) | y;
> 	dev_priv->pch_pf_size = (width << 16) | height;
> }
>+
>+static void intel_pch_panel_set_backlight(struct drm_device *dev, u8 level)
>+{
>+	struct drm_i915_private *dev_priv = dev->dev_private;
>+	u32 max, val;
>+
>+	max = (I915_READ(BLC_PWM_PCH_CTL2) >> 16) & BACKLIGHT_DUTY_CYCLE_MASK;
>+	val = I915_READ(BLC_PWM_CPU_CTL) & ~(BACKLIGHT_DUTY_CYCLE_MASK);
>+
>+	I915_WRITE(BLC_PWM_CPU_CTL, val | (level * max) / 255);
>+}
>+
>+void intel_panel_set_backlight(struct drm_device *dev, u8 level)
>+{
>+	struct drm_i915_private *dev_priv = dev->dev_private;
>+
>+	if (HAS_PCH_SPLIT(dev))
>+		return intel_pch_panel_set_backlight(dev, level);
>+
>+	if (IS_I965G(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE)) {
>+		pci_write_config_dword(dev->pdev, PCI_LBPC, level);
>+	} else {
>+		u32 tmp = I915_READ(BLC_PWM_CTL);
>+		u32 max= ((tmp & BACKLIGHT_MODULATION_FREQ_MASK) >>
>+			  BACKLIGHT_MODULATION_FREQ_SHIFT);
>+		u32 shift = BACKLIGHT_DUTY_CYCLE_SHIFT;
>+
>+		if (IS_PINEVIEW(dev)) {
>+			tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
>+			shift++;
>+		} else {
>+			tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
>+			max *= 2;
>+		}
>+
>+		I915_WRITE(BLC_PWM_CTL, tmp | ((level * max/ 255) << shift));
>+	}
>+}
>+
>+static u8 intel_pch_panel_get_backlight(struct drm_device *dev)
>+{
>+	struct drm_i915_private *dev_priv = dev->dev_private;
>+	u32 max, val;
>+
>+	max = (I915_READ(BLC_PWM_PCH_CTL2) >> 16) & BACKLIGHT_DUTY_CYCLE_MASK;
>+	val = I915_READ(BLC_PWM_CPU_CTL);
>+
>+	return val * 255 / max;
>+}
>+
>+u8 intel_panel_get_backlight(struct drm_device *dev)
>+{
>+	struct drm_i915_private *dev_priv = dev->dev_private;
>+	u32 val, max, shift;
>+
>+	if (HAS_PCH_SPLIT(dev))
>+		return intel_pch_panel_get_backlight(dev);
>+
>+	if (IS_I965G(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE)) {
>+		pci_read_config_dword(dev->pdev, PCI_LBPC, &val);
>+		return val;
>+	}
>+
>+	val = I915_READ(BLC_PWM_CTL);
>+	max = ((val & BACKLIGHT_MODULATION_FREQ_MASK) >>
>+	       BACKLIGHT_MODULATION_FREQ_SHIFT);
>+	shift = BACKLIGHT_DUTY_CYCLE_SHIFT;
>+
>+	if (IS_PINEVIEW(dev)) {
>+		val &= BACKLIGHT_DUTY_CYCLE_MASK - 1;
>+		shift++;
>+	} else {
>+		val &= BACKLIGHT_DUTY_CYCLE_MASK;
>+		max *= 2;
>+	}
>+	val >>= shift;
>+
>+	return val * 255 / max;
>+}
>diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>index a308b4f..ea2f4ab 100644
>--- a/drivers/gpu/drm/i915/intel_sdvo.c
>+++ b/drivers/gpu/drm/i915/intel_sdvo.c
>@@ -1827,8 +1827,6 @@ intel_sdvo_set_property(struct drm_connector *connector,
> set_value:
> 	if (!intel_sdvo_set_value(intel_sdvo, cmd, &temp_value, 2))
> 		return -EIO;
>-
>-
> done:
> 	if (intel_sdvo->base.base.crtc) {
> 		struct drm_crtc *crtc = intel_sdvo->base.base.crtc;
>-- 
>1.7.1
>

Actions: View | Splinter Review

Attachments on bug 29716: 38059 | 39609 | 40154

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.