Re: [Intel-gfx] [PATCH v5] drm/i915/bxt: eDP Panel Power sequencing
Looks good to me.
Reviewed-by: Sonika Jindal sonika.jin...@intel.com
-Original Message-
From: Kannan, Vandana
Sent: Thursday, June 18, 2015 11:01 AM
To: intel-gfx@lists.freedesktop.org
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 vandana.kan...@intel.com
Signed-off-by: A.Sunil Kamath sunil.kam...@intel.com
---
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 (0x 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,
Re: [Intel-gfx] [PATCH v5] drm/i915/bxt: eDP Panel Power sequencing
On Thu, Jun 18, 2015 at 06:08:46AM +, Jindal, Sonika wrote:
Looks good to me.
Reviewed-by: Sonika Jindal sonika.jin...@intel.com
-Original Message-
From: Kannan, Vandana
Sent: Thursday, June 18, 2015 11:01 AM
To: intel-gfx@lists.freedesktop.org
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 vandana.kan...@intel.com
Signed-off-by: A.Sunil Kamath sunil.kam...@intel.com
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
JaniSonika.
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 (0x 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_STATUS20xc7300
+#define _BXT_PP_CONTROL2 0xc7304
+#define _BXT_PP_ON_DELAYS2 0xc7308
+#define _BXT_PP_OFF_DELAYS20xc730c
+
+#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
[Intel-gfx] [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 vandana.kan...@intel.com
Signed-off-by: A.Sunil Kamath sunil.kam...@intel.com
---
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 (0x 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_STATUS20xc7300
+#define _BXT_PP_CONTROL2 0xc7304
+#define _BXT_PP_ON_DELAYS2 0xc7308
+#define _BXT_PP_OFF_DELAYS20xc730c
+
+#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
