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

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

[Jesse Barnes]

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.

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

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;
 	}
 
 	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;
+		I915_WRITE(BLC_PWM_CTL, (0xf42 << 16) | cur_val);
+	}
+}
+
 static void intel_panel_init_backlight(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	intel_panel_init_backlight_regs(dev);
+
 	dev_priv->backlight.level = intel_panel_get_backlight(dev);
 	dev_priv->backlight.enabled = dev_priv->backlight.level != 0;
 }
-- 
1.8.3.1

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

[Jani Nikula]

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?

> +		I915_WRITE(BLC_PWM_CTL, (0xf42 << 16) | cur_val);
> +	}
> +}
> +
>  static void intel_panel_init_backlight(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> +	intel_panel_init_backlight_regs(dev);
> +
>  	dev_priv->backlight.level = intel_panel_get_backlight(dev);
>  	dev_priv->backlight.enabled = dev_priv->backlight.level != 0;
>  }
> -- 
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jesse Barnes]

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... :/

Simply doing it here isn't enough, because we never actually set the
modulation freq bits.  But yes, we could use the same magic value in
both places.  Seems to work either way though.

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

No I'm explicitly ignoring the low 16 bits, trying to set the bits for
the modulation frequency.  Without those, setting a value just gives
you a blinking display as the backlight goes on and off about once per
500ms.


-- 
Jesse Barnes, Intel Open Source Technology Center

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

[Jesse Barnes]

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

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

[Daniel Vetter]

On Wed, Sep 25, 2013 at 08:18:39PM +0300, Jani Nikula 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.

Afaik ChromeOS doesn't have a vbt, so I think we need to shovel some
failsafe (yeah, failsafe and backlight doesn't compute, I know) default
into the regs in case all else fails.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch