Re: [Intel-gfx] [PATCH] drm/i915: write backlight harder

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 06 Feb 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> 770c12312ad617172b1a65b911d3e6564fc5aca8 is the first bad commit
> commit 770c12312ad617172b1a65b911d3e6564fc5aca8
> Author: Takashi Iwai <tiwai@suse.de>
> Date:   Sat Aug 11 08:56:42 2012 +0200
>
>     drm/i915: Fix blank panel at reopening lid
>
> changed the register write sequence for restoring the backlight, which
> helped prevent non-working backlights on some machines. Turns out that
> the original sequence was the right thing to do for a different set of
> machines. Worse, setting the backlight level _after_ enabling it seems
> to reset it somehow. So we need to make that one conditional upon the
> backlight having been reset to zero, and add the old one back.
>
> Cargo-culting at it's best, but it seems to work.
>
> Cc: stable@vger.kernel.org
> Cc: Takashi Iwai <tiwai@suse.de>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=47941
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_panel.c |   13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index bee8cb6..a3730e0 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -321,6 +321,9 @@ void intel_panel_enable_backlight(struct drm_device *dev,
>  	if (dev_priv->backlight_level == 0)
>  		dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
>  
> +	dev_priv->backlight_enabled = true;
> +	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
> +
>  	if (INTEL_INFO(dev)->gen >= 4) {
>  		uint32_t reg, tmp;
>  
> @@ -356,12 +359,12 @@ void intel_panel_enable_backlight(struct drm_device *dev,
>  	}
>  
>  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.
> +	/* Check the current backlight level and try to set again if it's zero.
> +	 * On some machines, BLC_PWM_CPU_CTL is cleared to zero automatically
> +	 * when BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1 are written.
>  	 */
> -	dev_priv->backlight_enabled = true;
> -	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
> +	if (!intel_panel_get_backlight(dev))
> +		intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);

Given how much pain the backlight keeps giving us, do you think we would
benefit from sticking a DRM_DEBUG_DRIVER in the if block there?

Either way, with a heavy *sigh*,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>  }
>  
>  static void intel_panel_init_backlight(struct drm_device *dev)
> -- 
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx