* [PATCH] drm/i915: Move init_clock_gating() back to where it was
@ 2017-11-03 13:03 ` Ville Syrjala
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2017-11-03 13:03 UTC (permalink / raw)
To: intel-gfx
Cc: stable, Chris Wilson, Mark Janes, Maarten Lankhorst,
Daniel Vetter, Joonas Lahtinen, Oscar Mateo, Mika Kuoppala
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Apparently setting up a bunch of GT registers before we've properly
initialized the rest of the GT hardware leads to these setting being
lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
Do .init_clock_gating() earlier to avoid it clobbering watermarks")
by doing init_clock_gating() too early. This should actually affect
other platforms as well, but apparently not to such a great degree.
What I was ultimately after in that commit was to move the
ilk_init_lp_watermarks() call earlier. So let's undo the damage and
move init_clock_gating() back to where it was, and call
ilk_init_lp_watermarks() just before the watermark state readout.
This highlights how fragile and messed up our init order really is.
I wonder why we even initialize the display before gem. The opposite
order would make much more sense to me...
Cc: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mark Janes <mark.a.janes@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reported-by: Mark Janes <mark.a.janes@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 5 +++--
drivers/gpu/drm/i915/intel_pm.c | 40 ++++++++++++++++--------------------
2 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..d4576c2ae31d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3676,6 +3676,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
intel_pps_unlock_regs_wa(dev_priv);
intel_modeset_init_hw(dev);
+ intel_init_clock_gating(dev_priv);
spin_lock_irq(&dev_priv->irq_lock);
if (dev_priv->display.hpd_irq_setup)
@@ -14365,8 +14366,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
intel_update_cdclk(dev_priv);
intel_dump_cdclk_state(&dev_priv->cdclk.hw, "Current CDCLK");
dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw;
-
- intel_init_clock_gating(dev_priv);
}
/*
@@ -15176,6 +15175,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
intel_init_gt_powersave(dev_priv);
+ intel_init_clock_gating(dev_priv);
+
intel_setup_overlay(dev_priv);
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 07118c0b69d3..352a6739ed70 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5754,12 +5754,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
mutex_unlock(&dev_priv->wm.wm_mutex);
}
+/*
+ * FIXME should probably kill this and improve
+ * the real watermark readout/sanitation instead
+ */
+static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
+{
+ I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
+ I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
+ I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
+
+ /*
+ * Don't touch WM1S_LP_EN here.
+ * Doing so could cause underruns.
+ */
+}
+
void ilk_wm_get_hw_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
struct ilk_wm_values *hw = &dev_priv->wm.hw;
struct drm_crtc *crtc;
+ ilk_init_lp_watermarks(dev_priv);
+
for_each_crtc(dev, crtc)
ilk_pipe_wm_get_hw_state(crtc);
@@ -8213,18 +8231,6 @@ static void g4x_disable_trickle_feed(struct drm_i915_private *dev_priv)
}
}
-static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
-{
- I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
- I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
- I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
-
- /*
- * Don't touch WM1S_LP_EN here.
- * Doing so could cause underruns.
- */
-}
-
static void ilk_init_clock_gating(struct drm_i915_private *dev_priv)
{
uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
@@ -8258,8 +8264,6 @@ static void ilk_init_clock_gating(struct drm_i915_private *dev_priv)
(I915_READ(DISP_ARB_CTL) |
DISP_FBC_WM_DIS));
- ilk_init_lp_watermarks(dev_priv);
-
/*
* Based on the document from hardware guys the following bits
* should be set unconditionally in order to enable FBC.
@@ -8372,8 +8376,6 @@ static void gen6_init_clock_gating(struct drm_i915_private *dev_priv)
I915_WRITE(GEN6_GT_MODE,
_MASKED_FIELD(GEN6_WIZ_HASHING_MASK, GEN6_WIZ_HASHING_16x4));
- ilk_init_lp_watermarks(dev_priv);
-
I915_WRITE(CACHE_MODE_0,
_MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
@@ -8600,8 +8602,6 @@ static void bdw_init_clock_gating(struct drm_i915_private *dev_priv)
I915_GTT_PAGE_SIZE_2M);
enum pipe pipe;
- ilk_init_lp_watermarks(dev_priv);
-
/* WaSwitchSolVfFArbitrationPriority:bdw */
I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
@@ -8652,8 +8652,6 @@ static void bdw_init_clock_gating(struct drm_i915_private *dev_priv)
static void hsw_init_clock_gating(struct drm_i915_private *dev_priv)
{
- ilk_init_lp_watermarks(dev_priv);
-
/* L3 caching of data atomics doesn't work -- disable it. */
I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
I915_WRITE(HSW_ROW_CHICKEN3,
@@ -8708,8 +8706,6 @@ static void ivb_init_clock_gating(struct drm_i915_private *dev_priv)
{
uint32_t snpcr;
- ilk_init_lp_watermarks(dev_priv);
-
I915_WRITE(ILK_DSPCLK_GATE_D, ILK_VRHUNIT_CLOCK_GATE_DISABLE);
/* WaDisableEarlyCull:ivb */
--
2.13.6
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH] drm/i915: Move init_clock_gating() back to where it was
@ 2017-11-03 13:03 ` Ville Syrjala
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2017-11-03 13:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, stable
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Apparently setting up a bunch of GT registers before we've properly
initialized the rest of the GT hardware leads to these setting being
lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
Do .init_clock_gating() earlier to avoid it clobbering watermarks")
by doing init_clock_gating() too early. This should actually affect
other platforms as well, but apparently not to such a great degree.
What I was ultimately after in that commit was to move the
ilk_init_lp_watermarks() call earlier. So let's undo the damage and
move init_clock_gating() back to where it was, and call
ilk_init_lp_watermarks() just before the watermark state readout.
This highlights how fragile and messed up our init order really is.
I wonder why we even initialize the display before gem. The opposite
order would make much more sense to me...
Cc: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mark Janes <mark.a.janes@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reported-by: Mark Janes <mark.a.janes@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 5 +++--
drivers/gpu/drm/i915/intel_pm.c | 40 ++++++++++++++++--------------------
2 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..d4576c2ae31d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3676,6 +3676,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
intel_pps_unlock_regs_wa(dev_priv);
intel_modeset_init_hw(dev);
+ intel_init_clock_gating(dev_priv);
spin_lock_irq(&dev_priv->irq_lock);
if (dev_priv->display.hpd_irq_setup)
@@ -14365,8 +14366,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
intel_update_cdclk(dev_priv);
intel_dump_cdclk_state(&dev_priv->cdclk.hw, "Current CDCLK");
dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw;
-
- intel_init_clock_gating(dev_priv);
}
/*
@@ -15176,6 +15175,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
intel_init_gt_powersave(dev_priv);
+ intel_init_clock_gating(dev_priv);
+
intel_setup_overlay(dev_priv);
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 07118c0b69d3..352a6739ed70 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5754,12 +5754,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
mutex_unlock(&dev_priv->wm.wm_mutex);
}
+/*
+ * FIXME should probably kill this and improve
+ * the real watermark readout/sanitation instead
+ */
+static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
+{
+ I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
+ I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
+ I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
+
+ /*
+ * Don't touch WM1S_LP_EN here.
+ * Doing so could cause underruns.
+ */
+}
+
void ilk_wm_get_hw_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
struct ilk_wm_values *hw = &dev_priv->wm.hw;
struct drm_crtc *crtc;
+ ilk_init_lp_watermarks(dev_priv);
+
for_each_crtc(dev, crtc)
ilk_pipe_wm_get_hw_state(crtc);
@@ -8213,18 +8231,6 @@ static void g4x_disable_trickle_feed(struct drm_i915_private *dev_priv)
}
}
-static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
-{
- I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
- I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
- I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
-
- /*
- * Don't touch WM1S_LP_EN here.
- * Doing so could cause underruns.
- */
-}
-
static void ilk_init_clock_gating(struct drm_i915_private *dev_priv)
{
uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
@@ -8258,8 +8264,6 @@ static void ilk_init_clock_gating(struct drm_i915_private *dev_priv)
(I915_READ(DISP_ARB_CTL) |
DISP_FBC_WM_DIS));
- ilk_init_lp_watermarks(dev_priv);
-
/*
* Based on the document from hardware guys the following bits
* should be set unconditionally in order to enable FBC.
@@ -8372,8 +8376,6 @@ static void gen6_init_clock_gating(struct drm_i915_private *dev_priv)
I915_WRITE(GEN6_GT_MODE,
_MASKED_FIELD(GEN6_WIZ_HASHING_MASK, GEN6_WIZ_HASHING_16x4));
- ilk_init_lp_watermarks(dev_priv);
-
I915_WRITE(CACHE_MODE_0,
_MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
@@ -8600,8 +8602,6 @@ static void bdw_init_clock_gating(struct drm_i915_private *dev_priv)
I915_GTT_PAGE_SIZE_2M);
enum pipe pipe;
- ilk_init_lp_watermarks(dev_priv);
-
/* WaSwitchSolVfFArbitrationPriority:bdw */
I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
@@ -8652,8 +8652,6 @@ static void bdw_init_clock_gating(struct drm_i915_private *dev_priv)
static void hsw_init_clock_gating(struct drm_i915_private *dev_priv)
{
- ilk_init_lp_watermarks(dev_priv);
-
/* L3 caching of data atomics doesn't work -- disable it. */
I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
I915_WRITE(HSW_ROW_CHICKEN3,
@@ -8708,8 +8706,6 @@ static void ivb_init_clock_gating(struct drm_i915_private *dev_priv)
{
uint32_t snpcr;
- ilk_init_lp_watermarks(dev_priv);
-
I915_WRITE(ILK_DSPCLK_GATE_D, ILK_VRHUNIT_CLOCK_GATE_DISABLE);
/* WaDisableEarlyCull:ivb */
--
2.13.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Move init_clock_gating() back to where it was
2017-11-03 13:03 ` Ville Syrjala
(?)
@ 2017-11-03 13:26 ` Patchwork
-1 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2017-11-03 13:26 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Move init_clock_gating() back to where it was
URL : https://patchwork.freedesktop.org/series/33124/
State : failure
== Summary ==
Series 33124v1 drm/i915: Move init_clock_gating() back to where it was
https://patchwork.freedesktop.org/api/1.0/series/33124/revisions/1/mbox/
Test chamelium:
Subgroup dp-crc-fast:
incomplete -> PASS (fi-kbl-7500u) fdo#102514
Test gem_exec_reloc:
Subgroup basic-write-gtt-noreloc:
pass -> INCOMPLETE (fi-glk-dsi)
Test kms_busy:
Subgroup basic-flip-b:
pass -> INCOMPLETE (fi-hsw-4770)
pass -> INCOMPLETE (fi-hsw-4770r)
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:442s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:455s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:386s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:531s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:277s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:510s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:508s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:504s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:490s
fi-cfl-s total:289 pass:254 dwarn:3 dfail:0 fail:0 skip:32 time:559s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:427s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:263s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:585s
fi-glk-dsi total:97 pass:77 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-4770 total:207 pass:187 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-4770r total:207 pass:187 dwarn:0 dfail:0 fail:0 skip:19
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:428s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:462s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:497s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:575s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:484s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:586s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:579s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:458s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:600s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:649s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:523s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:504s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:463s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:570s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:432s
2f945d948d05de1dd4bd1ca14eac40ff563d9d77 drm-tip: 2017y-11m-03d-09h-28m-20s UTC integration manifest
03b4a3fe7a2b drm/i915: Move init_clock_gating() back to where it was
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6937/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Move init_clock_gating() back to where it was
2017-11-03 13:03 ` Ville Syrjala
@ 2017-11-03 13:27 ` Chris Wilson
-1 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2017-11-03 13:27 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
Cc: stable, Mark Janes, Maarten Lankhorst, Daniel Vetter,
Joonas Lahtinen, Oscar Mateo, Mika Kuoppala
Quoting Ville Syrjala (2017-11-03 13:03:37)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Apparently setting up a bunch of GT registers before we've properly
> initialized the rest of the GT hardware leads to these setting being
> lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> by doing init_clock_gating() too early. This should actually affect
> other platforms as well, but apparently not to such a great degree.
>
> What I was ultimately after in that commit was to move the
> ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> move init_clock_gating() back to where it was, and call
> ilk_init_lp_watermarks() just before the watermark state readout.
>
> This highlights how fragile and messed up our init order really is.
> I wonder why we even initialize the display before gem. The opposite
> order would make much more sense to me...
Indeed this will cause some fun for me momentarily as I will have to
move init_clock_gating() to i915_gem_init(). We can only wish for that
magic wand to be waved sooner.
Does that make sense, or will I have to start carving up
init_clock_gating()?
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 07118c0b69d3..352a6739ed70 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5754,12 +5754,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
> mutex_unlock(&dev_priv->wm.wm_mutex);
> }
>
> +/*
> + * FIXME should probably kill this and improve
> + * the real watermark readout/sanitation instead
> + */
> +static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
> +{
> + I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
> + I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
> + I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
> +
> + /*
> + * Don't touch WM1S_LP_EN here.
> + * Doing so could cause underruns.
> + */
> +}
> +
> void ilk_wm_get_hw_state(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct ilk_wm_values *hw = &dev_priv->wm.hw;
> struct drm_crtc *crtc;
>
> + ilk_init_lp_watermarks(dev_priv);
Wasn't expecting this, but the rest lgtm. Could you explain you decision
to put it here in a bit more detail?
-Chris
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Move init_clock_gating() back to where it was
@ 2017-11-03 13:27 ` Chris Wilson
0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2017-11-03 13:27 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx; +Cc: Daniel Vetter, stable
Quoting Ville Syrjala (2017-11-03 13:03:37)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Apparently setting up a bunch of GT registers before we've properly
> initialized the rest of the GT hardware leads to these setting being
> lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> by doing init_clock_gating() too early. This should actually affect
> other platforms as well, but apparently not to such a great degree.
>
> What I was ultimately after in that commit was to move the
> ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> move init_clock_gating() back to where it was, and call
> ilk_init_lp_watermarks() just before the watermark state readout.
>
> This highlights how fragile and messed up our init order really is.
> I wonder why we even initialize the display before gem. The opposite
> order would make much more sense to me...
Indeed this will cause some fun for me momentarily as I will have to
move init_clock_gating() to i915_gem_init(). We can only wish for that
magic wand to be waved sooner.
Does that make sense, or will I have to start carving up
init_clock_gating()?
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 07118c0b69d3..352a6739ed70 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5754,12 +5754,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
> mutex_unlock(&dev_priv->wm.wm_mutex);
> }
>
> +/*
> + * FIXME should probably kill this and improve
> + * the real watermark readout/sanitation instead
> + */
> +static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
> +{
> + I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
> + I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
> + I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
> +
> + /*
> + * Don't touch WM1S_LP_EN here.
> + * Doing so could cause underruns.
> + */
> +}
> +
> void ilk_wm_get_hw_state(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct ilk_wm_values *hw = &dev_priv->wm.hw;
> struct drm_crtc *crtc;
>
> + ilk_init_lp_watermarks(dev_priv);
Wasn't expecting this, but the rest lgtm. Could you explain you decision
to put it here in a bit more detail?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Move init_clock_gating() back to where it was
2017-11-03 13:27 ` Chris Wilson
@ 2017-11-03 14:20 ` Ville Syrjälä
-1 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2017-11-03 14:20 UTC (permalink / raw)
To: Chris Wilson
Cc: intel-gfx, stable, Mark Janes, Maarten Lankhorst, Daniel Vetter,
Joonas Lahtinen, Oscar Mateo, Mika Kuoppala
On Fri, Nov 03, 2017 at 01:27:55PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-03 13:03:37)
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> >
> > Apparently setting up a bunch of GT registers before we've properly
> > initialized the rest of the GT hardware leads to these setting being
> > lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> > Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > by doing init_clock_gating() too early. This should actually affect
> > other platforms as well, but apparently not to such a great degree.
> >
> > What I was ultimately after in that commit was to move the
> > ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> > move init_clock_gating() back to where it was, and call
> > ilk_init_lp_watermarks() just before the watermark state readout.
> >
> > This highlights how fragile and messed up our init order really is.
> > I wonder why we even initialize the display before gem. The opposite
> > order would make much more sense to me...
>
> Indeed this will cause some fun for me momentarily as I will have to
> move init_clock_gating() to i915_gem_init(). We can only wish for that
> magic wand to be waved sooner.
>
> Does that make sense, or will I have to start carving up
> init_clock_gating()?
i915_gem_init() should be fine as far as the display is concerned
at least, albeit a bit unexpected. Do we need to do that already in
this patch, or a followup?
>
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 07118c0b69d3..352a6739ed70 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5754,12 +5754,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
> > mutex_unlock(&dev_priv->wm.wm_mutex);
> > }
> >
> > +/*
> > + * FIXME should probably kill this and improve
> > + * the real watermark readout/sanitation instead
> > + */
> > +static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
> > +{
> > + I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
> > + I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
> > + I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
> > +
> > + /*
> > + * Don't touch WM1S_LP_EN here.
> > + * Doing so could cause underruns.
> > + */
> > +}
> > +
> > void ilk_wm_get_hw_state(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct ilk_wm_values *hw = &dev_priv->wm.hw;
> > struct drm_crtc *crtc;
> >
> > + ilk_init_lp_watermarks(dev_priv);
>
> Wasn't expecting this, but the rest lgtm. Could you explain you decision
> to put it here in a bit more detail?
The original problem was that this guy turned off the LP1+ watermarks
after we'd already done the state readout in ilk_wm_get_hw_state(). So
the state we had read out no longer matched the hardware state.
To keep the software and hardware states in sync we just need to make
sure ilk_init_lp_watermarks() is called before the readout. And the
obvious thing to do then is to call it immediately before to the
readout.
--
Ville Syrj�l�
Intel OTC
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Move init_clock_gating() back to where it was
@ 2017-11-03 14:20 ` Ville Syrjälä
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2017-11-03 14:20 UTC (permalink / raw)
To: Chris Wilson
Cc: intel-gfx, stable, Mark Janes, Maarten Lankhorst, Daniel Vetter,
Joonas Lahtinen, Oscar Mateo, Mika Kuoppala
On Fri, Nov 03, 2017 at 01:27:55PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-03 13:03:37)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Apparently setting up a bunch of GT registers before we've properly
> > initialized the rest of the GT hardware leads to these setting being
> > lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> > Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > by doing init_clock_gating() too early. This should actually affect
> > other platforms as well, but apparently not to such a great degree.
> >
> > What I was ultimately after in that commit was to move the
> > ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> > move init_clock_gating() back to where it was, and call
> > ilk_init_lp_watermarks() just before the watermark state readout.
> >
> > This highlights how fragile and messed up our init order really is.
> > I wonder why we even initialize the display before gem. The opposite
> > order would make much more sense to me...
>
> Indeed this will cause some fun for me momentarily as I will have to
> move init_clock_gating() to i915_gem_init(). We can only wish for that
> magic wand to be waved sooner.
>
> Does that make sense, or will I have to start carving up
> init_clock_gating()?
i915_gem_init() should be fine as far as the display is concerned
at least, albeit a bit unexpected. Do we need to do that already in
this patch, or a followup?
>
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 07118c0b69d3..352a6739ed70 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5754,12 +5754,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
> > mutex_unlock(&dev_priv->wm.wm_mutex);
> > }
> >
> > +/*
> > + * FIXME should probably kill this and improve
> > + * the real watermark readout/sanitation instead
> > + */
> > +static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
> > +{
> > + I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
> > + I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
> > + I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
> > +
> > + /*
> > + * Don't touch WM1S_LP_EN here.
> > + * Doing so could cause underruns.
> > + */
> > +}
> > +
> > void ilk_wm_get_hw_state(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct ilk_wm_values *hw = &dev_priv->wm.hw;
> > struct drm_crtc *crtc;
> >
> > + ilk_init_lp_watermarks(dev_priv);
>
> Wasn't expecting this, but the rest lgtm. Could you explain you decision
> to put it here in a bit more detail?
The original problem was that this guy turned off the LP1+ watermarks
after we'd already done the state readout in ilk_wm_get_hw_state(). So
the state we had read out no longer matched the hardware state.
To keep the software and hardware states in sync we just need to make
sure ilk_init_lp_watermarks() is called before the readout. And the
obvious thing to do then is to call it immediately before to the
readout.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Move init_clock_gating() back to where it was
2017-11-03 14:20 ` Ville Syrjälä
(?)
@ 2017-11-03 14:41 ` Chris Wilson
-1 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2017-11-03 14:41 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx, stable
Quoting Ville Syrjälä (2017-11-03 14:20:33)
> On Fri, Nov 03, 2017 at 01:27:55PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjala (2017-11-03 13:03:37)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Apparently setting up a bunch of GT registers before we've properly
> > > initialized the rest of the GT hardware leads to these setting being
> > > lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> > > Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > > by doing init_clock_gating() too early. This should actually affect
> > > other platforms as well, but apparently not to such a great degree.
> > >
> > > What I was ultimately after in that commit was to move the
> > > ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> > > move init_clock_gating() back to where it was, and call
> > > ilk_init_lp_watermarks() just before the watermark state readout.
> > >
> > > This highlights how fragile and messed up our init order really is.
> > > I wonder why we even initialize the display before gem. The opposite
> > > order would make much more sense to me...
> >
> > Indeed this will cause some fun for me momentarily as I will have to
> > move init_clock_gating() to i915_gem_init(). We can only wish for that
> > magic wand to be waved sooner.
> >
> > Does that make sense, or will I have to start carving up
> > init_clock_gating()?
>
> i915_gem_init() should be fine as far as the display is concerned
> at least, albeit a bit unexpected. Do we need to do that already in
> this patch, or a followup?
After this.
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 07118c0b69d3..352a6739ed70 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -5754,12 +5754,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
> > > mutex_unlock(&dev_priv->wm.wm_mutex);
> > > }
> > >
> > > +/*
> > > + * FIXME should probably kill this and improve
> > > + * the real watermark readout/sanitation instead
> > > + */
> > > +static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
> > > +{
> > > + I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
> > > + I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
> > > + I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
> > > +
> > > + /*
> > > + * Don't touch WM1S_LP_EN here.
> > > + * Doing so could cause underruns.
> > > + */
> > > +}
> > > +
> > > void ilk_wm_get_hw_state(struct drm_device *dev)
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > struct ilk_wm_values *hw = &dev_priv->wm.hw;
> > > struct drm_crtc *crtc;
> > >
> > > + ilk_init_lp_watermarks(dev_priv);
> >
> > Wasn't expecting this, but the rest lgtm. Could you explain you decision
> > to put it here in a bit more detail?
>
> The original problem was that this guy turned off the LP1+ watermarks
> after we'd already done the state readout in ilk_wm_get_hw_state(). So
> the state we had read out no longer matched the hardware state.
Ah. Dim light bulb flickers.
> To keep the software and hardware states in sync we just need to make
> sure ilk_init_lp_watermarks() is called before the readout. And the
> obvious thing to do then is to call it immediately before to the
> readout.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Move init_clock_gating() back to where it was
2017-11-03 13:03 ` Ville Syrjala
` (2 preceding siblings ...)
(?)
@ 2017-11-03 14:58 ` Chris Wilson
-1 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2017-11-03 14:58 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
Cc: stable, Mark Janes, Maarten Lankhorst, Daniel Vetter,
Joonas Lahtinen, Oscar Mateo, Mika Kuoppala
Quoting Ville Syrjala (2017-11-03 13:03:37)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Apparently setting up a bunch of GT registers before we've properly
> initialized the rest of the GT hardware leads to these setting being
> lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> by doing init_clock_gating() too early. This should actually affect
> other platforms as well, but apparently not to such a great degree.
>
> What I was ultimately after in that commit was to move the
> ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> move init_clock_gating() back to where it was, and call
> ilk_init_lp_watermarks() just before the watermark state readout.
>
> This highlights how fragile and messed up our init order really is.
> I wonder why we even initialize the display before gem. The opposite
> order would make much more sense to me...
>
> Cc: stable@vger.kernel.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mark Janes <mark.a.janes@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reported-by: Mark Janes <mark.a.janes@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
> Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Wrt to the reported CTS regressions, they are fixed.
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Move init_clock_gating() back to where it was
2017-11-03 13:03 ` Ville Syrjala
` (3 preceding siblings ...)
(?)
@ 2017-11-03 17:05 ` Patchwork
2017-11-03 17:08 ` Chris Wilson
-1 siblings, 1 reply; 23+ messages in thread
From: Patchwork @ 2017-11-03 17:05 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Move init_clock_gating() back to where it was
URL : https://patchwork.freedesktop.org/series/33124/
State : failure
== Summary ==
Series 33124v1 drm/i915: Move init_clock_gating() back to where it was
https://patchwork.freedesktop.org/api/1.0/series/33124/revisions/1/mbox/
Test chamelium:
Subgroup dp-crc-fast:
pass -> FAIL (fi-kbl-7500u) fdo#102514
Test gem_exec_reloc:
Subgroup basic-write-gtt-active:
fail -> PASS (fi-gdg-551) fdo#102582
Test kms_busy:
Subgroup basic-flip-b:
fail -> PASS (fi-bwr-2160)
pass -> INCOMPLETE (fi-hsw-4770r)
Subgroup basic-flip-c:
pass -> INCOMPLETE (fi-hsw-4770)
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:441s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:456s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:543s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:279s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:504s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:504s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:504s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:485s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:431s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:265s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:584s
fi-hsw-4770 total:208 pass:188 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-4770r total:207 pass:187 dwarn:0 dfail:0 fail:0 skip:19
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:428s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:505s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:460s
fi-kbl-7500u total:289 pass:263 dwarn:1 dfail:0 fail:1 skip:24 time:477s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:572s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:480s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:589s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:571s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:453s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:592s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:654s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:516s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:500s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:459s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:568s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:426s
fi-cfl-s failed to connect after reboot
de359919ae463cdaef6bc6890156df84e19dee2a drm-tip: 2017y-11m-03d-14h-00m-37s UTC integration manifest
0c75534fbce0 drm/i915: Move init_clock_gating() back to where it was
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6944/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: ✗ Fi.CI.BAT: failure for drm/i915: Move init_clock_gating() back to where it was
2017-11-03 17:05 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2017-11-03 17:08 ` Chris Wilson
2017-11-03 17:37 ` Ville Syrjälä
0 siblings, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2017-11-03 17:08 UTC (permalink / raw)
To: Patchwork, Ville Syrjälä; +Cc: intel-gfx
Quoting Patchwork (2017-11-03 17:05:20)
> == Series Details ==
>
> Series: drm/i915: Move init_clock_gating() back to where it was
> URL : https://patchwork.freedesktop.org/series/33124/
> State : failure
>
> == Summary ==
>
> Series 33124v1 drm/i915: Move init_clock_gating() back to where it was
> https://patchwork.freedesktop.org/api/1.0/series/33124/revisions/1/mbox/
>
> Test chamelium:
> Subgroup dp-crc-fast:
> pass -> FAIL (fi-kbl-7500u) fdo#102514
> Test gem_exec_reloc:
> Subgroup basic-write-gtt-active:
> fail -> PASS (fi-gdg-551) fdo#102582
> Test kms_busy:
> Subgroup basic-flip-b:
> fail -> PASS (fi-bwr-2160)
> pass -> INCOMPLETE (fi-hsw-4770r)
> Subgroup basic-flip-c:
> pass -> INCOMPLETE (fi-hsw-4770)
Darn persistent. Third time lucky?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: ✗ Fi.CI.BAT: failure for drm/i915: Move init_clock_gating() back to where it was
2017-11-03 17:08 ` Chris Wilson
@ 2017-11-03 17:37 ` Ville Syrjälä
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2017-11-03 17:37 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Fri, Nov 03, 2017 at 05:08:07PM +0000, Chris Wilson wrote:
> Quoting Patchwork (2017-11-03 17:05:20)
> > == Series Details ==
> >
> > Series: drm/i915: Move init_clock_gating() back to where it was
> > URL : https://patchwork.freedesktop.org/series/33124/
> > State : failure
> >
> > == Summary ==
> >
> > Series 33124v1 drm/i915: Move init_clock_gating() back to where it was
> > https://patchwork.freedesktop.org/api/1.0/series/33124/revisions/1/mbox/
> >
> > Test chamelium:
> > Subgroup dp-crc-fast:
> > pass -> FAIL (fi-kbl-7500u) fdo#102514
> > Test gem_exec_reloc:
> > Subgroup basic-write-gtt-active:
> > fail -> PASS (fi-gdg-551) fdo#102582
> > Test kms_busy:
> > Subgroup basic-flip-b:
> > fail -> PASS (fi-bwr-2160)
> > pass -> INCOMPLETE (fi-hsw-4770r)
> > Subgroup basic-flip-c:
> > pass -> INCOMPLETE (fi-hsw-4770)
>
> Darn persistent. Third time lucky?
Let's find out.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Move init_clock_gating() back to where it was
2017-11-03 13:03 ` Ville Syrjala
` (4 preceding siblings ...)
(?)
@ 2017-11-03 18:20 ` Patchwork
-1 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2017-11-03 18:20 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Move init_clock_gating() back to where it was
URL : https://patchwork.freedesktop.org/series/33124/
State : failure
== Summary ==
Series 33124v1 drm/i915: Move init_clock_gating() back to where it was
https://patchwork.freedesktop.org/api/1.0/series/33124/revisions/1/mbox/
Test chamelium:
Subgroup dp-crc-fast:
pass -> FAIL (fi-kbl-7500u) fdo#102514
Test gem_exec_reloc:
Subgroup basic-write-gtt-active:
fail -> PASS (fi-gdg-551) fdo#102582
Test kms_busy:
Subgroup basic-flip-b:
fail -> PASS (fi-bwr-2160)
pass -> INCOMPLETE (fi-hsw-4770)
pass -> INCOMPLETE (fi-hsw-4770r)
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:442s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:453s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:539s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:274s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:508s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:508s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:506s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:491s
fi-cfl-s total:289 pass:254 dwarn:3 dfail:0 fail:0 skip:32 time:551s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:431s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:265s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:584s
fi-hsw-4770 total:207 pass:187 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-4770r total:207 pass:187 dwarn:0 dfail:0 fail:0 skip:19
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:424s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:497s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:461s
fi-kbl-7500u total:289 pass:263 dwarn:1 dfail:0 fail:1 skip:24 time:482s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:575s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:480s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:586s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:570s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:462s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:599s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:645s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:524s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:507s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:462s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:576s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:430s
de359919ae463cdaef6bc6890156df84e19dee2a drm-tip: 2017y-11m-03d-14h-00m-37s UTC integration manifest
638ed9079681 drm/i915: Move init_clock_gating() back to where it was
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6948/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2] drm/i915: Move init_clock_gating() back to where it was
2017-11-03 13:03 ` Ville Syrjala
@ 2017-11-08 13:35 ` Ville Syrjala
-1 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2017-11-08 13:35 UTC (permalink / raw)
To: intel-gfx
Cc: stable, Chris Wilson, Mark Janes, Maarten Lankhorst,
Daniel Vetter, Joonas Lahtinen, Oscar Mateo, Mika Kuoppala
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Apparently setting up a bunch of GT registers before we've properly
initialized the rest of the GT hardware leads to these setting being
lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
Do .init_clock_gating() earlier to avoid it clobbering watermarks")
by doing init_clock_gating() too early. This should actually affect
other platforms as well, but apparently not to such a great degree.
What I was ultimately after in that commit was to move the
ilk_init_lp_watermarks() call earlier. So let's undo the damage and
move init_clock_gating() back to where it was, and call
ilk_init_lp_watermarks() just before the watermark state readout.
This highlights how fragile and messed up our init order really is.
I wonder why we even initialize the display before gem. The opposite
order would make much more sense to me...
v2: Keep WaRsPkgCStateDisplayPMReq:hsw early as it really must
be done before all planes might get disabled.
Cc: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mark Janes <mark.a.janes@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reported-by: Mark Janes <mark.a.janes@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++--
drivers/gpu/drm/i915/intel_pm.c | 44 +++++++++++++++---------------------
2 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..8174392acc18 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3676,6 +3676,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
intel_pps_unlock_regs_wa(dev_priv);
intel_modeset_init_hw(dev);
+ intel_init_clock_gating(dev_priv);
spin_lock_irq(&dev_priv->irq_lock);
if (dev_priv->display.hpd_irq_setup)
@@ -14365,8 +14366,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
intel_update_cdclk(dev_priv);
intel_dump_cdclk_state(&dev_priv->cdclk.hw, "Current CDCLK");
dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw;
-
- intel_init_clock_gating(dev_priv);
}
/*
@@ -15079,6 +15078,15 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
struct intel_encoder *encoder;
int i;
+ if (IS_HASWELL(dev_priv)) {
+ /*
+ * WaRsPkgCStateDisplayPMReq:hsw
+ * System hang if this isn't done before disabling all planes!
+ */
+ I915_WRITE(CHICKEN_PAR1_1,
+ I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
+ }
+
intel_modeset_readout_hw_state(dev);
/* HW state is read out, now we need to sanitize this mess. */
@@ -15176,6 +15184,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
intel_init_gt_powersave(dev_priv);
+ intel_init_clock_gating(dev_priv);
+
intel_setup_overlay(dev_priv);
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 07118c0b69d3..b712ee30a06c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5754,12 +5754,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
mutex_unlock(&dev_priv->wm.wm_mutex);
}
+/*
+ * FIXME should probably kill this and improve
+ * the real watermark readout/sanitation instead
+ */
+static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
+{
+ I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
+ I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
+ I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
+
+ /*
+ * Don't touch WM1S_LP_EN here.
+ * Doing so could cause underruns.
+ */
+}
+
void ilk_wm_get_hw_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
struct ilk_wm_values *hw = &dev_priv->wm.hw;
struct drm_crtc *crtc;
+ ilk_init_lp_watermarks(dev_priv);
+
for_each_crtc(dev, crtc)
ilk_pipe_wm_get_hw_state(crtc);
@@ -8213,18 +8231,6 @@ static void g4x_disable_trickle_feed(struct drm_i915_private *dev_priv)
}
}
-static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
-{
- I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
- I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
- I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
-
- /*
- * Don't touch WM1S_LP_EN here.
- * Doing so could cause underruns.
- */
-}
-
static void ilk_init_clock_gating(struct drm_i915_private *dev_priv)
{
uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
@@ -8258,8 +8264,6 @@ static void ilk_init_clock_gating(struct drm_i915_private *dev_priv)
(I915_READ(DISP_ARB_CTL) |
DISP_FBC_WM_DIS));
- ilk_init_lp_watermarks(dev_priv);
-
/*
* Based on the document from hardware guys the following bits
* should be set unconditionally in order to enable FBC.
@@ -8372,8 +8376,6 @@ static void gen6_init_clock_gating(struct drm_i915_private *dev_priv)
I915_WRITE(GEN6_GT_MODE,
_MASKED_FIELD(GEN6_WIZ_HASHING_MASK, GEN6_WIZ_HASHING_16x4));
- ilk_init_lp_watermarks(dev_priv);
-
I915_WRITE(CACHE_MODE_0,
_MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
@@ -8600,8 +8602,6 @@ static void bdw_init_clock_gating(struct drm_i915_private *dev_priv)
I915_GTT_PAGE_SIZE_2M);
enum pipe pipe;
- ilk_init_lp_watermarks(dev_priv);
-
/* WaSwitchSolVfFArbitrationPriority:bdw */
I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
@@ -8652,8 +8652,6 @@ static void bdw_init_clock_gating(struct drm_i915_private *dev_priv)
static void hsw_init_clock_gating(struct drm_i915_private *dev_priv)
{
- ilk_init_lp_watermarks(dev_priv);
-
/* L3 caching of data atomics doesn't work -- disable it. */
I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
I915_WRITE(HSW_ROW_CHICKEN3,
@@ -8697,10 +8695,6 @@ static void hsw_init_clock_gating(struct drm_i915_private *dev_priv)
/* WaSwitchSolVfFArbitrationPriority:hsw */
I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
- /* WaRsPkgCStateDisplayPMReq:hsw */
- I915_WRITE(CHICKEN_PAR1_1,
- I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
-
lpt_init_clock_gating(dev_priv);
}
@@ -8708,8 +8702,6 @@ static void ivb_init_clock_gating(struct drm_i915_private *dev_priv)
{
uint32_t snpcr;
- ilk_init_lp_watermarks(dev_priv);
-
I915_WRITE(ILK_DSPCLK_GATE_D, ILK_VRHUNIT_CLOCK_GATE_DISABLE);
/* WaDisableEarlyCull:ivb */
--
2.13.6
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2] drm/i915: Move init_clock_gating() back to where it was
@ 2017-11-08 13:35 ` Ville Syrjala
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2017-11-08 13:35 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, stable
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Apparently setting up a bunch of GT registers before we've properly
initialized the rest of the GT hardware leads to these setting being
lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
Do .init_clock_gating() earlier to avoid it clobbering watermarks")
by doing init_clock_gating() too early. This should actually affect
other platforms as well, but apparently not to such a great degree.
What I was ultimately after in that commit was to move the
ilk_init_lp_watermarks() call earlier. So let's undo the damage and
move init_clock_gating() back to where it was, and call
ilk_init_lp_watermarks() just before the watermark state readout.
This highlights how fragile and messed up our init order really is.
I wonder why we even initialize the display before gem. The opposite
order would make much more sense to me...
v2: Keep WaRsPkgCStateDisplayPMReq:hsw early as it really must
be done before all planes might get disabled.
Cc: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mark Janes <mark.a.janes@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reported-by: Mark Janes <mark.a.janes@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++--
drivers/gpu/drm/i915/intel_pm.c | 44 +++++++++++++++---------------------
2 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..8174392acc18 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3676,6 +3676,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
intel_pps_unlock_regs_wa(dev_priv);
intel_modeset_init_hw(dev);
+ intel_init_clock_gating(dev_priv);
spin_lock_irq(&dev_priv->irq_lock);
if (dev_priv->display.hpd_irq_setup)
@@ -14365,8 +14366,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
intel_update_cdclk(dev_priv);
intel_dump_cdclk_state(&dev_priv->cdclk.hw, "Current CDCLK");
dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw;
-
- intel_init_clock_gating(dev_priv);
}
/*
@@ -15079,6 +15078,15 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
struct intel_encoder *encoder;
int i;
+ if (IS_HASWELL(dev_priv)) {
+ /*
+ * WaRsPkgCStateDisplayPMReq:hsw
+ * System hang if this isn't done before disabling all planes!
+ */
+ I915_WRITE(CHICKEN_PAR1_1,
+ I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
+ }
+
intel_modeset_readout_hw_state(dev);
/* HW state is read out, now we need to sanitize this mess. */
@@ -15176,6 +15184,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
intel_init_gt_powersave(dev_priv);
+ intel_init_clock_gating(dev_priv);
+
intel_setup_overlay(dev_priv);
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 07118c0b69d3..b712ee30a06c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5754,12 +5754,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
mutex_unlock(&dev_priv->wm.wm_mutex);
}
+/*
+ * FIXME should probably kill this and improve
+ * the real watermark readout/sanitation instead
+ */
+static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
+{
+ I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
+ I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
+ I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
+
+ /*
+ * Don't touch WM1S_LP_EN here.
+ * Doing so could cause underruns.
+ */
+}
+
void ilk_wm_get_hw_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
struct ilk_wm_values *hw = &dev_priv->wm.hw;
struct drm_crtc *crtc;
+ ilk_init_lp_watermarks(dev_priv);
+
for_each_crtc(dev, crtc)
ilk_pipe_wm_get_hw_state(crtc);
@@ -8213,18 +8231,6 @@ static void g4x_disable_trickle_feed(struct drm_i915_private *dev_priv)
}
}
-static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
-{
- I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
- I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
- I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
-
- /*
- * Don't touch WM1S_LP_EN here.
- * Doing so could cause underruns.
- */
-}
-
static void ilk_init_clock_gating(struct drm_i915_private *dev_priv)
{
uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
@@ -8258,8 +8264,6 @@ static void ilk_init_clock_gating(struct drm_i915_private *dev_priv)
(I915_READ(DISP_ARB_CTL) |
DISP_FBC_WM_DIS));
- ilk_init_lp_watermarks(dev_priv);
-
/*
* Based on the document from hardware guys the following bits
* should be set unconditionally in order to enable FBC.
@@ -8372,8 +8376,6 @@ static void gen6_init_clock_gating(struct drm_i915_private *dev_priv)
I915_WRITE(GEN6_GT_MODE,
_MASKED_FIELD(GEN6_WIZ_HASHING_MASK, GEN6_WIZ_HASHING_16x4));
- ilk_init_lp_watermarks(dev_priv);
-
I915_WRITE(CACHE_MODE_0,
_MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
@@ -8600,8 +8602,6 @@ static void bdw_init_clock_gating(struct drm_i915_private *dev_priv)
I915_GTT_PAGE_SIZE_2M);
enum pipe pipe;
- ilk_init_lp_watermarks(dev_priv);
-
/* WaSwitchSolVfFArbitrationPriority:bdw */
I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
@@ -8652,8 +8652,6 @@ static void bdw_init_clock_gating(struct drm_i915_private *dev_priv)
static void hsw_init_clock_gating(struct drm_i915_private *dev_priv)
{
- ilk_init_lp_watermarks(dev_priv);
-
/* L3 caching of data atomics doesn't work -- disable it. */
I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
I915_WRITE(HSW_ROW_CHICKEN3,
@@ -8697,10 +8695,6 @@ static void hsw_init_clock_gating(struct drm_i915_private *dev_priv)
/* WaSwitchSolVfFArbitrationPriority:hsw */
I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
- /* WaRsPkgCStateDisplayPMReq:hsw */
- I915_WRITE(CHICKEN_PAR1_1,
- I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
-
lpt_init_clock_gating(dev_priv);
}
@@ -8708,8 +8702,6 @@ static void ivb_init_clock_gating(struct drm_i915_private *dev_priv)
{
uint32_t snpcr;
- ilk_init_lp_watermarks(dev_priv);
-
I915_WRITE(ILK_DSPCLK_GATE_D, ILK_VRHUNIT_CLOCK_GATE_DISABLE);
/* WaDisableEarlyCull:ivb */
--
2.13.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Move init_clock_gating() back to where it was (rev2)
2017-11-03 13:03 ` Ville Syrjala
` (6 preceding siblings ...)
(?)
@ 2017-11-08 15:31 ` Patchwork
-1 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2017-11-08 15:31 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Move init_clock_gating() back to where it was (rev2)
URL : https://patchwork.freedesktop.org/series/33124/
State : failure
== Summary ==
Series 33124v2 drm/i915: Move init_clock_gating() back to where it was
https://patchwork.freedesktop.org/api/1.0/series/33124/revisions/2/mbox/
Test chamelium:
Subgroup dp-crc-fast:
fail -> PASS (fi-kbl-7500u) fdo#102514
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> INCOMPLETE (fi-bxt-j4205)
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:446s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:457s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:381s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:528s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:273s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:497s
fi-bxt-j4205 total:217 pass:195 dwarn:0 dfail:0 fail:0 skip:21
fi-byt-j1900 total:289 pass:254 dwarn:0 dfail:0 fail:0 skip:35 time:491s
fi-byt-n2820 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:484s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:428s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:264s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:542s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:429s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:437s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:480s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:459s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:486s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:478s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:533s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:456s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:547s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:565s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:517s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:492s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:461s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:552s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:423s
Blacklisted hosts:
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:554s
748f2c6b4046b23a623b4af3799563ef3110bb0d drm-tip: 2017y-11m-08d-07h-50m-13s UTC integration manifest
b1eea3cd9403 drm/i915: Move init_clock_gating() back to where it was
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7013/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] drm/i915: Move init_clock_gating() back to where it was
2017-11-08 13:35 ` Ville Syrjala
@ 2017-11-08 16:18 ` Chris Wilson
-1 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2017-11-08 16:18 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
Cc: stable, Mark Janes, Maarten Lankhorst, Daniel Vetter,
Joonas Lahtinen, Oscar Mateo, Mika Kuoppala
Quoting Ville Syrjala (2017-11-08 13:35:55)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Apparently setting up a bunch of GT registers before we've properly
> initialized the rest of the GT hardware leads to these setting being
> lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> by doing init_clock_gating() too early. This should actually affect
> other platforms as well, but apparently not to such a great degree.
>
> What I was ultimately after in that commit was to move the
> ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> move init_clock_gating() back to where it was, and call
> ilk_init_lp_watermarks() just before the watermark state readout.
>
> This highlights how fragile and messed up our init order really is.
> I wonder why we even initialize the display before gem. The opposite
> order would make much more sense to me...
>
> v2: Keep WaRsPkgCStateDisplayPMReq:hsw early as it really must
> be done before all planes might get disabled.
>
> Cc: stable@vger.kernel.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mark Janes <mark.a.janes@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reported-by: Mark Janes <mark.a.janes@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
> Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
CTS remains fixed,
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
I know of no reason why this should work in this order,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
(but I didn't know about the v2 requirement either!)
-Chris
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] drm/i915: Move init_clock_gating() back to where it was
@ 2017-11-08 16:18 ` Chris Wilson
0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2017-11-08 16:18 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx; +Cc: Daniel Vetter, stable
Quoting Ville Syrjala (2017-11-08 13:35:55)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Apparently setting up a bunch of GT registers before we've properly
> initialized the rest of the GT hardware leads to these setting being
> lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> by doing init_clock_gating() too early. This should actually affect
> other platforms as well, but apparently not to such a great degree.
>
> What I was ultimately after in that commit was to move the
> ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> move init_clock_gating() back to where it was, and call
> ilk_init_lp_watermarks() just before the watermark state readout.
>
> This highlights how fragile and messed up our init order really is.
> I wonder why we even initialize the display before gem. The opposite
> order would make much more sense to me...
>
> v2: Keep WaRsPkgCStateDisplayPMReq:hsw early as it really must
> be done before all planes might get disabled.
>
> Cc: stable@vger.kernel.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mark Janes <mark.a.janes@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reported-by: Mark Janes <mark.a.janes@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
> Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
CTS remains fixed,
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
I know of no reason why this should work in this order,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
(but I didn't know about the v2 requirement either!)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Move init_clock_gating() back to where it was (rev2)
2017-11-03 13:03 ` Ville Syrjala
` (7 preceding siblings ...)
(?)
@ 2017-11-08 16:25 ` Patchwork
-1 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2017-11-08 16:25 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Move init_clock_gating() back to where it was (rev2)
URL : https://patchwork.freedesktop.org/series/33124/
State : success
== Summary ==
Series 33124v2 drm/i915: Move init_clock_gating() back to where it was
https://patchwork.freedesktop.org/api/1.0/series/33124/revisions/2/mbox/
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:439s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:448s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:377s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:537s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:274s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:496s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:501s
fi-byt-j1900 total:289 pass:254 dwarn:0 dfail:0 fail:0 skip:35 time:496s
fi-byt-n2820 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:491s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:431s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:262s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:539s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:428s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:438s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:429s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:480s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:460s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:483s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:523s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:474s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:533s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:569s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:457s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:546s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:570s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:521s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:494s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:457s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:557s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:421s
Blacklisted hosts:
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:566s
fi-glk-dsi total:289 pass:181 dwarn:1 dfail:4 fail:0 skip:103 time:356s
087c404bd6d56a52e0656ac7c79faa376c25b796 drm-tip: 2017y-11m-08d-15h-44m-06s UTC integration manifest
da17e32c5fc5 drm/i915: Move init_clock_gating() back to where it was
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7015/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] drm/i915: Move init_clock_gating() back to where it was
2017-11-08 16:18 ` Chris Wilson
(?)
@ 2017-11-08 16:32 ` Chris Wilson
-1 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2017-11-08 16:32 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
Cc: stable, Mark Janes, Maarten Lankhorst, Daniel Vetter,
Joonas Lahtinen, Oscar Mateo, Mika Kuoppala
Quoting Chris Wilson (2017-11-08 16:18:27)
> Quoting Ville Syrjala (2017-11-08 13:35:55)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Apparently setting up a bunch of GT registers before we've properly
> > initialized the rest of the GT hardware leads to these setting being
> > lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> > Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > by doing init_clock_gating() too early. This should actually affect
> > other platforms as well, but apparently not to such a great degree.
> >
> > What I was ultimately after in that commit was to move the
> > ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> > move init_clock_gating() back to where it was, and call
> > ilk_init_lp_watermarks() just before the watermark state readout.
> >
> > This highlights how fragile and messed up our init order really is.
> > I wonder why we even initialize the display before gem. The opposite
> > order would make much more sense to me...
> >
> > v2: Keep WaRsPkgCStateDisplayPMReq:hsw early as it really must
> > be done before all planes might get disabled.
> >
> > Cc: stable@vger.kernel.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mark Janes <mark.a.janes@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Oscar Mateo <oscar.mateo@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Reported-by: Mark Janes <mark.a.janes@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
> > Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> CTS remains fixed,
> Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> I know of no reason why this should work in this order,
s/should work/should not/work ;)
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> (but I didn't know about the v2 requirement either!)
> -Chris
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] drm/i915: Move init_clock_gating() back to where it was
2017-11-08 16:18 ` Chris Wilson
@ 2017-11-08 17:38 ` Ville Syrjälä
-1 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2017-11-08 17:38 UTC (permalink / raw)
To: Chris Wilson
Cc: intel-gfx, stable, Mark Janes, Maarten Lankhorst, Daniel Vetter,
Joonas Lahtinen, Oscar Mateo, Mika Kuoppala
On Wed, Nov 08, 2017 at 04:18:27PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-08 13:35:55)
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> >
> > Apparently setting up a bunch of GT registers before we've properly
> > initialized the rest of the GT hardware leads to these setting being
> > lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> > Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > by doing init_clock_gating() too early. This should actually affect
> > other platforms as well, but apparently not to such a great degree.
> >
> > What I was ultimately after in that commit was to move the
> > ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> > move init_clock_gating() back to where it was, and call
> > ilk_init_lp_watermarks() just before the watermark state readout.
> >
> > This highlights how fragile and messed up our init order really is.
> > I wonder why we even initialize the display before gem. The opposite
> > order would make much more sense to me...
> >
> > v2: Keep WaRsPkgCStateDisplayPMReq:hsw early as it really must
> > be done before all planes might get disabled.
> >
> > Cc: stable@vger.kernel.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mark Janes <mark.a.janes@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Oscar Mateo <oscar.mateo@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Reported-by: Mark Janes <mark.a.janes@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
> > Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
> > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
>
> CTS remains fixed,
> Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> I know of no reason why this should work in this order,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> (but I didn't know about the v2 requirement either!)
It's somewhat baffling how this stuff managed to work before I
moved .init_clock_gating() to happen earlier. AFAICS that plane
disabling vs. the chicken bit should have bitten us back then as
well.
Patch pushed to dinq. Thanks for doing all the heavy lifting on
this one.
--
Ville Syrj�l�
Intel OTC
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] drm/i915: Move init_clock_gating() back to where it was
@ 2017-11-08 17:38 ` Ville Syrjälä
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2017-11-08 17:38 UTC (permalink / raw)
To: Chris Wilson
Cc: intel-gfx, stable, Mark Janes, Maarten Lankhorst, Daniel Vetter,
Joonas Lahtinen, Oscar Mateo, Mika Kuoppala
On Wed, Nov 08, 2017 at 04:18:27PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-08 13:35:55)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Apparently setting up a bunch of GT registers before we've properly
> > initialized the rest of the GT hardware leads to these setting being
> > lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
> > Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > by doing init_clock_gating() too early. This should actually affect
> > other platforms as well, but apparently not to such a great degree.
> >
> > What I was ultimately after in that commit was to move the
> > ilk_init_lp_watermarks() call earlier. So let's undo the damage and
> > move init_clock_gating() back to where it was, and call
> > ilk_init_lp_watermarks() just before the watermark state readout.
> >
> > This highlights how fragile and messed up our init order really is.
> > I wonder why we even initialize the display before gem. The opposite
> > order would make much more sense to me...
> >
> > v2: Keep WaRsPkgCStateDisplayPMReq:hsw early as it really must
> > be done before all planes might get disabled.
> >
> > Cc: stable@vger.kernel.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mark Janes <mark.a.janes@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Oscar Mateo <oscar.mateo@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Reported-by: Mark Janes <mark.a.janes@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
> > Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> > References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> CTS remains fixed,
> Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> I know of no reason why this should work in this order,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> (but I didn't know about the v2 requirement either!)
It's somewhat baffling how this stuff managed to work before I
moved .init_clock_gating() to happen earlier. AFAICS that plane
disabling vs. the chicken bit should have bitten us back then as
well.
Patch pushed to dinq. Thanks for doing all the heavy lifting on
this one.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Move init_clock_gating() back to where it was (rev2)
2017-11-03 13:03 ` Ville Syrjala
` (8 preceding siblings ...)
(?)
@ 2017-11-08 19:59 ` Patchwork
-1 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2017-11-08 19:59 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Move init_clock_gating() back to where it was (rev2)
URL : https://patchwork.freedesktop.org/series/33124/
State : success
== Summary ==
Test kms_flip:
Subgroup plain-flip-fb-recreate-interruptible:
pass -> SKIP (shard-hsw) fdo#100368
Test perf:
Subgroup blocking:
fail -> PASS (shard-hsw) fdo#102252 +1
Test kms_busy:
Subgroup extended-modeset-hang-oldfb-with-reset-render-b:
pass -> DMESG-WARN (shard-hsw) fdo#102249
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
shard-hsw total:2540 pass:1430 dwarn:2 dfail:0 fail:10 skip:1098 time:9220s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7015/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-11-08 19:59 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-03 13:03 [PATCH] drm/i915: Move init_clock_gating() back to where it was Ville Syrjala
2017-11-03 13:03 ` Ville Syrjala
2017-11-03 13:26 ` ✗ Fi.CI.BAT: failure for " Patchwork
2017-11-03 13:27 ` [PATCH] " Chris Wilson
2017-11-03 13:27 ` Chris Wilson
2017-11-03 14:20 ` Ville Syrjälä
2017-11-03 14:20 ` Ville Syrjälä
2017-11-03 14:41 ` Chris Wilson
2017-11-03 14:58 ` Chris Wilson
2017-11-03 17:05 ` ✗ Fi.CI.BAT: failure for " Patchwork
2017-11-03 17:08 ` Chris Wilson
2017-11-03 17:37 ` Ville Syrjälä
2017-11-03 18:20 ` Patchwork
2017-11-08 13:35 ` [PATCH v2] " Ville Syrjala
2017-11-08 13:35 ` Ville Syrjala
2017-11-08 16:18 ` Chris Wilson
2017-11-08 16:18 ` Chris Wilson
2017-11-08 16:32 ` Chris Wilson
2017-11-08 17:38 ` Ville Syrjälä
2017-11-08 17:38 ` Ville Syrjälä
2017-11-08 15:31 ` ✗ Fi.CI.BAT: failure for drm/i915: Move init_clock_gating() back to where it was (rev2) Patchwork
2017-11-08 16:25 ` ✓ Fi.CI.BAT: success " Patchwork
2017-11-08 19:59 ` ✓ Fi.CI.IGT: " Patchwork
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.