| Message ID | 1404485998-4580-2-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 04, 2014 at 11:59:58AM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > On HSW, the D_COMP register can be accessed through the mailbox (read > and write) or through MMIO on a MCHBAR offset (read only). On BDW, the > access should be done through MMIO on another address. So to account > for all these cases, create hsw_read_dcomp() with the correct > implementation for reading, and also fix hsw_write_dcomp() to do the > correct thing on BDW. > > With this patch, we can now get back from the PC8+ state on BDW. We > were previously getting a black screen and lots of dmesg errors. > Please notice that the bug only happens when you actually reach the > PC8+ states, not when you only allow it. > > Testcase: igt/pm_rpm/rte > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
On Wed, Jul 09, 2014 at 05:35:52PM +0100, Damien Lespiau wrote: > On Fri, Jul 04, 2014 at 11:59:58AM -0300, Paulo Zanoni wrote: > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > On HSW, the D_COMP register can be accessed through the mailbox (read > > and write) or through MMIO on a MCHBAR offset (read only). On BDW, the > > access should be done through MMIO on another address. So to account > > for all these cases, create hsw_read_dcomp() with the correct > > implementation for reading, and also fix hsw_write_dcomp() to do the > > correct thing on BDW. > > > > With this patch, we can now get back from the PC8+ state on BDW. We > > were previously getting a black screen and lots of dmesg errors. > > Please notice that the bug only happens when you actually reach the > > PC8+ states, not when you only allow it. > > > > Testcase: igt/pm_rpm/rte > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> Both patches merged, thanks. -Daniel > > -- > Damien > > > --- > > drivers/gpu/drm/i915/i915_reg.h | 5 ++++- > > drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++++----- > > 2 files changed, 20 insertions(+), 6 deletions(-) > > > > I tested this before my BDW machine died and it worked. I couldn't test it again > > since then. Please notice that this is an important patch that prevents the > > machine from being useless after runtime PM is enabled (and we actually reach > > package C8 or deeper states). This will be even easier to reproduce after we > > merge Daniel's "RPM on DPMS" series. > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 3488567..3e4b13e 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -5962,7 +5962,10 @@ enum punit_power_well { > > #define LCPLL_CD_SOURCE_FCLK (1<<21) > > #define LCPLL_CD_SOURCE_FCLK_DONE (1<<19) > > > > -#define D_COMP (MCHBAR_MIRROR_BASE_SNB + 0x5F0C) > > +/* Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register, > > + * since on HSW we can't write to it using I915_WRITE. */ > > +#define D_COMP_HSW (MCHBAR_MIRROR_BASE_SNB + 0x5F0C) > > +#define D_COMP_BDW 0x138144 > > #define D_COMP_RCOMP_IN_PROGRESS (1<<9) > > #define D_COMP_COMP_FORCE (1<<8) > > #define D_COMP_COMP_DISABLE (1<<0) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 6986594..3a8c9a7 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -7362,6 +7362,16 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv) > > WARN(!dev_priv->pm.irqs_disabled, "IRQs enabled\n"); > > } > > > > +static uint32_t hsw_read_dcomp(struct drm_i915_private *dev_priv) > > +{ > > + struct drm_device *dev = dev_priv->dev; > > + > > + if (IS_HASWELL(dev)) > > + return I915_READ(D_COMP_HSW); > > + else > > + return I915_READ(D_COMP_BDW); > > +} > > + > > static void hsw_write_dcomp(struct drm_i915_private *dev_priv, uint32_t val) > > { > > struct drm_device *dev = dev_priv->dev; > > @@ -7373,9 +7383,9 @@ static void hsw_write_dcomp(struct drm_i915_private *dev_priv, uint32_t val) > > DRM_ERROR("Failed to write D_COMP\n"); > > mutex_unlock(&dev_priv->rps.hw_lock); > > } else { > > - I915_WRITE(D_COMP, val); > > + I915_WRITE(D_COMP_BDW, val); > > + POSTING_READ(D_COMP_BDW); > > } > > - POSTING_READ(D_COMP); > > } > > > > /* > > @@ -7413,12 +7423,13 @@ static void hsw_disable_lcpll(struct drm_i915_private *dev_priv, > > if (wait_for((I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK) == 0, 1)) > > DRM_ERROR("LCPLL still locked\n"); > > > > - val = I915_READ(D_COMP); > > + val = hsw_read_dcomp(dev_priv); > > val |= D_COMP_COMP_DISABLE; > > hsw_write_dcomp(dev_priv, val); > > ndelay(100); > > > > - if (wait_for((I915_READ(D_COMP) & D_COMP_RCOMP_IN_PROGRESS) == 0, 1)) > > + if (wait_for((hsw_read_dcomp(dev_priv) & D_COMP_RCOMP_IN_PROGRESS) == 0, > > + 1)) > > DRM_ERROR("D_COMP RCOMP still in progress\n"); > > > > if (allow_power_down) { > > @@ -7467,7 +7478,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv) > > POSTING_READ(LCPLL_CTL); > > } > > > > - val = I915_READ(D_COMP); > > + val = hsw_read_dcomp(dev_priv); > > val |= D_COMP_COMP_FORCE; > > val &= ~D_COMP_COMP_DISABLE; > > hsw_write_dcomp(dev_priv, val); > > -- > > 2.0.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 3488567..3e4b13e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5962,7 +5962,10 @@ enum punit_power_well { #define LCPLL_CD_SOURCE_FCLK (1<<21) #define LCPLL_CD_SOURCE_FCLK_DONE (1<<19) -#define D_COMP (MCHBAR_MIRROR_BASE_SNB + 0x5F0C) +/* Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register, + * since on HSW we can't write to it using I915_WRITE. */ +#define D_COMP_HSW (MCHBAR_MIRROR_BASE_SNB + 0x5F0C) +#define D_COMP_BDW 0x138144 #define D_COMP_RCOMP_IN_PROGRESS (1<<9) #define D_COMP_COMP_FORCE (1<<8) #define D_COMP_COMP_DISABLE (1<<0) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6986594..3a8c9a7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7362,6 +7362,16 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv) WARN(!dev_priv->pm.irqs_disabled, "IRQs enabled\n"); } +static uint32_t hsw_read_dcomp(struct drm_i915_private *dev_priv) +{ + struct drm_device *dev = dev_priv->dev; + + if (IS_HASWELL(dev)) + return I915_READ(D_COMP_HSW); + else + return I915_READ(D_COMP_BDW); +} + static void hsw_write_dcomp(struct drm_i915_private *dev_priv, uint32_t val) { struct drm_device *dev = dev_priv->dev; @@ -7373,9 +7383,9 @@ static void hsw_write_dcomp(struct drm_i915_private *dev_priv, uint32_t val) DRM_ERROR("Failed to write D_COMP\n"); mutex_unlock(&dev_priv->rps.hw_lock); } else { - I915_WRITE(D_COMP, val); + I915_WRITE(D_COMP_BDW, val); + POSTING_READ(D_COMP_BDW); } - POSTING_READ(D_COMP); } /* @@ -7413,12 +7423,13 @@ static void hsw_disable_lcpll(struct drm_i915_private *dev_priv, if (wait_for((I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK) == 0, 1)) DRM_ERROR("LCPLL still locked\n"); - val = I915_READ(D_COMP); + val = hsw_read_dcomp(dev_priv); val |= D_COMP_COMP_DISABLE; hsw_write_dcomp(dev_priv, val); ndelay(100); - if (wait_for((I915_READ(D_COMP) & D_COMP_RCOMP_IN_PROGRESS) == 0, 1)) + if (wait_for((hsw_read_dcomp(dev_priv) & D_COMP_RCOMP_IN_PROGRESS) == 0, + 1)) DRM_ERROR("D_COMP RCOMP still in progress\n"); if (allow_power_down) { @@ -7467,7 +7478,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv) POSTING_READ(LCPLL_CTL); } - val = I915_READ(D_COMP); + val = hsw_read_dcomp(dev_priv); val |= D_COMP_COMP_FORCE; val &= ~D_COMP_COMP_DISABLE; hsw_write_dcomp(dev_priv, val);