[Intel-gfx] [PATCH 08/13] drm/i915: use the initialized backlight max value instead of reading it

[Daniel Vetter]

On Wed, Nov 13, 2013 at 10:39:43AM +0200, Jani Nikula wrote:
> On Wed, 13 Nov 2013, Imre Deak <imre.deak at intel.com> wrote:
> > On Fri, 2013-11-08 at 16:49 +0200, Jani Nikula wrote:
> >> We now have the max backlight value cached. Use it.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_panel.c |   45 +++++++++++++++++++-----------------
> >>  1 file changed, 24 insertions(+), 21 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> >> index ed6b1ec..9a55b36 100644
> >> --- a/drivers/gpu/drm/i915/intel_panel.c
> >> +++ b/drivers/gpu/drm/i915/intel_panel.c
> >> @@ -436,9 +436,6 @@ static u32 vlv_get_max_backlight(struct intel_connector *connector)
> >>  	return _vlv_get_max_backlight(dev, pipe);
> >>  }
> >>  
> >> -/* XXX: query mode clock or hardware clock and program max PWM appropriately
> >> - * when it's 0.
> >> - */
> >>  static u32 intel_panel_get_max_backlight(struct intel_connector *connector)
> >>  {
> >>  	struct drm_device *dev = connector->base.dev;
> >> @@ -466,15 +463,16 @@ static u32 intel_panel_compute_brightness(struct intel_connector *connector,
> >>  {
> >>  	struct drm_device *dev = connector->base.dev;
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >> +	struct intel_panel *panel = &connector->panel;
> >> +
> >> +	WARN_ON(panel->backlight.max == 0);
> >
> > Just to clarify that I understood this: we can't hit this any more since
> > setup_backlight returns now -ENODEV if backlight.max == 0 and so we
> > won't register the backlight device in that case. I assume then that
> > during booting the BIOS always leaves us with a valid max PWM value in
> > one of the PWM_CTL regs and it can only get zeroed due to suspend, which
> > isn't a problem after patch 4. With this:
> 
> You've correctly understood what I meant. I do hope what I meant is the
> right thing to do! ;)
> 
> Now that I think about it, there are hints that (at least on some
> platforms) a GPU reset might cause the registers to be reset too:
> 
> commit 0b0b053a3949f5c467c3b3ba135d4c161f9fbd00
> Author: Chris Wilson <chris at chris-wilson.co.uk>
> Date:   Tue Nov 23 09:45:50 2010 +0000
> 
>     drm/i915/panel: Restore saved value of BLC_PWM_CTL
> 
> The big question is, are we okay with re-writing the potentially zeroed
> out registers at crtc enable time? The current upstream code would do
> that at reading the max backlight level, which happens also at adjusting
> the backlight. But there are no guarantees about anyone adjusting the
> backlight either.
> 
> I don't understand our GPU reset enough to say for sure, but my gut
> feeling is that what I have in this series should be enough.

Also not that nowadays we don't reinitialize the display stuff any more
since it caused trouble. And it wasn't necessary either. So if we'd indeed
need to readjust the backlight I think we'd have a bug report by now:

commit 523bcb28c3fb92baecda6daa60560f67eb4a6177
Author: Daniel Vetter <daniel.vetter at ffwll.ch>
Date:   Fri Apr 27 15:17:46 2012 +0200

    drm/i915: remove modeset reset from i915_reset

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch