[PATCH 1/3] drm/i915: Disable sempahores on Sandybridge

[PATCH 1/3] drm/i915: Disable sempahores on Sandybridge

[Chris Wilson]

I should have admitted defeat long ago as there has been a rare but
persistent error on Sandybridge where semaphore signaling did not
propagate to the waiter, leading to a GPU hang.

With the work on fence signaling for v4.9, the impact of using CPU driven
signaling was greatly reduced wrt to the latency of GPU semaphores,
though without logical rings support, the benefit of reordering work to
avoid bubbles is not realised (i.e. as it stands fence signaling is just
a slower, more costly version of HW semaphores; but works more
consistently). As a rough indicator of the difference,

with semaphores:
Sequential (3 engines, 1 processes): average 5.470us per cycle [expected 4.988us]

w/o semaphores:
Sequential (3 engines, 1 processes): average 15.771us per cycle [expected 4.923us]

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54226
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 24d795802eaf..eee699dd376e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4950,20 +4950,12 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 
 bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
 {
-	if (INTEL_GEN(dev_priv) < 6)
-		return false;
-
-	/* TODO: make semaphores and Execlists play nicely together */
-	if (HAS_EXECLISTS(dev_priv))
+	if (!IS_GEN7(dev_priv))
 		return false;
 
 	if (value >= 0)
 		return value;
 
-	/* Enable semaphores on SNB when IO remapping is off */
-	if (IS_GEN6(dev_priv) && intel_vtd_active())
-		return false;
-
 	return true;
 }
 
-- 
2.15.0.rc2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info

[Chris Wilson]

As the semaphores is just part of the engine, include it with the
general pretty printer universally used for debugging.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c    | 32 --------------------------------
 drivers/gpu/drm/i915/intel_engine_cs.c |  9 +++++++++
 2 files changed, 9 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b83fee536a2d..a8ad94382dff 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3197,37 +3197,6 @@ static int i915_shrinker_info(struct seq_file *m, void *unused)
 	return 0;
 }
 
-static int i915_semaphore_status(struct seq_file *m, void *unused)
-{
-	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	struct drm_device *dev = &dev_priv->drm;
-	struct intel_engine_cs *engine;
-	int num_rings = INTEL_INFO(dev_priv)->num_rings;
-	enum intel_engine_id id;
-	int j, ret;
-
-	if (!i915_modparams.semaphores) {
-		seq_puts(m, "Semaphores are disabled\n");
-		return 0;
-	}
-
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
-	if (ret)
-		return ret;
-	intel_runtime_pm_get(dev_priv);
-
-	seq_puts(m, "  Last signal:");
-	for_each_engine(engine, dev_priv, id)
-		for (j = 0; j < num_rings; j++)
-			seq_printf(m, "0x%08x\n",
-				   I915_READ(engine->semaphore.mbox.signal[j]));
-	seq_putc(m, '\n');
-
-	intel_runtime_pm_put(dev_priv);
-	mutex_unlock(&dev->struct_mutex);
-	return 0;
-}
-
 static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4707,7 +4676,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_display_info", i915_display_info, 0},
 	{"i915_engine_info", i915_engine_info, 0},
 	{"i915_shrinker_info", i915_shrinker_info, 0},
-	{"i915_semaphore_status", i915_semaphore_status, 0},
 	{"i915_shared_dplls_info", i915_shared_dplls_info, 0},
 	{"i915_dp_mst_info", i915_dp_mst_info, 0},
 	{"i915_wa_registers", i915_wa_registers, 0},
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 00431cec0914..6c54bfb1706a 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1733,6 +1733,15 @@ void intel_engine_dump(struct intel_engine_cs *engine, struct drm_printer *m)
 			   I915_READ(RING_MI_MODE(engine->mmio_base)),
 			   I915_READ(RING_MI_MODE(engine->mmio_base)) & (MODE_IDLE) ? " [idle]" : "");
 	}
+	if (i915_modparams.semaphores) {
+		drm_printf(m, "\tSYNC_0: 0x%08x\n",
+			   I915_READ(RING_SYNC_0(engine->mmio_base)));
+		drm_printf(m, "\tSYNC_1: 0x%08x\n",
+			   I915_READ(RING_SYNC_1(engine->mmio_base)));
+		if (HAS_VEBOX(dev_priv))
+			drm_printf(m, "\tSYNC_2: 0x%08x\n",
+				   I915_READ(RING_SYNC_2(engine->mmio_base)));
+	}
 
 	rcu_read_unlock();
 
-- 
2.15.0.rc2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Remove i915.semaphores modparam

[Chris Wilson]

Having disabled the broken semaphores on Sandybridge, there is no need
for a modparam any more, so remove it in favour of a simple
HAS_LEGACY_SEMAPHORES() guard.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  7 +------
 drivers/gpu/drm/i915/i915_drv.h         |  4 ++--
 drivers/gpu/drm/i915/i915_gem.c         | 11 -----------
 drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
 drivers/gpu/drm/i915/i915_params.c      |  4 ----
 drivers/gpu/drm/i915/i915_params.h      |  1 -
 drivers/gpu/drm/i915/intel_engine_cs.c  |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c |  4 ++--
 8 files changed, 7 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e021ae585f70..a23b3edaca23 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -321,7 +321,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
 		value = USES_PPGTT(dev_priv);
 		break;
 	case I915_PARAM_HAS_SEMAPHORES:
-		value = i915_modparams.semaphores;
+		value = HAS_LEGACY_SEMAPHORES(dev_priv);
 		break;
 	case I915_PARAM_HAS_SECURE_BATCHES:
 		value = capable(CAP_SYS_ADMIN);
@@ -1056,11 +1056,6 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
 					    i915_modparams.enable_ppgtt);
 	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915_modparams.enable_ppgtt);
 
-	i915_modparams.semaphores =
-		intel_sanitize_semaphores(dev_priv, i915_modparams.semaphores);
-	DRM_DEBUG_DRIVER("use GPU semaphores? %s\n",
-			 yesno(i915_modparams.semaphores));
-
 	intel_uc_sanitize_options(dev_priv);
 
 	intel_gvt_sanitize_options(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cd0bd8d16ef0..326a06f844f9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3127,6 +3127,8 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define HAS_BLT(dev_priv)	HAS_ENGINE(dev_priv, BCS)
 #define HAS_VEBOX(dev_priv)	HAS_ENGINE(dev_priv, VECS)
 
+#define HAS_LEGACY_SEMAPHORES(dev_priv) IS_GEN7(dev_priv)
+
 #define HAS_LLC(dev_priv)	((dev_priv)->info.has_llc)
 #define HAS_SNOOP(dev_priv)	((dev_priv)->info.has_snoop)
 #define HAS_EDRAM(dev_priv)	(!!((dev_priv)->edram_cap & EDRAM_ENABLED))
@@ -3291,8 +3293,6 @@ intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *dev_priv)
 int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 				int enable_ppgtt);
 
-bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value);
-
 /* i915_drv.c */
 void __printf(3, 4)
 __i915_printk(struct drm_i915_private *dev_priv, const char *level,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index eee699dd376e..ab4a0a2b99fe 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4948,17 +4948,6 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
-{
-	if (!IS_GEN7(dev_priv))
-		return false;
-
-	if (value >= 0)
-		return value;
-
-	return true;
-}
-
 int i915_gem_init(struct drm_i915_private *dev_priv)
 {
 	int ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index a843e35f00db..103a633754f3 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -596,7 +596,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 flags)
 	enum intel_engine_id id;
 	const int num_rings =
 		/* Use an extended w/a on gen7 if signalling from other rings */
-		(i915_modparams.semaphores && INTEL_GEN(dev_priv) == 7) ?
+		(HAS_LEGACY_SEMAPHORES(dev_priv) && IS_GEN7(dev_priv)) ?
 		INTEL_INFO(dev_priv)->num_rings - 1 :
 		0;
 	int len;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 72056a9721b7..7bc538687871 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -46,10 +46,6 @@ i915_param_named_unsafe(panel_ignore_lid, int, 0600,
 	"Override lid status (0=autodetect, 1=autodetect disabled [default], "
 	"-1=force lid closed, -2=force lid open)");
 
-i915_param_named_unsafe(semaphores, int, 0400,
-	"Use semaphores for inter-ring sync "
-	"(default: -1 (use per-chip defaults))");
-
 i915_param_named_unsafe(enable_dc, int, 0400,
 	"Enable power-saving display C-states. "
 	"(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 17298698c362..c48c88bb95e8 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -31,7 +31,6 @@
 	param(char *, vbt_firmware, NULL) \
 	param(int, modeset, -1) \
 	param(int, panel_ignore_lid, 1) \
-	param(int, semaphores, -1) \
 	param(int, lvds_channel_mode, 0) \
 	param(int, panel_use_ssc, -1) \
 	param(int, vbt_sdvo_panel_type, -1) \
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 6c54bfb1706a..5f708c14177f 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1733,7 +1733,7 @@ void intel_engine_dump(struct intel_engine_cs *engine, struct drm_printer *m)
 			   I915_READ(RING_MI_MODE(engine->mmio_base)),
 			   I915_READ(RING_MI_MODE(engine->mmio_base)) & (MODE_IDLE) ? " [idle]" : "");
 	}
-	if (i915_modparams.semaphores) {
+	if (HAS_LEGACY_SEMAPHORES(dev_priv)) {
 		drm_printf(m, "\tSYNC_0: 0x%08x\n",
 			   I915_READ(RING_SYNC_0(engine->mmio_base)));
 		drm_printf(m, "\tSYNC_1: 0x%08x\n",
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a05c0006f6fd..85e80a6787d2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1668,7 +1668,7 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 {
 	int i;
 
-	if (!i915_modparams.semaphores)
+	if (!HAS_LEGACY_SEMAPHORES(dev_priv))
 		return;
 
 	GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
@@ -1779,7 +1779,7 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 
 	engine->emit_breadcrumb = i9xx_emit_breadcrumb;
 	engine->emit_breadcrumb_sz = i9xx_emit_breadcrumb_sz;
-	if (i915_modparams.semaphores) {
+	if (HAS_LEGACY_SEMAPHORES(dev_priv)) {
 		int num_rings;
 
 		engine->emit_breadcrumb = gen6_sema_emit_breadcrumb;
-- 
2.15.0.rc2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Disable sempahores on Sandybridge

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Disable sempahores on Sandybridge
URL   : https://patchwork.freedesktop.org/series/32800/
State : failure

== Summary ==

Series 32800 revision 1 was fully merged or fully failed: no git log

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Disable sempahores on Sandybridge

[Chris Wilson]

Quoting Chris Wilson (2017-10-28 12:27:23)
> I should have admitted defeat long ago as there has been a rare but
> persistent error on Sandybridge where semaphore signaling did not
> propagate to the waiter, leading to a GPU hang.
> 
> With the work on fence signaling for v4.9, the impact of using CPU driven
> signaling was greatly reduced wrt to the latency of GPU semaphores,
> though without logical rings support, the benefit of reordering work to
> avoid bubbles is not realised (i.e. as it stands fence signaling is just
> a slower, more costly version of HW semaphores; but works more
> consistently). As a rough indicator of the difference,
> 
> with semaphores:
> Sequential (3 engines, 1 processes): average 5.470us per cycle [expected 4.988us]
> 
> w/o semaphores:
> Sequential (3 engines, 1 processes): average 15.771us per cycle [expected 4.923us]

In comparison, v3.4:

with semaphores:
Sequential (3 engines, 1 processes): average 16.066us per cycle [expected 11.842us]

w/o semaphores:
Sequential (3 engines, 1 processes): average 23.460us per cycle [expected 11.839us]

Interesting in that this microbenchmark doesn't show as big as an impact
that drove adoption of semaphores (originally it gave ~3x better
performance for x11perf), and that know even without semaphores we are
faster than a few years ago. Further, since the split engines in
Sandybridge userspace has learnt not to frequently jump between engines.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Remove i915.semaphores modparam

[Maarten Lankhorst]

Op 28-10-17 om 13:27 schreef Chris Wilson:
> Having disabled the broken semaphores on Sandybridge, there is no need
> for a modparam any more, so remove it in favour of a simple
> HAS_LEGACY_SEMAPHORES() guard.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  7 +------
>  drivers/gpu/drm/i915/i915_drv.h         |  4 ++--
>  drivers/gpu/drm/i915/i915_gem.c         | 11 -----------
>  drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
>  drivers/gpu/drm/i915/i915_params.c      |  4 ----
>  drivers/gpu/drm/i915/i915_params.h      |  1 -
>  drivers/gpu/drm/i915/intel_engine_cs.c  |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  4 ++--
>  8 files changed, 7 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e021ae585f70..a23b3edaca23 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -321,7 +321,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  		value = USES_PPGTT(dev_priv);
>  		break;
>  	case I915_PARAM_HAS_SEMAPHORES:
> -		value = i915_modparams.semaphores;
> +		value = HAS_LEGACY_SEMAPHORES(dev_priv);
>  		break;
>  	case I915_PARAM_HAS_SECURE_BATCHES:
>  		value = capable(CAP_SYS_ADMIN);
> @@ -1056,11 +1056,6 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
>  					    i915_modparams.enable_ppgtt);
>  	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915_modparams.enable_ppgtt);
>  
> -	i915_modparams.semaphores =
> -		intel_sanitize_semaphores(dev_priv, i915_modparams.semaphores);
> -	DRM_DEBUG_DRIVER("use GPU semaphores? %s\n",
> -			 yesno(i915_modparams.semaphores));
> -
>  	intel_uc_sanitize_options(dev_priv);
>  
>  	intel_gvt_sanitize_options(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cd0bd8d16ef0..326a06f844f9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3127,6 +3127,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>  #define HAS_BLT(dev_priv)	HAS_ENGINE(dev_priv, BCS)
>  #define HAS_VEBOX(dev_priv)	HAS_ENGINE(dev_priv, VECS)
>  
> +#define HAS_LEGACY_SEMAPHORES(dev_priv) IS_GEN7(dev_priv)
> +
>  #define HAS_LLC(dev_priv)	((dev_priv)->info.has_llc)
>  #define HAS_SNOOP(dev_priv)	((dev_priv)->info.has_snoop)
>  #define HAS_EDRAM(dev_priv)	(!!((dev_priv)->edram_cap & EDRAM_ENABLED))
> @@ -3291,8 +3293,6 @@ intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *dev_priv)
>  int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
>  				int enable_ppgtt);
>  
> -bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value);
> -
>  /* i915_drv.c */
>  void __printf(3, 4)
>  __i915_printk(struct drm_i915_private *dev_priv, const char *level,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index eee699dd376e..ab4a0a2b99fe 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4948,17 +4948,6 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
>  	return ret;
>  }
>  
> -bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
> -{
> -	if (!IS_GEN7(dev_priv))
> -		return false;
> -
> -	if (value >= 0)
> -		return value;
> -
> -	return true;
> -}
> -
>  int i915_gem_init(struct drm_i915_private *dev_priv)
>  {
>  	int ret;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index a843e35f00db..103a633754f3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -596,7 +596,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 flags)
>  	enum intel_engine_id id;
>  	const int num_rings =
>  		/* Use an extended w/a on gen7 if signalling from other rings */
> -		(i915_modparams.semaphores && INTEL_GEN(dev_priv) == 7) ?
> +		(HAS_LEGACY_SEMAPHORES(dev_priv) && IS_GEN7(dev_priv)) ?
The HAS_LEGACY_SEMAPHORES becomes redundant here. :)

Also typo in commit 1, sempahores -> semaphores

If you can get an ack from someone who knows more about snb semaphores and if this actually gets through CI then

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Disable sempahores on Sandybridge

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> I should have admitted defeat long ago as there has been a rare but
> persistent error on Sandybridge where semaphore signaling did not
> propagate to the waiter, leading to a GPU hang.
>
> With the work on fence signaling for v4.9, the impact of using CPU driven
> signaling was greatly reduced wrt to the latency of GPU semaphores,
> though without logical rings support, the benefit of reordering work to
> avoid bubbles is not realised (i.e. as it stands fence signaling is just
> a slower, more costly version of HW semaphores; but works more
> consistently). As a rough indicator of the difference,
>
> with semaphores:
> Sequential (3 engines, 1 processes): average 5.470us per cycle [expected 4.988us]
>
> w/o semaphores:
> Sequential (3 engines, 1 processes): average 15.771us per cycle [expected 4.923us]
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54226
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 24d795802eaf..eee699dd376e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4950,20 +4950,12 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
>  
>  bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
>  {
> -	if (INTEL_GEN(dev_priv) < 6)
> -		return false;
> -
> -	/* TODO: make semaphores and Execlists play nicely together */
> -	if (HAS_EXECLISTS(dev_priv))
> +	if (!IS_GEN7(dev_priv))
>  		return false;
>  
>  	if (value >= 0)
>  		return value;
>  
> -	/* Enable semaphores on SNB when IO remapping is off */
> -	if (IS_GEN6(dev_priv) && intel_vtd_active())
> -		return false;
> -
>  	return true;
>  }
>  
> -- 
> 2.15.0.rc2
>
> _______________________________________________
> 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