On 6/12/2015 3:57 PM, Vandana Kannan wrote:
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

Signed-off-by: Vandana Kannan <vandana.kan...@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kam...@intel.com>
Cc: Jani Nikula <jani.nik...@linux.intel.com>
---
  drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
  drivers/gpu/drm/i915/intel_dp.c | 76 ++++++++++++++++++++++++++++++++---------
  2 files changed, 72 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7213224..cc03b5b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6377,6 +6377,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)
@@ -6405,6 +6407,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
Not aligned?

+#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 15e8892..ad9cb0e9 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));
@@ -1701,8 +1705,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;
  }

@@ -5091,8 +5097,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);

@@ -5100,7 +5106,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;
@@ -5116,12 +5131,13 @@ 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);
+       I915_WRITE(pp_ctrl_reg, pp_ctl);
Since you are not doing anything in ironlake_get_pp_control, you can move this write to !IS_BROXTON below


        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))
+               pp_div = I915_READ(pp_div_reg);

        /* Pull timing values out of registers */
        cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
@@ -5136,7 +5152,11 @@ 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))
+               cur.t11_t12 = (((pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
+                       BXT_POWER_CYCLE_DELAY_SHIFT) - 1) * 1000;
Why a -1 here for broxton and not for other platforms?
Also, since 0 is also a valid value and cur.t11_t12 is u16, this -1 will result in undefined value.

Regards,
Sonika
+       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",
@@ -5194,13 +5214,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;
@@ -5226,9 +5256,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. */
@@ -5245,11 +5282,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));
  }


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to