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

[Ville Syrjälä]

On Wed, Nov 21, 2018 at 08:16:06AM +0100, Tolga Cakir wrote:
> 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 think fastboot should work the same for s4 resume as boot, but
only if your boot kernel doesn't load i915.ko. That way the
resuming kernel will directly take over from the BIOS programmed
display configuration. OTOH if the boot kernel loads i915 then
it will shut down all displays before handing control over to
the resuming kernel.

> 
> 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

-- 
Ville Syrjälä
Intel