* [PATCH 2/3] drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info
2017-10-28 11:27 [PATCH 1/3] drm/i915: Disable sempahores on Sandybridge Chris Wilson
@ 2017-10-28 11:27 ` Chris Wilson
2017-10-28 11:27 ` [PATCH 3/3] drm/i915: Remove i915.semaphores modparam Chris Wilson
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2017-10-28 11:27 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] drm/i915: Remove i915.semaphores modparam
2017-10-28 11:27 [PATCH 1/3] drm/i915: Disable sempahores on Sandybridge Chris Wilson
2017-10-28 11:27 ` [PATCH 2/3] drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info Chris Wilson
@ 2017-10-28 11:27 ` Chris Wilson
2017-10-30 10:22 ` Maarten Lankhorst
2017-10-28 11:34 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Disable sempahores on Sandybridge Patchwork
` (2 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2017-10-28 11:27 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] drm/i915: Remove i915.semaphores modparam
2017-10-28 11:27 ` [PATCH 3/3] drm/i915: Remove i915.semaphores modparam Chris Wilson
@ 2017-10-30 10:22 ` Maarten Lankhorst
0 siblings, 0 replies; 7+ messages in thread
From: Maarten Lankhorst @ 2017-10-30 10:22 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Disable sempahores on Sandybridge
2017-10-28 11:27 [PATCH 1/3] drm/i915: Disable sempahores on Sandybridge Chris Wilson
2017-10-28 11:27 ` [PATCH 2/3] drm/i915: Move debugfs/i915_semaphore_status to i915_engine_info Chris Wilson
2017-10-28 11:27 ` [PATCH 3/3] drm/i915: Remove i915.semaphores modparam Chris Wilson
@ 2017-10-28 11:34 ` Patchwork
2017-10-28 15:37 ` [PATCH 1/3] " Chris Wilson
2017-10-30 12:20 ` Mika Kuoppala
4 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-10-28 11:34 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] drm/i915: Disable sempahores on Sandybridge
2017-10-28 11:27 [PATCH 1/3] drm/i915: Disable sempahores on Sandybridge Chris Wilson
` (2 preceding siblings ...)
2017-10-28 11:34 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Disable sempahores on Sandybridge Patchwork
@ 2017-10-28 15:37 ` Chris Wilson
2017-10-30 12:20 ` Mika Kuoppala
4 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2017-10-28 15:37 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] drm/i915: Disable sempahores on Sandybridge
2017-10-28 11:27 [PATCH 1/3] drm/i915: Disable sempahores on Sandybridge Chris Wilson
` (3 preceding siblings ...)
2017-10-28 15:37 ` [PATCH 1/3] " Chris Wilson
@ 2017-10-30 12:20 ` Mika Kuoppala
4 siblings, 0 replies; 7+ messages in thread
From: Mika Kuoppala @ 2017-10-30 12:20 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
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
^ permalink raw reply [flat|nested] 7+ messages in thread