On Thu, Jun 18, 2015 at 06:08:46AM +0000, Jindal, Sonika wrote:
> Looks good to me.
> Reviewed-by: Sonika Jindal <[email protected]>
>
> > -----Original Message-----
> > From: Kannan, Vandana
> > Sent: Thursday, June 18, 2015 11:01 AM
> > To: [email protected]
> > Cc: Jindal, Sonika; Kannan, Vandana
> > Subject: [PATCH v5] drm/i915/bxt: eDP Panel Power sequencing
> >
> > Changes for BXT - added a IS_BROXTON check to use the macro related to
> > PPS registers for BXT.
> > BXT does not have PP_DIV register. Making changes to handle this.
> > Second set of PPS registers have been defined but will be used when VBT
> > provides a selection between the 2 sets of registers.
> >
> > v2:
> > [Jani] Added 2nd set of PPS registers and the macro Jani's review comments
> > - remove reference in i915_suspend.c
> > - Use BXT PP macro
> > Squashing all PPS related patches into one.
> >
> > v3: Jani's review comments addressed
> > - Use pp_ctl instead of pp
> > - ironlake_get_pp_control() is not required for BXT
> > - correct the use of && in the print statement
> > - drop the shift in the print statement
> >
> > v4: Jani's comments
> > - modify ironlake_get_pp_control() - dont set unlock key for bxt
> >
> > v5: Sonika's comments addressed
> > - check alignment
> > - move pp_ctrl_reg write (after ironlake_get_pp_control())
> > to !IS_BROXTON case.
> > - check before subtracting 1 for t11_t12
> >
> > Signed-off-by: Vandana Kannan <[email protected]>
> > Signed-off-by: A.Sunil Kamath <[email protected]>
When resending patches please always include reviewers from previous
rounds in the Cc: list of the patch. Otherwise it's very likely that
they'll miss the patch in the overall mail flood on intel-gfx. Adding
Jani&Sonika.
Queued for -next (assuming Jani's ok too), thanks for the patch.
-Daniel
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
> > drivers/gpu/drm/i915/intel_dp.c | 82
> > ++++++++++++++++++++++++++++++++---------
> > 2 files changed, 78 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 0b979ad..27eb49e 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6391,6 +6391,8 @@ enum skl_disp_power_wells {
> > #define PCH_PP_CONTROL 0xc7204
> > #define PANEL_UNLOCK_REGS (0xabcd << 16)
> > #define PANEL_UNLOCK_MASK (0xffff << 16)
> > +#define BXT_POWER_CYCLE_DELAY_MASK (0x1f0)
> > +#define BXT_POWER_CYCLE_DELAY_SHIFT 4
> > #define EDP_FORCE_VDD (1 << 3)
> > #define EDP_BLC_ENABLE (1 << 2)
> > #define PANEL_POWER_RESET (1 << 1)
> > @@ -6419,6 +6421,17 @@ enum skl_disp_power_wells {
> > #define PANEL_POWER_CYCLE_DELAY_MASK (0x1f)
> > #define PANEL_POWER_CYCLE_DELAY_SHIFT 0
> >
> > +/* BXT PPS changes - 2nd set of PPS registers */
> > +#define _BXT_PP_STATUS2 0xc7300
> > +#define _BXT_PP_CONTROL2 0xc7304
> > +#define _BXT_PP_ON_DELAYS2 0xc7308
> > +#define _BXT_PP_OFF_DELAYS2 0xc730c
> > +
> > +#define BXT_PP_STATUS(n) ((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
> > +#define BXT_PP_CONTROL(n) ((!n) ? PCH_PP_CONTROL :
> > _BXT_PP_CONTROL2)
> > +#define BXT_PP_ON_DELAYS(n) ((!n) ? PCH_PP_ON_DELAYS :
> > _BXT_PP_ON_DELAYS2)
> > +#define BXT_PP_OFF_DELAYS(n) ((!n) ? PCH_PP_OFF_DELAYS :
> > _BXT_PP_OFF_DELAYS2)
> > +
> > #define PCH_DP_B 0xe4100
> > #define PCH_DPB_AUX_CH_CTL 0xe4110
> > #define PCH_DPB_AUX_CH_DATA1 0xe4114
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c index f52eef1..c2145e7 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -567,7 +567,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp) {
> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >
> > - if (HAS_PCH_SPLIT(dev))
> > + if (IS_BROXTON(dev))
> > + return BXT_PP_CONTROL(0);
> > + else if (HAS_PCH_SPLIT(dev))
> > return PCH_PP_CONTROL;
> > else
> > return
> > VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> > @@ -577,7 +579,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp) {
> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >
> > - if (HAS_PCH_SPLIT(dev))
> > + if (IS_BROXTON(dev))
> > + return BXT_PP_STATUS(0);
> > + else if (HAS_PCH_SPLIT(dev))
> > return PCH_PP_STATUS;
> > else
> > return
> > VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> > @@ -1702,8 +1706,10 @@ static u32 ironlake_get_pp_control(struct
> > intel_dp *intel_dp)
> > lockdep_assert_held(&dev_priv->pps_mutex);
> >
> > control = I915_READ(_pp_ctrl_reg(intel_dp));
> > - control &= ~PANEL_UNLOCK_MASK;
> > - control |= PANEL_UNLOCK_REGS;
> > + if (!IS_BROXTON(dev)) {
> > + control &= ~PANEL_UNLOCK_MASK;
> > + control |= PANEL_UNLOCK_REGS;
> > + }
> > return control;
> > }
> >
> > @@ -5092,8 +5098,8 @@ intel_dp_init_panel_power_sequencer(struct
> > drm_device *dev,
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct edp_power_seq cur, vbt, spec,
> > *final = &intel_dp->pps_delays;
> > - u32 pp_on, pp_off, pp_div, pp;
> > - int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg;
> > + u32 pp_on, pp_off, pp_div = 0, pp_ctl = 0;
> > + int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
> >
> > lockdep_assert_held(&dev_priv->pps_mutex);
> >
> > @@ -5101,7 +5107,16 @@ intel_dp_init_panel_power_sequencer(struct
> > drm_device *dev,
> > if (final->t11_t12 != 0)
> > return;
> >
> > - if (HAS_PCH_SPLIT(dev)) {
> > + if (IS_BROXTON(dev)) {
> > + /*
> > + * TODO: BXT has 2 sets of PPS registers.
> > + * Correct Register for Broxton need to be identified
> > + * using VBT. hardcoding for now
> > + */
> > + pp_ctrl_reg = BXT_PP_CONTROL(0);
> > + pp_on_reg = BXT_PP_ON_DELAYS(0);
> > + pp_off_reg = BXT_PP_OFF_DELAYS(0);
> > + } else if (HAS_PCH_SPLIT(dev)) {
> > pp_ctrl_reg = PCH_PP_CONTROL;
> > pp_on_reg = PCH_PP_ON_DELAYS;
> > pp_off_reg = PCH_PP_OFF_DELAYS;
> > @@ -5117,12 +5132,14 @@ intel_dp_init_panel_power_sequencer(struct
> > drm_device *dev,
> >
> > /* Workaround: Need to write PP_CONTROL with the unlock key as
> > * the very first thing. */
> > - pp = ironlake_get_pp_control(intel_dp);
> > - I915_WRITE(pp_ctrl_reg, pp);
> > + pp_ctl = ironlake_get_pp_control(intel_dp);
> >
> > pp_on = I915_READ(pp_on_reg);
> > pp_off = I915_READ(pp_off_reg);
> > - pp_div = I915_READ(pp_div_reg);
> > + if (!IS_BROXTON(dev)) {
> > + I915_WRITE(pp_ctrl_reg, pp_ctl);
> > + pp_div = I915_READ(pp_div_reg);
> > + }
> >
> > /* Pull timing values out of registers */
> > cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >> @@ -
> > 5137,8 +5154,17 @@ intel_dp_init_panel_power_sequencer(struct
> > drm_device *dev,
> > cur.t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >>
> > PANEL_POWER_DOWN_DELAY_SHIFT;
> >
> > - cur.t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
> > + if (IS_BROXTON(dev)) {
> > + u16 tmp = (pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
> > + BXT_POWER_CYCLE_DELAY_SHIFT;
> > + if (tmp > 0)
> > + cur.t11_t12 = (tmp - 1) * 1000;
> > + else
> > + cur.t11_t12 = 0;
> > + } else {
> > + cur.t11_t12 = ((pp_div &
> > PANEL_POWER_CYCLE_DELAY_MASK) >>
> > PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
> > + }
> >
> > DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
> > cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12); @@
> > -5195,13
> > +5221,23 @@ intel_dp_init_panel_power_sequencer_registers(struct
> > drm_device *dev,
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > u32 pp_on, pp_off, pp_div, port_sel = 0;
> > int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) :
> > intel_hrawclk(dev);
> > - int pp_on_reg, pp_off_reg, pp_div_reg;
> > + int pp_on_reg, pp_off_reg, pp_div_reg = 0, pp_ctrl_reg;
> > enum port port = dp_to_dig_port(intel_dp)->port;
> > const struct edp_power_seq *seq = &intel_dp->pps_delays;
> >
> > lockdep_assert_held(&dev_priv->pps_mutex);
> >
> > - if (HAS_PCH_SPLIT(dev)) {
> > + if (IS_BROXTON(dev)) {
> > + /*
> > + * TODO: BXT has 2 sets of PPS registers.
> > + * Correct Register for Broxton need to be identified
> > + * using VBT. hardcoding for now
> > + */
> > + pp_ctrl_reg = BXT_PP_CONTROL(0);
> > + pp_on_reg = BXT_PP_ON_DELAYS(0);
> > + pp_off_reg = BXT_PP_OFF_DELAYS(0);
> > +
> > + } else if (HAS_PCH_SPLIT(dev)) {
> > pp_on_reg = PCH_PP_ON_DELAYS;
> > pp_off_reg = PCH_PP_OFF_DELAYS;
> > pp_div_reg = PCH_PP_DIVISOR;
> > @@ -5227,9 +5263,16 @@
> > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> > (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT);
> > /* Compute the divisor for the pp clock, simply match the Bspec
> > * formula. */
> > - pp_div = ((100 * div)/2 - 1) << PP_REFERENCE_DIVIDER_SHIFT;
> > - pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
> > - << PANEL_POWER_CYCLE_DELAY_SHIFT);
> > + if (IS_BROXTON(dev)) {
> > + pp_div = I915_READ(pp_ctrl_reg);
> > + pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK;
> > + pp_div |= (DIV_ROUND_UP((seq->t11_t12 + 1), 1000)
> > + << BXT_POWER_CYCLE_DELAY_SHIFT);
> > + } else {
> > + pp_div = ((100 * div)/2 - 1) <<
> > PP_REFERENCE_DIVIDER_SHIFT;
> > + pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
> > + << PANEL_POWER_CYCLE_DELAY_SHIFT);
> > + }
> >
> > /* Haswell doesn't have any port selection bits for the panel
> > * power sequencer any more. */
> > @@ -5246,11 +5289,16 @@
> > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> >
> > I915_WRITE(pp_on_reg, pp_on);
> > I915_WRITE(pp_off_reg, pp_off);
> > - I915_WRITE(pp_div_reg, pp_div);
> > + if (IS_BROXTON(dev))
> > + I915_WRITE(pp_ctrl_reg, pp_div);
> > + else
> > + I915_WRITE(pp_div_reg, pp_div);
> >
> > DRM_DEBUG_KMS("panel power sequencer register settings:
> > PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n",
> > I915_READ(pp_on_reg),
> > I915_READ(pp_off_reg),
> > + IS_BROXTON(dev) ?
> > + (I915_READ(pp_ctrl_reg) &
> > BXT_POWER_CYCLE_DELAY_MASK) :
> > I915_READ(pp_div_reg));
> > }
> >
> > --
> > 2.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
