Re: [PATCH] drm/i915/vlv: hack to init backlight regs if BIOS fails to do so

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Wed, 25 Sep 2013 20:18:39 +0300
Jani Nikula <jani.nikula@linux.intel.com> wrote:

> On Wed, 25 Sep 2013, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > Still digging up the actual VBT info for this, but wanted to get this
> > out there for testing, or in case others are also bugged by this.
> 
> I had a look at this a few weeks back. The VBT value for max backlight
> is in Hz (as is the value you get through opregion) and transforming
> that into the value the registers eat needs some digging. I tried, but
> none of the real world examples of VBT and PWM freq matched any of that,
> so I moved on...
> 
> > This can happen if you boot with an external display connected.  In that
> > case, the attached eDP backlight modulation frequency may not be
> > programmed, so we need to use something (in this case the value my BIOS
> > normally programs with just the internal display enabled).
> 
> Something similar is required for non-vlv ChromeOS stuff too AFAIK.
> 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_panel.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 3bc89a6..a3536785 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -372,6 +372,9 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev)
> >  				I915_WRITE(BLC_PWM_CTL2,
> >  					   dev_priv->regfile.saveBLC_PWM_CTL2);
> >  		}
> > +
> > +		if (IS_VALLEYVIEW(dev) && !val)
> > +			val = 0xffffffff;
> 
> Huh, that's a lot... why don't you use the same value here and below?
> 
> In fact, it should be sufficient to do the hack right here, as this gets
> called through intel_panel_setup_backlight(). Then again, this hole
> function is a kludge... :/
> 
> >  	}
> >  
> >  	return val;
> > @@ -629,10 +632,24 @@ set_level:
> >  	spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
> >  }
> >  
> > +/* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */
> > +static void intel_panel_init_backlight_regs(struct drm_device *dev)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +
> > +	if (IS_VALLEYVIEW(dev)) {
> > +		u32 cur_val = I915_READ(BLC_PWM_CTL) &
> > +			~BACKLIGHT_DUTY_CYCLE_MASK;
> 
> That should be without the NOT, right?

Oops yes, rather than preserving the value I'm about to clobber... :)

-- 
Jesse Barnes, Intel Open Source Technology Center