When I submitted the PPS patch in April, I got an input from Jani to not
make changes in i915_suspend.c as it was to become obsolete. Below mail
for your reference.
Jani,
Does your initial comment still hold good?
On Thu, 30 Apr 2015, Vandana Kannan <vandana.kannan@xxxxxxxxx> wrote:
> Changes based on future platform readiness patches related to
> HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT
>
> Signed-off-by: Vandana Kannan <vandana.kannan@xxxxxxxxx>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
> drivers/gpu/drm/i915/intel_dp.c | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c
> b/drivers/gpu/drm/i915/i915_suspend.c
> index cf67f82..e91d637 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
> dev_priv->regfile.saveLVDS = I915_READ(LVDS);
>
> /* Panel power sequencer */
> - if (HAS_PCH_SPLIT(dev)) {
> + if (!HAS_GMCH_DISPLAY(dev)) {
> dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
> dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
> dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
> @@ -79,7 +79,7 @@ static void i915_restore_display(struct drm_device
*dev)
> I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
>
> /* Panel power sequencer */
> - if (HAS_PCH_SPLIT(dev)) {
> + if (!HAS_GMCH_DISPLAY(dev)) {
> I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
> I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
> I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
I don't think you should touch i915_suspend.c at all. We're trying to
get rid of this blind register save/restore, and make everything work in
the encoder/connector code. Do note that we already skip this for
vlv/chv power sequencer registers.
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c index 937ba31..68e10c1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -559,7 +559,7 @@ 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 (!HAS_GMCH_DISPLAY(dev))
> return PCH_PP_CONTROL;
> else
> return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> @@ -569,7 +569,7 @@ 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 (!HAS_GMCH_DISPLAY(dev))
> return PCH_PP_STATUS;
> else
> return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> @@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct
drm_device *dev,
> if (final->t11_t12 != 0)
> return;
>
> - if (HAS_PCH_SPLIT(dev)) {
> + if (!HAS_GMCH_DISPLAY(dev)) {
> pp_ctrl_reg = PCH_PP_CONTROL;
> pp_on_reg = PCH_PP_ON_DELAYS;
> pp_off_reg = PCH_PP_OFF_DELAYS;
> @@ -5063,7 +5063,7 @@
> intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>
> lockdep_assert_held(&dev_priv->pps_mutex);
>
> - if (HAS_PCH_SPLIT(dev)) {
> + if (!HAS_GMCH_DISPLAY(dev)) {
> pp_on_reg = PCH_PP_ON_DELAYS;
> pp_off_reg = PCH_PP_OFF_DELAYS;
> pp_div_reg = PCH_PP_DIVISOR;
For the rest, I'd like you to do what I suggested in my reply to patch
3, i.e. add a new if (IS_BROXTON(dev)) at the top of each of these, with
BXT_PP_WHATEVER(0) as the register. Later on, we can add code to choose
the power sequencer, a bit similar to what vlv/chv do now (except there
it's based on pipe).
BR,
Jani.
> --
> 2.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
- Vandana
On 12/31/2015 4:26 AM, Matt Roper wrote:
Broxton-specific panel power sequencing was added in commit
commit b0a08bec96318be54db97c3f0b9e37b52561f9ea
Author: Vandana Kannan <vandana.kannan@xxxxxxxxx>
Date: Thu Jun 18 11:00:55 2015 +0530
drm/i915/bxt: eDP Panel Power sequencing
As noted in that commit, Broxton has two sets of registers (and we're
supposed to read which to use from the VBT, although we're punting on
that part at the moment) and also doesn't have a divisor register.
Although we take the BXT-specific details into account at initial system
init, we fail to save/restore the appropriate registers around system
suspend/resume, which leads to unclaimed register warnings at the
moment, and may also fail to save the proper registers once we start
actually paying attention to the VBT.
Cc: Vandana Kannan <vandana.kannan@xxxxxxxxx>
Cc: drm-intel-fixes@xxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_suspend.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index bf582fe..61bec8e 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -44,7 +44,18 @@ static void i915_save_display(struct drm_device *dev)
dev_priv->regfile.saveLVDS = I915_READ(LVDS);
/* Panel power sequencer */
- 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.
+ *
+ * There's also no divisor register on Broxton.
+ */
+ dev_priv->regfile.savePP_CONTROL = I915_READ(BXT_PP_CONTROL(0));
+ dev_priv->regfile.savePP_ON_DELAYS = I915_READ(BXT_PP_ON_DELAYS(0));
+ dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(BXT_PP_OFF_DELAYS(0));
+ } else if (HAS_PCH_SPLIT(dev)) {
dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
@@ -83,7 +94,16 @@ static void i915_restore_display(struct drm_device *dev)
I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
/* Panel power sequencer */
- 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
+ */
+ I915_WRITE(BXT_PP_ON_DELAYS(0), dev_priv->regfile.savePP_ON_DELAYS);
+ I915_WRITE(BXT_PP_OFF_DELAYS(0), dev_priv->regfile.savePP_OFF_DELAYS);
+ I915_WRITE(BXT_PP_CONTROL(0), dev_priv->regfile.savePP_CONTROL);
+ } else if (HAS_PCH_SPLIT(dev)) {
I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
