* [PATCH] drm/i915: Modify reset func to handle per engine resets
@ 2016-03-16 11:52 Mika Kuoppala
2016-03-16 12:37 ` Chris Wilson
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Mika Kuoppala @ 2016-03-16 11:52 UTC (permalink / raw)
To: intel-gfx
In full gpu reset we prime all engines and reset domains corresponding to
each engine. Per engine reset is just a special case of this process
wherein only a single engine is reset. This change is aimed to modify
relevant functions to achieve this. There are some other steps we carry out
in case of engine reset which are addressed in later patches.
Reset func now accepts a mask of all engines that need to be reset. Where
per engine resets are supported, error handler populates the mask
accordingly otherwise all engines are specified.
v2: ALL_ENGINES mask fixup, better for_each_ring_masked (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 8 ++-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 2 +
drivers/gpu/drm/i915/intel_uncore.c | 88 ++++++++++++++++++++++++---------
6 files changed, 77 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 20e82008b8b6..3648b73b48da 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -881,7 +881,7 @@ int i915_reset(struct drm_device *dev)
simulated = dev_priv->gpu_error.stop_rings != 0;
- ret = intel_gpu_reset(dev);
+ ret = intel_gpu_reset(dev, ALL_ENGINES);
/* Also reset the gpu hangman. */
if (simulated) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 80b14f1ba302..0386310e59c5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1971,6 +1971,10 @@ static inline struct drm_i915_private *guc_to_i915(struct intel_guc *guc)
for ((i__) = 0; (i__) < I915_NUM_RINGS; (i__)++) \
for_each_if ((((ring__) = &(dev_priv__)->ring[(i__)]), intel_ring_initialized((ring__))))
+#define for_each_ring_masked(ring__, dev_priv__, mask__) \
+ for ((ring__) = &dev_priv->ring[0]; (ring__) < &dev_priv->ring[I915_NUM_RINGS]; (ring__)++) \
+ for_each_if (intel_ring_flag((ring__)) & (mask__) && intel_ring_initialized((ring__)))
+
enum hdmi_force_audio {
HDMI_AUDIO_OFF_DVI = -2, /* no aux data for HDMI-DVI converter */
HDMI_AUDIO_OFF, /* force turn off HDMI audio */
@@ -2553,6 +2557,8 @@ struct drm_i915_cmd_table {
#define BLT_RING (1<<BCS)
#define VEBOX_RING (1<<VECS)
#define BSD2_RING (1<<VCS2)
+#define ALL_ENGINES (~0)
+
#define HAS_BSD(dev) (INTEL_INFO(dev)->ring_mask & BSD_RING)
#define HAS_BSD2(dev) (INTEL_INFO(dev)->ring_mask & BSD2_RING)
#define HAS_BLT(dev) (INTEL_INFO(dev)->ring_mask & BLT_RING)
@@ -2681,7 +2687,7 @@ extern void i915_driver_postclose(struct drm_device *dev,
extern long i915_compat_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg);
#endif
-extern int intel_gpu_reset(struct drm_device *dev);
+extern int intel_gpu_reset(struct drm_device *dev, u32 engine_mask);
extern bool intel_has_gpu_reset(struct drm_device *dev);
extern int i915_reset(struct drm_device *dev);
extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b854af2c4141..68d766af2ed0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5018,7 +5018,7 @@ i915_gem_cleanup_ringbuffer(struct drm_device *dev)
* expects the system to be in execlists mode on startup,
* so we need to reset the GPU back to legacy mode.
*/
- intel_gpu_reset(dev);
+ intel_gpu_reset(dev, ALL_ENGINES);
}
static void
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 5dd84e148bba..3251539a6cbe 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -413,7 +413,7 @@ void i915_gem_context_fini(struct drm_device *dev)
/* The only known way to stop the gpu from accessing the hw context is
* to reset it. Do this as the very last operation to avoid confusing
* other code, leading to spurious errors. */
- intel_gpu_reset(dev);
+ intel_gpu_reset(dev, ALL_ENGINES);
/* When default context is created and switched to, base object refcount
* will be 2 (+1 from object creation and +1 from do_switch()).
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7dfc4007f3fa..69359d918a4a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -164,6 +164,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define GEN6_GRDOM_RENDER (1 << 1)
#define GEN6_GRDOM_MEDIA (1 << 2)
#define GEN6_GRDOM_BLT (1 << 3)
+#define GEN6_GRDOM_VECS (1 << 4)
+#define GEN8_GRDOM_MEDIA2 (1 << 7)
#define RING_PP_DIR_BASE(ring) _MMIO((ring)->mmio_base+0x228)
#define RING_PP_DIR_BASE_READ(ring) _MMIO((ring)->mmio_base+0x518)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index d31447f6fa32..cce5003b0dde 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1433,7 +1433,7 @@ static int i915_reset_complete(struct drm_device *dev)
return (gdrst & GRDOM_RESET_STATUS) == 0;
}
-static int i915_do_reset(struct drm_device *dev)
+static int i915_do_reset(struct drm_device *dev, unsigned engine_mask)
{
/* assert reset for at least 20 usec */
pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
@@ -1450,13 +1450,13 @@ static int g4x_reset_complete(struct drm_device *dev)
return (gdrst & GRDOM_RESET_ENABLE) == 0;
}
-static int g33_do_reset(struct drm_device *dev)
+static int g33_do_reset(struct drm_device *dev, unsigned engine_mask)
{
pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
return wait_for(g4x_reset_complete(dev), 500);
}
-static int g4x_do_reset(struct drm_device *dev)
+static int g4x_do_reset(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
@@ -1486,7 +1486,7 @@ static int g4x_do_reset(struct drm_device *dev)
return 0;
}
-static int ironlake_do_reset(struct drm_device *dev)
+static int ironlake_do_reset(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
@@ -1510,21 +1510,63 @@ static int ironlake_do_reset(struct drm_device *dev)
return 0;
}
-static int gen6_do_reset(struct drm_device *dev)
+/* Reset the hardware domains (GENX_GRDOM_*) specified by mask */
+static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
+ u32 hw_domain_mask)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- int ret;
-
- /* Reset the chip */
+ int ret;
/* GEN6_GDRST is not in the gt power well, no need to check
* for fifo space for the write or forcewake the chip for
* the read
*/
- __raw_i915_write32(dev_priv, GEN6_GDRST, GEN6_GRDOM_FULL);
+ __raw_i915_write32(dev_priv, GEN6_GDRST, hw_domain_mask);
+
+#define ACKED ((__raw_i915_read32(dev_priv, GEN6_GDRST) & hw_domain_mask) == 0)
+ /* Spin waiting for the device to ack the reset requests */
+ ret = wait_for(ACKED, 500);
+#undef ACKED
+
+ return ret;
+}
+
+/**
+ * gen6_reset_engines - reset individual engines
+ * @dev: DRM device
+ * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset
+ *
+ * This function will reset the individual engines that are set in engine_mask.
+ * If you provide ALL_ENGINES as mask, full global domain reset will be issued.
+ *
+ * Note: It is responsibility of the caller to handle the difference between
+ * asking full domain reset versus reset for all available individual engines.
+ *
+ * Returns 0 on success, nonzero on error.
+ */
+static int gen6_reset_engines(struct drm_device *dev, unsigned engine_mask)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_engine_cs *engine;
+ const u32 hw_engine_mask[I915_NUM_RINGS] = {
+ [RCS] = GEN6_GRDOM_RENDER,
+ [BCS] = GEN6_GRDOM_BLT,
+ [VCS] = GEN6_GRDOM_MEDIA,
+ [VCS2] = GEN8_GRDOM_MEDIA2,
+ [VECS] = GEN6_GRDOM_VECS,
+ };
+ int ret;
+ u32 hw_mask;
- /* Spin waiting for the device to ack the reset request */
- ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) & GEN6_GRDOM_FULL) == 0, 500);
+ if (engine_mask == ALL_ENGINES) {
+ hw_mask = GEN6_GRDOM_FULL;
+ } else {
+ hw_mask = 0;
+
+ for_each_ring_masked(engine, dev_priv, engine_mask)
+ hw_mask |= hw_engine_mask[engine->id];
+ }
+
+ ret = gen6_hw_domain_reset(dev_priv, hw_mask);
intel_uncore_forcewake_reset(dev, true);
@@ -1567,34 +1609,34 @@ static void gen8_unrequest_engine_reset(struct intel_engine_cs *engine)
_MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
}
-static int gen8_do_reset(struct drm_device *dev)
+static int gen8_reset_engines(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
- int i;
- for_each_ring(engine, dev_priv, i)
+ for_each_ring_masked(engine, dev_priv, engine_mask)
if (gen8_request_engine_reset(engine))
goto not_ready;
- return gen6_do_reset(dev);
+ return gen6_reset_engines(dev, engine_mask);
not_ready:
- for_each_ring(engine, dev_priv, i)
+ for_each_ring_masked(engine, dev_priv, engine_mask)
gen8_unrequest_engine_reset(engine);
return -EIO;
}
-static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
+static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *,
+ unsigned engine_mask)
{
if (!i915.reset)
return NULL;
if (INTEL_INFO(dev)->gen >= 8)
- return gen8_do_reset;
+ return gen8_reset_engines;
else if (INTEL_INFO(dev)->gen >= 6)
- return gen6_do_reset;
+ return gen6_reset_engines;
else if (IS_GEN5(dev))
return ironlake_do_reset;
else if (IS_G4X(dev))
@@ -1607,10 +1649,10 @@ static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
return NULL;
}
-int intel_gpu_reset(struct drm_device *dev)
+int intel_gpu_reset(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- int (*reset)(struct drm_device *);
+ int (*reset)(struct drm_device *, unsigned);
int ret;
reset = intel_get_gpu_reset(dev);
@@ -1621,7 +1663,7 @@ int intel_gpu_reset(struct drm_device *dev)
* request may be dropped and never completes (causing -EIO).
*/
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
- ret = reset(dev);
+ ret = reset(dev, engine_mask);
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
return ret;
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Modify reset func to handle per engine resets
2016-03-16 11:52 [PATCH] drm/i915: Modify reset func to handle per engine resets Mika Kuoppala
@ 2016-03-16 12:37 ` Chris Wilson
2016-03-16 15:30 ` ✗ Fi.CI.BAT: failure for " Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2016-03-16 12:37 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
On Wed, Mar 16, 2016 at 01:52:04PM +0200, Mika Kuoppala wrote:
> In full gpu reset we prime all engines and reset domains corresponding to
> each engine. Per engine reset is just a special case of this process
> wherein only a single engine is reset. This change is aimed to modify
> relevant functions to achieve this. There are some other steps we carry out
> in case of engine reset which are addressed in later patches.
>
> Reset func now accepts a mask of all engines that need to be reset. Where
> per engine resets are supported, error handler populates the mask
> accordingly otherwise all engines are specified.
>
> v2: ALL_ENGINES mask fixup, better for_each_ring_masked (Chris)
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index b854af2c4141..68d766af2ed0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5018,7 +5018,7 @@ i915_gem_cleanup_ringbuffer(struct drm_device *dev)
> * expects the system to be in execlists mode on startup,
> * so we need to reset the GPU back to legacy mode.
> */
> - intel_gpu_reset(dev);
> + intel_gpu_reset(dev, ALL_ENGINES);
Hmm, whitespace cleanup required around here?
> +/**
> + * gen6_reset_engines - reset individual engines
> + * @dev: DRM device
> + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset
> + *
> + * This function will reset the individual engines that are set in engine_mask.
> + * If you provide ALL_ENGINES as mask, full global domain reset will be issued.
> + *
> + * Note: It is responsibility of the caller to handle the difference between
> + * asking full domain reset versus reset for all available individual engines.
> + *
> + * Returns 0 on success, nonzero on error.
> + */
> +static int gen6_reset_engines(struct drm_device *dev, unsigned engine_mask)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_engine_cs *engine;
> + const u32 hw_engine_mask[I915_NUM_RINGS] = {
> + [RCS] = GEN6_GRDOM_RENDER,
> + [BCS] = GEN6_GRDOM_BLT,
> + [VCS] = GEN6_GRDOM_MEDIA,
> + [VCS2] = GEN8_GRDOM_MEDIA2,
> + [VECS] = GEN6_GRDOM_VECS,
> + };
> + int ret;
> + u32 hw_mask;
u32 hw_mask;
int ret;
looks neater;
>
> - /* Spin waiting for the device to ack the reset request */
> - ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) & GEN6_GRDOM_FULL) == 0, 500);
> + if (engine_mask == ALL_ENGINES) {
> + hw_mask = GEN6_GRDOM_FULL;
> + } else {
> + hw_mask = 0;
> +
and this whitespace doesn't help since this block is entirely about the
computation of hw_mask
That's the sum total of my criticism.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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] 8+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Modify reset func to handle per engine resets
2016-03-16 11:52 [PATCH] drm/i915: Modify reset func to handle per engine resets Mika Kuoppala
2016-03-16 12:37 ` Chris Wilson
@ 2016-03-16 15:30 ` Patchwork
2016-03-16 15:40 ` [PATCH] " Mika Kuoppala
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2016-03-16 15:30 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Modify reset func to handle per engine resets
URL : https://patchwork.freedesktop.org/series/4510/
State : failure
== Summary ==
Series 4510v1 drm/i915: Modify reset func to handle per engine resets
http://patchwork.freedesktop.org/api/1.0/series/4510/revisions/1/mbox/
Test drv_module_reload_basic:
skip -> PASS (bdw-nuci7)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> DMESG-WARN (bdw-ultra)
Test kms_frontbuffer_tracking:
Subgroup basic:
dmesg-warn -> PASS (hsw-brixbox)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-b:
pass -> DMESG-WARN (snb-x220t)
Subgroup nonblocking-crc-pipe-b-frame-sequence:
pass -> DMESG-WARN (hsw-gt2)
dmesg-warn -> PASS (bdw-ultra)
Subgroup read-crc-pipe-b:
dmesg-warn -> PASS (hsw-gt2)
Subgroup read-crc-pipe-b-frame-sequence:
dmesg-warn -> PASS (hsw-brixbox)
Test pm_rpm:
Subgroup basic-rte:
dmesg-warn -> PASS (snb-x220t)
bdw-nuci7 total:194 pass:182 dwarn:0 dfail:0 fail:0 skip:12
bdw-ultra total:194 pass:172 dwarn:1 dfail:0 fail:0 skip:21
byt-nuc total:194 pass:155 dwarn:4 dfail:0 fail:0 skip:35
hsw-brixbox total:194 pass:172 dwarn:0 dfail:0 fail:0 skip:22
hsw-gt2 total:194 pass:176 dwarn:1 dfail:0 fail:0 skip:17
ivb-t430s total:194 pass:169 dwarn:0 dfail:0 fail:0 skip:25
skl-i5k-2 total:194 pass:171 dwarn:0 dfail:0 fail:0 skip:23
skl-i7k-2 total:194 pass:171 dwarn:0 dfail:0 fail:0 skip:23
skl-nuci5 total:194 pass:183 dwarn:0 dfail:0 fail:0 skip:11
snb-x220t total:194 pass:159 dwarn:1 dfail:0 fail:1 skip:33
Results at /archive/results/CI_IGT_test/Patchwork_1617/
a5c43f5d1b4968a370f54bdda5387ce213aca785 drm-intel-nightly: 2016y-03m-16d-14h-09m-03s UTC integration manifest
05e8832f029f89fa0a70993a6489f717bf7f4588 drm/i915: Modify reset func to handle per engine resets
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] drm/i915: Modify reset func to handle per engine resets
2016-03-16 11:52 [PATCH] drm/i915: Modify reset func to handle per engine resets Mika Kuoppala
2016-03-16 12:37 ` Chris Wilson
2016-03-16 15:30 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-03-16 15:40 ` Mika Kuoppala
2016-03-16 15:54 ` Mika Kuoppala
2016-03-17 7:36 ` ✗ Fi.CI.BAT: failure for drm/i915: Modify reset func to handle per engine resets (rev3) Patchwork
4 siblings, 0 replies; 8+ messages in thread
From: Mika Kuoppala @ 2016-03-16 15:40 UTC (permalink / raw)
To: intel-gfx
In full gpu reset we prime all engines and reset domains corresponding to
each engine. Per engine reset is just a special case of this process
wherein only a single engine is reset. This change is aimed to modify
relevant functions to achieve this. There are some other steps we carry out
in case of engine reset which are addressed in later patches.
Reset func now accepts a mask of all engines that need to be reset. Where
per engine resets are supported, error handler populates the mask
accordingly otherwise all engines are specified.
v2: ALL_ENGINES mask fixup, better for_each_ring_masked (Chris)
v3: Whitespace fixes (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 8 ++-
drivers/gpu/drm/i915/i915_gem.c | 14 +++---
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 2 +
drivers/gpu/drm/i915/intel_uncore.c | 87 ++++++++++++++++++++++++---------
6 files changed, 82 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 20e82008b8b6..3648b73b48da 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -881,7 +881,7 @@ int i915_reset(struct drm_device *dev)
simulated = dev_priv->gpu_error.stop_rings != 0;
- ret = intel_gpu_reset(dev);
+ ret = intel_gpu_reset(dev, ALL_ENGINES);
/* Also reset the gpu hangman. */
if (simulated) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 80b14f1ba302..0386310e59c5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1971,6 +1971,10 @@ static inline struct drm_i915_private *guc_to_i915(struct intel_guc *guc)
for ((i__) = 0; (i__) < I915_NUM_RINGS; (i__)++) \
for_each_if ((((ring__) = &(dev_priv__)->ring[(i__)]), intel_ring_initialized((ring__))))
+#define for_each_ring_masked(ring__, dev_priv__, mask__) \
+ for ((ring__) = &dev_priv->ring[0]; (ring__) < &dev_priv->ring[I915_NUM_RINGS]; (ring__)++) \
+ for_each_if (intel_ring_flag((ring__)) & (mask__) && intel_ring_initialized((ring__)))
+
enum hdmi_force_audio {
HDMI_AUDIO_OFF_DVI = -2, /* no aux data for HDMI-DVI converter */
HDMI_AUDIO_OFF, /* force turn off HDMI audio */
@@ -2553,6 +2557,8 @@ struct drm_i915_cmd_table {
#define BLT_RING (1<<BCS)
#define VEBOX_RING (1<<VECS)
#define BSD2_RING (1<<VCS2)
+#define ALL_ENGINES (~0)
+
#define HAS_BSD(dev) (INTEL_INFO(dev)->ring_mask & BSD_RING)
#define HAS_BSD2(dev) (INTEL_INFO(dev)->ring_mask & BSD2_RING)
#define HAS_BLT(dev) (INTEL_INFO(dev)->ring_mask & BLT_RING)
@@ -2681,7 +2687,7 @@ extern void i915_driver_postclose(struct drm_device *dev,
extern long i915_compat_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg);
#endif
-extern int intel_gpu_reset(struct drm_device *dev);
+extern int intel_gpu_reset(struct drm_device *dev, u32 engine_mask);
extern bool intel_has_gpu_reset(struct drm_device *dev);
extern int i915_reset(struct drm_device *dev);
extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b854af2c4141..4c0cdd5ee8cf 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5012,13 +5012,13 @@ i915_gem_cleanup_ringbuffer(struct drm_device *dev)
for_each_ring(ring, dev_priv, i)
dev_priv->gt.cleanup_ring(ring);
- if (i915.enable_execlists)
- /*
- * Neither the BIOS, ourselves or any other kernel
- * expects the system to be in execlists mode on startup,
- * so we need to reset the GPU back to legacy mode.
- */
- intel_gpu_reset(dev);
+ if (i915.enable_execlists)
+ /*
+ * Neither the BIOS, ourselves or any other kernel
+ * expects the system to be in execlists mode on startup,
+ * so we need to reset the GPU back to legacy mode.
+ */
+ intel_gpu_reset(dev, ALL_ENGINES);
}
static void
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 5dd84e148bba..3251539a6cbe 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -413,7 +413,7 @@ void i915_gem_context_fini(struct drm_device *dev)
/* The only known way to stop the gpu from accessing the hw context is
* to reset it. Do this as the very last operation to avoid confusing
* other code, leading to spurious errors. */
- intel_gpu_reset(dev);
+ intel_gpu_reset(dev, ALL_ENGINES);
/* When default context is created and switched to, base object refcount
* will be 2 (+1 from object creation and +1 from do_switch()).
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7dfc4007f3fa..69359d918a4a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -164,6 +164,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define GEN6_GRDOM_RENDER (1 << 1)
#define GEN6_GRDOM_MEDIA (1 << 2)
#define GEN6_GRDOM_BLT (1 << 3)
+#define GEN6_GRDOM_VECS (1 << 4)
+#define GEN8_GRDOM_MEDIA2 (1 << 7)
#define RING_PP_DIR_BASE(ring) _MMIO((ring)->mmio_base+0x228)
#define RING_PP_DIR_BASE_READ(ring) _MMIO((ring)->mmio_base+0x518)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index d31447f6fa32..280426de6425 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1433,7 +1433,7 @@ static int i915_reset_complete(struct drm_device *dev)
return (gdrst & GRDOM_RESET_STATUS) == 0;
}
-static int i915_do_reset(struct drm_device *dev)
+static int i915_do_reset(struct drm_device *dev, unsigned engine_mask)
{
/* assert reset for at least 20 usec */
pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
@@ -1450,13 +1450,13 @@ static int g4x_reset_complete(struct drm_device *dev)
return (gdrst & GRDOM_RESET_ENABLE) == 0;
}
-static int g33_do_reset(struct drm_device *dev)
+static int g33_do_reset(struct drm_device *dev, unsigned engine_mask)
{
pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
return wait_for(g4x_reset_complete(dev), 500);
}
-static int g4x_do_reset(struct drm_device *dev)
+static int g4x_do_reset(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
@@ -1486,7 +1486,7 @@ static int g4x_do_reset(struct drm_device *dev)
return 0;
}
-static int ironlake_do_reset(struct drm_device *dev)
+static int ironlake_do_reset(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
@@ -1510,21 +1510,62 @@ static int ironlake_do_reset(struct drm_device *dev)
return 0;
}
-static int gen6_do_reset(struct drm_device *dev)
+/* Reset the hardware domains (GENX_GRDOM_*) specified by mask */
+static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
+ u32 hw_domain_mask)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- int ret;
-
- /* Reset the chip */
+ int ret;
/* GEN6_GDRST is not in the gt power well, no need to check
* for fifo space for the write or forcewake the chip for
* the read
*/
- __raw_i915_write32(dev_priv, GEN6_GDRST, GEN6_GRDOM_FULL);
+ __raw_i915_write32(dev_priv, GEN6_GDRST, hw_domain_mask);
- /* Spin waiting for the device to ack the reset request */
- ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) & GEN6_GRDOM_FULL) == 0, 500);
+#define ACKED ((__raw_i915_read32(dev_priv, GEN6_GDRST) & hw_domain_mask) == 0)
+ /* Spin waiting for the device to ack the reset requests */
+ ret = wait_for(ACKED, 500);
+#undef ACKED
+
+ return ret;
+}
+
+/**
+ * gen6_reset_engines - reset individual engines
+ * @dev: DRM device
+ * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset
+ *
+ * This function will reset the individual engines that are set in engine_mask.
+ * If you provide ALL_ENGINES as mask, full global domain reset will be issued.
+ *
+ * Note: It is responsibility of the caller to handle the difference between
+ * asking full domain reset versus reset for all available individual engines.
+ *
+ * Returns 0 on success, nonzero on error.
+ */
+static int gen6_reset_engines(struct drm_device *dev, unsigned engine_mask)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_engine_cs *engine;
+ const u32 hw_engine_mask[I915_NUM_RINGS] = {
+ [RCS] = GEN6_GRDOM_RENDER,
+ [BCS] = GEN6_GRDOM_BLT,
+ [VCS] = GEN6_GRDOM_MEDIA,
+ [VCS2] = GEN8_GRDOM_MEDIA2,
+ [VECS] = GEN6_GRDOM_VECS,
+ };
+ u32 hw_mask;
+ int ret;
+
+ if (engine_mask == ALL_ENGINES) {
+ hw_mask = GEN6_GRDOM_FULL;
+ } else {
+ hw_mask = 0;
+ for_each_ring_masked(engine, dev_priv, engine_mask)
+ hw_mask |= hw_engine_mask[engine->id];
+ }
+
+ ret = gen6_hw_domain_reset(dev_priv, hw_mask);
intel_uncore_forcewake_reset(dev, true);
@@ -1567,34 +1608,34 @@ static void gen8_unrequest_engine_reset(struct intel_engine_cs *engine)
_MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
}
-static int gen8_do_reset(struct drm_device *dev)
+static int gen8_reset_engines(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
- int i;
- for_each_ring(engine, dev_priv, i)
+ for_each_ring_masked(engine, dev_priv, engine_mask)
if (gen8_request_engine_reset(engine))
goto not_ready;
- return gen6_do_reset(dev);
+ return gen6_reset_engines(dev, engine_mask);
not_ready:
- for_each_ring(engine, dev_priv, i)
+ for_each_ring_masked(engine, dev_priv, engine_mask)
gen8_unrequest_engine_reset(engine);
return -EIO;
}
-static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
+static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *,
+ unsigned engine_mask)
{
if (!i915.reset)
return NULL;
if (INTEL_INFO(dev)->gen >= 8)
- return gen8_do_reset;
+ return gen8_reset_engines;
else if (INTEL_INFO(dev)->gen >= 6)
- return gen6_do_reset;
+ return gen6_reset_engines;
else if (IS_GEN5(dev))
return ironlake_do_reset;
else if (IS_G4X(dev))
@@ -1607,10 +1648,10 @@ static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
return NULL;
}
-int intel_gpu_reset(struct drm_device *dev)
+int intel_gpu_reset(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- int (*reset)(struct drm_device *);
+ int (*reset)(struct drm_device *, unsigned);
int ret;
reset = intel_get_gpu_reset(dev);
@@ -1621,7 +1662,7 @@ int intel_gpu_reset(struct drm_device *dev)
* request may be dropped and never completes (causing -EIO).
*/
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
- ret = reset(dev);
+ ret = reset(dev, engine_mask);
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
return ret;
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] drm/i915: Modify reset func to handle per engine resets
2016-03-16 11:52 [PATCH] drm/i915: Modify reset func to handle per engine resets Mika Kuoppala
` (2 preceding siblings ...)
2016-03-16 15:40 ` [PATCH] " Mika Kuoppala
@ 2016-03-16 15:54 ` Mika Kuoppala
2016-03-17 13:14 ` Mika Kuoppala
2016-03-17 7:36 ` ✗ Fi.CI.BAT: failure for drm/i915: Modify reset func to handle per engine resets (rev3) Patchwork
4 siblings, 1 reply; 8+ messages in thread
From: Mika Kuoppala @ 2016-03-16 15:54 UTC (permalink / raw)
To: intel-gfx
In full gpu reset we prime all engines and reset domains corresponding to
each engine. Per engine reset is just a special case of this process
wherein only a single engine is reset. This change is aimed to modify
relevant functions to achieve this. There are some other steps we carry out
in case of engine reset which are addressed in later patches.
Reset func now accepts a mask of all engines that need to be reset. Where
per engine resets are supported, error handler populates the mask
accordingly otherwise all engines are specified.
v2: ALL_ENGINES mask fixup, better for_each_ring_masked (Chris)
v3: Whitespace fixes (Chris)
v4: Rebase due to s/ring/engine
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 8 ++-
drivers/gpu/drm/i915/i915_gem.c | 14 +++---
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 2 +
drivers/gpu/drm/i915/intel_uncore.c | 87 ++++++++++++++++++++++++---------
6 files changed, 82 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 20e82008b8b6..3648b73b48da 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -881,7 +881,7 @@ int i915_reset(struct drm_device *dev)
simulated = dev_priv->gpu_error.stop_rings != 0;
- ret = intel_gpu_reset(dev);
+ ret = intel_gpu_reset(dev, ALL_ENGINES);
/* Also reset the gpu hangman. */
if (simulated) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fd1ed66dd298..8940b38ed57f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1971,6 +1971,10 @@ static inline struct drm_i915_private *guc_to_i915(struct intel_guc *guc)
for ((i__) = 0; (i__) < I915_NUM_ENGINES; (i__)++) \
for_each_if ((((ring__) = &(dev_priv__)->engine[(i__)]), intel_engine_initialized((ring__))))
+#define for_each_engine_masked(engine__, dev_priv__, mask__) \
+ for ((engine__) = &dev_priv->engine[0]; (engine__) < &dev_priv->engine[I915_NUM_ENGINES]; (engine__)++) \
+ for_each_if (intel_engine_flag((engine__)) & (mask__) && intel_engine_initialized((engine__)))
+
enum hdmi_force_audio {
HDMI_AUDIO_OFF_DVI = -2, /* no aux data for HDMI-DVI converter */
HDMI_AUDIO_OFF, /* force turn off HDMI audio */
@@ -2553,6 +2557,8 @@ struct drm_i915_cmd_table {
#define BLT_RING (1<<BCS)
#define VEBOX_RING (1<<VECS)
#define BSD2_RING (1<<VCS2)
+#define ALL_ENGINES (~0)
+
#define HAS_BSD(dev) (INTEL_INFO(dev)->ring_mask & BSD_RING)
#define HAS_BSD2(dev) (INTEL_INFO(dev)->ring_mask & BSD2_RING)
#define HAS_BLT(dev) (INTEL_INFO(dev)->ring_mask & BLT_RING)
@@ -2681,7 +2687,7 @@ extern void i915_driver_postclose(struct drm_device *dev,
extern long i915_compat_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg);
#endif
-extern int intel_gpu_reset(struct drm_device *dev);
+extern int intel_gpu_reset(struct drm_device *dev, u32 engine_mask);
extern bool intel_has_gpu_reset(struct drm_device *dev);
extern int i915_reset(struct drm_device *dev);
extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 31652c1da761..a25109aa033c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5012,13 +5012,13 @@ i915_gem_cleanup_engines(struct drm_device *dev)
for_each_engine(engine, dev_priv, i)
dev_priv->gt.cleanup_engine(engine);
- if (i915.enable_execlists)
- /*
- * Neither the BIOS, ourselves or any other kernel
- * expects the system to be in execlists mode on startup,
- * so we need to reset the GPU back to legacy mode.
- */
- intel_gpu_reset(dev);
+ if (i915.enable_execlists)
+ /*
+ * Neither the BIOS, ourselves or any other kernel
+ * expects the system to be in execlists mode on startup,
+ * so we need to reset the GPU back to legacy mode.
+ */
+ intel_gpu_reset(dev, ALL_ENGINES);
}
static void
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 1993449ab7c5..c114665a24b3 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -413,7 +413,7 @@ void i915_gem_context_fini(struct drm_device *dev)
/* The only known way to stop the gpu from accessing the hw context is
* to reset it. Do this as the very last operation to avoid confusing
* other code, leading to spurious errors. */
- intel_gpu_reset(dev);
+ intel_gpu_reset(dev, ALL_ENGINES);
/* When default context is created and switched to, base object refcount
* will be 2 (+1 from object creation and +1 from do_switch()).
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7dfc4007f3fa..69359d918a4a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -164,6 +164,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define GEN6_GRDOM_RENDER (1 << 1)
#define GEN6_GRDOM_MEDIA (1 << 2)
#define GEN6_GRDOM_BLT (1 << 3)
+#define GEN6_GRDOM_VECS (1 << 4)
+#define GEN8_GRDOM_MEDIA2 (1 << 7)
#define RING_PP_DIR_BASE(ring) _MMIO((ring)->mmio_base+0x228)
#define RING_PP_DIR_BASE_READ(ring) _MMIO((ring)->mmio_base+0x518)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 02add02e0ce4..512b7faedefd 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1433,7 +1433,7 @@ static int i915_reset_complete(struct drm_device *dev)
return (gdrst & GRDOM_RESET_STATUS) == 0;
}
-static int i915_do_reset(struct drm_device *dev)
+static int i915_do_reset(struct drm_device *dev, unsigned engine_mask)
{
/* assert reset for at least 20 usec */
pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
@@ -1450,13 +1450,13 @@ static int g4x_reset_complete(struct drm_device *dev)
return (gdrst & GRDOM_RESET_ENABLE) == 0;
}
-static int g33_do_reset(struct drm_device *dev)
+static int g33_do_reset(struct drm_device *dev, unsigned engine_mask)
{
pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
return wait_for(g4x_reset_complete(dev), 500);
}
-static int g4x_do_reset(struct drm_device *dev)
+static int g4x_do_reset(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
@@ -1486,7 +1486,7 @@ static int g4x_do_reset(struct drm_device *dev)
return 0;
}
-static int ironlake_do_reset(struct drm_device *dev)
+static int ironlake_do_reset(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
@@ -1510,21 +1510,62 @@ static int ironlake_do_reset(struct drm_device *dev)
return 0;
}
-static int gen6_do_reset(struct drm_device *dev)
+/* Reset the hardware domains (GENX_GRDOM_*) specified by mask */
+static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
+ u32 hw_domain_mask)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- int ret;
-
- /* Reset the chip */
+ int ret;
/* GEN6_GDRST is not in the gt power well, no need to check
* for fifo space for the write or forcewake the chip for
* the read
*/
- __raw_i915_write32(dev_priv, GEN6_GDRST, GEN6_GRDOM_FULL);
+ __raw_i915_write32(dev_priv, GEN6_GDRST, hw_domain_mask);
- /* Spin waiting for the device to ack the reset request */
- ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) & GEN6_GRDOM_FULL) == 0, 500);
+#define ACKED ((__raw_i915_read32(dev_priv, GEN6_GDRST) & hw_domain_mask) == 0)
+ /* Spin waiting for the device to ack the reset requests */
+ ret = wait_for(ACKED, 500);
+#undef ACKED
+
+ return ret;
+}
+
+/**
+ * gen6_reset_engines - reset individual engines
+ * @dev: DRM device
+ * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset
+ *
+ * This function will reset the individual engines that are set in engine_mask.
+ * If you provide ALL_ENGINES as mask, full global domain reset will be issued.
+ *
+ * Note: It is responsibility of the caller to handle the difference between
+ * asking full domain reset versus reset for all available individual engines.
+ *
+ * Returns 0 on success, nonzero on error.
+ */
+static int gen6_reset_engines(struct drm_device *dev, unsigned engine_mask)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_engine_cs *engine;
+ const u32 hw_engine_mask[I915_NUM_ENGINES] = {
+ [RCS] = GEN6_GRDOM_RENDER,
+ [BCS] = GEN6_GRDOM_BLT,
+ [VCS] = GEN6_GRDOM_MEDIA,
+ [VCS2] = GEN8_GRDOM_MEDIA2,
+ [VECS] = GEN6_GRDOM_VECS,
+ };
+ u32 hw_mask;
+ int ret;
+
+ if (engine_mask == ALL_ENGINES) {
+ hw_mask = GEN6_GRDOM_FULL;
+ } else {
+ hw_mask = 0;
+ for_each_engine_masked(engine, dev_priv, engine_mask)
+ hw_mask |= hw_engine_mask[engine->id];
+ }
+
+ ret = gen6_hw_domain_reset(dev_priv, hw_mask);
intel_uncore_forcewake_reset(dev, true);
@@ -1567,34 +1608,34 @@ static void gen8_unrequest_engine_reset(struct intel_engine_cs *engine)
_MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
}
-static int gen8_do_reset(struct drm_device *dev)
+static int gen8_reset_engines(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
- int i;
- for_each_engine(engine, dev_priv, i)
+ for_each_engine_masked(engine, dev_priv, engine_mask)
if (gen8_request_engine_reset(engine))
goto not_ready;
- return gen6_do_reset(dev);
+ return gen6_reset_engines(dev, engine_mask);
not_ready:
- for_each_engine(engine, dev_priv, i)
+ for_each_engine_masked(engine, dev_priv, engine_mask)
gen8_unrequest_engine_reset(engine);
return -EIO;
}
-static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
+static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *,
+ unsigned engine_mask)
{
if (!i915.reset)
return NULL;
if (INTEL_INFO(dev)->gen >= 8)
- return gen8_do_reset;
+ return gen8_reset_engines;
else if (INTEL_INFO(dev)->gen >= 6)
- return gen6_do_reset;
+ return gen6_reset_engines;
else if (IS_GEN5(dev))
return ironlake_do_reset;
else if (IS_G4X(dev))
@@ -1607,10 +1648,10 @@ static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
return NULL;
}
-int intel_gpu_reset(struct drm_device *dev)
+int intel_gpu_reset(struct drm_device *dev, unsigned engine_mask)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- int (*reset)(struct drm_device *);
+ int (*reset)(struct drm_device *, unsigned);
int ret;
reset = intel_get_gpu_reset(dev);
@@ -1621,7 +1662,7 @@ int intel_gpu_reset(struct drm_device *dev)
* request may be dropped and never completes (causing -EIO).
*/
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
- ret = reset(dev);
+ ret = reset(dev, engine_mask);
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
return ret;
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Modify reset func to handle per engine resets (rev3)
2016-03-16 11:52 [PATCH] drm/i915: Modify reset func to handle per engine resets Mika Kuoppala
` (3 preceding siblings ...)
2016-03-16 15:54 ` Mika Kuoppala
@ 2016-03-17 7:36 ` Patchwork
2016-03-17 12:02 ` Mika Kuoppala
4 siblings, 1 reply; 8+ messages in thread
From: Patchwork @ 2016-03-17 7:36 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Modify reset func to handle per engine resets (rev3)
URL : https://patchwork.freedesktop.org/series/4510/
State : failure
== Summary ==
Series 4510v3 drm/i915: Modify reset func to handle per engine resets
http://patchwork.freedesktop.org/api/1.0/series/4510/revisions/3/mbox/
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
dmesg-warn -> PASS (hsw-brixbox)
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-b:
pass -> DMESG-WARN (hsw-brixbox)
Subgroup nonblocking-crc-pipe-c:
pass -> DMESG-WARN (hsw-brixbox)
Subgroup read-crc-pipe-a-frame-sequence:
pass -> DMESG-WARN (bdw-ultra)
bdw-nuci7 total:194 pass:182 dwarn:0 dfail:0 fail:0 skip:12
bdw-ultra total:194 pass:172 dwarn:1 dfail:0 fail:0 skip:21
byt-nuc total:194 pass:155 dwarn:4 dfail:0 fail:0 skip:35
hsw-brixbox total:194 pass:170 dwarn:2 dfail:0 fail:0 skip:22
hsw-gt2 total:194 pass:175 dwarn:2 dfail:0 fail:0 skip:17
ivb-t430s total:194 pass:169 dwarn:0 dfail:0 fail:0 skip:25
skl-i5k-2 total:194 pass:171 dwarn:0 dfail:0 fail:0 skip:23
skl-i7k-2 total:194 pass:171 dwarn:0 dfail:0 fail:0 skip:23
skl-nuci5 total:194 pass:183 dwarn:0 dfail:0 fail:0 skip:11
Results at /archive/results/CI_IGT_test/Patchwork_1620/
dbbc6d276864d7b7a3a1edb04f0511153f9c3852 drm-intel-nightly: 2016y-03m-16d-22h-27m-36s UTC integration manifest
5511c10cc346289acc0ecbd55b58c2a21adc18f0 drm/i915: Modify reset func to handle per engine resets
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ✗ Fi.CI.BAT: failure for drm/i915: Modify reset func to handle per engine resets (rev3)
2016-03-17 7:36 ` ✗ Fi.CI.BAT: failure for drm/i915: Modify reset func to handle per engine resets (rev3) Patchwork
@ 2016-03-17 12:02 ` Mika Kuoppala
0 siblings, 0 replies; 8+ messages in thread
From: Mika Kuoppala @ 2016-03-17 12:02 UTC (permalink / raw)
To: Patchwork; +Cc: intel-gfx
Patchwork <patchwork@emeril.freedesktop.org> writes:
> [ text/plain ]
> == Series Details ==
>
> Series: drm/i915: Modify reset func to handle per engine resets (rev3)
> URL : https://patchwork.freedesktop.org/series/4510/
> State : failure
>
> == Summary ==
>
> Series 4510v3 drm/i915: Modify reset func to handle per engine resets
> http://patchwork.freedesktop.org/api/1.0/series/4510/revisions/3/mbox/
>
> Test kms_flip:
> Subgroup basic-flip-vs-wf_vblank:
> dmesg-warn -> PASS (hsw-brixbox)
> Test kms_pipe_crc_basic:
> Subgroup nonblocking-crc-pipe-b:
> pass -> DMESG-WARN (hsw-brixbox)
https://bugs.freedesktop.org/show_bug.cgi?id=94349
> Subgroup nonblocking-crc-pipe-c:
> pass -> DMESG-WARN (hsw-brixbox)
https://bugs.freedesktop.org/show_bug.cgi?id=94384
> Subgroup read-crc-pipe-a-frame-sequence:
> pass -> DMESG-WARN (bdw-ultra)
>
https://bugs.freedesktop.org/show_bug.cgi?id=94349
-Mika
> bdw-nuci7 total:194 pass:182 dwarn:0 dfail:0 fail:0 skip:12
> bdw-ultra total:194 pass:172 dwarn:1 dfail:0 fail:0 skip:21
> byt-nuc total:194 pass:155 dwarn:4 dfail:0 fail:0 skip:35
> hsw-brixbox total:194 pass:170 dwarn:2 dfail:0 fail:0 skip:22
> hsw-gt2 total:194 pass:175 dwarn:2 dfail:0 fail:0 skip:17
> ivb-t430s total:194 pass:169 dwarn:0 dfail:0 fail:0 skip:25
> skl-i5k-2 total:194 pass:171 dwarn:0 dfail:0 fail:0 skip:23
> skl-i7k-2 total:194 pass:171 dwarn:0 dfail:0 fail:0 skip:23
> skl-nuci5 total:194 pass:183 dwarn:0 dfail:0 fail:0 skip:11
>
> Results at /archive/results/CI_IGT_test/Patchwork_1620/
>
> dbbc6d276864d7b7a3a1edb04f0511153f9c3852 drm-intel-nightly: 2016y-03m-16d-22h-27m-36s UTC integration manifest
> 5511c10cc346289acc0ecbd55b58c2a21adc18f0 drm/i915: Modify reset func to handle per engine resets
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Modify reset func to handle per engine resets
2016-03-16 15:54 ` Mika Kuoppala
@ 2016-03-17 13:14 ` Mika Kuoppala
0 siblings, 0 replies; 8+ messages in thread
From: Mika Kuoppala @ 2016-03-17 13:14 UTC (permalink / raw)
To: intel-gfx
Mika Kuoppala <mika.kuoppala@linux.intel.com> writes:
> [ text/plain ]
> In full gpu reset we prime all engines and reset domains corresponding to
> each engine. Per engine reset is just a special case of this process
> wherein only a single engine is reset. This change is aimed to modify
> relevant functions to achieve this. There are some other steps we carry out
> in case of engine reset which are addressed in later patches.
>
> Reset func now accepts a mask of all engines that need to be reset. Where
> per engine resets are supported, error handler populates the mask
> accordingly otherwise all engines are specified.
>
> v2: ALL_ENGINES mask fixup, better for_each_ring_masked (Chris)
> v3: Whitespace fixes (Chris)
> v4: Rebase due to s/ring/engine
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Pushed to dinq. Thank you all.
-Mika
> ---
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 8 ++-
> drivers/gpu/drm/i915/i915_gem.c | 14 +++---
> drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 2 +
> drivers/gpu/drm/i915/intel_uncore.c | 87 ++++++++++++++++++++++++---------
> 6 files changed, 82 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 20e82008b8b6..3648b73b48da 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -881,7 +881,7 @@ int i915_reset(struct drm_device *dev)
>
> simulated = dev_priv->gpu_error.stop_rings != 0;
>
> - ret = intel_gpu_reset(dev);
> + ret = intel_gpu_reset(dev, ALL_ENGINES);
>
> /* Also reset the gpu hangman. */
> if (simulated) {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index fd1ed66dd298..8940b38ed57f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1971,6 +1971,10 @@ static inline struct drm_i915_private *guc_to_i915(struct intel_guc *guc)
> for ((i__) = 0; (i__) < I915_NUM_ENGINES; (i__)++) \
> for_each_if ((((ring__) = &(dev_priv__)->engine[(i__)]), intel_engine_initialized((ring__))))
>
> +#define for_each_engine_masked(engine__, dev_priv__, mask__) \
> + for ((engine__) = &dev_priv->engine[0]; (engine__) < &dev_priv->engine[I915_NUM_ENGINES]; (engine__)++) \
> + for_each_if (intel_engine_flag((engine__)) & (mask__) && intel_engine_initialized((engine__)))
> +
> enum hdmi_force_audio {
> HDMI_AUDIO_OFF_DVI = -2, /* no aux data for HDMI-DVI converter */
> HDMI_AUDIO_OFF, /* force turn off HDMI audio */
> @@ -2553,6 +2557,8 @@ struct drm_i915_cmd_table {
> #define BLT_RING (1<<BCS)
> #define VEBOX_RING (1<<VECS)
> #define BSD2_RING (1<<VCS2)
> +#define ALL_ENGINES (~0)
> +
> #define HAS_BSD(dev) (INTEL_INFO(dev)->ring_mask & BSD_RING)
> #define HAS_BSD2(dev) (INTEL_INFO(dev)->ring_mask & BSD2_RING)
> #define HAS_BLT(dev) (INTEL_INFO(dev)->ring_mask & BLT_RING)
> @@ -2681,7 +2687,7 @@ extern void i915_driver_postclose(struct drm_device *dev,
> extern long i915_compat_ioctl(struct file *filp, unsigned int cmd,
> unsigned long arg);
> #endif
> -extern int intel_gpu_reset(struct drm_device *dev);
> +extern int intel_gpu_reset(struct drm_device *dev, u32 engine_mask);
> extern bool intel_has_gpu_reset(struct drm_device *dev);
> extern int i915_reset(struct drm_device *dev);
> extern unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 31652c1da761..a25109aa033c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5012,13 +5012,13 @@ i915_gem_cleanup_engines(struct drm_device *dev)
> for_each_engine(engine, dev_priv, i)
> dev_priv->gt.cleanup_engine(engine);
>
> - if (i915.enable_execlists)
> - /*
> - * Neither the BIOS, ourselves or any other kernel
> - * expects the system to be in execlists mode on startup,
> - * so we need to reset the GPU back to legacy mode.
> - */
> - intel_gpu_reset(dev);
> + if (i915.enable_execlists)
> + /*
> + * Neither the BIOS, ourselves or any other kernel
> + * expects the system to be in execlists mode on startup,
> + * so we need to reset the GPU back to legacy mode.
> + */
> + intel_gpu_reset(dev, ALL_ENGINES);
> }
>
> static void
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 1993449ab7c5..c114665a24b3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -413,7 +413,7 @@ void i915_gem_context_fini(struct drm_device *dev)
> /* The only known way to stop the gpu from accessing the hw context is
> * to reset it. Do this as the very last operation to avoid confusing
> * other code, leading to spurious errors. */
> - intel_gpu_reset(dev);
> + intel_gpu_reset(dev, ALL_ENGINES);
>
> /* When default context is created and switched to, base object refcount
> * will be 2 (+1 from object creation and +1 from do_switch()).
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7dfc4007f3fa..69359d918a4a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -164,6 +164,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #define GEN6_GRDOM_RENDER (1 << 1)
> #define GEN6_GRDOM_MEDIA (1 << 2)
> #define GEN6_GRDOM_BLT (1 << 3)
> +#define GEN6_GRDOM_VECS (1 << 4)
> +#define GEN8_GRDOM_MEDIA2 (1 << 7)
>
> #define RING_PP_DIR_BASE(ring) _MMIO((ring)->mmio_base+0x228)
> #define RING_PP_DIR_BASE_READ(ring) _MMIO((ring)->mmio_base+0x518)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 02add02e0ce4..512b7faedefd 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1433,7 +1433,7 @@ static int i915_reset_complete(struct drm_device *dev)
> return (gdrst & GRDOM_RESET_STATUS) == 0;
> }
>
> -static int i915_do_reset(struct drm_device *dev)
> +static int i915_do_reset(struct drm_device *dev, unsigned engine_mask)
> {
> /* assert reset for at least 20 usec */
> pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
> @@ -1450,13 +1450,13 @@ static int g4x_reset_complete(struct drm_device *dev)
> return (gdrst & GRDOM_RESET_ENABLE) == 0;
> }
>
> -static int g33_do_reset(struct drm_device *dev)
> +static int g33_do_reset(struct drm_device *dev, unsigned engine_mask)
> {
> pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
> return wait_for(g4x_reset_complete(dev), 500);
> }
>
> -static int g4x_do_reset(struct drm_device *dev)
> +static int g4x_do_reset(struct drm_device *dev, unsigned engine_mask)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> int ret;
> @@ -1486,7 +1486,7 @@ static int g4x_do_reset(struct drm_device *dev)
> return 0;
> }
>
> -static int ironlake_do_reset(struct drm_device *dev)
> +static int ironlake_do_reset(struct drm_device *dev, unsigned engine_mask)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> int ret;
> @@ -1510,21 +1510,62 @@ static int ironlake_do_reset(struct drm_device *dev)
> return 0;
> }
>
> -static int gen6_do_reset(struct drm_device *dev)
> +/* Reset the hardware domains (GENX_GRDOM_*) specified by mask */
> +static int gen6_hw_domain_reset(struct drm_i915_private *dev_priv,
> + u32 hw_domain_mask)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - int ret;
> -
> - /* Reset the chip */
> + int ret;
>
> /* GEN6_GDRST is not in the gt power well, no need to check
> * for fifo space for the write or forcewake the chip for
> * the read
> */
> - __raw_i915_write32(dev_priv, GEN6_GDRST, GEN6_GRDOM_FULL);
> + __raw_i915_write32(dev_priv, GEN6_GDRST, hw_domain_mask);
>
> - /* Spin waiting for the device to ack the reset request */
> - ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) & GEN6_GRDOM_FULL) == 0, 500);
> +#define ACKED ((__raw_i915_read32(dev_priv, GEN6_GDRST) & hw_domain_mask) == 0)
> + /* Spin waiting for the device to ack the reset requests */
> + ret = wait_for(ACKED, 500);
> +#undef ACKED
> +
> + return ret;
> +}
> +
> +/**
> + * gen6_reset_engines - reset individual engines
> + * @dev: DRM device
> + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset
> + *
> + * This function will reset the individual engines that are set in engine_mask.
> + * If you provide ALL_ENGINES as mask, full global domain reset will be issued.
> + *
> + * Note: It is responsibility of the caller to handle the difference between
> + * asking full domain reset versus reset for all available individual engines.
> + *
> + * Returns 0 on success, nonzero on error.
> + */
> +static int gen6_reset_engines(struct drm_device *dev, unsigned engine_mask)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_engine_cs *engine;
> + const u32 hw_engine_mask[I915_NUM_ENGINES] = {
> + [RCS] = GEN6_GRDOM_RENDER,
> + [BCS] = GEN6_GRDOM_BLT,
> + [VCS] = GEN6_GRDOM_MEDIA,
> + [VCS2] = GEN8_GRDOM_MEDIA2,
> + [VECS] = GEN6_GRDOM_VECS,
> + };
> + u32 hw_mask;
> + int ret;
> +
> + if (engine_mask == ALL_ENGINES) {
> + hw_mask = GEN6_GRDOM_FULL;
> + } else {
> + hw_mask = 0;
> + for_each_engine_masked(engine, dev_priv, engine_mask)
> + hw_mask |= hw_engine_mask[engine->id];
> + }
> +
> + ret = gen6_hw_domain_reset(dev_priv, hw_mask);
>
> intel_uncore_forcewake_reset(dev, true);
>
> @@ -1567,34 +1608,34 @@ static void gen8_unrequest_engine_reset(struct intel_engine_cs *engine)
> _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
> }
>
> -static int gen8_do_reset(struct drm_device *dev)
> +static int gen8_reset_engines(struct drm_device *dev, unsigned engine_mask)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_engine_cs *engine;
> - int i;
>
> - for_each_engine(engine, dev_priv, i)
> + for_each_engine_masked(engine, dev_priv, engine_mask)
> if (gen8_request_engine_reset(engine))
> goto not_ready;
>
> - return gen6_do_reset(dev);
> + return gen6_reset_engines(dev, engine_mask);
>
> not_ready:
> - for_each_engine(engine, dev_priv, i)
> + for_each_engine_masked(engine, dev_priv, engine_mask)
> gen8_unrequest_engine_reset(engine);
>
> return -EIO;
> }
>
> -static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
> +static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *,
> + unsigned engine_mask)
> {
> if (!i915.reset)
> return NULL;
>
> if (INTEL_INFO(dev)->gen >= 8)
> - return gen8_do_reset;
> + return gen8_reset_engines;
> else if (INTEL_INFO(dev)->gen >= 6)
> - return gen6_do_reset;
> + return gen6_reset_engines;
> else if (IS_GEN5(dev))
> return ironlake_do_reset;
> else if (IS_G4X(dev))
> @@ -1607,10 +1648,10 @@ static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
> return NULL;
> }
>
> -int intel_gpu_reset(struct drm_device *dev)
> +int intel_gpu_reset(struct drm_device *dev, unsigned engine_mask)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> - int (*reset)(struct drm_device *);
> + int (*reset)(struct drm_device *, unsigned);
> int ret;
>
> reset = intel_get_gpu_reset(dev);
> @@ -1621,7 +1662,7 @@ int intel_gpu_reset(struct drm_device *dev)
> * request may be dropped and never completes (causing -EIO).
> */
> intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> - ret = reset(dev);
> + ret = reset(dev, engine_mask);
> intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>
> return ret;
> --
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-03-17 13:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-16 11:52 [PATCH] drm/i915: Modify reset func to handle per engine resets Mika Kuoppala
2016-03-16 12:37 ` Chris Wilson
2016-03-16 15:30 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-03-16 15:40 ` [PATCH] " Mika Kuoppala
2016-03-16 15:54 ` Mika Kuoppala
2016-03-17 13:14 ` Mika Kuoppala
2016-03-17 7:36 ` ✗ Fi.CI.BAT: failure for drm/i915: Modify reset func to handle per engine resets (rev3) Patchwork
2016-03-17 12:02 ` Mika Kuoppala
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.