| Message ID | 1491506163-14587-6-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 06, 2017 at 12:15:02PM -0700, Rodrigo Vivi wrote: > As for BXT, PP_DIVISOR was removed from CNP PCH and power > cycle delay has been moved to PP_CONTROL. > > Cc: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index b38cba7..da111cb 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -788,7 +788,7 @@ static void intel_pps_get_registers(struct drm_i915_private *dev_priv, > regs->pp_stat = PP_STATUS(pps_idx); > regs->pp_on = PP_ON_DELAYS(pps_idx); > regs->pp_off = PP_OFF_DELAYS(pps_idx); > - if (!IS_GEN9_LP(dev_priv)) > + if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv)) GEN >= 10 all over might be more future proof. > regs->pp_div = PP_DIVISOR(pps_idx); > } > > @@ -5198,7 +5198,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > > pp_on = I915_READ(regs.pp_on); > pp_off = I915_READ(regs.pp_off); > - if (!IS_GEN9_LP(dev_priv)) { > + if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv)) { > I915_WRITE(regs.pp_ctrl, pp_ctl); Slightly unrelated, but I wonder what this write is doing in the BXT+ branch. I'm thinking it should either be done unconditionally, or we should just nuke it since I think Imre's early pps unlock thing should have already done it if needed, I think. > pp_div = I915_READ(regs.pp_div); > } > @@ -5216,7 +5216,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > seq->t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >> > PANEL_POWER_DOWN_DELAY_SHIFT; > > - if (IS_GEN9_LP(dev_priv)) { > + if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) { > u16 tmp = (pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >> > BXT_POWER_CYCLE_DELAY_SHIFT; > if (tmp > 0) > @@ -5373,7 +5373,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT); > /* Compute the divisor for the pp clock, simply match the Bspec > * formula. */ > - if (IS_GEN9_LP(dev_priv)) { > + if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) { > pp_div = I915_READ(regs.pp_ctrl); > pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK; > pp_div |= (DIV_ROUND_UP((seq->t11_t12 + 1), 1000) > @@ -5407,7 +5407,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n", > I915_READ(regs.pp_on), > I915_READ(regs.pp_off), > - IS_GEN9_LP(dev_priv) ? > + (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) ? > (I915_READ(regs.pp_ctrl) & BXT_POWER_CYCLE_DELAY_MASK) : > I915_READ(regs.pp_div)); > } > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 2017-04-07 at 17:48 +0300, Ville Syrjälä wrote: > On Thu, Apr 06, 2017 at 12:15:02PM -0700, Rodrigo Vivi wrote: > > As for BXT, PP_DIVISOR was removed from CNP PCH and power > > cycle delay has been moved to PP_CONTROL. > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index b38cba7..da111cb 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -788,7 +788,7 @@ static void intel_pps_get_registers(struct drm_i915_private *dev_priv, > > regs->pp_stat = PP_STATUS(pps_idx); > > regs->pp_on = PP_ON_DELAYS(pps_idx); > > regs->pp_off = PP_OFF_DELAYS(pps_idx); > > - if (!IS_GEN9_LP(dev_priv)) > > + if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv)) > > GEN >= 10 all over might be more future proof. True, but I didn't want to loose the track that this part is on the PCH. for the core.... Could we let it like this and in the future if we decide that this is the case we change?! > > > regs->pp_div = PP_DIVISOR(pps_idx); > > } > > > > @@ -5198,7 +5198,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > > > > pp_on = I915_READ(regs.pp_on); > > pp_off = I915_READ(regs.pp_off); > > - if (!IS_GEN9_LP(dev_priv)) { > > + if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv)) { > > I915_WRITE(regs.pp_ctrl, pp_ctl); > > Slightly unrelated, but I wonder what this write is doing in the BXT+ > branch. I'm thinking it should either be done unconditionally, or we > should just nuke it since I think Imre's early pps unlock thing should > have already done it if needed, I think. > > > pp_div = I915_READ(regs.pp_div); > > } > > @@ -5216,7 +5216,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > > seq->t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >> > > PANEL_POWER_DOWN_DELAY_SHIFT; > > > > - if (IS_GEN9_LP(dev_priv)) { > > + if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) { > > u16 tmp = (pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >> > > BXT_POWER_CYCLE_DELAY_SHIFT; > > if (tmp > 0) > > @@ -5373,7 +5373,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > > (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT); > > /* Compute the divisor for the pp clock, simply match the Bspec > > * formula. */ > > - if (IS_GEN9_LP(dev_priv)) { > > + if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) { > > pp_div = I915_READ(regs.pp_ctrl); > > pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK; > > pp_div |= (DIV_ROUND_UP((seq->t11_t12 + 1), 1000) > > @@ -5407,7 +5407,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > > DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n", > > I915_READ(regs.pp_on), > > I915_READ(regs.pp_off), > > - IS_GEN9_LP(dev_priv) ? > > + (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) ? > > (I915_READ(regs.pp_ctrl) & BXT_POWER_CYCLE_DELAY_MASK) : > > I915_READ(regs.pp_div)); > > } > > -- > > 1.9.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Thu, Apr 13, 2017 at 11:48:41PM +0000, Vivi, Rodrigo wrote: > On Fri, 2017-04-07 at 17:48 +0300, Ville Syrjälä wrote: > > On Thu, Apr 06, 2017 at 12:15:02PM -0700, Rodrigo Vivi wrote: > > > As for BXT, PP_DIVISOR was removed from CNP PCH and power > > > cycle delay has been moved to PP_CONTROL. > > > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_dp.c | 10 +++++----- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > index b38cba7..da111cb 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -788,7 +788,7 @@ static void intel_pps_get_registers(struct drm_i915_private *dev_priv, > > > regs->pp_stat = PP_STATUS(pps_idx); > > > regs->pp_on = PP_ON_DELAYS(pps_idx); > > > regs->pp_off = PP_OFF_DELAYS(pps_idx); > > > - if (!IS_GEN9_LP(dev_priv)) > > > + if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv)) > > > > GEN >= 10 all over might be more future proof. > > True, but I didn't want to loose the track that this part is on the PCH. > for the core.... > > Could we let it like this and in the future if we decide that this is > the case we change?! Sorry, I dropped this discussion through the cracks somehow. IIRC the patch didn't seem to have any real issues, so I think we can go with it as is, so Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > regs->pp_div = PP_DIVISOR(pps_idx); > > > } > > > > > > @@ -5198,7 +5198,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > > > > > > pp_on = I915_READ(regs.pp_on); > > > pp_off = I915_READ(regs.pp_off); > > > - if (!IS_GEN9_LP(dev_priv)) { > > > + if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv)) { > > > I915_WRITE(regs.pp_ctrl, pp_ctl); > > > > Slightly unrelated, but I wonder what this write is doing in the BXT+ > > branch. I'm thinking it should either be done unconditionally, or we > > should just nuke it since I think Imre's early pps unlock thing should > > have already done it if needed, I think. > > > > > pp_div = I915_READ(regs.pp_div); > > > } > > > @@ -5216,7 +5216,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > > > seq->t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >> > > > PANEL_POWER_DOWN_DELAY_SHIFT; > > > > > > - if (IS_GEN9_LP(dev_priv)) { > > > + if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) { > > > u16 tmp = (pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >> > > > BXT_POWER_CYCLE_DELAY_SHIFT; > > > if (tmp > 0) > > > @@ -5373,7 +5373,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > > > (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT); > > > /* Compute the divisor for the pp clock, simply match the Bspec > > > * formula. */ > > > - if (IS_GEN9_LP(dev_priv)) { > > > + if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) { > > > pp_div = I915_READ(regs.pp_ctrl); > > > pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK; > > > pp_div |= (DIV_ROUND_UP((seq->t11_t12 + 1), 1000) > > > @@ -5407,7 +5407,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) > > > DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n", > > > I915_READ(regs.pp_on), > > > I915_READ(regs.pp_off), > > > - IS_GEN9_LP(dev_priv) ? > > > + (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) ? > > > (I915_READ(regs.pp_ctrl) & BXT_POWER_CYCLE_DELAY_MASK) : > > > I915_READ(regs.pp_div)); > > > } > > > -- > > > 1.9.1 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > >
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index b38cba7..da111cb 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -788,7 +788,7 @@ static void intel_pps_get_registers(struct drm_i915_private *dev_priv, regs->pp_stat = PP_STATUS(pps_idx); regs->pp_on = PP_ON_DELAYS(pps_idx); regs->pp_off = PP_OFF_DELAYS(pps_idx); - if (!IS_GEN9_LP(dev_priv)) + if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv)) regs->pp_div = PP_DIVISOR(pps_idx); } @@ -5198,7 +5198,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) pp_on = I915_READ(regs.pp_on); pp_off = I915_READ(regs.pp_off); - if (!IS_GEN9_LP(dev_priv)) { + if (!IS_GEN9_LP(dev_priv) && !HAS_PCH_CNP(dev_priv)) { I915_WRITE(regs.pp_ctrl, pp_ctl); pp_div = I915_READ(regs.pp_div); } @@ -5216,7 +5216,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) seq->t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >> PANEL_POWER_DOWN_DELAY_SHIFT; - if (IS_GEN9_LP(dev_priv)) { + if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) { u16 tmp = (pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >> BXT_POWER_CYCLE_DELAY_SHIFT; if (tmp > 0) @@ -5373,7 +5373,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT); /* Compute the divisor for the pp clock, simply match the Bspec * formula. */ - if (IS_GEN9_LP(dev_priv)) { + if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) { pp_div = I915_READ(regs.pp_ctrl); pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK; pp_div |= (DIV_ROUND_UP((seq->t11_t12 + 1), 1000) @@ -5407,7 +5407,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n", I915_READ(regs.pp_on), I915_READ(regs.pp_off), - IS_GEN9_LP(dev_priv) ? + (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) ? (I915_READ(regs.pp_ctrl) & BXT_POWER_CYCLE_DELAY_MASK) : I915_READ(regs.pp_div)); }
As for BXT, PP_DIVISOR was removed from CNP PCH and power cycle delay has been moved to PP_CONTROL. Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)