Re: [PATCH 05/17] drm/i915: Don't initialize power seqeuencer delays more than once

Imre Deak
2014-10-27T14:43:14Z

On Thu, 2014-10-16 at 21:27 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Since we read the current power seqeuncer delays from the registers
> (as well as looking at the vbt and spec values) we may end up
> corrupting delays we already initialized when we switch to another
> pipe and the power seqeuncer there has different values currently
> in the registers.
>
> So make sure we only initialize the delays once even if
> intel_dp_init_panel_power_sequencer() gets called multiple times.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7a10464..9a1295d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4756,6 +4756,10 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>
> lockdep_assert_held(&dev_priv->pps_mutex);
>
> + /* already initialized? */
> + if (final->t11_t12 != 0)
> + return;
> +

I wonder if some other place depends on the PP_CONTROL unlocking done
here. At least intel_dp_init_panel_power_sequencer_registers() doesn't
do the unlocking when writing to other PP regs. Maybe the locking
mechanism has an effect only while power sequencing is active, so it
wouldn't matter but the comment in this function suggests that we need
to unlock as a first step. The VLV spec is unclear if unlocking is
needed.

> if (HAS_PCH_SPLIT(dev)) {
> pp_ctrl_reg = PCH_PP_CONTROL;
> pp_on_reg = PCH_PP_ON_DELAYS;