diff mbox

[053/190] drm/i915: Convert i915_semaphores_is_enabled over to early sanitize

Message ID 1452503961-14837-53-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Jan. 11, 2016, 9:17 a.m. UTC
Rather than recomputing whether semaphores are enabled, we can do that
computation once during early initialisation as the i915.semaphores
module parameter is now read-only.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |  2 +-
 drivers/gpu/drm/i915/i915_dma.c         |  2 +-
 drivers/gpu/drm/i915/i915_drv.c         | 25 -----------------------
 drivers/gpu/drm/i915/i915_drv.h         |  1 -
 drivers/gpu/drm/i915/i915_gem.c         | 35 ++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c   |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 20 +++++++++----------
 8 files changed, 46 insertions(+), 43 deletions(-)

Comments

Dave Gordon Jan. 12, 2016, 7:07 p.m. UTC | #1
On 11/01/16 09:17, Chris Wilson wrote:
> Rather than recomputing whether semaphores are enabled, we can do that
> computation once during early initialisation as the i915.semaphores
> module parameter is now read-only.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c     |  2 +-
>   drivers/gpu/drm/i915/i915_dma.c         |  2 +-
>   drivers/gpu/drm/i915/i915_drv.c         | 25 -----------------------
>   drivers/gpu/drm/i915/i915_drv.h         |  1 -
>   drivers/gpu/drm/i915/i915_gem.c         | 35 ++++++++++++++++++++++++++++++---
>   drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
>   drivers/gpu/drm/i915/i915_gpu_error.c   |  2 +-
>   drivers/gpu/drm/i915/intel_ringbuffer.c | 20 +++++++++----------
>   8 files changed, 46 insertions(+), 43 deletions(-)

LGTM.

Reviewed-by: Dave Gordon <david.s.gordon@intel.com>

[aside]
The conditions below seem to exclude a lot of systems. It looks like 
semaphores can only be used on GEN 6 (if no IOMMU) and GEN 7. Nothing 
before or after that range, as GEN9+ supports execlist only.

So is it even worth continuing to support semaphores at all? Especially 
as we have customers who say that the scheduler gives more performance 
gain than semaphores ...
[/aside]

.Dave.

> +static bool i915_gem_sanitize_semaphore(struct drm_i915_private *dev_priv,
> +					int param_value)
> +{
> +	if (INTEL_INFO(dev_priv)->gen < 6)
> +		return false;
> +
> +	if (param_value >= 0)
> +		return param_value;
> +
> +	/* TODO: make semaphores and Execlists play nicely together */
> +	if (i915.enable_execlists)
> +		return false;
> +
> +	/* Until we get further testing... */
> +	if (IS_GEN8(dev_priv))
> +		return false;
> +
> +#ifdef CONFIG_INTEL_IOMMU
> +	/* Enable semaphores on SNB when IO remapping is off */
> +	if (INTEL_INFO(dev_priv)->gen == 6 && intel_iommu_gfx_mapped)
> +		return false;
> +#endif
> +
> +	return true;
> +}
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5335072f2047..387ae77d3c29 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3146,7 +3146,7 @@  static int i915_semaphore_status(struct seq_file *m, void *unused)
 	int num_rings = hweight32(INTEL_INFO(dev)->ring_mask);
 	int i, j, ret;
 
-	if (!i915_semaphore_is_enabled(dev)) {
+	if (!i915.semaphores) {
 		seq_puts(m, "Semaphores are disabled\n");
 		return 0;
 	}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 9e49e304dd8e..4c72c83cfa28 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -126,7 +126,7 @@  static int i915_getparam(struct drm_device *dev, void *data,
 		value = 1;
 		break;
 	case I915_PARAM_HAS_SEMAPHORES:
-		value = i915_semaphore_is_enabled(dev);
+		value = i915.semaphores;
 		break;
 	case I915_PARAM_HAS_PRIME_VMAP_FLUSH:
 		value = 1;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e9f85fd0542f..cc831a34f7bb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -515,31 +515,6 @@  void intel_detect_pch(struct drm_device *dev)
 	pci_dev_put(pch);
 }
 
-bool i915_semaphore_is_enabled(struct drm_device *dev)
-{
-	if (INTEL_INFO(dev)->gen < 6)
-		return false;
-
-	if (i915.semaphores >= 0)
-		return i915.semaphores;
-
-	/* TODO: make semaphores and Execlists play nicely together */
-	if (i915.enable_execlists)
-		return false;
-
-	/* Until we get further testing... */
-	if (IS_GEN8(dev))
-		return false;
-
-#ifdef CONFIG_INTEL_IOMMU
-	/* Enable semaphores on SNB when IO remapping is off */
-	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
-		return false;
-#endif
-
-	return true;
-}
-
 static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 56cf2ffc1eac..58e9e5e50769 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3226,7 +3226,6 @@  extern void intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
 extern void intel_detect_pch(struct drm_device *dev);
 extern int intel_enable_rc6(const struct drm_device *dev);
 
-extern bool i915_semaphore_is_enabled(struct drm_device *dev);
 int i915_reg_read_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file);
 int i915_get_reset_stats_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a4f9c5bbb883..31926a4fb42a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2567,7 +2567,7 @@  __i915_gem_object_sync(struct drm_i915_gem_object *obj,
 	if (i915_gem_request_completed(from_req))
 		return 0;
 
-	if (!i915_semaphore_is_enabled(obj->base.dev)) {
+	if (!i915.semaphores) {
 		struct drm_i915_private *i915 = to_i915(obj->base.dev);
 		ret = __i915_wait_request(from_req,
 					  i915->mm.interruptible,
@@ -4304,13 +4304,42 @@  out:
 	return ret;
 }
 
+static bool i915_gem_sanitize_semaphore(struct drm_i915_private *dev_priv,
+					int param_value)
+{
+	if (INTEL_INFO(dev_priv)->gen < 6)
+		return false;
+
+	if (param_value >= 0)
+		return param_value;
+
+	/* TODO: make semaphores and Execlists play nicely together */
+	if (i915.enable_execlists)
+		return false;
+
+	/* Until we get further testing... */
+	if (IS_GEN8(dev_priv))
+		return false;
+
+#ifdef CONFIG_INTEL_IOMMU
+	/* Enable semaphores on SNB when IO remapping is off */
+	if (INTEL_INFO(dev_priv)->gen == 6 && intel_iommu_gfx_mapped)
+		return false;
+#endif
+
+	return true;
+}
+
 int i915_gem_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
-	i915.enable_execlists = intel_sanitize_enable_execlists(dev,
-			i915.enable_execlists);
+	i915.enable_execlists =
+		intel_sanitize_enable_execlists(dev, i915.enable_execlists);
+
+	i915.semaphores =
+		i915_gem_sanitize_semaphore(dev_priv, i915.semaphores);
 
 	mutex_lock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 0aea5ccf6d68..361be1085a18 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -523,7 +523,7 @@  mi_set_context(struct drm_i915_gem_request *req, u32 hw_flags)
 	u32 flags = hw_flags | MI_MM_SPACE_GTT;
 	const int num_rings =
 		/* Use an extended w/a on ivb+ if signalling from other rings */
-		i915_semaphore_is_enabled(ring->dev) ?
+		i915.semaphores ?
 		hweight32(INTEL_INFO(ring->dev)->ring_mask) - 1 :
 		0;
 	int len, i, ret;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 05f054898a95..84ce91275fdd 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -823,7 +823,7 @@  static void gen8_record_semaphore_state(struct drm_i915_private *dev_priv,
 	struct intel_engine_cs *to;
 	int i;
 
-	if (!i915_semaphore_is_enabled(dev_priv->dev))
+	if (!i915.semaphores)
 		return;
 
 	if (!error->semaphore_obj)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 02b7032e16e0..e143da96dcfa 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2510,7 +2510,7 @@  int intel_init_render_ring_buffer(struct drm_device *dev)
 	ring->mmio_base = RENDER_RING_BASE;
 
 	if (INTEL_INFO(dev)->gen >= 8) {
-		if (i915_semaphore_is_enabled(dev)) {
+		if (i915.semaphores) {
 			obj = i915_gem_alloc_object(dev, 4096);
 			if (obj == NULL) {
 				DRM_ERROR("Failed to allocate semaphore bo. Disabling semaphores\n");
@@ -2534,7 +2534,7 @@  int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->irq_disable = gen8_ring_disable_irq;
 		ring->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
 		ring->irq_seqno_barrier = gen6_seqno_barrier;
-		if (i915_semaphore_is_enabled(dev)) {
+		if (i915.semaphores) {
 			WARN_ON(!dev_priv->semaphore_obj);
 			ring->semaphore.sync_to = gen8_ring_sync;
 			ring->semaphore.signal = gen8_rcs_signal;
@@ -2550,7 +2550,7 @@  int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->irq_disable = gen6_ring_disable_irq;
 		ring->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
 		ring->irq_seqno_barrier = gen6_seqno_barrier;
-		if (i915_semaphore_is_enabled(dev)) {
+		if (i915.semaphores) {
 			ring->semaphore.sync_to = gen6_ring_sync;
 			ring->semaphore.signal = gen6_signal;
 			/*
@@ -2666,7 +2666,7 @@  int intel_init_bsd_ring_buffer(struct drm_device *dev)
 			ring->irq_disable = gen8_ring_disable_irq;
 			ring->dispatch_execbuffer =
 				gen8_ring_dispatch_execbuffer;
-			if (i915_semaphore_is_enabled(dev)) {
+			if (i915.semaphores) {
 				ring->semaphore.sync_to = gen8_ring_sync;
 				ring->semaphore.signal = gen8_xcs_signal;
 				GEN8_RING_SEMAPHORE_INIT;
@@ -2677,7 +2677,7 @@  int intel_init_bsd_ring_buffer(struct drm_device *dev)
 			ring->irq_disable = gen6_ring_disable_irq;
 			ring->dispatch_execbuffer =
 				gen6_ring_dispatch_execbuffer;
-			if (i915_semaphore_is_enabled(dev)) {
+			if (i915.semaphores) {
 				ring->semaphore.sync_to = gen6_ring_sync;
 				ring->semaphore.signal = gen6_signal;
 				ring->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_VR;
@@ -2734,7 +2734,7 @@  int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	ring->irq_disable = gen8_ring_disable_irq;
 	ring->dispatch_execbuffer =
 			gen8_ring_dispatch_execbuffer;
-	if (i915_semaphore_is_enabled(dev)) {
+	if (i915.semaphores) {
 		ring->semaphore.sync_to = gen8_ring_sync;
 		ring->semaphore.signal = gen8_xcs_signal;
 		GEN8_RING_SEMAPHORE_INIT;
@@ -2763,7 +2763,7 @@  int intel_init_blt_ring_buffer(struct drm_device *dev)
 		ring->irq_enable = gen8_ring_enable_irq;
 		ring->irq_disable = gen8_ring_disable_irq;
 		ring->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
-		if (i915_semaphore_is_enabled(dev)) {
+		if (i915.semaphores) {
 			ring->semaphore.sync_to = gen8_ring_sync;
 			ring->semaphore.signal = gen8_xcs_signal;
 			GEN8_RING_SEMAPHORE_INIT;
@@ -2773,7 +2773,7 @@  int intel_init_blt_ring_buffer(struct drm_device *dev)
 		ring->irq_enable = gen6_ring_enable_irq;
 		ring->irq_disable = gen6_ring_disable_irq;
 		ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
-		if (i915_semaphore_is_enabled(dev)) {
+		if (i915.semaphores) {
 			ring->semaphore.signal = gen6_signal;
 			ring->semaphore.sync_to = gen6_ring_sync;
 			/*
@@ -2820,7 +2820,7 @@  int intel_init_vebox_ring_buffer(struct drm_device *dev)
 		ring->irq_enable = gen8_ring_enable_irq;
 		ring->irq_disable = gen8_ring_disable_irq;
 		ring->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
-		if (i915_semaphore_is_enabled(dev)) {
+		if (i915.semaphores) {
 			ring->semaphore.sync_to = gen8_ring_sync;
 			ring->semaphore.signal = gen8_xcs_signal;
 			GEN8_RING_SEMAPHORE_INIT;
@@ -2830,7 +2830,7 @@  int intel_init_vebox_ring_buffer(struct drm_device *dev)
 		ring->irq_enable = hsw_vebox_enable_irq;
 		ring->irq_disable = hsw_vebox_disable_irq;
 		ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
-		if (i915_semaphore_is_enabled(dev)) {
+		if (i915.semaphores) {
 			ring->semaphore.sync_to = gen6_ring_sync;
 			ring->semaphore.signal = gen6_signal;
 			ring->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_VER;