| Message ID | 1380006867-20771-1-git-send-email-chon.ming.lee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 24, 2013 at 03:14:27PM +0800, Chon Ming Lee wrote: > Without the DPIO cmnreset, the PLL fail to lock. This should have > done by BIOS. > > v2: Move this to intel_uncore_sanitize to allow it to get call during > resume path. (Daniel) > v3: Remove redundant write 0 to DPIO_CTL, and use DPIO_RESET instead of > just 0x1 (Ville) > Without BIOS, DPIO/render well/media well may still power gated. > Turn it off. > > Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++ > drivers/gpu/drm/i915/intel_uncore.c | 23 +++++++++++++++++++++++ > 2 files changed, 32 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c4f9bef..c02f893 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -361,6 +361,15 @@ > #define PUNIT_OPCODE_REG_READ 6 > #define PUNIT_OPCODE_REG_WRITE 7 > > +#define PUNIT_REG_PWRGT_CTRL 0x60 > +#define PUNIT_REG_PWRGT_STATUS 0x61 > +#define PUNIT_CLK_GATE 1 > +#define PUNIT_PWR_RESET 2 > +#define PUNIT_PWR_GATE 3 > +#define RENDER_PWRGT (PUNIT_PWR_GATE << 0) > +#define MEDIA_PWRGT (PUNIT_PWR_GATE << 2) > +#define DPIO_PWRGT (PUNIT_PWR_GATE << 6) Subsys 6 seems to be one of four TX lanes, and there's also the common lane subsys, and the disp2d is one as well. RX supposedly is not relevant for display PHY, not sure why it has subsys allocated too. Anyways my point would be that shouldn't we check all subsys ie render + media + disp2d + common lane + all tx lanes? And should we maybe power gate the RX lanes always as those shouldn't be needed for display? > + > #define PUNIT_REG_GPU_LFM 0xd3 > #define PUNIT_REG_GPU_FREQ_REQ 0xd4 > #define PUNIT_REG_GPU_FREQ_STS 0xd8 > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 8649f1c..6923b4d 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -276,10 +276,33 @@ static void intel_uncore_forcewake_reset(struct drm_device *dev) > > void intel_uncore_sanitize(struct drm_device *dev) > { > + struct drm_i915_private *dev_priv = dev->dev_private; > + u32 reg_val; > + > intel_uncore_forcewake_reset(dev); > > /* BIOS often leaves RC6 enabled, but disable it for hw init */ > intel_disable_gt_powersave(dev); > + > + /* Trigger DPIO CMN RESET and turn off power gate, require > + * especially in BIOS less system > + */ > + if (IS_VALLEYVIEW(dev)) { > + > + mutex_lock(&dev_priv->rps.hw_lock); > + reg_val = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS); > + > + if (reg_val & (RENDER_PWRGT | MEDIA_PWRGT | DPIO_PWRGT)) > + vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, 0x0); > + > + mutex_unlock(&dev_priv->rps.hw_lock); > + > + reg_val = I915_READ(DPIO_CTL); > + if (!(reg_val & DPIO_RESET)) { > + I915_WRITE(DPIO_CTL, DPIO_RESET); > + POSTING_READ(DPIO_CTL); > + } > + } > } > > /* > -- > 1.7.7.6 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 09/24 18:18, Ville Syrjälä wrote: > On Tue, Sep 24, 2013 at 03:14:27PM +0800, Chon Ming Lee wrote: > > Without the DPIO cmnreset, the PLL fail to lock. This should have > > done by BIOS. > > > > v2: Move this to intel_uncore_sanitize to allow it to get call during > > resume path. (Daniel) > > v3: Remove redundant write 0 to DPIO_CTL, and use DPIO_RESET instead of > > just 0x1 (Ville) > > Without BIOS, DPIO/render well/media well may still power gated. > > Turn it off. > > > > Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++ > > drivers/gpu/drm/i915/intel_uncore.c | 23 +++++++++++++++++++++++ > > 2 files changed, 32 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index c4f9bef..c02f893 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -361,6 +361,15 @@ > > #define PUNIT_OPCODE_REG_READ 6 > > #define PUNIT_OPCODE_REG_WRITE 7 > > > > +#define PUNIT_REG_PWRGT_CTRL 0x60 > > +#define PUNIT_REG_PWRGT_STATUS 0x61 > > +#define PUNIT_CLK_GATE 1 > > +#define PUNIT_PWR_RESET 2 > > +#define PUNIT_PWR_GATE 3 > > +#define RENDER_PWRGT (PUNIT_PWR_GATE << 0) > > +#define MEDIA_PWRGT (PUNIT_PWR_GATE << 2) > > +#define DPIO_PWRGT (PUNIT_PWR_GATE << 6) > > Subsys 6 seems to be one of four TX lanes, and there's also the common > lane subsys, and the disp2d is one as well. RX supposedly is not relevant > for display PHY, not sure why it has subsys allocated too. > > Anyways my point would be that shouldn't we check all subsys ie render + media + > disp2d + common lane + all tx lanes? > By default, the common lane + all tx lanes are not power gated during cold boot or system resume. Unless S0ix entry actually power gate it by driver, then, this will need to add to turn off it. > And should we maybe power gate the RX lanes always as those shouldn't be needed > for display? Yes, you are correct. I believe there should be another patch to do it, to enable power gate the VLV correctly for SOix entry or exit. > > > + > > #define PUNIT_REG_GPU_LFM 0xd3 > > #define PUNIT_REG_GPU_FREQ_REQ 0xd4 > > #define PUNIT_REG_GPU_FREQ_STS 0xd8 > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > index 8649f1c..6923b4d 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > @@ -276,10 +276,33 @@ static void intel_uncore_forcewake_reset(struct drm_device *dev) > > > > void intel_uncore_sanitize(struct drm_device *dev) > > { > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + u32 reg_val; > > + > > intel_uncore_forcewake_reset(dev); > > > > /* BIOS often leaves RC6 enabled, but disable it for hw init */ > > intel_disable_gt_powersave(dev); > > + > > + /* Trigger DPIO CMN RESET and turn off power gate, require > > + * especially in BIOS less system > > + */ > > + if (IS_VALLEYVIEW(dev)) { > > + > > + mutex_lock(&dev_priv->rps.hw_lock); > > + reg_val = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS); > > + > > + if (reg_val & (RENDER_PWRGT | MEDIA_PWRGT | DPIO_PWRGT)) > > + vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, 0x0); > > + > > + mutex_unlock(&dev_priv->rps.hw_lock); > > + > > + reg_val = I915_READ(DPIO_CTL); > > + if (!(reg_val & DPIO_RESET)) { > > + I915_WRITE(DPIO_CTL, DPIO_RESET); > > + POSTING_READ(DPIO_CTL); > > + } > > + } > > } > > > > /* > > -- > > 1.7.7.6 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC
On Wed, Sep 25, 2013 at 07:14:34AM +0800, Lee, Chon Ming wrote: > On 09/24 18:18, Ville Syrjälä wrote: > > On Tue, Sep 24, 2013 at 03:14:27PM +0800, Chon Ming Lee wrote: > > > Without the DPIO cmnreset, the PLL fail to lock. This should have > > > done by BIOS. > > > > > > v2: Move this to intel_uncore_sanitize to allow it to get call during > > > resume path. (Daniel) > > > v3: Remove redundant write 0 to DPIO_CTL, and use DPIO_RESET instead of > > > just 0x1 (Ville) > > > Without BIOS, DPIO/render well/media well may still power gated. > > > Turn it off. > > > > > > Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++ > > > drivers/gpu/drm/i915/intel_uncore.c | 23 +++++++++++++++++++++++ > > > 2 files changed, 32 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index c4f9bef..c02f893 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -361,6 +361,15 @@ > > > #define PUNIT_OPCODE_REG_READ 6 > > > #define PUNIT_OPCODE_REG_WRITE 7 > > > > > > +#define PUNIT_REG_PWRGT_CTRL 0x60 > > > +#define PUNIT_REG_PWRGT_STATUS 0x61 > > > +#define PUNIT_CLK_GATE 1 > > > +#define PUNIT_PWR_RESET 2 > > > +#define PUNIT_PWR_GATE 3 > > > +#define RENDER_PWRGT (PUNIT_PWR_GATE << 0) > > > +#define MEDIA_PWRGT (PUNIT_PWR_GATE << 2) > > > +#define DPIO_PWRGT (PUNIT_PWR_GATE << 6) > > > > Subsys 6 seems to be one of four TX lanes, and there's also the common > > lane subsys, and the disp2d is one as well. RX supposedly is not relevant > > for display PHY, not sure why it has subsys allocated too. > > > > Anyways my point would be that shouldn't we check all subsys ie render + media + > > disp2d + common lane + all tx lanes? > > > By default, the common lane + all tx lanes are not power gated during cold boot > or system resume. Unless S0ix entry actually power gate it by driver, then, > this will need to add to turn off it. OK. And as Imre pointed out to me the '<< 6' isn't a TX lane as I claimed but the display subsystems (3). I assume that's the same thing as the disp2d block, ie. the pipes. So the DPIO in the name is wrong. It should be called display or disp2d I think. If you say the PHY side isn't power gated during cold boot, I think we can ignore it for now. So if you rename the DPIO thing, this patch should be OK. > > > And should we maybe power gate the RX lanes always as those shouldn't be needed > > for display? > > Yes, you are correct. I believe there should be another patch to do it, to > enable power gate the VLV correctly for SOix entry or exit. My plan is to power gate everything we can during runtime, not just s0ix. But that's a biger topic we can discuss once Imre gets some relevant patches ready. > > > > > + > > > #define PUNIT_REG_GPU_LFM 0xd3 > > > #define PUNIT_REG_GPU_FREQ_REQ 0xd4 > > > #define PUNIT_REG_GPU_FREQ_STS 0xd8 > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > > index 8649f1c..6923b4d 100644 > > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > > @@ -276,10 +276,33 @@ static void intel_uncore_forcewake_reset(struct drm_device *dev) > > > > > > void intel_uncore_sanitize(struct drm_device *dev) > > > { > > > + struct drm_i915_private *dev_priv = dev->dev_private; > > > + u32 reg_val; > > > + > > > intel_uncore_forcewake_reset(dev); > > > > > > /* BIOS often leaves RC6 enabled, but disable it for hw init */ > > > intel_disable_gt_powersave(dev); > > > + > > > + /* Trigger DPIO CMN RESET and turn off power gate, require > > > + * especially in BIOS less system > > > + */ > > > + if (IS_VALLEYVIEW(dev)) { > > > + > > > + mutex_lock(&dev_priv->rps.hw_lock); > > > + reg_val = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS); > > > + > > > + if (reg_val & (RENDER_PWRGT | MEDIA_PWRGT | DPIO_PWRGT)) > > > + vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, 0x0); > > > + > > > + mutex_unlock(&dev_priv->rps.hw_lock); > > > + > > > + reg_val = I915_READ(DPIO_CTL); > > > + if (!(reg_val & DPIO_RESET)) { > > > + I915_WRITE(DPIO_CTL, DPIO_RESET); > > > + POSTING_READ(DPIO_CTL); > > > + } > > > + } > > > } > > > > > > /* > > > -- > > > 1.7.7.6 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c4f9bef..c02f893 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -361,6 +361,15 @@ #define PUNIT_OPCODE_REG_READ 6 #define PUNIT_OPCODE_REG_WRITE 7 +#define PUNIT_REG_PWRGT_CTRL 0x60 +#define PUNIT_REG_PWRGT_STATUS 0x61 +#define PUNIT_CLK_GATE 1 +#define PUNIT_PWR_RESET 2 +#define PUNIT_PWR_GATE 3 +#define RENDER_PWRGT (PUNIT_PWR_GATE << 0) +#define MEDIA_PWRGT (PUNIT_PWR_GATE << 2) +#define DPIO_PWRGT (PUNIT_PWR_GATE << 6) + #define PUNIT_REG_GPU_LFM 0xd3 #define PUNIT_REG_GPU_FREQ_REQ 0xd4 #define PUNIT_REG_GPU_FREQ_STS 0xd8 diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 8649f1c..6923b4d 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -276,10 +276,33 @@ static void intel_uncore_forcewake_reset(struct drm_device *dev) void intel_uncore_sanitize(struct drm_device *dev) { + struct drm_i915_private *dev_priv = dev->dev_private; + u32 reg_val; + intel_uncore_forcewake_reset(dev); /* BIOS often leaves RC6 enabled, but disable it for hw init */ intel_disable_gt_powersave(dev); + + /* Trigger DPIO CMN RESET and turn off power gate, require + * especially in BIOS less system + */ + if (IS_VALLEYVIEW(dev)) { + + mutex_lock(&dev_priv->rps.hw_lock); + reg_val = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS); + + if (reg_val & (RENDER_PWRGT | MEDIA_PWRGT | DPIO_PWRGT)) + vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, 0x0); + + mutex_unlock(&dev_priv->rps.hw_lock); + + reg_val = I915_READ(DPIO_CTL); + if (!(reg_val & DPIO_RESET)) { + I915_WRITE(DPIO_CTL, DPIO_RESET); + POSTING_READ(DPIO_CTL); + } + } } /*
Without the DPIO cmnreset, the PLL fail to lock. This should have done by BIOS. v2: Move this to intel_uncore_sanitize to allow it to get call during resume path. (Daniel) v3: Remove redundant write 0 to DPIO_CTL, and use DPIO_RESET instead of just 0x1 (Ville) Without BIOS, DPIO/render well/media well may still power gated. Turn it off. Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++ drivers/gpu/drm/i915/intel_uncore.c | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 0 deletions(-)