* [PATCH 1/2] drm/i915: Split GEM resetting into 3 phases
2017-03-13 16:14 Fixes that failed to backport to v4.11-rc1 Jani Nikula
@ 2017-03-13 16:59 ` Chris Wilson
2017-03-13 16:59 ` [PATCH 2/2] drm/i915: Disable engine->irq_tasklet around resets Chris Wilson
2017-03-13 17:02 ` [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled Chris Wilson
` (5 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: Chris Wilson @ 2017-03-13 16:59 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Mika Kuoppala
Currently we do a reset prepare/finish around the call to reset the GPU,
but it looks like we need a later stage after the hw has been
reinitialised to allow GEM to restart itself. Start by splitting the 2
GEM phases into 3:
prepare - before the reset, check if GEM recovered, then stop GEM
reset - after the reset, update GEM bookkeeping
finish - after the re-initialisation following the reset, restart GEM
Cherry-pick: d80270931314 ("drm/i915: Split GEM resetting into 3 phases")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170208143033.11651-2-chris@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_drv.c | 3 ++-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 7 ++++++-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e703556eba99..2093d203665d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1788,7 +1788,7 @@ void i915_reset(struct drm_i915_private *dev_priv)
goto error;
}
- i915_gem_reset_finish(dev_priv);
+ i915_gem_reset(dev_priv);
intel_overlay_reset(dev_priv);
/* Ok, now get things going again... */
@@ -1811,6 +1811,7 @@ void i915_reset(struct drm_i915_private *dev_priv)
goto error;
}
+ i915_gem_reset_finish(dev_priv);
i915_queue_hangcheck(dev_priv);
wakeup:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7febe6eecf72..1d20c2d00f42 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3342,6 +3342,7 @@ static inline u32 i915_reset_count(struct i915_gpu_error *error)
}
int i915_gem_reset_prepare(struct drm_i915_private *dev_priv);
+void i915_gem_reset(struct drm_i915_private *dev_priv);
void i915_gem_reset_finish(struct drm_i915_private *dev_priv);
void i915_gem_set_wedged(struct drm_i915_private *dev_priv);
void i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 10777da73039..27fe5a9315f0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2834,7 +2834,7 @@ static void i915_gem_reset_engine(struct intel_engine_cs *engine)
engine->reset_hw(engine, request);
}
-void i915_gem_reset_finish(struct drm_i915_private *dev_priv)
+void i915_gem_reset(struct drm_i915_private *dev_priv)
{
struct intel_engine_cs *engine;
enum intel_engine_id id;
@@ -2856,6 +2856,11 @@ void i915_gem_reset_finish(struct drm_i915_private *dev_priv)
}
}
+void i915_gem_reset_finish(struct drm_i915_private *dev_priv)
+{
+ lockdep_assert_held(&dev_priv->drm.struct_mutex);
+}
+
static void nop_submit_request(struct drm_i915_gem_request *request)
{
dma_fence_set_error(&request->fence, -EIO);
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 2/2] drm/i915: Disable engine->irq_tasklet around resets
2017-03-13 16:59 ` [PATCH 1/2] drm/i915: Split GEM resetting into 3 phases Chris Wilson
@ 2017-03-13 16:59 ` Chris Wilson
0 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2017-03-13 16:59 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Mika Kuoppala
When we restart the engines, and we have active requests, a request on
the first engine may complete and queue a request to the second engine
before we try to restart the second engine. That queueing of the
request may race with the engine to restart, and so may corrupt the
current state. Disabling the engine->irq_tasklet prevents the two paths
from writing into ELSP simultaneously (and modifyin the execlists_port[]
at the same time).
Include fixup 1d309634bcf4 ("drm/i915: Kill the tasklet then disable")
Cherry-pick: 1f7b847d72c3 ("drm/i915: Disable engine->irq_tasklet around resets")
Fixes: 821ed7df6e2a ("drm/i915: Update reset path to fix incomplete requests")
Testcase: igt/gem_exec_fence/await-hang
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170208143033.11651-3-chris@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_gem.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 27fe5a9315f0..1051fdf37d20 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2719,7 +2719,16 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
for_each_engine(engine, dev_priv, id) {
struct drm_i915_gem_request *request;
+ /* Prevent request submission to the hardware until we have
+ * completed the reset in i915_gem_reset_finish(). If a request
+ * is completed by one engine, it may then queue a request
+ * to a second via its engine->irq_tasklet *just* as we are
+ * calling engine->init_hw() and also writing the ELSP.
+ * Turning off the engine->irq_tasklet until the reset is over
+ * prevents the race.
+ */
tasklet_kill(&engine->irq_tasklet);
+ tasklet_disable(&engine->irq_tasklet);
if (engine_stalled(engine)) {
request = i915_gem_find_active_request(engine);
@@ -2858,7 +2867,13 @@ void i915_gem_reset(struct drm_i915_private *dev_priv)
void i915_gem_reset_finish(struct drm_i915_private *dev_priv)
{
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+
lockdep_assert_held(&dev_priv->drm.struct_mutex);
+
+ for_each_engine(engine, dev_priv, id)
+ tasklet_enable(&engine->irq_tasklet);
}
static void nop_submit_request(struct drm_i915_gem_request *request)
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
2017-03-13 16:14 Fixes that failed to backport to v4.11-rc1 Jani Nikula
2017-03-13 16:59 ` [PATCH 1/2] drm/i915: Split GEM resetting into 3 phases Chris Wilson
@ 2017-03-13 17:02 ` Chris Wilson
2017-03-15 20:39 ` Lyude Paul
2017-03-13 17:04 ` [PATCH] drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters Chris Wilson
` (4 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: Chris Wilson @ 2017-03-13 17:02 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Daniel Vetter, stable, Hans de Goede
In order to prevent accessing the hpd registers outside of the display
power wells, we should refrain from writing to the registers before the
display interrupts are enabled.
[ 4.740136] WARNING: CPU: 1 PID: 221 at drivers/gpu/drm/i915/intel_uncore.c:795 __unclaimed_reg_debug+0x44/0x50 [i915]
[ 4.740155] Unclaimed read from register 0x1e1110
[ 4.740168] Modules linked in: i915(+) intel_gtt drm_kms_helper prime_numbers
[ 4.740190] CPU: 1 PID: 221 Comm: systemd-udevd Not tainted 4.10.0-rc6+ #384
[ 4.740203] Hardware name: / , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[ 4.740220] Call Trace:
[ 4.740236] dump_stack+0x4d/0x6f
[ 4.740251] __warn+0xc1/0xe0
[ 4.740265] warn_slowpath_fmt+0x4a/0x50
[ 4.740281] ? insert_work+0x77/0xc0
[ 4.740355] ? fwtable_write32+0x90/0x130 [i915]
[ 4.740431] __unclaimed_reg_debug+0x44/0x50 [i915]
[ 4.740507] fwtable_read32+0xd8/0x130 [i915]
[ 4.740575] i915_hpd_irq_setup+0xa5/0x100 [i915]
[ 4.740649] intel_hpd_init+0x68/0x80 [i915]
[ 4.740716] i915_driver_load+0xe19/0x1380 [i915]
[ 4.740784] i915_pci_probe+0x32/0x90 [i915]
[ 4.740799] pci_device_probe+0x8b/0xf0
[ 4.740815] driver_probe_device+0x2b6/0x450
[ 4.740828] __driver_attach+0xda/0xe0
[ 4.740841] ? driver_probe_device+0x450/0x450
[ 4.740853] bus_for_each_dev+0x5b/0x90
[ 4.740865] driver_attach+0x19/0x20
[ 4.740878] bus_add_driver+0x166/0x260
[ 4.740892] driver_register+0x5b/0xd0
[ 4.740906] ? 0xffffffffa0166000
[ 4.740920] __pci_register_driver+0x47/0x50
[ 4.740985] i915_init+0x5c/0x5e [i915]
[ 4.740999] do_one_initcall+0x3e/0x160
[ 4.741015] ? __vunmap+0x7c/0xc0
[ 4.741029] ? kmem_cache_alloc+0xcf/0x120
[ 4.741045] do_init_module+0x55/0x1c4
[ 4.741060] load_module+0x1f3f/0x25b0
[ 4.741073] ? __symbol_put+0x40/0x40
[ 4.741086] ? kernel_read_file+0x100/0x190
[ 4.741100] SYSC_finit_module+0xbc/0xf0
[ 4.741112] SyS_finit_module+0x9/0x10
[ 4.741125] entry_SYSCALL_64_fastpath+0x17/0x98
[ 4.741135] RIP: 0033:0x7f8559a140f9
[ 4.741145] RSP: 002b:00007fff7509a3e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[ 4.741161] RAX: ffffffffffffffda RBX: 00007f855aba02d1 RCX: 00007f8559a140f9
[ 4.741172] RDX: 0000000000000000 RSI: 000055b6db0914f0 RDI: 0000000000000011
[ 4.741183] RBP: 0000000000020000 R08: 0000000000000000 R09: 000000000000000e
[ 4.741193] R10: 0000000000000011 R11: 0000000000000246 R12: 000055b6db0854d0
[ 4.741204] R13: 000055b6db091150 R14: 0000000000000000 R15: 000055b6db035924
v2: Set dev_priv->display_irqs_enabled to true for all platforms other
than vlv/chv that manually control the display power domain.
Cherry-pick: 262fd485ac6b ("drm/i915: Only enable hotplug interrupts if the display interrupts are enabled")
Fixes: 19625e85c6ec ("drm/i915: Enable polling when we don't have hpd")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97798
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lyude <cpaul@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
Cc: stable@vger.kernel.org
Link: http://patchwork.freedesktop.org/patch/msgid/20170215131547.5064-1-chris@chris-wilson.co.uk
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++
drivers/gpu/drm/i915/intel_hotplug.c | 14 ++++++++------
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e6ffef2f707a..4fc8973744b4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4266,6 +4266,16 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
if (!IS_GEN2(dev_priv))
dev->vblank_disable_immediate = true;
+ /* Most platforms treat the display irq block as an always-on
+ * power domain. vlv/chv can disable it at runtime and need
+ * special care to avoid writing any of the display block registers
+ * outside of the power domain. We defer setting up the display irqs
+ * in this case to the runtime pm.
+ */
+ dev_priv->display_irqs_enabled = true;
+ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+ dev_priv->display_irqs_enabled = false;
+
dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index b62e3f8ad415..54208bef7a83 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -219,7 +219,7 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
}
}
}
- if (dev_priv->display.hpd_irq_setup)
+ if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup)
dev_priv->display.hpd_irq_setup(dev_priv);
spin_unlock_irq(&dev_priv->irq_lock);
@@ -425,7 +425,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
}
}
- if (storm_detected)
+ if (storm_detected && dev_priv->display_irqs_enabled)
dev_priv->display.hpd_irq_setup(dev_priv);
spin_unlock(&dev_priv->irq_lock);
@@ -471,10 +471,12 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
* Interrupt setup is already guaranteed to be single-threaded, this is
* just to make the assert_spin_locked checks happy.
*/
- spin_lock_irq(&dev_priv->irq_lock);
- if (dev_priv->display.hpd_irq_setup)
- dev_priv->display.hpd_irq_setup(dev_priv);
- spin_unlock_irq(&dev_priv->irq_lock);
+ if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
+ spin_lock_irq(&dev_priv->irq_lock);
+ if (dev_priv->display_irqs_enabled)
+ dev_priv->display.hpd_irq_setup(dev_priv);
+ spin_unlock_irq(&dev_priv->irq_lock);
+ }
}
static void i915_hpd_poll_init_work(struct work_struct *work)
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled
2017-03-13 17:02 ` [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled Chris Wilson
@ 2017-03-15 20:39 ` Lyude Paul
0 siblings, 0 replies; 28+ messages in thread
From: Lyude Paul @ 2017-03-15 20:39 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: jani.nikula, Daniel Vetter, stable, Hans de Goede
Acked-by: Lyude <lyude@redhat.com>
On Mon, 2017-03-13 at 17:02 +0000, Chris Wilson wrote:
> In order to prevent accessing the hpd registers outside of the
> display
> power wells, we should refrain from writing to the registers before
> the
> display interrupts are enabled.
>
> [ 4.740136] WARNING: CPU: 1 PID: 221 at
> drivers/gpu/drm/i915/intel_uncore.c:795
> __unclaimed_reg_debug+0x44/0x50 [i915]
> [ 4.740155] Unclaimed read from register 0x1e1110
> [ 4.740168] Modules linked in: i915(+) intel_gtt drm_kms_helper
> prime_numbers
> [ 4.740190] CPU: 1 PID: 221 Comm: systemd-udevd Not tainted
> 4.10.0-rc6+ #384
> [ 4.740203] Hardware name: / , BIOS
> PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> [ 4.740220] Call Trace:
> [ 4.740236] dump_stack+0x4d/0x6f
> [ 4.740251] __warn+0xc1/0xe0
> [ 4.740265] warn_slowpath_fmt+0x4a/0x50
> [ 4.740281] ? insert_work+0x77/0xc0
> [ 4.740355] ? fwtable_write32+0x90/0x130 [i915]
> [ 4.740431] __unclaimed_reg_debug+0x44/0x50 [i915]
> [ 4.740507] fwtable_read32+0xd8/0x130 [i915]
> [ 4.740575] i915_hpd_irq_setup+0xa5/0x100 [i915]
> [ 4.740649] intel_hpd_init+0x68/0x80 [i915]
> [ 4.740716] i915_driver_load+0xe19/0x1380 [i915]
> [ 4.740784] i915_pci_probe+0x32/0x90 [i915]
> [ 4.740799] pci_device_probe+0x8b/0xf0
> [ 4.740815] driver_probe_device+0x2b6/0x450
> [ 4.740828] __driver_attach+0xda/0xe0
> [ 4.740841] ? driver_probe_device+0x450/0x450
> [ 4.740853] bus_for_each_dev+0x5b/0x90
> [ 4.740865] driver_attach+0x19/0x20
> [ 4.740878] bus_add_driver+0x166/0x260
> [ 4.740892] driver_register+0x5b/0xd0
> [ 4.740906] ? 0xffffffffa0166000
> [ 4.740920] __pci_register_driver+0x47/0x50
> [ 4.740985] i915_init+0x5c/0x5e [i915]
> [ 4.740999] do_one_initcall+0x3e/0x160
> [ 4.741015] ? __vunmap+0x7c/0xc0
> [ 4.741029] ? kmem_cache_alloc+0xcf/0x120
> [ 4.741045] do_init_module+0x55/0x1c4
> [ 4.741060] load_module+0x1f3f/0x25b0
> [ 4.741073] ? __symbol_put+0x40/0x40
> [ 4.741086] ? kernel_read_file+0x100/0x190
> [ 4.741100] SYSC_finit_module+0xbc/0xf0
> [ 4.741112] SyS_finit_module+0x9/0x10
> [ 4.741125] entry_SYSCALL_64_fastpath+0x17/0x98
> [ 4.741135] RIP: 0033:0x7f8559a140f9
> [ 4.741145] RSP: 002b:00007fff7509a3e8 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000139
> [ 4.741161] RAX: ffffffffffffffda RBX: 00007f855aba02d1 RCX:
> 00007f8559a140f9
> [ 4.741172] RDX: 0000000000000000 RSI: 000055b6db0914f0 RDI:
> 0000000000000011
> [ 4.741183] RBP: 0000000000020000 R08: 0000000000000000 R09:
> 000000000000000e
> [ 4.741193] R10: 0000000000000011 R11: 0000000000000246 R12:
> 000055b6db0854d0
> [ 4.741204] R13: 000055b6db091150 R14: 0000000000000000 R15:
> 000055b6db035924
>
> v2: Set dev_priv->display_irqs_enabled to true for all platforms
> other
> than vlv/chv that manually control the display power domain.
>
> Cherry-pick: 262fd485ac6b ("drm/i915: Only enable hotplug interrupts
> if the display interrupts are enabled")
> Fixes: 19625e85c6ec ("drm/i915: Enable polling when we don't have
> hpd")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97798
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> Cc: stable@vger.kernel.org
> Link: http://patchwork.freedesktop.org/patch/msgid/20170215131547.506
> 4-1-chris@chris-wilson.co.uk
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++
> drivers/gpu/drm/i915/intel_hotplug.c | 14 ++++++++------
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index e6ffef2f707a..4fc8973744b4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4266,6 +4266,16 @@ void intel_irq_init(struct drm_i915_private
> *dev_priv)
> if (!IS_GEN2(dev_priv))
> dev->vblank_disable_immediate = true;
>
> + /* Most platforms treat the display irq block as an always-
> on
> + * power domain. vlv/chv can disable it at runtime and need
> + * special care to avoid writing any of the display block
> registers
> + * outside of the power domain. We defer setting up the
> display irqs
> + * in this case to the runtime pm.
> + */
> + dev_priv->display_irqs_enabled = true;
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + dev_priv->display_irqs_enabled = false;
> +
> dev->driver->get_vblank_timestamp =
> i915_get_vblank_timestamp;
> dev->driver->get_scanout_position =
> i915_get_crtc_scanoutpos;
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index b62e3f8ad415..54208bef7a83 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -219,7 +219,7 @@ static void
> intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> }
> }
> }
> - if (dev_priv->display.hpd_irq_setup)
> + if (dev_priv->display_irqs_enabled && dev_priv-
> >display.hpd_irq_setup)
> dev_priv->display.hpd_irq_setup(dev_priv);
> spin_unlock_irq(&dev_priv->irq_lock);
>
> @@ -425,7 +425,7 @@ void intel_hpd_irq_handler(struct
> drm_i915_private *dev_priv,
> }
> }
>
> - if (storm_detected)
> + if (storm_detected && dev_priv->display_irqs_enabled)
> dev_priv->display.hpd_irq_setup(dev_priv);
> spin_unlock(&dev_priv->irq_lock);
>
> @@ -471,10 +471,12 @@ void intel_hpd_init(struct drm_i915_private
> *dev_priv)
> * Interrupt setup is already guaranteed to be single-
> threaded, this is
> * just to make the assert_spin_locked checks happy.
> */
> - spin_lock_irq(&dev_priv->irq_lock);
> - if (dev_priv->display.hpd_irq_setup)
> - dev_priv->display.hpd_irq_setup(dev_priv);
> - spin_unlock_irq(&dev_priv->irq_lock);
> + if (dev_priv->display_irqs_enabled && dev_priv-
> >display.hpd_irq_setup) {
> + spin_lock_irq(&dev_priv->irq_lock);
> + if (dev_priv->display_irqs_enabled)
> + dev_priv->display.hpd_irq_setup(dev_priv);
> + spin_unlock_irq(&dev_priv->irq_lock);
> + }
> }
>
> static void i915_hpd_poll_init_work(struct work_struct *work)
--
Cheers,
Lyude
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters.
2017-03-13 16:14 Fixes that failed to backport to v4.11-rc1 Jani Nikula
2017-03-13 16:59 ` [PATCH 1/2] drm/i915: Split GEM resetting into 3 phases Chris Wilson
2017-03-13 17:02 ` [PATCH] drm/i915: Only enable hotplug interrupts if the display interrupts are enabled Chris Wilson
@ 2017-03-13 17:04 ` Chris Wilson
2017-03-13 17:06 ` [PATCH] drm/i915: Stop using RP_DOWN_EI on Baytrail Chris Wilson
` (3 subsequent siblings)
6 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2017-03-13 17:04 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Kenneth Graunke, stable, Chris Wilson
From: Kenneth Graunke <kenneth@whitecape.org>
This patch makes the I915_PARAM_HAS_EXEC_CONSTANTS getparam return 0
(indicating the optional feature is not supported), and makes execbuf
always return -EINVAL if the flags are used.
Apparently, no userspace ever shipped which used this optional feature:
I checked the git history of Mesa, xf86-video-intel, libva, and Beignet,
and there were zero commits showing a use of these flags. Kernel commit
72bfa19c8deb4 apparently introduced the feature prematurely. According
to Chris, the intention was to use this in cairo-drm, but "the use was
broken for gen6", so I don't think it ever happened.
'relative_constants_mode' has always been tracked per-device, but this
has actually been wrong ever since hardware contexts were introduced, as
the INSTPM register is saved (and automatically restored) as part of the
render ring context. The software per-device value could therefore get
out of sync with the hardware per-context value. This meant that using
them is actually unsafe: a client which tried to use them could damage
the state of other clients, causing the GPU to interpret their BO
offsets as absolute pointers, leading to bogus memory reads.
These flags were also never ported to execlist mode, making them no-ops
on Gen9+ (which requires execlists), and Gen8 in the default mode.
On Gen8+, userspace can write these registers directly, achieving the
same effect. On Gen6-7.5, it likely makes sense to extend the command
parser to support them. I don't think anyone wants this on Gen4-5.
Based on a patch by Dave Gordon.
v3: Return -ENODEV for the getparam, as this is what we do for other
obsolete features. Suggested by Chris Wilson.
Cherry-pick: ef0f411f5147 ("drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters.")
Cc: stable@vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92448
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/20170215093446.21291-1-kenneth@whitecape.org
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_drv.c | 4 +--
drivers/gpu/drm/i915/i915_drv.h | 2 --
drivers/gpu/drm/i915/i915_gem.c | 2 --
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 52 ++----------------------------
4 files changed, 3 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2093d203665d..6cd78bb2064d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -248,6 +248,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_IRQ_ACTIVE:
case I915_PARAM_ALLOW_BATCHBUFFER:
case I915_PARAM_LAST_DISPATCH:
+ case I915_PARAM_HAS_EXEC_CONSTANTS:
/* Reject all old ums/dri params. */
return -ENODEV;
case I915_PARAM_CHIPSET_ID:
@@ -274,9 +275,6 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_HAS_BSD2:
value = !!dev_priv->engine[VCS2];
break;
- case I915_PARAM_HAS_EXEC_CONSTANTS:
- value = INTEL_GEN(dev_priv) >= 4;
- break;
case I915_PARAM_HAS_LLC:
value = HAS_LLC(dev_priv);
break;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1d20c2d00f42..80be09831a52 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2064,8 +2064,6 @@ struct drm_i915_private {
const struct intel_device_info info;
- int relative_constants_mode;
-
void __iomem *regs;
struct intel_uncore uncore;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1051fdf37d20..67b1fc5a0331 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4694,8 +4694,6 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
init_waitqueue_head(&dev_priv->gpu_error.wait_queue);
init_waitqueue_head(&dev_priv->gpu_error.reset_queue);
- dev_priv->relative_constants_mode = I915_EXEC_CONSTANTS_REL_GENERAL;
-
init_waitqueue_head(&dev_priv->pending_flip_queue);
dev_priv->mm.interruptible = true;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d02cfaefe1c8..30e0675fd7da 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1408,10 +1408,7 @@ execbuf_submit(struct i915_execbuffer_params *params,
struct drm_i915_gem_execbuffer2 *args,
struct list_head *vmas)
{
- struct drm_i915_private *dev_priv = params->request->i915;
u64 exec_start, exec_len;
- int instp_mode;
- u32 instp_mask;
int ret;
ret = i915_gem_execbuffer_move_to_gpu(params->request, vmas);
@@ -1422,56 +1419,11 @@ execbuf_submit(struct i915_execbuffer_params *params,
if (ret)
return ret;
- instp_mode = args->flags & I915_EXEC_CONSTANTS_MASK;
- instp_mask = I915_EXEC_CONSTANTS_MASK;
- switch (instp_mode) {
- case I915_EXEC_CONSTANTS_REL_GENERAL:
- case I915_EXEC_CONSTANTS_ABSOLUTE:
- case I915_EXEC_CONSTANTS_REL_SURFACE:
- if (instp_mode != 0 && params->engine->id != RCS) {
- DRM_DEBUG("non-0 rel constants mode on non-RCS\n");
- return -EINVAL;
- }
-
- if (instp_mode != dev_priv->relative_constants_mode) {
- if (INTEL_INFO(dev_priv)->gen < 4) {
- DRM_DEBUG("no rel constants on pre-gen4\n");
- return -EINVAL;
- }
-
- if (INTEL_INFO(dev_priv)->gen > 5 &&
- instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) {
- DRM_DEBUG("rel surface constants mode invalid on gen5+\n");
- return -EINVAL;
- }
-
- /* The HW changed the meaning on this bit on gen6 */
- if (INTEL_INFO(dev_priv)->gen >= 6)
- instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE;
- }
- break;
- default:
- DRM_DEBUG("execbuf with unknown constants: %d\n", instp_mode);
+ if (args->flags & I915_EXEC_CONSTANTS_MASK) {
+ DRM_DEBUG("I915_EXEC_CONSTANTS_* unsupported\n");
return -EINVAL;
}
- if (params->engine->id == RCS &&
- instp_mode != dev_priv->relative_constants_mode) {
- struct intel_ring *ring = params->request->ring;
-
- ret = intel_ring_begin(params->request, 4);
- if (ret)
- return ret;
-
- intel_ring_emit(ring, MI_NOOP);
- intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
- intel_ring_emit_reg(ring, INSTPM);
- intel_ring_emit(ring, instp_mask << 16 | instp_mode);
- intel_ring_advance(ring);
-
- dev_priv->relative_constants_mode = instp_mode;
- }
-
if (args->flags & I915_EXEC_GEN7_SOL_RESET) {
ret = i915_reset_gen7_sol_offsets(params->request);
if (ret)
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH] drm/i915: Stop using RP_DOWN_EI on Baytrail
2017-03-13 16:14 Fixes that failed to backport to v4.11-rc1 Jani Nikula
` (2 preceding siblings ...)
2017-03-13 17:04 ` [PATCH] drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters Chris Wilson
@ 2017-03-13 17:06 ` Chris Wilson
2017-03-13 21:23 ` Fixes that failed to backport to v4.11-rc1 Chris Wilson
` (2 subsequent siblings)
6 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2017-03-13 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Chris Wilson, Mika Kuoppala, # v4 . 1+
On Baytrail, we manually calculate busyness over the evaluation interval
to avoid issues with miscaluations with RC6 enabled. However, it turns
out that the DOWN_EI interrupt generator is completely bust - it
operates in two modes, continuous or never. Neither of which are
conducive to good behaviour. Stop unmask the DOWN_EI interrupt and just
compute everything from the UP_EI which does seem to correspond to the
desired interval.
v2: Fixup gen6_rps_pm_mask() as well
v3: Inline vlv_c0_above() to combine the now identical elapsed
calculation for up/down and simplify the threshold testing
Cherry-pick: e0e8c7cb6eb6 ("drm/i915: Stop using RP_DOWN_EI on Baytrail")
Fixes: 43cf3bf084ba ("drm/i915: Improved w/a for rps on Baytrail")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: <stable@vger.kernel.org> # v4.1+
Link: http://patchwork.freedesktop.org/patch/msgid/20170309211232.28878-1-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_irq.c | 73 ++++++++++++++++-------------------------
drivers/gpu/drm/i915/intel_pm.c | 5 +--
3 files changed, 32 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 80be09831a52..1e53c31b6826 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1325,7 +1325,7 @@ struct intel_gen6_power_mgmt {
unsigned boosts;
/* manual wa residency calculations */
- struct intel_rps_ei up_ei, down_ei;
+ struct intel_rps_ei ei;
/*
* Protects RPS/RC6 register access and PCU communication.
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4fc8973744b4..b6c886ac901b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1046,68 +1046,51 @@ static void vlv_c0_read(struct drm_i915_private *dev_priv,
ei->media_c0 = I915_READ(VLV_MEDIA_C0_COUNT);
}
-static bool vlv_c0_above(struct drm_i915_private *dev_priv,
- const struct intel_rps_ei *old,
- const struct intel_rps_ei *now,
- int threshold)
-{
- u64 time, c0;
- unsigned int mul = 100;
-
- if (old->cz_clock == 0)
- return false;
-
- if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
- mul <<= 8;
-
- time = now->cz_clock - old->cz_clock;
- time *= threshold * dev_priv->czclk_freq;
-
- /* Workload can be split between render + media, e.g. SwapBuffers
- * being blitted in X after being rendered in mesa. To account for
- * this we need to combine both engines into our activity counter.
- */
- c0 = now->render_c0 - old->render_c0;
- c0 += now->media_c0 - old->media_c0;
- c0 *= mul * VLV_CZ_CLOCK_TO_MILLI_SEC;
-
- return c0 >= time;
-}
-
void gen6_rps_reset_ei(struct drm_i915_private *dev_priv)
{
- vlv_c0_read(dev_priv, &dev_priv->rps.down_ei);
- dev_priv->rps.up_ei = dev_priv->rps.down_ei;
+ memset(&dev_priv->rps.ei, 0, sizeof(dev_priv->rps.ei));
}
static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir)
{
+ const struct intel_rps_ei *prev = &dev_priv->rps.ei;
struct intel_rps_ei now;
u32 events = 0;
- if ((pm_iir & (GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED)) == 0)
+ if ((pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) == 0)
return 0;
vlv_c0_read(dev_priv, &now);
if (now.cz_clock == 0)
return 0;
- if (pm_iir & GEN6_PM_RP_DOWN_EI_EXPIRED) {
- if (!vlv_c0_above(dev_priv,
- &dev_priv->rps.down_ei, &now,
- dev_priv->rps.down_threshold))
- events |= GEN6_PM_RP_DOWN_THRESHOLD;
- dev_priv->rps.down_ei = now;
- }
+ if (prev->cz_clock) {
+ u64 time, c0;
+ unsigned int mul;
- if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) {
- if (vlv_c0_above(dev_priv,
- &dev_priv->rps.up_ei, &now,
- dev_priv->rps.up_threshold))
- events |= GEN6_PM_RP_UP_THRESHOLD;
- dev_priv->rps.up_ei = now;
+ mul = VLV_CZ_CLOCK_TO_MILLI_SEC * 100; /* scale to threshold% */
+ if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
+ mul <<= 8;
+
+ time = now.cz_clock - prev->cz_clock;
+ time *= dev_priv->czclk_freq;
+
+ /* Workload can be split between render + media,
+ * e.g. SwapBuffers being blitted in X after being rendered in
+ * mesa. To account for this we need to combine both engines
+ * into our activity counter.
+ */
+ c0 = now.render_c0 - prev->render_c0;
+ c0 += now.media_c0 - prev->media_c0;
+ c0 *= mul;
+
+ if (c0 > time * dev_priv->rps.up_threshold)
+ events = GEN6_PM_RP_UP_THRESHOLD;
+ else if (c0 < time * dev_priv->rps.down_threshold)
+ events = GEN6_PM_RP_DOWN_THRESHOLD;
}
+ dev_priv->rps.ei = now;
return events;
}
@@ -4228,7 +4211,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
/* Let's track the enabled rps events */
if (IS_VALLEYVIEW(dev_priv))
/* WaGsvRC0ResidencyMethod:vlv */
- dev_priv->pm_rps_events = GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED;
+ dev_priv->pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED;
else
dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 940bab22d464..6a29784d2b41 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4928,8 +4928,9 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val)
{
u32 mask = 0;
+ /* We use UP_EI_EXPIRED interupts for both up/down in manual mode */
if (val > dev_priv->rps.min_freq_softlimit)
- mask |= GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT;
+ mask |= GEN6_PM_RP_UP_EI_EXPIRED | GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT;
if (val < dev_priv->rps.max_freq_softlimit)
mask |= GEN6_PM_RP_UP_EI_EXPIRED | GEN6_PM_RP_UP_THRESHOLD;
@@ -5039,7 +5040,7 @@ void gen6_rps_busy(struct drm_i915_private *dev_priv)
{
mutex_lock(&dev_priv->rps.hw_lock);
if (dev_priv->rps.enabled) {
- if (dev_priv->pm_rps_events & (GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED))
+ if (dev_priv->pm_rps_events & GEN6_PM_RP_UP_EI_EXPIRED)
gen6_rps_reset_ei(dev_priv);
I915_WRITE(GEN6_PMINTRMSK,
gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: Fixes that failed to backport to v4.11-rc1
2017-03-13 16:14 Fixes that failed to backport to v4.11-rc1 Jani Nikula
` (3 preceding siblings ...)
2017-03-13 17:06 ` [PATCH] drm/i915: Stop using RP_DOWN_EI on Baytrail Chris Wilson
@ 2017-03-13 21:23 ` Chris Wilson
2017-03-14 7:41 ` Jani Nikula
2017-03-15 9:24 ` Jani Nikula
2017-03-15 14:31 ` [PATCH 4.11] drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks ville.syrjala
2017-03-21 14:23 ` Fixes that failed to backport to v4.11-rc1 Jani Nikula
6 siblings, 2 replies; 28+ messages in thread
From: Chris Wilson @ 2017-03-13 21:23 UTC (permalink / raw)
To: Jani Nikula
Cc: Conselvan De Oliveira, Ander, Kenneth Graunke, intel-gfx,
Daniel Vetter
On Mon, Mar 13, 2017 at 06:14:56PM +0200, Jani Nikula wrote:
> 1f7b847d72c3 ("drm/i915: Disable engine->irq_tasklet around resets")
Done.
> 370a81fb89cb ("drm/i915: Remove unused function intel_ddi_get_link_dpll()")
Don't bother, just code tidy?
> 262fd485ac6b ("drm/i915: Only enable hotplug interrupts if the display interrupts are enabled")
Done
> ef0f411f5147 ("drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters.")
Done.
> c998e8a0f43a ("drm/i915: Hold rpm during GEM suspend in driver unload/suspend")
No one else has noticed, so user impact is zero.
> a5509abda48e ("drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW")
> 5be6e3340099 ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
Ville?
> d9321a03efcd ("drm/i915/glk: Remove MODULE_FIRMWARE() tag from Geminilake's DMC")
Jani, I'd say that's your call as to whether it make sense to backport a
minor user confusion fix.
> e0e8c7cb6eb6 ("drm/i915: Stop using RP_DOWN_EI on Baytrail")
Done.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: Fixes that failed to backport to v4.11-rc1
2017-03-13 21:23 ` Fixes that failed to backport to v4.11-rc1 Chris Wilson
@ 2017-03-14 7:41 ` Jani Nikula
2017-03-14 10:56 ` Jani Nikula
2017-03-15 9:24 ` Jani Nikula
1 sibling, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2017-03-14 7:41 UTC (permalink / raw)
To: Chris Wilson
Cc: Conselvan De Oliveira, Ander, Kenneth Graunke, intel-gfx,
Daniel Vetter
On Mon, 13 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, Mar 13, 2017 at 06:14:56PM +0200, Jani Nikula wrote:
>> 1f7b847d72c3 ("drm/i915: Disable engine->irq_tasklet around resets")
> Done.
>
>> 370a81fb89cb ("drm/i915: Remove unused function intel_ddi_get_link_dpll()")
> Don't bother, just code tidy?
Agreed; I didn't look at or filter the list at all before sending.
>
>> 262fd485ac6b ("drm/i915: Only enable hotplug interrupts if the display interrupts are enabled")
> Done
>
>> ef0f411f5147 ("drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters.")
> Done.
>
>> c998e8a0f43a ("drm/i915: Hold rpm during GEM suspend in driver unload/suspend")
> No one else has noticed, so user impact is zero.
>
>> a5509abda48e ("drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW")
>> 5be6e3340099 ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
>
> Ville?
>
>> d9321a03efcd ("drm/i915/glk: Remove MODULE_FIRMWARE() tag from Geminilake's DMC")
>
> Jani, I'd say that's your call as to whether it make sense to backport a
> minor user confusion fix.
This causes initrd tools to whine if the firmware is missing. I think we
want this.
Thank you for the backports. I'll likely send a pull request of the
stuff I have in -fixes now, apply these, and let them simmer until next
week's pull.
BR,
Jani.
>
>> e0e8c7cb6eb6 ("drm/i915: Stop using RP_DOWN_EI on Baytrail")
> Done.
> -Chris
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: Fixes that failed to backport to v4.11-rc1
2017-03-14 7:41 ` Jani Nikula
@ 2017-03-14 10:56 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-03-14 10:56 UTC (permalink / raw)
To: Chris Wilson
Cc: Conselvan De Oliveira, Ander, Kenneth Graunke, intel-gfx,
Daniel Vetter
On Tue, 14 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> Thank you for the backports. I'll likely send a pull request of the
> stuff I have in -fixes now, apply these, and let them simmer until next
> week's pull.
I sent the fixes pull request for -rc3, and pushed Chris' backports
after that, thanks again.
I changed the "Cherry-pick: $sha" style annotations to "(cherry picked
from commit $sha)" style, because that's what my current scripts use and
expect. Better to avoid confusion, regardless of which style would
objectively make more sense. I just picked the style because it comes
out of the box with git cherry-pick -x.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Fixes that failed to backport to v4.11-rc1
2017-03-13 21:23 ` Fixes that failed to backport to v4.11-rc1 Chris Wilson
2017-03-14 7:41 ` Jani Nikula
@ 2017-03-15 9:24 ` Jani Nikula
2017-03-15 9:57 ` [PATCH] drm/i915/glk: Remove MODULE_FIRMWARE() tag from Geminilake's DMC Ander Conselvan de Oliveira
2017-03-15 11:04 ` Fixes that failed to backport to v4.11-rc1 Ville Syrjälä
1 sibling, 2 replies; 28+ messages in thread
From: Jani Nikula @ 2017-03-15 9:24 UTC (permalink / raw)
To: Chris Wilson
Cc: Conselvan De Oliveira, Ander, Kenneth Graunke, intel-gfx,
Daniel Vetter
On Mon, 13 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, Mar 13, 2017 at 06:14:56PM +0200, Jani Nikula wrote:
>> a5509abda48e ("drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW")
>> 5be6e3340099 ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
>
> Ville?
Ville, ping, are these required for v4.11? Backport from dinq feasible?
>> d9321a03efcd ("drm/i915/glk: Remove MODULE_FIRMWARE() tag from Geminilake's DMC")
Ander, please send a backported version of this that applies to
drm-intel-fixes. It's dead simple, but I don't want to set a precedent
of me backporting fixes that don't apply. ;)
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH] drm/i915/glk: Remove MODULE_FIRMWARE() tag from Geminilake's DMC
2017-03-15 9:24 ` Jani Nikula
@ 2017-03-15 9:57 ` Ander Conselvan de Oliveira
2017-03-15 10:44 ` Jani Nikula
2017-03-15 11:04 ` Fixes that failed to backport to v4.11-rc1 Ville Syrjälä
1 sibling, 1 reply; 28+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-03-15 9:57 UTC (permalink / raw)
To: intel-gfx, jani.nikula
Cc: Ander Conselvan de Oliveira, drm-intel-fixes, Rodrigo Vivi,
Daniel Vetter
Geminilake's DMC is not yet available in the linux-firmware repository.
To prevent userspace tools such as mkinitramfs to complain about
missing firmware, remove the MODULE_FIRMWARE() tag for now.
Fixes: dbb28b5c3d3c ("drm/i915/DMC/GLK: Load DMC on GLK")
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: <drm-intel-fixes@lists.freedesktop.org>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170306085651.14008-1-ander.conselvan.de.oliveira@intel.com
(cherry picked from commit d9321a03efcda867b3a8c6327e01808516f0acd7)
---
drivers/gpu/drm/i915/intel_csr.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 0085bc7..de219b7 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -35,7 +35,6 @@
*/
#define I915_CSR_GLK "i915/glk_dmc_ver1_01.bin"
-MODULE_FIRMWARE(I915_CSR_GLK);
#define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 1)
#define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH] drm/i915/glk: Remove MODULE_FIRMWARE() tag from Geminilake's DMC
2017-03-15 9:57 ` [PATCH] drm/i915/glk: Remove MODULE_FIRMWARE() tag from Geminilake's DMC Ander Conselvan de Oliveira
@ 2017-03-15 10:44 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-03-15 10:44 UTC (permalink / raw)
To: intel-gfx
Cc: Daniel Vetter, Ander Conselvan de Oliveira, drm-intel-fixes,
Rodrigo Vivi
On Wed, 15 Mar 2017, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote:
> Geminilake's DMC is not yet available in the linux-firmware repository.
> To prevent userspace tools such as mkinitramfs to complain about
> missing firmware, remove the MODULE_FIRMWARE() tag for now.
>
> Fixes: dbb28b5c3d3c ("drm/i915/DMC/GLK: Load DMC on GLK")
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: <drm-intel-fixes@lists.freedesktop.org>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Link: http://patchwork.freedesktop.org/patch/msgid/20170306085651.14008-1-ander.conselvan.de.oliveira@intel.com
> (cherry picked from commit d9321a03efcda867b3a8c6327e01808516f0acd7)
Pushed to drm-intel-fixes, obrigado.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/intel_csr.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 0085bc7..de219b7 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -35,7 +35,6 @@
> */
>
> #define I915_CSR_GLK "i915/glk_dmc_ver1_01.bin"
> -MODULE_FIRMWARE(I915_CSR_GLK);
> #define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 1)
>
> #define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin"
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Fixes that failed to backport to v4.11-rc1
2017-03-15 9:24 ` Jani Nikula
2017-03-15 9:57 ` [PATCH] drm/i915/glk: Remove MODULE_FIRMWARE() tag from Geminilake's DMC Ander Conselvan de Oliveira
@ 2017-03-15 11:04 ` Ville Syrjälä
1 sibling, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2017-03-15 11:04 UTC (permalink / raw)
To: Jani Nikula
Cc: Daniel Vetter, Conselvan De Oliveira, Ander, Kenneth Graunke,
intel-gfx
On Wed, Mar 15, 2017 at 11:24:43AM +0200, Jani Nikula wrote:
> On Mon, 13 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Mon, Mar 13, 2017 at 06:14:56PM +0200, Jani Nikula wrote:
> >> a5509abda48e ("drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW")
> >> 5be6e3340099 ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
> >
> > Ville?
>
> Ville, ping, are these required for v4.11? Backport from dinq feasible?
The clock_gating one we want, I think. Let me hack that one up...
The cursor thing I'm not so sure about. Apparently I already broke SKL+
with that so we'd have to suck in further fixes on top. Probably safer
to let it simmer longer.
--
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] 28+ messages in thread
* [PATCH 4.11] drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks
2017-03-13 16:14 Fixes that failed to backport to v4.11-rc1 Jani Nikula
` (4 preceding siblings ...)
2017-03-13 21:23 ` Fixes that failed to backport to v4.11-rc1 Chris Wilson
@ 2017-03-15 14:31 ` ville.syrjala
2017-03-16 8:06 ` Jani Nikula
2017-03-21 14:23 ` Fixes that failed to backport to v4.11-rc1 Jani Nikula
6 siblings, 1 reply; 28+ messages in thread
From: ville.syrjala @ 2017-03-15 14:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Currently ILK-BDW explicitly disable LP1+ watermarks from their
.init_clock_gating() hooks. Unfortunately that hook gets called way too
late since by that time we've already initialized all the watermark
state tracking which then gets out of sync with the hardware state.
We may eventually want to consider killing off the explicit LP1+
disable from .init_clock_gating(). In the meantime however, we can
avoid the problem by reordering the init sequence such that
intel_modeset_init_hw()->intel_init_clock_gating() gets called
prior to the hardware state takeover.
I suppose prior to the two stage watermark programming we were
magically saved by something that forced the watermarks to be
reprogrammed fully after .init_clock_gating() got called. But
now that no longer happens.
Note that the diff might look a bit odd as it kills off one
call of intel_update_cdclk(), but that's fine because
intel_modeset_init_hw() does the exact same thing. Previously
we just did it twice.
Actually even this new init sequence is pretty bogus as
.init_clock_gating() really should be called before any gem
hardware init since it can configure various clock gating
workarounds and whatnot that affect the GT side as well. Also
intel_modeset_init() really should get split up into better
defined init stages. Another "fun" detail is that
intel_modeset_gem_init() is where RPS/RC6 gets configured.
Why that is done from the display code is beyond me. I've
decided to leave all this be for now, and just try to fix
the init sequence enough for watermarks to work.
Cc: stable@vger.kernel.org
Cc: Gabriele Mazzotta <gabriele.mzt@gmail.com>
Cc: David Purton <dcpurton@marshwiggle.net>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reported-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
Reported-by: David Purton <dcpurton@marshwiggle.net>
Tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96645
Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170220140443.30891-1-ville.syrjala@linux.intel.com
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 01341670738f..70be773b3e70 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16696,12 +16696,11 @@ int intel_modeset_init(struct drm_device *dev)
}
}
- intel_update_czclk(dev_priv);
- intel_update_cdclk(dev_priv);
- dev_priv->atomic_cdclk_freq = dev_priv->cdclk_freq;
-
intel_shared_dpll_init(dev);
+ intel_update_czclk(dev_priv);
+ intel_modeset_init_hw(dev);
+
if (dev_priv->max_cdclk_freq == 0)
intel_update_max_cdclk(dev_priv);
@@ -17258,8 +17257,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
intel_init_gt_powersave(dev_priv);
- intel_modeset_init_hw(dev);
-
intel_setup_overlay(dev_priv);
}
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 4.11] drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks
2017-03-15 14:31 ` [PATCH 4.11] drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks ville.syrjala
@ 2017-03-16 8:06 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-03-16 8:06 UTC (permalink / raw)
To: ville.syrjala, intel-gfx
On Wed, 15 Mar 2017, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently ILK-BDW explicitly disable LP1+ watermarks from their
> .init_clock_gating() hooks. Unfortunately that hook gets called way too
> late since by that time we've already initialized all the watermark
> state tracking which then gets out of sync with the hardware state.
>
> We may eventually want to consider killing off the explicit LP1+
> disable from .init_clock_gating(). In the meantime however, we can
> avoid the problem by reordering the init sequence such that
> intel_modeset_init_hw()->intel_init_clock_gating() gets called
> prior to the hardware state takeover.
>
> I suppose prior to the two stage watermark programming we were
> magically saved by something that forced the watermarks to be
> reprogrammed fully after .init_clock_gating() got called. But
> now that no longer happens.
>
> Note that the diff might look a bit odd as it kills off one
> call of intel_update_cdclk(), but that's fine because
> intel_modeset_init_hw() does the exact same thing. Previously
> we just did it twice.
>
> Actually even this new init sequence is pretty bogus as
> .init_clock_gating() really should be called before any gem
> hardware init since it can configure various clock gating
> workarounds and whatnot that affect the GT side as well. Also
> intel_modeset_init() really should get split up into better
> defined init stages. Another "fun" detail is that
> intel_modeset_gem_init() is where RPS/RC6 gets configured.
> Why that is done from the display code is beyond me. I've
> decided to leave all this be for now, and just try to fix
> the init sequence enough for watermarks to work.
>
> Cc: stable@vger.kernel.org
> Cc: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> Cc: David Purton <dcpurton@marshwiggle.net>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reported-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> Reported-by: David Purton <dcpurton@marshwiggle.net>
> Tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96645
> Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: http://patchwork.freedesktop.org/patch/msgid/20170220140443.30891-1-ville.syrjala@linux.intel.com
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Thanks, pushed to drm-intel-fixes.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/intel_display.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 01341670738f..70be773b3e70 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -16696,12 +16696,11 @@ int intel_modeset_init(struct drm_device *dev)
> }
> }
>
> - intel_update_czclk(dev_priv);
> - intel_update_cdclk(dev_priv);
> - dev_priv->atomic_cdclk_freq = dev_priv->cdclk_freq;
> -
> intel_shared_dpll_init(dev);
>
> + intel_update_czclk(dev_priv);
> + intel_modeset_init_hw(dev);
> +
> if (dev_priv->max_cdclk_freq == 0)
> intel_update_max_cdclk(dev_priv);
>
> @@ -17258,8 +17257,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
>
> intel_init_gt_powersave(dev_priv);
>
> - intel_modeset_init_hw(dev);
> -
> intel_setup_overlay(dev_priv);
> }
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Fixes that failed to backport to v4.11-rc1
2017-03-13 16:14 Fixes that failed to backport to v4.11-rc1 Jani Nikula
` (5 preceding siblings ...)
2017-03-15 14:31 ` [PATCH 4.11] drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks ville.syrjala
@ 2017-03-21 14:23 ` Jani Nikula
2017-03-21 14:45 ` [PATCH] drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker) Chris Wilson
` (3 more replies)
6 siblings, 4 replies; 28+ messages in thread
From: Jani Nikula @ 2017-03-21 14:23 UTC (permalink / raw)
To: Chris Wilson, Changbin Du; +Cc: Daniel Vetter, intel-gfx
On Mon, 13 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> I'm already scripting my fixes backports quite a bit, and frankly don't
> really manually backport anything that doesn't apply cleanly. I'm
> thinking of automating some "failed to backport" reporting to authors,
> not unlike the failed stable backport reports.
>
> This is a manual report that the following commits have been marked as
> Cc: stable or fixing something in v4.11-rc1, but failed to cherry-pick
> to drm-intel-fixes. Please see if they are worth backporting, and please
> do so if they are.
>
> Feedback about the idea of this reporting is also appreciated.
Refreshed list as of today:
bd784b7cc41a ("drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker)")
3fc03069bc6e ("drm/i915: make context status notifier head be per engine")
2e8f9d322948 ("drm/i915: Restore engine->submit_request before unwedging")
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH] drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker)
2017-03-21 14:23 ` Fixes that failed to backport to v4.11-rc1 Jani Nikula
@ 2017-03-21 14:45 ` Chris Wilson
2017-03-21 14:47 ` [PATCH] drm/i915: make context status notifier head be per engine Chris Wilson
` (2 subsequent siblings)
3 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2017-03-21 14:45 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Chris Wilson, Daniel Vetter, # v4 . 9+
The rcu_barrier() takes the cpu_hotplug mutex which itself is not
reclaim-safe, and so rcu_barrier() is illegal from inside the shrinker.
[ 309.661373] =========================================================
[ 309.661376] [ INFO: possible irq lock inversion dependency detected ]
[ 309.661380] 4.11.0-rc1-CI-CI_DRM_2333+ #1 Tainted: G W
[ 309.661383] ---------------------------------------------------------
[ 309.661386] gem_exec_gttfil/6435 just changed the state of lock:
[ 309.661389] (rcu_preempt_state.barrier_mutex){+.+.-.}, at: [<ffffffff81100731>] _rcu_barrier+0x31/0x160
[ 309.661399] but this lock took another, RECLAIM_FS-unsafe lock in the past:
[ 309.661402] (cpu_hotplug.lock){+.+.+.}
[ 309.661404]
and interrupts could create inverse lock ordering between them.
[ 309.661410]
other info that might help us debug this:
[ 309.661414] Possible interrupt unsafe locking scenario:
[ 309.661417] CPU0 CPU1
[ 309.661419] ---- ----
[ 309.661421] lock(cpu_hotplug.lock);
[ 309.661425] local_irq_disable();
[ 309.661432] lock(rcu_preempt_state.barrier_mutex);
[ 309.661441] lock(cpu_hotplug.lock);
[ 309.661446] <Interrupt>
[ 309.661448] lock(rcu_preempt_state.barrier_mutex);
[ 309.661453]
*** DEADLOCK ***
[ 309.661460] 4 locks held by gem_exec_gttfil/6435:
[ 309.661464] #0: (sb_writers#10){.+.+.+}, at: [<ffffffff8120d83d>] vfs_write+0x17d/0x1f0
[ 309.661475] #1: (debugfs_srcu){......}, at: [<ffffffff81320491>] debugfs_use_file_start+0x41/0xa0
[ 309.661486] #2: (&attr->mutex){+.+.+.}, at: [<ffffffff8123a3e7>] simple_attr_write+0x37/0xe0
[ 309.661495] #3: (&dev->struct_mutex){+.+.+.}, at: [<ffffffffa0091b4a>] i915_drop_caches_set+0x3a/0x150 [i915]
[ 309.661540]
the shortest dependencies between 2nd lock and 1st lock:
[ 309.661547] -> (cpu_hotplug.lock){+.+.+.} ops: 829 {
[ 309.661553] HARDIRQ-ON-W at:
[ 309.661560] __lock_acquire+0x5e5/0x1b50
[ 309.661565] lock_acquire+0xc9/0x220
[ 309.661572] __mutex_lock+0x6e/0x990
[ 309.661576] mutex_lock_nested+0x16/0x20
[ 309.661583] get_online_cpus+0x61/0x80
[ 309.661590] kmem_cache_create+0x25/0x1d0
[ 309.661596] debug_objects_mem_init+0x30/0x249
[ 309.661602] start_kernel+0x341/0x3fe
[ 309.661607] x86_64_start_reservations+0x2a/0x2c
[ 309.661612] x86_64_start_kernel+0x173/0x186
[ 309.661619] verify_cpu+0x0/0xfc
[ 309.661622] SOFTIRQ-ON-W at:
[ 309.661627] __lock_acquire+0x611/0x1b50
[ 309.661632] lock_acquire+0xc9/0x220
[ 309.661636] __mutex_lock+0x6e/0x990
[ 309.661641] mutex_lock_nested+0x16/0x20
[ 309.661646] get_online_cpus+0x61/0x80
[ 309.661650] kmem_cache_create+0x25/0x1d0
[ 309.661655] debug_objects_mem_init+0x30/0x249
[ 309.661660] start_kernel+0x341/0x3fe
[ 309.661664] x86_64_start_reservations+0x2a/0x2c
[ 309.661669] x86_64_start_kernel+0x173/0x186
[ 309.661674] verify_cpu+0x0/0xfc
[ 309.661677] RECLAIM_FS-ON-W at:
[ 309.661682] mark_held_locks+0x6f/0xa0
[ 309.661687] lockdep_trace_alloc+0xb3/0x100
[ 309.661693] kmem_cache_alloc_trace+0x31/0x2e0
[ 309.661699] __smpboot_create_thread.part.1+0x27/0xe0
[ 309.661704] smpboot_create_threads+0x61/0x90
[ 309.661709] cpuhp_invoke_callback+0x9c/0x8a0
[ 309.661713] cpuhp_up_callbacks+0x31/0xb0
[ 309.661718] _cpu_up+0x7a/0xc0
[ 309.661723] do_cpu_up+0x5f/0x80
[ 309.661727] cpu_up+0xe/0x10
[ 309.661734] smp_init+0x71/0xb3
[ 309.661738] kernel_init_freeable+0x94/0x19e
[ 309.661743] kernel_init+0x9/0xf0
[ 309.661748] ret_from_fork+0x2e/0x40
[ 309.661752] INITIAL USE at:
[ 309.661757] __lock_acquire+0x234/0x1b50
[ 309.661761] lock_acquire+0xc9/0x220
[ 309.661766] __mutex_lock+0x6e/0x990
[ 309.661771] mutex_lock_nested+0x16/0x20
[ 309.661775] get_online_cpus+0x61/0x80
[ 309.661780] __cpuhp_setup_state+0x44/0x170
[ 309.661785] page_alloc_init+0x23/0x3a
[ 309.661790] start_kernel+0x124/0x3fe
[ 309.661794] x86_64_start_reservations+0x2a/0x2c
[ 309.661799] x86_64_start_kernel+0x173/0x186
[ 309.661804] verify_cpu+0x0/0xfc
[ 309.661807] }
[ 309.661813] ... key at: [<ffffffff81e37690>] cpu_hotplug+0xb0/0x100
[ 309.661817] ... acquired at:
[ 309.661821] lock_acquire+0xc9/0x220
[ 309.661825] __mutex_lock+0x6e/0x990
[ 309.661829] mutex_lock_nested+0x16/0x20
[ 309.661833] get_online_cpus+0x61/0x80
[ 309.661837] _rcu_barrier+0x9f/0x160
[ 309.661841] rcu_barrier+0x10/0x20
[ 309.661847] netdev_run_todo+0x5f/0x310
[ 309.661852] rtnl_unlock+0x9/0x10
[ 309.661856] default_device_exit_batch+0x133/0x150
[ 309.661862] ops_exit_list.isra.0+0x4d/0x60
[ 309.661866] cleanup_net+0x1d8/0x2c0
[ 309.661872] process_one_work+0x1f4/0x6d0
[ 309.661876] worker_thread+0x49/0x4a0
[ 309.661881] kthread+0x107/0x140
[ 309.661884] ret_from_fork+0x2e/0x40
[ 309.661890] -> (rcu_preempt_state.barrier_mutex){+.+.-.} ops: 179 {
[ 309.661896] HARDIRQ-ON-W at:
[ 309.661901] __lock_acquire+0x5e5/0x1b50
[ 309.661905] lock_acquire+0xc9/0x220
[ 309.661910] __mutex_lock+0x6e/0x990
[ 309.661914] mutex_lock_nested+0x16/0x20
[ 309.661919] _rcu_barrier+0x31/0x160
[ 309.661923] rcu_barrier+0x10/0x20
[ 309.661928] netdev_run_todo+0x5f/0x310
[ 309.661932] rtnl_unlock+0x9/0x10
[ 309.661936] default_device_exit_batch+0x133/0x150
[ 309.661941] ops_exit_list.isra.0+0x4d/0x60
[ 309.661946] cleanup_net+0x1d8/0x2c0
[ 309.661951] process_one_work+0x1f4/0x6d0
[ 309.661955] worker_thread+0x49/0x4a0
[ 309.661960] kthread+0x107/0x140
[ 309.661964] ret_from_fork+0x2e/0x40
[ 309.661968] SOFTIRQ-ON-W at:
[ 309.661972] __lock_acquire+0x611/0x1b50
[ 309.661977] lock_acquire+0xc9/0x220
[ 309.661981] __mutex_lock+0x6e/0x990
[ 309.661986] mutex_lock_nested+0x16/0x20
[ 309.661990] _rcu_barrier+0x31/0x160
[ 309.661995] rcu_barrier+0x10/0x20
[ 309.661999] netdev_run_todo+0x5f/0x310
[ 309.662003] rtnl_unlock+0x9/0x10
[ 309.662008] default_device_exit_batch+0x133/0x150
[ 309.662013] ops_exit_list.isra.0+0x4d/0x60
[ 309.662017] cleanup_net+0x1d8/0x2c0
[ 309.662022] process_one_work+0x1f4/0x6d0
[ 309.662027] worker_thread+0x49/0x4a0
[ 309.662031] kthread+0x107/0x140
[ 309.662035] ret_from_fork+0x2e/0x40
[ 309.662039] IN-RECLAIM_FS-W at:
[ 309.662043] __lock_acquire+0x638/0x1b50
[ 309.662048] lock_acquire+0xc9/0x220
[ 309.662053] __mutex_lock+0x6e/0x990
[ 309.662058] mutex_lock_nested+0x16/0x20
[ 309.662062] _rcu_barrier+0x31/0x160
[ 309.662067] rcu_barrier+0x10/0x20
[ 309.662089] i915_gem_shrink_all+0x33/0x40 [i915]
[ 309.662109] i915_drop_caches_set+0x141/0x150 [i915]
[ 309.662114] simple_attr_write+0xc7/0xe0
[ 309.662119] full_proxy_write+0x4f/0x70
[ 309.662124] __vfs_write+0x23/0x120
[ 309.662128] vfs_write+0xc6/0x1f0
[ 309.662133] SyS_write+0x44/0xb0
[ 309.662138] entry_SYSCALL_64_fastpath+0x1c/0xb1
[ 309.662142] INITIAL USE at:
[ 309.662147] __lock_acquire+0x234/0x1b50
[ 309.662151] lock_acquire+0xc9/0x220
[ 309.662156] __mutex_lock+0x6e/0x990
[ 309.662160] mutex_lock_nested+0x16/0x20
[ 309.662165] _rcu_barrier+0x31/0x160
[ 309.662169] rcu_barrier+0x10/0x20
[ 309.662174] netdev_run_todo+0x5f/0x310
[ 309.662178] rtnl_unlock+0x9/0x10
[ 309.662183] default_device_exit_batch+0x133/0x150
[ 309.662188] ops_exit_list.isra.0+0x4d/0x60
[ 309.662192] cleanup_net+0x1d8/0x2c0
[ 309.662197] process_one_work+0x1f4/0x6d0
[ 309.662202] worker_thread+0x49/0x4a0
[ 309.662206] kthread+0x107/0x140
[ 309.662210] ret_from_fork+0x2e/0x40
[ 309.662214] }
[ 309.662220] ... key at: [<ffffffff81e4e1c8>] rcu_preempt_state+0x508/0x780
[ 309.662225] ... acquired at:
[ 309.662229] check_usage_forwards+0x12b/0x130
[ 309.662233] mark_lock+0x360/0x6f0
[ 309.662237] __lock_acquire+0x638/0x1b50
[ 309.662241] lock_acquire+0xc9/0x220
[ 309.662245] __mutex_lock+0x6e/0x990
[ 309.662249] mutex_lock_nested+0x16/0x20
[ 309.662253] _rcu_barrier+0x31/0x160
[ 309.662257] rcu_barrier+0x10/0x20
[ 309.662279] i915_gem_shrink_all+0x33/0x40 [i915]
[ 309.662298] i915_drop_caches_set+0x141/0x150 [i915]
[ 309.662303] simple_attr_write+0xc7/0xe0
[ 309.662307] full_proxy_write+0x4f/0x70
[ 309.662311] __vfs_write+0x23/0x120
[ 309.662315] vfs_write+0xc6/0x1f0
[ 309.662319] SyS_write+0x44/0xb0
[ 309.662323] entry_SYSCALL_64_fastpath+0x1c/0xb1
[ 309.662329]
stack backtrace:
[ 309.662335] CPU: 1 PID: 6435 Comm: gem_exec_gttfil Tainted: G W 4.11.0-rc1-CI-CI_DRM_2333+ #1
[ 309.662342] Hardware name: Hewlett-Packard HP Compaq 8100 Elite SFF PC/304Ah, BIOS 786H1 v01.13 07/14/2011
[ 309.662348] Call Trace:
[ 309.662354] dump_stack+0x67/0x92
[ 309.662359] print_irq_inversion_bug.part.19+0x1a4/0x1b0
[ 309.662365] check_usage_forwards+0x12b/0x130
[ 309.662369] mark_lock+0x360/0x6f0
[ 309.662374] ? print_shortest_lock_dependencies+0x1a0/0x1a0
[ 309.662379] __lock_acquire+0x638/0x1b50
[ 309.662383] ? __mutex_unlock_slowpath+0x3e/0x2e0
[ 309.662388] ? trace_hardirqs_on+0xd/0x10
[ 309.662392] ? _rcu_barrier+0x31/0x160
[ 309.662396] lock_acquire+0xc9/0x220
[ 309.662400] ? _rcu_barrier+0x31/0x160
[ 309.662404] ? _rcu_barrier+0x31/0x160
[ 309.662409] __mutex_lock+0x6e/0x990
[ 309.662412] ? _rcu_barrier+0x31/0x160
[ 309.662416] ? _rcu_barrier+0x31/0x160
[ 309.662421] ? synchronize_rcu_expedited+0x35/0xb0
[ 309.662426] ? _raw_spin_unlock_irqrestore+0x52/0x60
[ 309.662434] mutex_lock_nested+0x16/0x20
[ 309.662438] _rcu_barrier+0x31/0x160
[ 309.662442] rcu_barrier+0x10/0x20
[ 309.662464] i915_gem_shrink_all+0x33/0x40 [i915]
[ 309.662484] i915_drop_caches_set+0x141/0x150 [i915]
[ 309.662489] simple_attr_write+0xc7/0xe0
[ 309.662494] full_proxy_write+0x4f/0x70
[ 309.662498] __vfs_write+0x23/0x120
[ 309.662503] ? rcu_read_lock_sched_held+0x75/0x80
[ 309.662507] ? rcu_sync_lockdep_assert+0x2a/0x50
[ 309.662512] ? __sb_start_write+0x102/0x210
[ 309.662516] ? vfs_write+0x17d/0x1f0
[ 309.662520] vfs_write+0xc6/0x1f0
[ 309.662524] ? trace_hardirqs_on_caller+0xe7/0x200
[ 309.662529] SyS_write+0x44/0xb0
[ 309.662533] entry_SYSCALL_64_fastpath+0x1c/0xb1
[ 309.662537] RIP: 0033:0x7f507eac24a0
[ 309.662541] RSP: 002b:00007fffda8720e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 309.662548] RAX: ffffffffffffffda RBX: ffffffff81482bd3 RCX: 00007f507eac24a0
[ 309.662552] RDX: 0000000000000005 RSI: 00007fffda8720f0 RDI: 0000000000000005
[ 309.662557] RBP: ffffc9000048bf88 R08: 0000000000000000 R09: 000000000000002c
[ 309.662561] R10: 0000000000000014 R11: 0000000000000246 R12: 00007fffda872230
[ 309.662566] R13: 00007fffda872228 R14: 0000000000000201 R15: 00007fffda8720f0
[ 309.662572] ? __this_cpu_preempt_check+0x13/0x20
Fixes: 0eafec6d3244 ("drm/i915: Enable lockless lookup of request tracking via RCU")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100192
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: <stable@vger.kernel.org> # v4.9+
Link: http://patchwork.freedesktop.org/patch/msgid/20170314115019.18127-1-chris@chris-wilson.co.uk
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
(cherry picked from commit bd784b7cc41af7a19cfb705fa6d800e511c4ab02)
---
drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 401006b4c6a3..d5d2b4c6ed38 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -263,7 +263,7 @@ unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv)
I915_SHRINK_BOUND |
I915_SHRINK_UNBOUND |
I915_SHRINK_ACTIVE);
- rcu_barrier(); /* wait until our RCU delayed slab frees are completed */
+ synchronize_rcu(); /* wait for our earlier RCU delayed slab frees */
return freed;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH] drm/i915: make context status notifier head be per engine
2017-03-21 14:23 ` Fixes that failed to backport to v4.11-rc1 Jani Nikula
2017-03-21 14:45 ` [PATCH] drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker) Chris Wilson
@ 2017-03-21 14:47 ` Chris Wilson
2017-03-21 14:48 ` Fixes that failed to backport to v4.11-rc1 Chris Wilson
2017-03-29 10:45 ` Jani Nikula
3 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2017-03-21 14:47 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
From: Changbin Du <changbin.du@intel.com>
GVTg has introduced the context status notifier to schedule the GVTg
workload. At that time, the notifier is bound to GVTg context only,
so GVTg is not aware of host workloads.
Now we are going to improve GVTg's guest workload scheduler policy,
and add Guc emulation support for new Gen graphics. Both these two
features require acknowledgment for all contexts running on hardware.
(But will not alter host workload.) So here try to make some change.
The change is simple:
1. Move the context status notifier head from i915_gem_context to
intel_engine_cs. Which means there is a notifier head per engine
instead of per context. Execlist driver still call notifier for
each context sched-in/out events of current engine.
2. At GVTg side, it binds a notifier_block for each physical engine
at GVTg initialization period. Then GVTg can hear all context
status events.
In this patch, GVTg do nothing for host context event, but later
will add a function there. But in any case, the notifier callback is
a noop if this is no active vGPU.
Since intel_gvt_init() is called at early initialization stage and
require the status notifier head has been initiated, I initiate it in
intel_engine_setup().
v2: remove a redundant newline. (chris)
Fixes: 3c7ba6359d70 ("drm/i915: Introduce execlist context status change notification")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100232
Signed-off-by: Changbin Du <changbin.du@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Zhi Wang <zhi.a.wang@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/20170313024711.28591-1-changbin.du@intel.com
Acked-by: Zhenyu Wang <zhenyuw@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
(cherry picked from commit 3fc03069bc6e6c316f19bb526e3c8ce784677477)
---
drivers/gpu/drm/i915/gvt/gvt.h | 2 +-
drivers/gpu/drm/i915/gvt/scheduler.c | 45 ++++++++++++++-------------------
drivers/gpu/drm/i915/i915_gem_context.c | 1 -
drivers/gpu/drm/i915/i915_gem_context.h | 3 ---
drivers/gpu/drm/i915/intel_engine_cs.c | 2 ++
drivers/gpu/drm/i915/intel_lrc.c | 3 ++-
drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +++
7 files changed, 27 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 23791920ced1..6dfc48b63b71 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -162,7 +162,6 @@ struct intel_vgpu {
atomic_t running_workload_num;
DECLARE_BITMAP(tlb_handle_pending, I915_NUM_ENGINES);
struct i915_gem_context *shadow_ctx;
- struct notifier_block shadow_ctx_notifier_block;
#if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
struct {
@@ -233,6 +232,7 @@ struct intel_gvt {
struct intel_gvt_gtt gtt;
struct intel_gvt_opregion opregion;
struct intel_gvt_workload_scheduler scheduler;
+ struct notifier_block shadow_ctx_notifier_block[I915_NUM_ENGINES];
DECLARE_HASHTABLE(cmd_table, GVT_CMD_HASH_BITS);
struct intel_vgpu_type *types;
unsigned int num_types;
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 39a83eb7aecc..c4353ed86d4b 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -130,12 +130,10 @@ static int populate_shadow_context(struct intel_vgpu_workload *workload)
static int shadow_context_status_change(struct notifier_block *nb,
unsigned long action, void *data)
{
- struct intel_vgpu *vgpu = container_of(nb,
- struct intel_vgpu, shadow_ctx_notifier_block);
- struct drm_i915_gem_request *req =
- (struct drm_i915_gem_request *)data;
- struct intel_gvt_workload_scheduler *scheduler =
- &vgpu->gvt->scheduler;
+ struct drm_i915_gem_request *req = (struct drm_i915_gem_request *)data;
+ struct intel_gvt *gvt = container_of(nb, struct intel_gvt,
+ shadow_ctx_notifier_block[req->engine->id]);
+ struct intel_gvt_workload_scheduler *scheduler = &gvt->scheduler;
struct intel_vgpu_workload *workload =
scheduler->current_workload[req->engine->id];
@@ -534,15 +532,16 @@ void intel_gvt_wait_vgpu_idle(struct intel_vgpu *vgpu)
void intel_gvt_clean_workload_scheduler(struct intel_gvt *gvt)
{
struct intel_gvt_workload_scheduler *scheduler = &gvt->scheduler;
- int i;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id i;
gvt_dbg_core("clean workload scheduler\n");
- for (i = 0; i < I915_NUM_ENGINES; i++) {
- if (scheduler->thread[i]) {
- kthread_stop(scheduler->thread[i]);
- scheduler->thread[i] = NULL;
- }
+ for_each_engine(engine, gvt->dev_priv, i) {
+ atomic_notifier_chain_unregister(
+ &engine->context_status_notifier,
+ &gvt->shadow_ctx_notifier_block[i]);
+ kthread_stop(scheduler->thread[i]);
}
}
@@ -550,18 +549,15 @@ int intel_gvt_init_workload_scheduler(struct intel_gvt *gvt)
{
struct intel_gvt_workload_scheduler *scheduler = &gvt->scheduler;
struct workload_thread_param *param = NULL;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id i;
int ret;
- int i;
gvt_dbg_core("init workload scheduler\n");
init_waitqueue_head(&scheduler->workload_complete_wq);
- for (i = 0; i < I915_NUM_ENGINES; i++) {
- /* check ring mask at init time */
- if (!HAS_ENGINE(gvt->dev_priv, i))
- continue;
-
+ for_each_engine(engine, gvt->dev_priv, i) {
init_waitqueue_head(&scheduler->waitq[i]);
param = kzalloc(sizeof(*param), GFP_KERNEL);
@@ -580,6 +576,11 @@ int intel_gvt_init_workload_scheduler(struct intel_gvt *gvt)
ret = PTR_ERR(scheduler->thread[i]);
goto err;
}
+
+ gvt->shadow_ctx_notifier_block[i].notifier_call =
+ shadow_context_status_change;
+ atomic_notifier_chain_register(&engine->context_status_notifier,
+ &gvt->shadow_ctx_notifier_block[i]);
}
return 0;
err:
@@ -591,9 +592,6 @@ int intel_gvt_init_workload_scheduler(struct intel_gvt *gvt)
void intel_vgpu_clean_gvt_context(struct intel_vgpu *vgpu)
{
- atomic_notifier_chain_unregister(&vgpu->shadow_ctx->status_notifier,
- &vgpu->shadow_ctx_notifier_block);
-
i915_gem_context_put_unlocked(vgpu->shadow_ctx);
}
@@ -608,10 +606,5 @@ int intel_vgpu_init_gvt_context(struct intel_vgpu *vgpu)
vgpu->shadow_ctx->engine[RCS].initialised = true;
- vgpu->shadow_ctx_notifier_block.notifier_call =
- shadow_context_status_change;
-
- atomic_notifier_chain_register(&vgpu->shadow_ctx->status_notifier,
- &vgpu->shadow_ctx_notifier_block);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 17f90c618208..e2d83b6d376b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -311,7 +311,6 @@ __create_hw_context(struct drm_i915_private *dev_priv,
ctx->ring_size = 4 * PAGE_SIZE;
ctx->desc_template = GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
GEN8_CTX_ADDRESSING_MODE_SHIFT;
- ATOMIC_INIT_NOTIFIER_HEAD(&ctx->status_notifier);
/* GuC requires the ring to be placed above GUC_WOPCM_TOP. If GuC is not
* present or not in use we still need a small bias as ring wraparound
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 0ac750b90f3d..e9c008fe14b1 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -160,9 +160,6 @@ struct i915_gem_context {
/** desc_template: invariant fields for the HW context descriptor */
u32 desc_template;
- /** status_notifier: list of callbacks for context-switch changes */
- struct atomic_notifier_head status_notifier;
-
/** guilty_count: How many times this context has caused a GPU hang. */
unsigned int guilty_count;
/**
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 371acf109e34..ab1be5c80ea5 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -105,6 +105,8 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
/* Nothing to do here, execute in order of dependencies */
engine->schedule = NULL;
+ ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier);
+
dev_priv->engine[id] = engine;
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ebf8023d21e6..471af3b480ad 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -345,7 +345,8 @@ execlists_context_status_change(struct drm_i915_gem_request *rq,
if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
return;
- atomic_notifier_call_chain(&rq->ctx->status_notifier, status, rq);
+ atomic_notifier_call_chain(&rq->engine->context_status_notifier,
+ status, rq);
}
static void
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 79c2b8d72322..13dccb18cd43 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -403,6 +403,9 @@ struct intel_engine_cs {
*/
struct i915_gem_context *legacy_active_context;
+ /* status_notifier: list of callbacks for context-switch changes */
+ struct atomic_notifier_head context_status_notifier;
+
struct intel_engine_hangcheck hangcheck;
bool needs_cmd_parser;
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: Fixes that failed to backport to v4.11-rc1
2017-03-21 14:23 ` Fixes that failed to backport to v4.11-rc1 Jani Nikula
2017-03-21 14:45 ` [PATCH] drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker) Chris Wilson
2017-03-21 14:47 ` [PATCH] drm/i915: make context status notifier head be per engine Chris Wilson
@ 2017-03-21 14:48 ` Chris Wilson
2017-03-21 15:02 ` Jani Nikula
2017-03-29 10:45 ` Jani Nikula
3 siblings, 1 reply; 28+ messages in thread
From: Chris Wilson @ 2017-03-21 14:48 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Daniel Vetter
On Tue, Mar 21, 2017 at 04:23:19PM +0200, Jani Nikula wrote:
> On Mon, 13 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> > I'm already scripting my fixes backports quite a bit, and frankly don't
> > really manually backport anything that doesn't apply cleanly. I'm
> > thinking of automating some "failed to backport" reporting to authors,
> > not unlike the failed stable backport reports.
> >
> > This is a manual report that the following commits have been marked as
> > Cc: stable or fixing something in v4.11-rc1, but failed to cherry-pick
> > to drm-intel-fixes. Please see if they are worth backporting, and please
> > do so if they are.
> >
> > Feedback about the idea of this reporting is also appreciated.
>
> Refreshed list as of today:
>
> bd784b7cc41a ("drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker)")
Done.
> 3fc03069bc6e ("drm/i915: make context status notifier head be per engine")
Done.
> 2e8f9d322948 ("drm/i915: Restore engine->submit_request before unwedging")
Don't care; I consider this is an debug-only feature. The expected
response to a wedged machine by a user are curse words followed by a
reboot.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: Fixes that failed to backport to v4.11-rc1
2017-03-21 14:48 ` Fixes that failed to backport to v4.11-rc1 Chris Wilson
@ 2017-03-21 15:02 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-03-21 15:02 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, Daniel Vetter
On Tue, 21 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Mar 21, 2017 at 04:23:19PM +0200, Jani Nikula wrote:
>> On Mon, 13 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
>> > I'm already scripting my fixes backports quite a bit, and frankly don't
>> > really manually backport anything that doesn't apply cleanly. I'm
>> > thinking of automating some "failed to backport" reporting to authors,
>> > not unlike the failed stable backport reports.
>> >
>> > This is a manual report that the following commits have been marked as
>> > Cc: stable or fixing something in v4.11-rc1, but failed to cherry-pick
>> > to drm-intel-fixes. Please see if they are worth backporting, and please
>> > do so if they are.
>> >
>> > Feedback about the idea of this reporting is also appreciated.
>>
>> Refreshed list as of today:
>>
>> bd784b7cc41a ("drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker)")
>
> Done.
>
>> 3fc03069bc6e ("drm/i915: make context status notifier head be per engine")
> Done.
>
>> 2e8f9d322948 ("drm/i915: Restore engine->submit_request before unwedging")
>
> Don't care; I consider this is an debug-only feature. The expected
> response to a wedged machine by a user are curse words followed by a
> reboot.
Thanks, pushed the backports to drm-intel-fixes.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Fixes that failed to backport to v4.11-rc1
2017-03-21 14:23 ` Fixes that failed to backport to v4.11-rc1 Jani Nikula
` (2 preceding siblings ...)
2017-03-21 14:48 ` Fixes that failed to backport to v4.11-rc1 Chris Wilson
@ 2017-03-29 10:45 ` Jani Nikula
2017-03-29 12:13 ` [PATCH] drm/i915/execlists: Wrap tail pointer after reset tweaking Chris Wilson
` (2 more replies)
3 siblings, 3 replies; 28+ messages in thread
From: Jani Nikula @ 2017-03-29 10:45 UTC (permalink / raw)
To: Chris Wilson, Changbin Du; +Cc: Daniel Vetter, intel-gfx
On Tue, 21 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> On Mon, 13 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
>> I'm already scripting my fixes backports quite a bit, and frankly don't
>> really manually backport anything that doesn't apply cleanly. I'm
>> thinking of automating some "failed to backport" reporting to authors,
>> not unlike the failed stable backport reports.
>>
>> This is a manual report that the following commits have been marked as
>> Cc: stable or fixing something in v4.11-rc1, but failed to cherry-pick
>> to drm-intel-fixes. Please see if they are worth backporting, and please
>> do so if they are.
>>
>> Feedback about the idea of this reporting is also appreciated.
>
> Refreshed list as of today:
>
> bd784b7cc41a ("drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker)")
> 3fc03069bc6e ("drm/i915: make context status notifier head be per engine")
> 2e8f9d322948 ("drm/i915: Restore engine->submit_request before unwedging")
Update:
e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
450362d3fe86 ("drm/i915/execlists: Wrap tail pointer after reset tweaking")
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH] drm/i915/execlists: Wrap tail pointer after reset tweaking
2017-03-29 10:45 ` Jani Nikula
@ 2017-03-29 12:13 ` Chris Wilson
2017-03-29 12:13 ` Fixes that failed to backport to v4.11-rc1 Chris Wilson
2017-04-11 12:21 ` Jani Nikula
2 siblings, 0 replies; 28+ messages in thread
From: Chris Wilson @ 2017-03-29 12:13 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, Chris Wilson, Mika Kuoppala, # v4 . 10+
If the request->wa_tail is 0 (because it landed exactly on the end of
the ringbuffer), when we reconstruct request->tail following a reset we
fill in an illegal value (-8 or 0x001ffff8). As a result, RING_HEAD is
never able to catch up with RING_TAIL and the GPU spins endlessly. If
the ring contains a couple of breadcrumbs, even our hangcheck is unable
to catch the busy-looping as the ACTHD and seqno continually advance.
v2: Move the wrap into a common intel_ring_wrap().
Fixes: a3aabe86a340 ("drm/i915/execlists: Reinitialise context image after GPU hang")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: <stable@vger.kernel.org> # v4.10+
Link: http://patchwork.freedesktop.org/patch/msgid/20170327130009.4678-1-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
(cherry picked from commit 450362d3fe866b14304f309b5fffba0c33fbfbc3)
---
drivers/gpu/drm/i915/intel_lrc.c | 4 +++-
drivers/gpu/drm/i915/intel_ringbuffer.h | 8 +++++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 471af3b480ad..91555d4e9129 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1440,7 +1440,9 @@ static void reset_common_ring(struct intel_engine_cs *engine,
GEM_BUG_ON(request->ctx != port[0].request->ctx);
/* Reset WaIdleLiteRestore:bdw,skl as well */
- request->tail = request->wa_tail - WA_TAIL_DWORDS * sizeof(u32);
+ request->tail =
+ intel_ring_wrap(request->ring,
+ request->wa_tail - WA_TAIL_DWORDS*sizeof(u32));
}
static int intel_logical_ring_emit_pdps(struct drm_i915_gem_request *req)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 13dccb18cd43..8cb2078c5bfc 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -521,11 +521,17 @@ static inline void intel_ring_advance(struct intel_ring *ring)
*/
}
+static inline u32
+intel_ring_wrap(const struct intel_ring *ring, u32 pos)
+{
+ return pos & (ring->size - 1);
+}
+
static inline u32 intel_ring_offset(struct intel_ring *ring, void *addr)
{
/* Don't write ring->size (equivalent to 0) as that hangs some GPUs. */
u32 offset = addr - ring->vaddr;
- return offset & (ring->size - 1);
+ return intel_ring_wrap(ring, offset);
}
int __intel_ring_space(int head, int tail, int size);
--
2.11.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: Fixes that failed to backport to v4.11-rc1
2017-03-29 10:45 ` Jani Nikula
2017-03-29 12:13 ` [PATCH] drm/i915/execlists: Wrap tail pointer after reset tweaking Chris Wilson
@ 2017-03-29 12:13 ` Chris Wilson
2017-03-29 12:49 ` Jani Nikula
2017-04-11 12:21 ` Jani Nikula
2 siblings, 1 reply; 28+ messages in thread
From: Chris Wilson @ 2017-03-29 12:13 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Daniel Vetter
On Wed, Mar 29, 2017 at 01:45:38PM +0300, Jani Nikula wrote:
> On Tue, 21 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> > On Mon, 13 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> >> I'm already scripting my fixes backports quite a bit, and frankly don't
> >> really manually backport anything that doesn't apply cleanly. I'm
> >> thinking of automating some "failed to backport" reporting to authors,
> >> not unlike the failed stable backport reports.
> >>
> >> This is a manual report that the following commits have been marked as
> >> Cc: stable or fixing something in v4.11-rc1, but failed to cherry-pick
> >> to drm-intel-fixes. Please see if they are worth backporting, and please
> >> do so if they are.
> >>
> >> Feedback about the idea of this reporting is also appreciated.
> >
> > Refreshed list as of today:
> >
> > bd784b7cc41a ("drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker)")
> > 3fc03069bc6e ("drm/i915: make context status notifier head be per engine")
> > 2e8f9d322948 ("drm/i915: Restore engine->submit_request before unwedging")
>
> Update:
>
> e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
Not worth it. The WARN is disabled in v4.11 so nobody will notice.
> 450362d3fe86 ("drm/i915/execlists: Wrap tail pointer after reset tweaking")
Done.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: Fixes that failed to backport to v4.11-rc1
2017-03-29 12:13 ` Fixes that failed to backport to v4.11-rc1 Chris Wilson
@ 2017-03-29 12:49 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-03-29 12:49 UTC (permalink / raw)
To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx
On Wed, 29 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Wed, Mar 29, 2017 at 01:45:38PM +0300, Jani Nikula wrote:
>> On Tue, 21 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
>> > On Mon, 13 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
>> >> I'm already scripting my fixes backports quite a bit, and frankly don't
>> >> really manually backport anything that doesn't apply cleanly. I'm
>> >> thinking of automating some "failed to backport" reporting to authors,
>> >> not unlike the failed stable backport reports.
>> >>
>> >> This is a manual report that the following commits have been marked as
>> >> Cc: stable or fixing something in v4.11-rc1, but failed to cherry-pick
>> >> to drm-intel-fixes. Please see if they are worth backporting, and please
>> >> do so if they are.
>> >>
>> >> Feedback about the idea of this reporting is also appreciated.
>> >
>> > Refreshed list as of today:
>> >
>> > bd784b7cc41a ("drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker)")
>> > 3fc03069bc6e ("drm/i915: make context status notifier head be per engine")
>> > 2e8f9d322948 ("drm/i915: Restore engine->submit_request before unwedging")
>>
>> Update:
>>
>> e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
>
> Not worth it. The WARN is disabled in v4.11 so nobody will notice.
>
>> 450362d3fe86 ("drm/i915/execlists: Wrap tail pointer after reset tweaking")
>
> Done.
Thanks, pushed to drm-intel-fixes.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Fixes that failed to backport to v4.11-rc1
2017-03-29 10:45 ` Jani Nikula
2017-03-29 12:13 ` [PATCH] drm/i915/execlists: Wrap tail pointer after reset tweaking Chris Wilson
2017-03-29 12:13 ` Fixes that failed to backport to v4.11-rc1 Chris Wilson
@ 2017-04-11 12:21 ` Jani Nikula
2 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2017-04-11 12:21 UTC (permalink / raw)
To: Chris Wilson, Changbin Du; +Cc: Daniel Vetter, intel-gfx
On Wed, 29 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> On Tue, 21 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
>> On Mon, 13 Mar 2017, Jani Nikula <jani.nikula@intel.com> wrote:
>>> I'm already scripting my fixes backports quite a bit, and frankly don't
>>> really manually backport anything that doesn't apply cleanly. I'm
>>> thinking of automating some "failed to backport" reporting to authors,
>>> not unlike the failed stable backport reports.
>>>
>>> This is a manual report that the following commits have been marked as
>>> Cc: stable or fixing something in v4.11-rc1, but failed to cherry-pick
>>> to drm-intel-fixes. Please see if they are worth backporting, and please
>>> do so if they are.
>>>
>>> Feedback about the idea of this reporting is also appreciated.
>>
>> Refreshed list as of today:
>>
>> bd784b7cc41a ("drm/i915: Avoid rcu_barrier() from reclaim paths (shrinker)")
>> 3fc03069bc6e ("drm/i915: make context status notifier head be per engine")
>> 2e8f9d322948 ("drm/i915: Restore engine->submit_request before unwedging")
>
> Update:
>
> e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
> 450362d3fe86 ("drm/i915/execlists: Wrap tail pointer after reset tweaking")
Update:
a7980a640cbd ("drm/i915: Apply a cond_resched() to the saturated signaler")
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread