* [PATCH] drm/i915: Correctly read backlight PWM for pipe B on vlv/chv @ 2014-07-29 12:44 rafael.barbalho 2014-07-29 13:13 ` Ville Syrjälä 0 siblings, 1 reply; 4+ messages in thread From: rafael.barbalho @ 2014-07-29 12:44 UTC (permalink / raw) To: intel-gfx From: Rafael Barbalho <rafael.barbalho@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.syrjala@linux.intel.com> Signed-off-by: Rafael Barbalho <rafael.barbalho@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); 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Correctly read backlight PWM for pipe B on vlv/chv 2014-07-29 12:44 [PATCH] drm/i915: Correctly read backlight PWM for pipe B on vlv/chv rafael.barbalho @ 2014-07-29 13:13 ` Ville Syrjälä 2014-07-29 13:38 ` Barbalho, Rafael 0 siblings, 1 reply; 4+ messages in thread From: Ville Syrjälä @ 2014-07-29 13:13 UTC (permalink / raw) To: rafael.barbalho; +Cc: intel-gfx On Tue, Jul 29, 2014 at 01:44:40PM +0100, rafael.barbalho@intel.com wrote: > From: Rafael Barbalho <rafael.barbalho@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.syrjala@linux.intel.com> > Signed-off-by: Rafael Barbalho <rafael.barbalho@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. > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Correctly read backlight PWM for pipe B on vlv/chv 2014-07-29 13:13 ` Ville Syrjälä @ 2014-07-29 13:38 ` Barbalho, Rafael 2014-07-29 14:02 ` Barbalho, Rafael 0 siblings, 1 reply; 4+ messages in thread From: Barbalho, Rafael @ 2014-07-29 13:38 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@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.barbalho@intel.com wrote: > > From: Rafael Barbalho <rafael.barbalho@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.syrjala@linux.intel.com> > > Signed-off-by: Rafael Barbalho <rafael.barbalho@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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Correctly read backlight PWM for pipe B on vlv/chv 2014-07-29 13:38 ` Barbalho, Rafael @ 2014-07-29 14:02 ` Barbalho, Rafael 0 siblings, 0 replies; 4+ messages in thread From: Barbalho, Rafael @ 2014-07-29 14:02 UTC (permalink / raw) To: Barbalho, Rafael, Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf > Of Barbalho, Rafael > Sent: Tuesday, July 29, 2014 2:38 PM > To: Ville Syrjälä > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Correctly read backlight PWM for > pipe B on vlv/chv > Importance: High > > > -----Original Message----- > > From: Ville Syrjälä [mailto:ville.syrjala@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.barbalho@intel.com > wrote: > > > From: Rafael Barbalho <rafael.barbalho@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.syrjala@linux.intel.com> > > > Signed-off-by: Rafael Barbalho <rafael.barbalho@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. > Thinking about it I need to still keep the initial for_each_pipe loop to very subtly initialise both pipe A & B to a known PWM modulation frequency, which would explain why setting the backlight brightness works on pipe B. However, inheriting it from the bios is not working, which is what I am seeing. Let me rejig the patch and I'll send out a new version. > > > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-07-29 14:02 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-29 12:44 [PATCH] drm/i915: Correctly read backlight PWM for pipe B on vlv/chv rafael.barbalho 2014-07-29 13:13 ` Ville Syrjälä 2014-07-29 13:38 ` Barbalho, Rafael 2014-07-29 14:02 ` Barbalho, Rafael
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.