* [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.