[Intel-gfx] [PATCH] drm/i915/backlight: Fix backlight takeover on LPT

[Tolga Cakir]

Am Di., 20. Nov. 2018 um 19:51 Uhr schrieb Ville Syrjälä
<ville.syrjala at 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 at linux.intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108225
> > Cc: Basil Eric Rabi <ericbasil.rabi at gmail.com>
> > Cc: Hans de Goede <jwrdegoede at fedoraproject.org>
> > Cc: Tolga Cakir <cevelnet at gmail.com>
> > Tested-by: Tolga Cakir <cevelnet at 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?
>

Hi,

I couldn't test this patch for S4 resume on my Dell M3800 laptop (Core
i7-4702HQ, Lynx Point), as fastboot=1 doesn't work for S4 resume, only
normal boot. Therefore, there are no issues on S4 wake after
hibernate. Backlight controls etc. work as expected; just like
fastboot=0.

I have applied "[1/2] drm/i915: Enable fastset for non-boot modesets"
by Maarten, but that doesn't give me fastboot for S4 resume aswell.
Same behavior as unpatched on my system.

Please let me know, when there is more I can test.

Cheers,
Tolga

> > +
> > +             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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel