Re: [PATCH] drm/i915/backlight: Fix backlight takeover on LPT

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Nov 20, 2018 at 11:39:26AM +0100, Maarten Lankhorst wrote:
> On lynxpoint the bios sometimes sets up the backlight using the CPU
> display, but the driver expects using the PWM PCH override register.
> 
> Read the value from the CPU register, then convert it to the other
> units by converting from the old duty cycle, to freq, to the new units.
> 
> This value is then programmed in the override register, after which
> we set the override and disable the CPU display control. This allows
> us to switch the source without flickering, and make the backlight
> controls work in the driver.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108225
> Cc: Basil Eric Rabi <ericbasil.rabi@gmail.com>
> Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> Cc: Tolga Cakir <cevelnet@gmail.com>
> Tested-by: Tolga Cakir <cevelnet@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_panel.c | 34 +++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index e6cd7b55c018..3357232f1b0e 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1485,7 +1485,7 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
>  	u32 pch_ctl1, pch_ctl2, val;
> -	bool alt;
> +	bool alt, cpu_mode = false;
>  
>  	if (HAS_PCH_LPT(dev_priv))
>  		alt = I915_READ(SOUTH_CHICKEN2) & LPT_PWM_GRANULARITY;
> @@ -1507,12 +1507,40 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
>  
>  	panel->backlight.min = get_backlight_min_vbt(connector);
>  
> -	val = lpt_get_backlight(connector);
> +	panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> +	if (panel->backlight.enabled && HAS_PCH_LPT(dev_priv) &&
> +	    (I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE) &&
> +	    !WARN_ON(pch_ctl1 & BLM_PCH_OVERRIDE_ENABLE)) {
> +		u32 freq;
> +
> +		cpu_mode = true;
> +		/*
> +		 * We're in cpu mode, convert to PCH units.
> +		 *
> +		 * Convert CPU pwm tick back to hz, back to new PCH units again.
> +		 * this is the same formula as pch_hz_to_pwm, but the other way
> +		 * around..
> +		 */
> +		val = pch_get_backlight(connector);
> +		freq = DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), val * 128);

The docs are telling me that HSW/BDW use cdclk for the CPU backlight,
and the increment is configurable as well.

Also what about S4 resume? Don't we need this trick there as well?

> +
> +		val = lpt_hz_to_pwm(connector, freq);
> +	} else
> +		val = lpt_get_backlight(connector);
>  	val = intel_panel_compute_brightness(connector, val);
>  	panel->backlight.level = clamp(val, panel->backlight.min,
>  				       panel->backlight.max);
>  
> -	panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> +	if (cpu_mode) {
> +		u32 tmp;
> +
> +		/* Use PWM mode, instead of cpu clock */
> +		lpt_set_backlight(connector->base.state, panel->backlight.level);
> +		I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_OVERRIDE_ENABLE);
> +
> +		tmp = I915_READ(BLC_PWM_CPU_CTL2);
> +		I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.19.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx