On Tue, Dec 20, 2016 at 06:51:17PM +0200, [email protected] wrote:
> From: Ville Syrjälä <[email protected]>
>
> Apparently some VLV BIOSen like to leave the VDD force bit enabled
> even for power seqeuncers that aren't properly hooked up to any
> port. That will result in a imbalance in the AUX power domain
> refcount when we stat to use said power sequencer as edp_panel_vdd_on()
> will not grab the power domain reference if it sees that the VDD is
> already on.
>
> To fix this let's make sure we turn off the VDD force bit when we
> initialize the power sequencer registers. That is, unless it's
> being done from the init path since there we are actually
> initializing the registers for the current power sequencer and
> we don't want to turn VDD off needlessly as that would require
> waiting for the power cycle delay before we turn it back on.
>
> This fixes the following kind of warnings:
> WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455
> intel_display_power_put+0x13a/0x170 [i915]()
> WARN_ON(!power_domains->domain_use_count[domain])
> ...
>
> Cc: [email protected]
> Cc: Matwey V. Kornilov <[email protected]>
> Tested-by: Matwey V. Kornilov <[email protected]>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695
> Signed-off-by: Ville Syrjälä <[email protected]>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 42
> ++++++++++++++++++++++++++++++++++-------
> 1 file changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 66b5bc80b781..4139122704b3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device
> *dev,
> struct intel_dp *intel_dp);
> static void
> intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> - struct intel_dp *intel_dp);
> + struct intel_dp *intel_dp,
> + bool force_disable_vdd);
> static void
> intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp);
>
> @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
>
> /* init power sequencer on this pipe and port */
> intel_dp_init_panel_power_sequencer(dev, intel_dp);
> - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true);
>
> /*
> * Even vdd force doesn't work until we've made
> @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp)
> * Only the HW needs to be reprogrammed, the SW state is fixed and
> * has been setup during connector init.
> */
> - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false);
>
> return 0;
> }
> @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp
> *intel_dp)
> port_name(port), pipe_name(intel_dp->pps_pipe));
>
> intel_dp_init_panel_power_sequencer(dev, intel_dp);
> - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false);
> }
>
> void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct
> intel_dp *intel_dp)
>
> /* init power sequencer on this pipe and port */
> intel_dp_init_panel_power_sequencer(dev, intel_dp);
> - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true);
> }
>
> static void vlv_pre_enable_dp(struct intel_encoder *encoder,
> @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device
> *dev,
>
> static void
> intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> - struct intel_dp *intel_dp)
> + struct intel_dp *intel_dp,
> + bool force_disable_vdd)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> u32 pp_on, pp_off, pp_div, port_sel = 0;
> @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct
> drm_device *dev,
>
> intel_pps_get_registers(dev_priv, intel_dp, ®s);
>
> + /*
> + * One some VLV machines the BIOS can leave the VDD
On
> + * enabled even on power seqeuencers which aren't
> + * even hooked up to any port. This would mess up
Remove even here
> + * the power domain tracking the first time we pick
> + * one of these power sequencers for use since
> + * edp_panel_vdd_on() would notice that the VDD was
> + * already on and therefore wouldn't even grab the
And here.
> + * power domain reference. Disable VDD first to avoid
> + * this. This also avoids spuriously turning the VDD
> + * on as soon as the new power seqeuencer gets
> + * initialized.
> + */
> + if (force_disable_vdd) {
> + u32 pp = ironlake_get_pp_control(intel_dp);
> +
> + WARN(pp & PANEL_POWER_ON, "Panel power already on\n");
Wouldn't this just replace one warning with another? Or is this
a subset of the other warning?
> +
> + if (pp & EDP_FORCE_VDD)
> + DRM_DEBUG_KMS("VDD already on, disabling first\n");
> +
> + pp &= ~EDP_FORCE_VDD;
> +
> + I915_WRITE(regs.pp_ctrl, pp);
> + }
> +
> pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) |
> (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT);
> pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) |
> @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev,
> vlv_initial_power_sequencer_setup(intel_dp);
> } else {
> intel_dp_init_panel_power_sequencer(dev, intel_dp);
> - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
> false);
> }
> }
>
> --
> 2.10.2
Kind regards, David
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
