> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Tuesday, July 29, 2014 2:13 PM
> To: Barbalho, Rafael
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915: Correctly read backlight PWM for pipe B on
> vlv/chv
>
> On Tue, Jul 29, 2014 at 01:44:40PM +0100, rafael.barba...@intel.com wrote:
> > From: Rafael Barbalho <rafael.barba...@intel.com>
> >
> > Make the vlv/chv backlight setup more generic by actually looking at which
> > pipe the panel is attached to and read the backlight PWM registers that
> were
> > setup by the bios from that pipe.
> >
> > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > Signed-off-by: Rafael Barbalho <rafael.barba...@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_panel.c | 24 ++++++++++--------------
> > 1 file changed, 10 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c
> b/drivers/gpu/drm/i915/intel_panel.c
> > index 59b028f..be75d76 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -1200,32 +1200,28 @@ static int vlv_setup_backlight(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;
> > - enum pipe pipe;
> > + enum pipe pipe = intel_get_pipe_from_connector(connector);
>
> This won't work unless the connector is already enabled.
>
> The power sequencer has a similar problem where we have to somehow
> deduce
> the correct pipe. vlv_power_sequencer_pipe() tries to guess the correct
> pipe
> there.
>
> We could start with intel_get_pipe_from_connector() and if that fails we'd
> do something like vlv_power_sequencer_pipe(). Hmm, except the backlight
> registers don't have the port information, so I guess we'd need to pick the
> pipe simply based on the DP port control register.
>
Fair point, I didn't realise that intel_get_pipe_from_connector could return
INVALID_PIPE.
How about if I add:
+ enum pipe pipe = intel_get_pipe_from_connector(connector);
+
+ if (pipe == INVALID_PIPE)
+ pipe = PIPE_A;
It would return the driver to the current behaviour but still enable pipe B
when that is present
at start up.
> > u32 ctl, ctl2, val;
> >
> > - for_each_pipe(pipe) {
> > - u32 cur_val = I915_READ(VLV_BLC_PWM_CTL(pipe));
> > + ctl2 = I915_READ(VLV_BLC_PWM_CTL2(pipe));
> > + panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965;
> >
> > - /* Skip if the modulation freq is already set */
> > - if (cur_val & ~BACKLIGHT_DUTY_CYCLE_MASK)
> > - continue;
> > + ctl = I915_READ(VLV_BLC_PWM_CTL(pipe));
> >
> > - cur_val &= BACKLIGHT_DUTY_CYCLE_MASK;
> > - I915_WRITE(VLV_BLC_PWM_CTL(pipe), (0xf42 << 16) |
> > - cur_val);
> > + /* Skip if the modulation freq is already set */
> > + if ((ctl & ~BACKLIGHT_DUTY_CYCLE_MASK) == 0) {
> > + ctl &= BACKLIGHT_DUTY_CYCLE_MASK;
> > + ctl |= (0xf42 << 16);
> > + I915_WRITE(VLV_BLC_PWM_CTL(pipe), ctl);
> > }
> >
> > - ctl2 = I915_READ(VLV_BLC_PWM_CTL2(PIPE_A));
> > - panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965;
> > -
> > - ctl = I915_READ(VLV_BLC_PWM_CTL(PIPE_A));
> > panel->backlight.max = ctl >> 16;
> > if (!panel->backlight.max)
> > return -ENODEV;
> >
> > panel->backlight.min = get_backlight_min_vbt(connector);
> >
> > - val = _vlv_get_backlight(dev, PIPE_A);
> > + val = _vlv_get_backlight(dev, pipe);
> > panel->backlight.level =
> intel_panel_compute_brightness(connector, val);
> >
> > panel->backlight.enabled = (ctl2 & BLM_PWM_ENABLE) &&
> > --
> > 2.0.3
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
