Re: Strange effect with i915 backlight controller

[Daniel Mack <zonque@gmail.com>]

On 11/14/2011 11:39 AM, Takashi Iwai wrote:
> OK, then perhaps a better fix is to change the check to be equivalent
> with pineview, as you mentioned in the original post.  The handling of
> bit 0 for old chips was lost during the refactoring of backlight code
> since 2.6.37.
> 
> Does the patch below work for you?
> 
> The only concern by this fix is that it changes the max value.  If
> apps expect some certain (e.g. recorded) value, it may screw up.  But
> I don't expect this would happen with sane apps.

Works perfectly - let's ship it :)


Thanks again,
Daniel


> ===
> From: Takashi Iwai <tiwai@suse.de>
> Subject: drm/i915: Fix invalid backpanel values for GEN3 or older chips
> 
> While refactoring of backlight control code in commit [a95735569:
> drm/i915: Refactor panel backlight controls], the handling of the bit
> 0 of duty-cycle was gone except for pineview.  This resulted in invalid
> register values for old chips like 915GM.  When the bit 0 is set, the
> backlight is turned off suddenly.
> 
> This patch changes the bit-0 check by replacing with the condition of
> gen < 4 (pineview is included in this condition, too).
> 
> Reported-by: Daniel Mack <zonque@gmail.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/i915/intel_panel.c |    8 +++-----
>  1 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 499d4c0..737d00f 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -178,12 +178,10 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev)
>  	if (HAS_PCH_SPLIT(dev)) {
>  		max >>= 16;
>  	} else {
> -		if (IS_PINEVIEW(dev)) {
> +		if (INTEL_INFO(dev)->gen < 4) {
>  			max >>= 17;
>  		} else {
>  			max >>= 16;
> -			if (INTEL_INFO(dev)->gen < 4)
> -				max &= ~1;
>  		}
>  
>  		if (is_backlight_combination_mode(dev))
> @@ -203,7 +201,7 @@ u32 intel_panel_get_backlight(struct drm_device *dev)
>  		val = I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
>  	} else {
>  		val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
> -		if (IS_PINEVIEW(dev))
> +		if (INTEL_INFO(dev)->gen < 4)
>  			val >>= 1;
>  
>  		if (is_backlight_combination_mode(dev)) {
> @@ -246,7 +244,7 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev, u32 level
>  	}
>  
>  	tmp = I915_READ(BLC_PWM_CTL);
> -	if (IS_PINEVIEW(dev)) {
> +	if (INTEL_INFO(dev)->gen < 4) {
>  		tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
>  		level <<= 1;
>  	} else