[PATCH v2] drivers: i915: Default max backlight brightness value

[PATCH v2] drivers: i915: Default max backlight brightness value

[Simon Que]

Use 0x1000 as the default backlight PWM max value and period.  This is
passed in as a module parameter to i915_drv and is used to program the
PWM registers.  It can be set to other values based on the needs of each
system.

Signed-off-by: Simon Que <sque@chromium.org>
---
 drivers/gpu/drm/i915/i915_drv.c    |    3 +++
 drivers/gpu/drm/i915/i915_drv.h    |    1 +
 drivers/gpu/drm/i915/i915_reg.h    |    1 +
 drivers/gpu/drm/i915/intel_panel.c |   29 +++++++++++++++++------------
 4 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index eb91e2d..fd06ce8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -70,6 +70,9 @@ module_param_named(vbt_sdvo_panel_type, i915_vbt_sdvo_panel_type, int, 0600);
 static bool i915_try_reset = true;
 module_param_named(reset, i915_try_reset, bool, 0600);
 
+unsigned int i915_max_backlight = 0x1000;
+module_param_named(max_backlight, i915_max_backlight, bool, 0600);
+
 static struct drm_driver driver;
 extern int intel_agp_enabled;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2d8fa6c..6096597 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -999,6 +999,7 @@ extern unsigned int i915_panel_use_ssc;
 extern int i915_vbt_sdvo_panel_type;
 extern unsigned int i915_enable_rc6;
 extern unsigned int i915_enable_fbc;
+extern unsigned int i915_max_backlight;
 
 extern int i915_suspend(struct drm_device *dev, pm_message_t state);
 extern int i915_resume(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5d5def7..a832028 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3275,6 +3275,7 @@
 #define  PWM_POLARITY_ACTIVE_HIGH2	(0 << 28)
 
 #define BLC_PWM_PCH_CTL2	0xc8254
+#define BLC_PWM_PCH_FREQ_SHIFT	16
 
 #define PCH_PP_STATUS		0xc7200
 #define PCH_PP_CONTROL		0xc7204
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 1af6888..82c6c05 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -133,27 +133,32 @@ static u32 i915_read_blc_pwm_ctl(struct drm_i915_private *dev_priv)
 {
 	u32 val;
 
-	/* Restore the CTL value if it lost, e.g. GPU reset */
-
+	/* Restore the CTL value if it was lost, e.g. GPU reset */
+	/* Use the default PWM max if none is available. */
 	if (HAS_PCH_SPLIT(dev_priv->dev)) {
 		val = I915_READ(BLC_PWM_PCH_CTL2);
-		if (dev_priv->saveBLC_PWM_CTL2 == 0) {
+		if (dev_priv->saveBLC_PWM_CTL2 == 0 && val == 0)
+			dev_priv->saveBLC_PWM_CTL2 =
+				i915_max_backlight << BLC_PWM_PCH_FREQ_SHIFT;
+		else if (dev_priv->saveBLC_PWM_CTL2 == 0)
 			dev_priv->saveBLC_PWM_CTL2 = val;
-		} else if (val == 0) {
+		if (val == 0) {
 			I915_WRITE(BLC_PWM_PCH_CTL2,
-				   dev_priv->saveBLC_PWM_CTL);
-			val = dev_priv->saveBLC_PWM_CTL;
+				   dev_priv->saveBLC_PWM_CTL2);
+			val = dev_priv->saveBLC_PWM_CTL2;
 		}
 	} else {
 		val = I915_READ(BLC_PWM_CTL);
-		if (dev_priv->saveBLC_PWM_CTL == 0) {
+		if (dev_priv->saveBLC_PWM_CTL == 0 && val == 0) {
+			dev_priv->saveBLC_PWM_CTL = i915_max_backlight
+				<< BACKLIGHT_MODULATION_FREQ_SHIFT;
+		} else if (dev_priv->saveBLC_PWM_CTL == 0) {
 			dev_priv->saveBLC_PWM_CTL = val;
 			dev_priv->saveBLC_PWM_CTL2 = I915_READ(BLC_PWM_CTL2);
-		} else if (val == 0) {
-			I915_WRITE(BLC_PWM_CTL,
-				   dev_priv->saveBLC_PWM_CTL);
-			I915_WRITE(BLC_PWM_CTL2,
-				   dev_priv->saveBLC_PWM_CTL2);
+		}
+		if (val == 0) {
+			I915_WRITE(BLC_PWM_CTL, dev_priv->saveBLC_PWM_CTL);
+			I915_WRITE(BLC_PWM_CTL2, dev_priv->saveBLC_PWM_CTL2);
 			val = dev_priv->saveBLC_PWM_CTL;
 		}
 	}
-- 
1.7.2.3

Re: [PATCH v2] drivers: i915: Default max backlight brightness value

[Matthew Garrett]

Again, adding arbitrary constants without any explanation for why you're 
making this the default really isn't acceptable. We have no way to 
determine whether fixing one machine is worth making things worse for 
another.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v2] drivers: i915: Default max backlight brightness value

[Simon Que]

On Tue, Nov 1, 2011 at 7:54 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> Again, adding arbitrary constants without any explanation for why you're
> making this the default really isn't acceptable. We have no way to
> determine whether fixing one machine is worth making things worse for
> another.

The default is applied only in the case where no valid register
settings have been found to be initialized by the BIOS.  Hence the
only systems that will be affected by this patch are the ones whose
backlight PWM is already improperly configured.  Hence, I think
affected systems will either benefit from this change or be made no
worse, as they were already broken.

That said, I do think the patch could use some more explanation in
comments and the patch description.