* [PATCH 1/2] drm/i915: fix hsw_write_dcomp() error message @ 2014-07-04 14:59 Paulo Zanoni 2014-07-04 14:59 ` [PATCH 2/2] drm/i915: fix D_COMP usage on BDW Paulo Zanoni 2014-07-09 16:16 ` [PATCH 1/2] drm/i915: fix hsw_write_dcomp() error message Damien Lespiau 0 siblings, 2 replies; 5+ messages in thread From: Paulo Zanoni @ 2014-07-04 14:59 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni From: Paulo Zanoni <paulo.r.zanoni@intel.com> That function can be used to write anything on D_COMP, not just disable it, so print a more appropriate message. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c12a5da..6986594 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7370,7 +7370,7 @@ static void hsw_write_dcomp(struct drm_i915_private *dev_priv, uint32_t val) mutex_lock(&dev_priv->rps.hw_lock); if (sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_D_COMP, val)) - DRM_ERROR("Failed to disable D_COMP\n"); + DRM_ERROR("Failed to write to D_COMP\n"); mutex_unlock(&dev_priv->rps.hw_lock); } else { I915_WRITE(D_COMP, val); -- 2.0.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/i915: fix D_COMP usage on BDW 2014-07-04 14:59 [PATCH 1/2] drm/i915: fix hsw_write_dcomp() error message Paulo Zanoni @ 2014-07-04 14:59 ` Paulo Zanoni 2014-07-09 16:35 ` Damien Lespiau 2014-07-09 16:16 ` [PATCH 1/2] drm/i915: fix hsw_write_dcomp() error message Damien Lespiau 1 sibling, 1 reply; 5+ messages in thread From: Paulo Zanoni @ 2014-07-04 14:59 UTC (permalink / raw) To: intel-gfx; +Cc: Paulo Zanoni 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> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: fix D_COMP usage on BDW 2014-07-04 14:59 ` [PATCH 2/2] drm/i915: fix D_COMP usage on BDW Paulo Zanoni @ 2014-07-09 16:35 ` Damien Lespiau 2014-07-10 6:28 ` Daniel Vetter 0 siblings, 1 reply; 5+ messages in thread From: Damien Lespiau @ 2014-07-09 16:35 UTC (permalink / raw) To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni 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> -- 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: fix D_COMP usage on BDW 2014-07-09 16:35 ` Damien Lespiau @ 2014-07-10 6:28 ` Daniel Vetter 0 siblings, 0 replies; 5+ messages in thread From: Daniel Vetter @ 2014-07-10 6:28 UTC (permalink / raw) To: Damien Lespiau; +Cc: intel-gfx, Paulo Zanoni 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/i915: fix hsw_write_dcomp() error message 2014-07-04 14:59 [PATCH 1/2] drm/i915: fix hsw_write_dcomp() error message Paulo Zanoni 2014-07-04 14:59 ` [PATCH 2/2] drm/i915: fix D_COMP usage on BDW Paulo Zanoni @ 2014-07-09 16:16 ` Damien Lespiau 1 sibling, 0 replies; 5+ messages in thread From: Damien Lespiau @ 2014-07-09 16:16 UTC (permalink / raw) To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni On Fri, Jul 04, 2014 at 11:59:57AM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > That function can be used to write anything on D_COMP, not just > disable it, so print a more appropriate message. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index c12a5da..6986594 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -7370,7 +7370,7 @@ static void hsw_write_dcomp(struct drm_i915_private *dev_priv, uint32_t val) > mutex_lock(&dev_priv->rps.hw_lock); > if (sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_D_COMP, > val)) > - DRM_ERROR("Failed to disable D_COMP\n"); > + DRM_ERROR("Failed to write to D_COMP\n"); > mutex_unlock(&dev_priv->rps.hw_lock); > } else { > I915_WRITE(D_COMP, val); > -- > 2.0.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-07-10 6:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-04 14:59 [PATCH 1/2] drm/i915: fix hsw_write_dcomp() error message Paulo Zanoni 2014-07-04 14:59 ` [PATCH 2/2] drm/i915: fix D_COMP usage on BDW Paulo Zanoni 2014-07-09 16:35 ` Damien Lespiau 2014-07-10 6:28 ` Daniel Vetter 2014-07-09 16:16 ` [PATCH 1/2] drm/i915: fix hsw_write_dcomp() error message Damien Lespiau
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.