Re: [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/17/2015 2:45 PM, Jindal, Sonika wrote:


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.kannan@xxxxxxxxx>
Signed-off-by: A.Sunil Kamath <sunil.kamath@xxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
---
  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

Agreed.

      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?
According to the BSpec, "The value should be programmed to (desired delay / 100 milliseconds) + 1". This -1 is to make up for that. This is mentioned for other platforms as well. So maybe we should make the change there as well ??
Also, since 0 is also a valid value and cur.t11_t12 is u16, this -1 will
result in undefined value.
Agreed. Making the change.

- Vandana

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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux