A simple batch of churn to make reference handling look consistent

A simple batch of churn to make reference handling look consistent

[Chris Wilson]

And a bonus 3 patches that do a minor tweak to intel_ringbuffer.c and
friends.
-Chris

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

[PATCH 1/9] drm/i915: Rename request reference/unreference to get/put

[Chris Wilson]

Now that we derive requests from struct fence, swap over to its
nomenclature for references. It's shorter and more idiomatic across the
kernel.

s/i915_gem_request_reference/i915_gem_request_get/
s/i915_gem_request_unreference/i915_gem_request_put/

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c          | 14 +++++++-------
 drivers/gpu/drm/i915/i915_gem_request.c  |  2 +-
 drivers/gpu/drm/i915/i915_gem_request.h  |  8 ++++----
 drivers/gpu/drm/i915/i915_gem_userptr.c  |  4 ++--
 drivers/gpu/drm/i915/intel_breadcrumbs.c |  4 ++--
 drivers/gpu/drm/i915/intel_display.c     |  5 ++---
 drivers/gpu/drm/i915/intel_lrc.c         | 10 +++++-----
 drivers/gpu/drm/i915/intel_pm.c          |  5 ++---
 8 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 37868cc594cb..d825db7b763d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1422,7 +1422,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 		if (req == NULL)
 			return 0;
 
-		requests[n++] = i915_gem_request_reference(req);
+		requests[n++] = i915_gem_request_get(req);
 	} else {
 		for (i = 0; i < I915_NUM_ENGINES; i++) {
 			struct drm_i915_gem_request *req;
@@ -1431,7 +1431,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 			if (req == NULL)
 				continue;
 
-			requests[n++] = i915_gem_request_reference(req);
+			requests[n++] = i915_gem_request_get(req);
 		}
 	}
 
@@ -1444,7 +1444,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 	for (i = 0; i < n; i++) {
 		if (ret == 0)
 			i915_gem_object_retire_request(obj, requests[i]);
-		i915_gem_request_unreference(requests[i]);
+		i915_gem_request_put(requests[i]);
 	}
 
 	return ret;
@@ -2820,7 +2820,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 		if (obj->last_read_req[i] == NULL)
 			continue;
 
-		req[n++] = i915_gem_request_reference(obj->last_read_req[i]);
+		req[n++] = i915_gem_request_get(obj->last_read_req[i]);
 	}
 
 	mutex_unlock(&dev->struct_mutex);
@@ -2830,7 +2830,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 			ret = __i915_wait_request(req[i], true,
 						  args->timeout_ns > 0 ? &args->timeout_ns : NULL,
 						  to_rps_client(file));
-		i915_gem_request_unreference(req[i]);
+		i915_gem_request_put(req[i]);
 	}
 	return ret;
 
@@ -3845,14 +3845,14 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
 		target = request;
 	}
 	if (target)
-		i915_gem_request_reference(target);
+		i915_gem_request_get(target);
 	spin_unlock(&file_priv->mm.lock);
 
 	if (target == NULL)
 		return 0;
 
 	ret = __i915_wait_request(target, true, NULL, NULL);
-	i915_gem_request_unreference(target);
+	i915_gem_request_put(target);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index f483e605a715..04ff97b10c08 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -181,7 +181,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	}
 
 	i915_gem_context_unreference(request->ctx);
-	i915_gem_request_unreference(request);
+	i915_gem_request_put(request);
 }
 
 void i915_gem_request_retire_upto(struct drm_i915_gem_request *req)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
index 0a01d01cac51..e06e81f459df 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -174,13 +174,13 @@ to_request(struct fence *fence)
 }
 
 static inline struct drm_i915_gem_request *
-i915_gem_request_reference(struct drm_i915_gem_request *req)
+i915_gem_request_get(struct drm_i915_gem_request *req)
 {
 	return to_request(fence_get(&req->fence));
 }
 
 static inline void
-i915_gem_request_unreference(struct drm_i915_gem_request *req)
+i915_gem_request_put(struct drm_i915_gem_request *req)
 {
 	fence_put(&req->fence);
 }
@@ -189,10 +189,10 @@ static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
 					   struct drm_i915_gem_request *src)
 {
 	if (src)
-		i915_gem_request_reference(src);
+		i915_gem_request_get(src);
 
 	if (*pdst)
-		i915_gem_request_unreference(*pdst);
+		i915_gem_request_put(*pdst);
 
 	*pdst = src;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 2314c88323e3..ba16e044fac6 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -78,7 +78,7 @@ static void wait_rendering(struct drm_i915_gem_object *obj)
 		if (req == NULL)
 			continue;
 
-		requests[n++] = i915_gem_request_reference(req);
+		requests[n++] = i915_gem_request_get(req);
 	}
 
 	mutex_unlock(&dev->struct_mutex);
@@ -89,7 +89,7 @@ static void wait_rendering(struct drm_i915_gem_object *obj)
 	mutex_lock(&dev->struct_mutex);
 
 	for (i = 0; i < n; i++)
-		i915_gem_request_unreference(requests[i]);
+		i915_gem_request_put(requests[i]);
 }
 
 static void cancel_userptr(struct work_struct *work)
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 32ada41dedfb..f0b56e3f4abe 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -453,7 +453,7 @@ static int intel_breadcrumbs_signaler(void *arg)
 			rb_erase(&request->signaling.node, &b->signals);
 			spin_unlock(&b->lock);
 
-			i915_gem_request_unreference(request);
+			i915_gem_request_put(request);
 		} else {
 			if (kthread_should_stop())
 				break;
@@ -484,7 +484,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request)
 
 	request->signaling.wait.tsk = b->signaler;
 	request->signaling.wait.seqno = request->fence.seqno;
-	i915_gem_request_reference(request);
+	i915_gem_request_get(request);
 
 	/* First add ourselves into the list of waiters, but register our
 	 * bottom-half as the signaller thread. As per usual, only the oldest
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 51fbca756cdb..2f7af855a585 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10954,11 +10954,10 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 	mutex_lock(&dev->struct_mutex);
 	intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
 	drm_gem_object_unreference(&work->pending_flip_obj->base);
-
-	if (work->flip_queued_req)
-		i915_gem_request_assign(&work->flip_queued_req, NULL);
 	mutex_unlock(&dev->struct_mutex);
 
+	i915_gem_request_put(work->flip_queued_req);
+
 	intel_frontbuffer_flip_complete(dev, to_intel_plane(primary)->frontbuffer_bit);
 	intel_fbc_post_update(crtc);
 	drm_framebuffer_unreference(work->old_fb);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 860dba2cf7b3..b23d4cc94cd5 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -441,7 +441,7 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
 			 * will update tail past first request's workload */
 			cursor->elsp_submitted = req0->elsp_submitted;
 			list_del(&req0->execlist_link);
-			i915_gem_request_unreference(req0);
+			i915_gem_request_put(req0);
 			req0 = cursor;
 		} else {
 			if (IS_ENABLED(CONFIG_DRM_I915_GVT)) {
@@ -514,7 +514,7 @@ execlists_check_remove_request(struct intel_engine_cs *engine, u32 ctx_id)
 	execlists_context_status_change(head_req, INTEL_CONTEXT_SCHEDULE_OUT);
 
 	list_del(&head_req->execlist_link);
-	i915_gem_request_unreference(head_req);
+	i915_gem_request_put(head_req);
 
 	return 1;
 }
@@ -632,11 +632,11 @@ static void execlists_context_queue(struct drm_i915_gem_request *request)
 			WARN(tail_req->elsp_submitted != 0,
 				"More than 2 already-submitted reqs queued\n");
 			list_del(&tail_req->execlist_link);
-			i915_gem_request_unreference(tail_req);
+			i915_gem_request_put(tail_req);
 		}
 	}
 
-	i915_gem_request_reference(request);
+	i915_gem_request_get(request);
 	list_add_tail(&request->execlist_link, &engine->execlist_queue);
 	request->ctx_hw_id = request->ctx->hw_id;
 	if (num_elements == 0)
@@ -904,7 +904,7 @@ void intel_execlists_cancel_requests(struct intel_engine_cs *engine)
 
 	list_for_each_entry_safe(req, tmp, &cancel_list, execlist_link) {
 		list_del(&req->execlist_link);
-		i915_gem_request_unreference(req);
+		i915_gem_request_put(req);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a1bf5f8fbb1c..4e9846e70b48 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7775,7 +7775,7 @@ static void __intel_rps_boost_work(struct work_struct *work)
 	if (!i915_gem_request_completed(req))
 		gen6_rps_boost(req->i915, NULL, req->emitted_jiffies);
 
-	i915_gem_request_unreference(req);
+	i915_gem_request_put(req);
 	kfree(boost);
 }
 
@@ -7793,8 +7793,7 @@ void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req)
 	if (boost == NULL)
 		return;
 
-	i915_gem_request_reference(req);
-	boost->req = req;
+	boost->req = i915_gem_request_get(req);
 
 	INIT_WORK(&boost->work, __intel_rps_boost_work);
 	queue_work(req->i915->wq, &boost->work);
-- 
2.8.1

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

[PATCH 2/9] drm/i915: Rename i915_gem_context_reference/unreference()

[Chris Wilson]

As these are wrappers around kref_get/kref_put() it is preferable to
follow the naming convention and use the same verb get/put in our
wrapper names for manipulating a reference to the context.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |  6 ++++--
 drivers/gpu/drm/i915/i915_gem_context.c    | 22 ++++++++++------------
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  6 +++---
 drivers/gpu/drm/i915/i915_gem_request.c    |  7 +++----
 drivers/gpu/drm/i915/intel_lrc.c           |  4 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.c    |  4 ++--
 6 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e163a9487750..2131e7f1e57a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3338,12 +3338,14 @@ i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
 	return ctx;
 }
 
-static inline void i915_gem_context_reference(struct i915_gem_context *ctx)
+static inline struct i915_gem_context *
+i915_gem_context_get(struct i915_gem_context *ctx)
 {
 	kref_get(&ctx->ref);
+	return ctx;
 }
 
-static inline void i915_gem_context_unreference(struct i915_gem_context *ctx)
+static inline void i915_gem_context_put(struct i915_gem_context *ctx)
 {
 	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
 	kref_put(&ctx->ref, i915_gem_context_free);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 3b636161256c..71517918853a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -305,7 +305,7 @@ __create_hw_context(struct drm_device *dev,
 	return ctx;
 
 err_out:
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 	return ERR_PTR(ret);
 }
 
@@ -333,7 +333,7 @@ i915_gem_create_context(struct drm_device *dev,
 			DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
 					 PTR_ERR(ppgtt));
 			idr_remove(&file_priv->context_idr, ctx->user_handle);
-			i915_gem_context_unreference(ctx);
+			i915_gem_context_put(ctx);
 			return ERR_CAST(ppgtt);
 		}
 
@@ -390,7 +390,7 @@ static void i915_gem_context_unpin(struct i915_gem_context *ctx,
 		if (ce->state)
 			i915_gem_object_ggtt_unpin(ce->state);
 
-		i915_gem_context_unreference(ctx);
+		i915_gem_context_put(ctx);
 	}
 }
 
@@ -504,7 +504,7 @@ void i915_gem_context_fini(struct drm_device *dev)
 
 	lockdep_assert_held(&dev->struct_mutex);
 
-	i915_gem_context_unreference(dctx);
+	i915_gem_context_put(dctx);
 	dev_priv->kernel_context = NULL;
 
 	ida_destroy(&dev_priv->context_hw_ida);
@@ -515,7 +515,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
 	struct i915_gem_context *ctx = p;
 
 	ctx->file_priv = ERR_PTR(-EBADF);
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 	return 0;
 }
 
@@ -827,10 +827,9 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 
 		/* obj is kept alive until the next request by its active ref */
 		i915_gem_object_ggtt_unpin(from->engine[RCS].state);
-		i915_gem_context_unreference(from);
+		i915_gem_context_put(from);
 	}
-	i915_gem_context_reference(to);
-	engine->last_context = to;
+	engine->last_context = i915_gem_context_get(to);
 
 	/* GEN8 does *not* require an explicit reload if the PDPs have been
 	 * setup, and we do not wish to move them.
@@ -914,10 +913,9 @@ int i915_switch_context(struct drm_i915_gem_request *req)
 		}
 
 		if (to != engine->last_context) {
-			i915_gem_context_reference(to);
 			if (engine->last_context)
-				i915_gem_context_unreference(engine->last_context);
-			engine->last_context = to;
+				i915_gem_context_put(engine->last_context);
+			engine->last_context = i915_gem_context_get(to);
 		}
 
 		return 0;
@@ -1014,7 +1012,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 	}
 
 	idr_remove(&file_priv->context_idr, ctx->user_handle);
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 	mutex_unlock(&dev->struct_mutex);
 
 	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index f6724ae210b3..551dd4f549fa 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1509,7 +1509,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		goto pre_mutex_err;
 	}
 
-	i915_gem_context_reference(ctx);
+	i915_gem_context_get(ctx);
 
 	if (ctx->ppgtt)
 		vm = &ctx->ppgtt->base;
@@ -1520,7 +1520,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
 	eb = eb_create(args);
 	if (eb == NULL) {
-		i915_gem_context_unreference(ctx);
+		i915_gem_context_put(ctx);
 		mutex_unlock(&dev->struct_mutex);
 		ret = -ENOMEM;
 		goto pre_mutex_err;
@@ -1664,7 +1664,7 @@ err_batch_unpin:
 
 err:
 	/* the request owns the ref now */
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 	eb_destroy(eb);
 
 	mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 04ff97b10c08..60a3a343b3a8 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -180,7 +180,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 					       request->engine);
 	}
 
-	i915_gem_context_unreference(request->ctx);
+	i915_gem_context_put(request->ctx);
 	i915_gem_request_put(request);
 }
 
@@ -341,8 +341,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
 
 	req->i915 = dev_priv;
 	req->engine = engine;
-	req->ctx = ctx;
-	i915_gem_context_reference(ctx);
+	req->ctx = i915_gem_context_get(ctx);
 
 	/*
 	 * Reserve space in the ring buffer for all the commands required to
@@ -364,7 +363,7 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
 	return 0;
 
 err_ctx:
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 err:
 	kmem_cache_free(dev_priv->requests, req);
 	return ret;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b23d4cc94cd5..6282fca6229c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -980,7 +980,6 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
 	if (ret)
 		goto unpin_map;
 
-	i915_gem_context_reference(ctx);
 	ce->lrc_vma = i915_gem_obj_to_ggtt(ce->state);
 	intel_lr_context_descriptor_update(ctx, engine);
 
@@ -992,6 +991,7 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx,
 	if (i915.enable_guc_submission)
 		I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
 
+	i915_gem_context_get(ctx);
 	return 0;
 
 unpin_map:
@@ -1023,7 +1023,7 @@ void intel_lr_context_unpin(struct i915_gem_context *ctx,
 	ce->lrc_desc = 0;
 	ce->lrc_reg_state = NULL;
 
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 }
 
 static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index af0bd71e3a36..3a7135d00033 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2139,7 +2139,7 @@ static int intel_ring_context_pin(struct i915_gem_context *ctx,
 	if (ctx == ctx->i915->kernel_context)
 		ce->initialised = true;
 
-	i915_gem_context_reference(ctx);
+	i915_gem_context_get(ctx);
 	return 0;
 
 error:
@@ -2160,7 +2160,7 @@ static void intel_ring_context_unpin(struct i915_gem_context *ctx,
 	if (ce->state)
 		i915_gem_object_ggtt_unpin(ce->state);
 
-	i915_gem_context_unreference(ctx);
+	i915_gem_context_put(ctx);
 }
 
 static int intel_init_ring_buffer(struct intel_engine_cs *engine)
-- 
2.8.1

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

[PATCH 3/9] drm/i915: Wrap drm_gem_object_lookup in i915_gem_object_lookup

[Chris Wilson]

For symmetry with a forthcoming i915_gem_object_get() and
i915_gem_object_pu().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h        | 20 +++++++++++-
 drivers/gpu/drm/i915/i915_gem.c        | 58 +++++++++++++++++-----------------
 drivers/gpu/drm/i915/i915_gem_tiling.c |  8 ++---
 drivers/gpu/drm/i915/intel_display.c   |  4 +--
 drivers/gpu/drm/i915/intel_overlay.c   |  5 ++-
 5 files changed, 56 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2131e7f1e57a..47f244f9c64e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2286,7 +2286,25 @@ struct drm_i915_gem_object {
 		} userptr;
 	};
 };
-#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
+
+static inline struct drm_i915_gem_object *
+to_intel_bo(struct drm_gem_object *gem)
+{
+	/* Assert that to_intel_bo(NULL) == NULL */
+	BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base));
+
+	return container_of(gem, struct drm_i915_gem_object, base);
+}
+
+static inline struct drm_i915_gem_object *
+i915_gem_object_lookup(struct drm_file *file, u32 handle)
+{
+	return to_intel_bo(drm_gem_object_lookup(file, handle));
+}
+
+__deprecated
+extern struct drm_gem_object *
+drm_gem_object_lookup(struct drm_file *file, u32 handle);
 
 static inline bool
 i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d825db7b763d..970496419098 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -864,8 +864,8 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		return ret;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
-	if (&obj->base == NULL) {
+	obj = i915_gem_object_lookup(file, args->handle);
+	if (!obj) {
 		ret = -ENOENT;
 		goto unlock;
 	}
@@ -1280,8 +1280,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		goto put_rpm;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
-	if (&obj->base == NULL) {
+	obj = i915_gem_object_lookup(file, args->handle);
+	if (!obj) {
 		ret = -ENOENT;
 		goto unlock;
 	}
@@ -1497,8 +1497,8 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		return ret;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
-	if (&obj->base == NULL) {
+	obj = i915_gem_object_lookup(file, args->handle);
+	if (!obj) {
 		ret = -ENOENT;
 		goto unlock;
 	}
@@ -1546,8 +1546,8 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		return ret;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
-	if (&obj->base == NULL) {
+	obj = i915_gem_object_lookup(file, args->handle);
+	if (!obj) {
 		ret = -ENOENT;
 		goto unlock;
 	}
@@ -1587,7 +1587,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 		    struct drm_file *file)
 {
 	struct drm_i915_gem_mmap *args = data;
-	struct drm_gem_object *obj;
+	struct drm_i915_gem_object *obj;
 	unsigned long addr;
 
 	if (args->flags & ~(I915_MMAP_WC))
@@ -1596,19 +1596,19 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 	if (args->flags & I915_MMAP_WC && !boot_cpu_has(X86_FEATURE_PAT))
 		return -ENODEV;
 
-	obj = drm_gem_object_lookup(file, args->handle);
-	if (obj == NULL)
+	obj = i915_gem_object_lookup(file, args->handle);
+	if (!obj)
 		return -ENOENT;
 
 	/* prime objects have no backing filp to GEM mmap
 	 * pages from.
 	 */
-	if (!obj->filp) {
-		drm_gem_object_unreference_unlocked(obj);
+	if (!obj->base.filp) {
+		drm_gem_object_unreference_unlocked(&obj->base);
 		return -EINVAL;
 	}
 
-	addr = vm_mmap(obj->filp, 0, args->size,
+	addr = vm_mmap(obj->base.filp, 0, args->size,
 		       PROT_READ | PROT_WRITE, MAP_SHARED,
 		       args->offset);
 	if (args->flags & I915_MMAP_WC) {
@@ -1616,7 +1616,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 		struct vm_area_struct *vma;
 
 		if (down_write_killable(&mm->mmap_sem)) {
-			drm_gem_object_unreference_unlocked(obj);
+			drm_gem_object_unreference_unlocked(&obj->base);
 			return -EINTR;
 		}
 		vma = find_vma(mm, addr);
@@ -1628,9 +1628,9 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 		up_write(&mm->mmap_sem);
 
 		/* This may race, but that's ok, it only gets set */
-		WRITE_ONCE(to_intel_bo(obj)->has_wc_mmap, true);
+		WRITE_ONCE(obj->has_wc_mmap, true);
 	}
-	drm_gem_object_unreference_unlocked(obj);
+	drm_gem_object_unreference_unlocked(&obj->base);
 	if (IS_ERR((void *)addr))
 		return addr;
 
@@ -1968,8 +1968,8 @@ i915_gem_mmap_gtt(struct drm_file *file,
 	if (ret)
 		return ret;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, handle));
-	if (&obj->base == NULL) {
+	obj = i915_gem_object_lookup(file, handle);
+	if (!obj) {
 		ret = -ENOENT;
 		goto unlock;
 	}
@@ -2792,8 +2792,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 	if (ret)
 		return ret;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, args->bo_handle));
-	if (&obj->base == NULL) {
+	obj = i915_gem_object_lookup(file, args->bo_handle);
+	if (!obj) {
 		mutex_unlock(&dev->struct_mutex);
 		return -ENOENT;
 	}
@@ -3596,8 +3596,8 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, void *data,
 	struct drm_i915_gem_caching *args = data;
 	struct drm_i915_gem_object *obj;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
-	if (&obj->base == NULL)
+	obj = i915_gem_object_lookup(file, args->handle);
+	if (!obj)
 		return -ENOENT;
 
 	switch (obj->cache_level) {
@@ -3657,8 +3657,8 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		goto rpm_put;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
-	if (&obj->base == NULL) {
+	obj = i915_gem_object_lookup(file, args->handle);
+	if (!obj) {
 		ret = -ENOENT;
 		goto unlock;
 	}
@@ -4026,8 +4026,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		return ret;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
-	if (&obj->base == NULL) {
+	obj = i915_gem_object_lookup(file, args->handle);
+	if (!obj) {
 		ret = -ENOENT;
 		goto unlock;
 	}
@@ -4091,8 +4091,8 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 	if (ret)
 		return ret;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file_priv, args->handle));
-	if (&obj->base == NULL) {
+	obj = i915_gem_object_lookup(file_priv, args->handle);
+	if (!obj) {
 		ret = -ENOENT;
 		goto unlock;
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 8030199731db..46e80f30afcd 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -166,8 +166,8 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
 	struct drm_i915_gem_object *obj;
 	int ret = 0;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
-	if (&obj->base == NULL)
+	obj = i915_gem_object_lookup(file, args->handle);
+	if (!obj)
 		return -ENOENT;
 
 	if (!i915_tiling_ok(dev,
@@ -297,8 +297,8 @@ i915_gem_get_tiling(struct drm_device *dev, void *data,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_i915_gem_object *obj;
 
-	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
-	if (&obj->base == NULL)
+	obj = i915_gem_object_lookup(file, args->handle);
+	if (!obj)
 		return -ENOENT;
 
 	mutex_lock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2f7af855a585..77d320584478 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15089,8 +15089,8 @@ intel_user_framebuffer_create(struct drm_device *dev,
 	struct drm_i915_gem_object *obj;
 	struct drm_mode_fb_cmd2 mode_cmd = *user_mode_cmd;
 
-	obj = to_intel_bo(drm_gem_object_lookup(filp, mode_cmd.handles[0]));
-	if (&obj->base == NULL)
+	obj = i915_gem_object_lookup(filp, mode_cmd.handles[0]);
+	if (!obj)
 		return ERR_PTR(-ENOENT);
 
 	fb = intel_framebuffer_create(dev, &mode_cmd, obj);
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 3212d8806b5a..5ca797b01ccb 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1122,9 +1122,8 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 	}
 	crtc = to_intel_crtc(drmmode_crtc);
 
-	new_bo = to_intel_bo(drm_gem_object_lookup(file_priv,
-						   put_image_rec->bo_handle));
-	if (&new_bo->base == NULL) {
+	new_bo = i915_gem_object_lookup(file_priv, put_image_rec->bo_handle);
+	if (!new_bo) {
 		ret = -ENOENT;
 		goto out_free;
 	}
-- 
2.8.1

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

[PATCH 4/9] drm/i915: Wrap drm_gem_object_reference in i915_gem_object_get

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h            | 11 +++++++++++
 drivers/gpu/drm/i915/i915_gem.c            |  4 ++--
 drivers/gpu/drm/i915/i915_gem_dmabuf.c     |  3 +--
 drivers/gpu/drm/i915/i915_gem_evict.c      |  2 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++--
 drivers/gpu/drm/i915/i915_gem_shrinker.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c    |  3 +--
 drivers/gpu/drm/i915/intel_display.c       |  3 +--
 8 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 47f244f9c64e..163d28e8e92c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2306,6 +2306,17 @@ __deprecated
 extern struct drm_gem_object *
 drm_gem_object_lookup(struct drm_file *file, u32 handle);
 
+__attribute__((nonnull))
+static inline struct drm_i915_gem_object *
+i915_gem_object_get(struct drm_i915_gem_object *obj)
+{
+	drm_gem_object_reference(&obj->base);
+	return obj;
+}
+
+__deprecated
+extern void drm_gem_object_reference(struct drm_gem_object *);
+
 static inline bool
 i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 970496419098..f886c0bb24ca 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -289,7 +289,7 @@ drop_pages(struct drm_i915_gem_object *obj)
 	struct i915_vma *vma, *next;
 	int ret;
 
-	drm_gem_object_reference(&obj->base);
+	i915_gem_object_get(obj);
 	list_for_each_entry_safe(vma, next, &obj->vma_list, obj_link)
 		if (i915_vma_unbind(vma))
 			break;
@@ -2361,7 +2361,7 @@ void i915_vma_move_to_active(struct i915_vma *vma,
 
 	/* Add a reference if we're newly entering the active list. */
 	if (obj->active == 0)
-		drm_gem_object_reference(&obj->base);
+		i915_gem_object_get(obj);
 	obj->active |= intel_engine_flag(engine);
 
 	list_move_tail(&obj->engine_list[engine->id], &engine->active_list);
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 7d08ac08c401..3a00ab3ad06e 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -278,8 +278,7 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
 			 * Importing dmabuf exported from out own gem increases
 			 * refcount on gem itself instead of f_count of dmabuf.
 			 */
-			drm_gem_object_reference(&obj->base);
-			return &obj->base;
+			return &i915_gem_object_get(obj)->base;
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index b1194c7c0a48..56103948ee16 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -183,7 +183,7 @@ found:
 				       exec_list);
 		if (drm_mm_scan_remove_block(&vma->node)) {
 			list_move(&vma->exec_list, &eviction_list);
-			drm_gem_object_reference(&vma->obj->base);
+			i915_gem_object_get(vma->obj);
 			continue;
 		}
 		list_del_init(&vma->exec_list);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 551dd4f549fa..28bcc99a0cc1 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -123,7 +123,7 @@ eb_lookup_vmas(struct eb_vmas *eb,
 			goto err;
 		}
 
-		drm_gem_object_reference(&obj->base);
+		i915_gem_object_get(obj);
 		list_add_tail(&obj->obj_exec_link, &objects);
 	}
 	spin_unlock(&file->table_lock);
@@ -1236,7 +1236,7 @@ i915_gem_execbuffer_parse(struct intel_engine_cs *engine,
 	vma = i915_gem_obj_to_ggtt(shadow_batch_obj);
 	vma->exec_entry = shadow_exec_entry;
 	vma->exec_entry->flags = __EXEC_OBJECT_HAS_PIN;
-	drm_gem_object_reference(&shadow_batch_obj->base);
+	i915_gem_object_get(shadow_batch_obj);
 	list_add_tail(&vma->exec_list, &eb->vmas);
 
 	shadow_batch_obj->base.pending_read_domains = I915_GEM_DOMAIN_COMMAND;
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 6f10b421487b..3aa76d3608ea 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -190,7 +190,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
 			if (!can_release_pages(obj))
 				continue;
 
-			drm_gem_object_reference(&obj->base);
+			i915_gem_object_get(obj);
 
 			/* For the unbound phase, this should be a no-op! */
 			list_for_each_entry_safe(vma, v,
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index ba16e044fac6..c41bf74f926e 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -622,8 +622,7 @@ __i915_gem_userptr_get_pages_schedule(struct drm_i915_gem_object *obj,
 	obj->userptr.work = &work->work;
 	obj->userptr.workers++;
 
-	work->obj = obj;
-	drm_gem_object_reference(&obj->base);
+	work->obj = i915_gem_object_get(obj);
 
 	work->task = current;
 	get_task_struct(work->task);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 77d320584478..84904a2d3fcd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11649,7 +11649,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 	/* Reference the objects for the scheduled work. */
 	drm_framebuffer_reference(work->old_fb);
-	drm_gem_object_reference(&obj->base);
 
 	crtc->primary->fb = fb;
 	update_state_fb(crtc->primary);
@@ -11657,7 +11656,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	intel_fbc_pre_update(intel_crtc, intel_crtc->config,
 			     to_intel_plane_state(primary->state));
 
-	work->pending_flip_obj = obj;
+	work->pending_flip_obj = i915_gem_object_get(obj);
 
 	ret = i915_mutex_lock_interruptible(dev);
 	if (ret)
-- 
2.8.1

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

[PATCH 5/9] drm/i915: Rename drm_gem_object_unreference in preparation for lockless free

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h              | 10 ++++++++++
 drivers/gpu/drm/i915/i915_gem.c              | 26 +++++++++++++-------------
 drivers/gpu/drm/i915/i915_gem_batch_pool.c   |  4 ++--
 drivers/gpu/drm/i915/i915_gem_context.c      |  4 ++--
 drivers/gpu/drm/i915/i915_gem_evict.c        |  7 ++++---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c   |  6 +++---
 drivers/gpu/drm/i915/i915_gem_render_state.c |  4 ++--
 drivers/gpu/drm/i915/i915_gem_shrinker.c     |  2 +-
 drivers/gpu/drm/i915/i915_gem_stolen.c       |  2 +-
 drivers/gpu/drm/i915/i915_gem_tiling.c       |  4 ++--
 drivers/gpu/drm/i915/i915_gem_userptr.c      |  4 ++--
 drivers/gpu/drm/i915/i915_guc_submission.c   |  6 +++---
 drivers/gpu/drm/i915/intel_display.c         |  6 +++---
 drivers/gpu/drm/i915/intel_fbdev.c           |  2 +-
 drivers/gpu/drm/i915/intel_guc_loader.c      |  8 +++++---
 drivers/gpu/drm/i915/intel_lrc.c             |  6 +++---
 drivers/gpu/drm/i915/intel_overlay.c         |  8 ++++----
 drivers/gpu/drm/i915/intel_ringbuffer.c      | 14 +++++++-------
 18 files changed, 68 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 163d28e8e92c..ffc97c5f9d2e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2317,6 +2317,16 @@ i915_gem_object_get(struct drm_i915_gem_object *obj)
 __deprecated
 extern void drm_gem_object_reference(struct drm_gem_object *);
 
+__attribute__((nonnull))
+static inline void
+i915_gem_object_put(struct drm_i915_gem_object *obj)
+{
+	drm_gem_object_unreference(&obj->base);
+}
+
+__deprecated
+extern void drm_gem_object_unreference(struct drm_gem_object *);
+
 static inline bool
 i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f886c0bb24ca..f23670fbc8a9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -295,7 +295,7 @@ drop_pages(struct drm_i915_gem_object *obj)
 			break;
 
 	ret = i915_gem_object_put_pages(obj);
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 
 	return ret;
 }
@@ -887,7 +887,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
 					args->offset, args->data_ptr);
 
 out:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
@@ -1320,7 +1320,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	}
 
 out:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
 put_rpm:
@@ -1522,7 +1522,7 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 		intel_fb_obj_invalidate(obj, write_origin(obj, write_domain));
 
 unref:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
@@ -1556,7 +1556,7 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
 	if (obj->pin_display)
 		i915_gem_object_flush_cpu_write_domain(obj);
 
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
@@ -1987,7 +1987,7 @@ i915_gem_mmap_gtt(struct drm_file *file,
 	*offset = drm_vma_node_offset_addr(&obj->base.vma_node);
 
 out:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
@@ -2411,7 +2411,7 @@ i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring)
 	}
 
 	i915_gem_request_assign(&obj->last_fenced_req, NULL);
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 }
 
 static bool i915_context_is_banned(const struct i915_gem_context *ctx)
@@ -2814,7 +2814,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 		goto out;
 	}
 
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 
 	for (i = 0; i < I915_NUM_ENGINES; i++) {
 		if (obj->last_read_req[i] == NULL)
@@ -2835,7 +2835,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 	return ret;
 
 out:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
 }
@@ -3665,7 +3665,7 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
 
 	ret = i915_gem_object_set_cache_level(obj, level);
 
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
 rpm_put:
@@ -4057,7 +4057,7 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 	}
 
 unref:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
@@ -4121,7 +4121,7 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 	args->retained = obj->madv != __I915_MADV_PURGED;
 
 out:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
@@ -4952,6 +4952,6 @@ i915_gem_object_create_from_data(struct drm_device *dev,
 	return obj;
 
 fail:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 	return ERR_PTR(ret);
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
index 3752d5daa4b2..3507b2753fd3 100644
--- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c
+++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
@@ -75,7 +75,7 @@ void i915_gem_batch_pool_fini(struct i915_gem_batch_pool *pool)
 						 batch_pool_link);
 
 			list_del(&obj->batch_pool_link);
-			drm_gem_object_unreference(&obj->base);
+			i915_gem_object_put(obj);
 		}
 	}
 }
@@ -121,7 +121,7 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool,
 		/* While we're looping, do some clean up */
 		if (tmp->madv == __I915_MADV_PURGED) {
 			list_del(&tmp->batch_pool_link);
-			drm_gem_object_unreference(&tmp->base);
+			i915_gem_object_put(tmp);
 			continue;
 		}
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 71517918853a..2600939fc2ee 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -176,7 +176,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
 		if (ce->ringbuf)
 			intel_ringbuffer_free(ce->ringbuf);
 
-		drm_gem_object_unreference(&ce->state->base);
+		i915_gem_object_put(ce->state);
 	}
 
 	list_del(&ctx->link);
@@ -216,7 +216,7 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
 		ret = i915_gem_object_set_cache_level(obj, I915_CACHE_L3_LLC);
 		/* Failure shouldn't ever happen this early */
 		if (WARN_ON(ret)) {
-			drm_gem_object_unreference(&obj->base);
+			i915_gem_object_put(obj);
 			return ERR_PTR(ret);
 		}
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 56103948ee16..81f7b4383d5e 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -191,17 +191,18 @@ found:
 
 	/* Unbinding will emit any required flushes */
 	while (!list_empty(&eviction_list)) {
-		struct drm_gem_object *obj;
+		struct drm_i915_gem_object *obj;
+
 		vma = list_first_entry(&eviction_list,
 				       struct i915_vma,
 				       exec_list);
 
-		obj =  &vma->obj->base;
+		obj =  vma->obj;
 		list_del_init(&vma->exec_list);
 		if (ret == 0)
 			ret = i915_vma_unbind(vma);
 
-		drm_gem_object_unreference(obj);
+		i915_gem_object_put(obj);
 	}
 
 	return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 28bcc99a0cc1..6482ec24ff3b 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -176,7 +176,7 @@ err:
 				       struct drm_i915_gem_object,
 				       obj_exec_link);
 		list_del_init(&obj->obj_exec_link);
-		drm_gem_object_unreference(&obj->base);
+		i915_gem_object_put(obj);
 	}
 	/*
 	 * Objects already transfered to the vmas list will be unreferenced by
@@ -264,7 +264,7 @@ static void eb_destroy(struct eb_vmas *eb)
 				       exec_list);
 		list_del_init(&vma->exec_list);
 		i915_gem_execbuffer_unreserve_vma(vma);
-		drm_gem_object_unreference(&vma->obj->base);
+		i915_gem_object_put(vma->obj);
 	}
 	kfree(eb);
 }
@@ -873,7 +873,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
 		vma = list_first_entry(&eb->vmas, struct i915_vma, exec_list);
 		list_del_init(&vma->exec_list);
 		i915_gem_execbuffer_unreserve_vma(vma);
-		drm_gem_object_unreference(&vma->obj->base);
+		i915_gem_object_put(vma->obj);
 	}
 
 	mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index f75bbd67a13a..b2be4676a5cf 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -70,7 +70,7 @@ static int render_state_init(struct render_state *so,
 	return 0;
 
 free_gem:
-	drm_gem_object_unreference(&so->obj->base);
+	i915_gem_object_put(so->obj);
 	return ret;
 }
 
@@ -195,7 +195,7 @@ err_out:
 void i915_gem_render_state_fini(struct render_state *so)
 {
 	i915_gem_object_ggtt_unpin(so->obj);
-	drm_gem_object_unreference(&so->obj->base);
+	i915_gem_object_put(so->obj);
 }
 
 int i915_gem_render_state_prepare(struct intel_engine_cs *engine,
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 3aa76d3608ea..afaa2597e35e 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -201,7 +201,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
 			if (i915_gem_object_put_pages(obj) == 0)
 				count += obj->base.size >> PAGE_SHIFT;
 
-			drm_gem_object_unreference(&obj->base);
+			i915_gem_object_put(obj);
 		}
 		list_splice(&still_in_list, phase->list);
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 66be299a1486..310756c30723 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -716,6 +716,6 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
 	return obj;
 
 err:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 	return NULL;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 46e80f30afcd..2d9d190bd6ef 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -268,7 +268,7 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
 	}
 
 err:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 	mutex_unlock(&dev->struct_mutex);
 
 	intel_runtime_pm_put(dev_priv);
@@ -328,7 +328,7 @@ i915_gem_get_tiling(struct drm_device *dev, void *data,
 	if (args->swizzle_mode == I915_BIT_6_SWIZZLE_9_10_17)
 		args->swizzle_mode = I915_BIT_6_SWIZZLE_9_10;
 
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 	mutex_unlock(&dev->struct_mutex);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index c41bf74f926e..cd4af22b8c59 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -119,7 +119,7 @@ static void cancel_userptr(struct work_struct *work)
 		dev_priv->mm.interruptible = was_interruptible;
 	}
 
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 	mutex_unlock(&dev->struct_mutex);
 }
 
@@ -577,7 +577,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
 	}
 
 	obj->userptr.workers--;
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 	mutex_unlock(&dev->struct_mutex);
 
 	release_pages(pvec, pinned, 0);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 1cc5de129e76..01c1c1671811 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -633,13 +633,13 @@ gem_allocate_guc_obj(struct drm_i915_private *dev_priv, u32 size)
 		return NULL;
 
 	if (i915_gem_object_get_pages(obj)) {
-		drm_gem_object_unreference(&obj->base);
+		i915_gem_object_put(obj);
 		return NULL;
 	}
 
 	if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE,
 			PIN_OFFSET_BIAS | GUC_WOPCM_TOP)) {
-		drm_gem_object_unreference(&obj->base);
+		i915_gem_object_put(obj);
 		return NULL;
 	}
 
@@ -661,7 +661,7 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 	if (i915_gem_obj_is_pinned(obj))
 		i915_gem_object_ggtt_unpin(obj);
 
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 84904a2d3fcd..a18a79c5db1c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2488,7 +2488,7 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
 	return true;
 
 out_unref_obj:
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 	mutex_unlock(&dev->struct_mutex);
 	return false;
 }
@@ -10953,7 +10953,7 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 
 	mutex_lock(&dev->struct_mutex);
 	intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
-	drm_gem_object_unreference(&work->pending_flip_obj->base);
+	i915_gem_object_put(work->pending_flip_obj);
 	mutex_unlock(&dev->struct_mutex);
 
 	i915_gem_request_put(work->flip_queued_req);
@@ -14851,7 +14851,7 @@ static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
 	drm_framebuffer_cleanup(fb);
 	mutex_lock(&dev->struct_mutex);
 	WARN_ON(!intel_fb->obj->framebuffer_references--);
-	drm_gem_object_unreference(&intel_fb->obj->base);
+	i915_gem_object_put(intel_fb->obj);
 	mutex_unlock(&dev->struct_mutex);
 	kfree(intel_fb);
 }
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 23129dcfba9d..b1074a2b2860 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -159,7 +159,7 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
 
 	fb = __intel_framebuffer_create(dev, &mode_cmd, obj);
 	if (IS_ERR(fb)) {
-		drm_gem_object_unreference(&obj->base);
+		i915_gem_object_put(obj);
 		ret = PTR_ERR(fb);
 		goto out;
 	}
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 605c69658d2c..75a1496ceb6f 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -662,7 +662,7 @@ fail:
 	mutex_lock(&dev->struct_mutex);
 	obj = guc_fw->guc_fw_obj;
 	if (obj)
-		drm_gem_object_unreference(&obj->base);
+		i915_gem_object_put(obj);
 	guc_fw->guc_fw_obj = NULL;
 	mutex_unlock(&dev->struct_mutex);
 
@@ -737,13 +737,15 @@ void intel_guc_fini(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
 
+	if (!guc_fw->guc_fw_obj)
+		return;
+
 	mutex_lock(&dev->struct_mutex);
 	direct_interrupts_to_host(dev_priv);
 	i915_guc_submission_disable(dev_priv);
 	i915_guc_submission_fini(dev_priv);
 
-	if (guc_fw->guc_fw_obj)
-		drm_gem_object_unreference(&guc_fw->guc_fw_obj->base);
+	i915_gem_object_put(guc_fw->guc_fw_obj);
 	guc_fw->guc_fw_obj = NULL;
 	mutex_unlock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6282fca6229c..439aeab807b1 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1367,7 +1367,7 @@ static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *engine, u32 size)
 	if (ret) {
 		DRM_DEBUG_DRIVER("pin LRC WA ctx backing obj failed: %d\n",
 				 ret);
-		drm_gem_object_unreference(&engine->wa_ctx.obj->base);
+		i915_gem_object_put(engine->wa_ctx.obj);
 		return ret;
 	}
 
@@ -1378,7 +1378,7 @@ static void lrc_destroy_wa_ctx_obj(struct intel_engine_cs *engine)
 {
 	if (engine->wa_ctx.obj) {
 		i915_gem_object_ggtt_unpin(engine->wa_ctx.obj);
-		drm_gem_object_unreference(&engine->wa_ctx.obj->base);
+		i915_gem_object_put(engine->wa_ctx.obj);
 		engine->wa_ctx.obj = NULL;
 	}
 }
@@ -2382,7 +2382,7 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 error_ringbuf:
 	intel_ringbuffer_free(ringbuf);
 error_deref_obj:
-	drm_gem_object_unreference(&ctx_obj->base);
+	i915_gem_object_put(ctx_obj);
 	ce->ringbuf = NULL;
 	ce->state = NULL;
 	return ret;
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 5ca797b01ccb..30d2f5db7d87 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -308,7 +308,7 @@ static void intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
 	struct drm_i915_gem_object *obj = overlay->old_vid_bo;
 
 	i915_gem_object_ggtt_unpin(obj);
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 
 	overlay->old_vid_bo = NULL;
 }
@@ -322,7 +322,7 @@ static void intel_overlay_off_tail(struct intel_overlay *overlay)
 		return;
 
 	i915_gem_object_ggtt_unpin(obj);
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 	overlay->vid_bo = NULL;
 
 	overlay->crtc->overlay = NULL;
@@ -1219,7 +1219,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 out_unlock:
 	mutex_unlock(&dev->struct_mutex);
 	drm_modeset_unlock_all(dev);
-	drm_gem_object_unreference_unlocked(&new_bo->base);
+	i915_gem_object_put(new_bo);
 out_free:
 	kfree(params);
 
@@ -1443,7 +1443,7 @@ out_unpin_bo:
 	if (!OVERLAY_NEEDS_PHYSICAL(dev_priv))
 		i915_gem_object_ggtt_unpin(reg_bo);
 out_free_bo:
-	drm_gem_object_unreference(&reg_bo->base);
+	i915_gem_object_put(reg_bo);
 out_free:
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 	kfree(overlay);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3a7135d00033..9c0a0b47628a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -641,7 +641,7 @@ void intel_fini_pipe_control(struct intel_engine_cs *engine)
 		return;
 
 	i915_gem_object_ggtt_unpin(engine->scratch.obj);
-	drm_gem_object_unreference(&engine->scratch.obj->base);
+	i915_gem_object_put(engine->scratch.obj);
 	engine->scratch.obj = NULL;
 }
 
@@ -672,7 +672,7 @@ int intel_init_pipe_control(struct intel_engine_cs *engine, int size)
 	return 0;
 
 err_unref:
-	drm_gem_object_unreference(&engine->scratch.obj->base);
+	i915_gem_object_put(engine->scratch.obj);
 err:
 	return ret;
 }
@@ -1312,7 +1312,7 @@ static void render_ring_cleanup(struct intel_engine_cs *engine)
 
 	if (dev_priv->semaphore_obj) {
 		i915_gem_object_ggtt_unpin(dev_priv->semaphore_obj);
-		drm_gem_object_unreference(&dev_priv->semaphore_obj->base);
+		i915_gem_object_put(dev_priv->semaphore_obj);
 		dev_priv->semaphore_obj = NULL;
 	}
 
@@ -1898,7 +1898,7 @@ static void cleanup_status_page(struct intel_engine_cs *engine)
 
 	kunmap(sg_page(obj->pages->sgl));
 	i915_gem_object_ggtt_unpin(obj);
-	drm_gem_object_unreference(&obj->base);
+	i915_gem_object_put(obj);
 	engine->status_page.obj = NULL;
 }
 
@@ -1936,7 +1936,7 @@ static int init_status_page(struct intel_engine_cs *engine)
 		ret = i915_gem_obj_ggtt_pin(obj, 4096, flags);
 		if (ret) {
 err_unref:
-			drm_gem_object_unreference(&obj->base);
+			i915_gem_object_put(obj);
 			return ret;
 		}
 
@@ -2039,7 +2039,7 @@ err_unpin:
 
 static void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
 {
-	drm_gem_object_unreference(&ringbuf->obj->base);
+	i915_gem_object_put(ringbuf->obj);
 	ringbuf->obj = NULL;
 }
 
@@ -2691,7 +2691,7 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 			i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
 			ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_NONBLOCK);
 			if (ret != 0) {
-				drm_gem_object_unreference(&obj->base);
+				i915_gem_object_put(obj);
 				DRM_ERROR("Failed to pin semaphore bo. Disabling semaphores\n");
 				i915.semaphores = 0;
 			} else {
-- 
2.8.1

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

[PATCH 6/9] drm/i915: Rename drm_gem_object_unreference_unlocked in preparation for lockless free

[Chris Wilson]

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h         | 10 ++++++++++
 drivers/gpu/drm/i915/i915_gem.c         | 10 +++++-----
 drivers/gpu/drm/i915/i915_gem_tiling.c  |  2 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c    |  6 +++---
 drivers/gpu/drm/i915/intel_overlay.c    |  2 +-
 drivers/gpu/drm/i915/intel_pm.c         |  2 +-
 7 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ffc97c5f9d2e..3ca1acec1522 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2327,6 +2327,16 @@ i915_gem_object_put(struct drm_i915_gem_object *obj)
 __deprecated
 extern void drm_gem_object_unreference(struct drm_gem_object *);
 
+__attribute__((nonnull))
+static inline void
+i915_gem_object_put_unlocked(struct drm_i915_gem_object *obj)
+{
+	drm_gem_object_unreference_unlocked(&obj->base);
+}
+
+__deprecated
+extern void drm_gem_object_unreference_unlocked(struct drm_gem_object *);
+
 static inline bool
 i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f23670fbc8a9..536acd527359 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -410,7 +410,7 @@ i915_gem_create(struct drm_file *file,
 
 	ret = drm_gem_handle_create(file, &obj->base, &handle);
 	/* drop reference from allocate - handle holds it now */
-	drm_gem_object_unreference_unlocked(&obj->base);
+	i915_gem_object_put_unlocked(obj);
 	if (ret)
 		return ret;
 
@@ -1604,7 +1604,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 	 * pages from.
 	 */
 	if (!obj->base.filp) {
-		drm_gem_object_unreference_unlocked(&obj->base);
+		i915_gem_object_put_unlocked(obj);
 		return -EINVAL;
 	}
 
@@ -1616,7 +1616,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 		struct vm_area_struct *vma;
 
 		if (down_write_killable(&mm->mmap_sem)) {
-			drm_gem_object_unreference_unlocked(&obj->base);
+			i915_gem_object_put_unlocked(obj);
 			return -EINTR;
 		}
 		vma = find_vma(mm, addr);
@@ -1630,7 +1630,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 		/* This may race, but that's ok, it only gets set */
 		WRITE_ONCE(obj->has_wc_mmap, true);
 	}
-	drm_gem_object_unreference_unlocked(&obj->base);
+	i915_gem_object_put_unlocked(obj);
 	if (IS_ERR((void *)addr))
 		return addr;
 
@@ -3615,7 +3615,7 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, void *data,
 		break;
 	}
 
-	drm_gem_object_unreference_unlocked(&obj->base);
+	i915_gem_object_put_unlocked(obj);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 2d9d190bd6ef..e83fc2d0433c 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -172,7 +172,7 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
 
 	if (!i915_tiling_ok(dev,
 			    args->stride, obj->base.size, args->tiling_mode)) {
-		drm_gem_object_unreference_unlocked(&obj->base);
+		i915_gem_object_put_unlocked(obj);
 		return -EINVAL;
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index cd4af22b8c59..ca8b82ab93d6 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -845,7 +845,7 @@ i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file
 		ret = drm_gem_handle_create(file, &obj->base, &handle);
 
 	/* drop reference from allocate - handle holds it now */
-	drm_gem_object_unreference_unlocked(&obj->base);
+	i915_gem_object_put_unlocked(obj);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a18a79c5db1c..78beb7e9d384 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10442,7 +10442,7 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
 
 	fb = intel_framebuffer_create(dev, &mode_cmd, obj);
 	if (IS_ERR(fb))
-		drm_gem_object_unreference_unlocked(&obj->base);
+		i915_gem_object_put_unlocked(obj);
 
 	return fb;
 }
@@ -11756,7 +11756,7 @@ cleanup:
 	crtc->primary->fb = old_fb;
 	update_state_fb(crtc->primary);
 
-	drm_gem_object_unreference_unlocked(&obj->base);
+	i915_gem_object_put_unlocked(obj);
 	drm_framebuffer_unreference(work->old_fb);
 
 	spin_lock_irq(&dev->event_lock);
@@ -15094,7 +15094,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
 
 	fb = intel_framebuffer_create(dev, &mode_cmd, obj);
 	if (IS_ERR(fb))
-		drm_gem_object_unreference_unlocked(&obj->base);
+		i915_gem_object_put_unlocked(obj);
 
 	return fb;
 }
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 30d2f5db7d87..8654a323722e 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1460,7 +1460,7 @@ void intel_cleanup_overlay(struct drm_i915_private *dev_priv)
 	 * hardware should be off already */
 	WARN_ON(dev_priv->overlay->active);
 
-	drm_gem_object_unreference_unlocked(&dev_priv->overlay->reg_bo->base);
+	i915_gem_object_put_unlocked(dev_priv->overlay->reg_bo);
 	kfree(dev_priv->overlay);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4e9846e70b48..64d628c915a3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5709,7 +5709,7 @@ static void valleyview_cleanup_pctx(struct drm_i915_private *dev_priv)
 	if (WARN_ON(!dev_priv->vlv_pctx))
 		return;
 
-	drm_gem_object_unreference_unlocked(&dev_priv->vlv_pctx->base);
+	i915_gem_object_put_unlocked(dev_priv->vlv_pctx);
 	dev_priv->vlv_pctx = NULL;
 }
 
-- 
2.8.1

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

[PATCH 7/9] drm/i915: Treat ringbuffer writes as write to normal memory

[Chris Wilson]

Ringbuffers are now being written to either through LLC or WC paths, so
treating them as simply iomem is no longer adequate. However, for the
older !llc hardware, the hardware is documentated as treating the TAIL
register update as serialising, so we can relax the barriers when filling
the rings (but even if it were not, it is still an uncached register write
and so serialising anyway.).

For simplicity, let's ignore the iomem annotation.

v2: Remove iomem from ringbuffer->virtual_address
v3: And for good measure add iomem elsewhere to keep sparse happy

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> #v2
---
 drivers/gpu/drm/i915/i915_gem_gtt.c     |  4 ++--
 drivers/gpu/drm/i915/i915_gem_gtt.h     |  1 +
 drivers/gpu/drm/i915/i915_irq.c         | 10 ++++++----
 drivers/gpu/drm/i915/intel_fbdev.c      |  2 +-
 drivers/gpu/drm/i915/intel_lrc.h        |  6 +++---
 drivers/gpu/drm/i915/intel_ringbuffer.c |  3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h | 28 ++++++++++++++++++++--------
 7 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 10f1e32767e6..30da543e1bdf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3681,7 +3681,7 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
 
 	lockdep_assert_held(&vma->vm->dev->struct_mutex);
 	if (WARN_ON(!vma->obj->map_and_fenceable))
-		return ERR_PTR(-ENODEV);
+		return IO_ERR_PTR(-ENODEV);
 
 	GEM_BUG_ON(!vma->is_ggtt);
 	GEM_BUG_ON((vma->bound & GLOBAL_BIND) == 0);
@@ -3692,7 +3692,7 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
 					vma->node.start,
 					vma->node.size);
 		if (ptr == NULL)
-			return ERR_PTR(-ENOMEM);
+			return IO_ERR_PTR(-ENOMEM);
 
 		vma->iomap = ptr;
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index aa5f31d1c2ed..c4a657973cc6 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -580,6 +580,7 @@ i915_ggtt_view_size(struct drm_i915_gem_object *obj,
  * Returns a valid iomapped pointer or ERR_PTR.
  */
 void __iomem *i915_vma_pin_iomap(struct i915_vma *vma);
+#define IO_ERR_PTR(x) ((void __iomem *)ERR_PTR(x))
 
 /**
  * i915_vma_unpin_iomap - unpins the mapping returned from i915_vma_iomap
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c8ed36765301..38d9ae90b613 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2860,6 +2860,7 @@ static struct intel_engine_cs *
 semaphore_waits_for(struct intel_engine_cs *engine, u32 *seqno)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	void __iomem *vaddr;
 	u32 cmd, ipehr, head;
 	u64 offset = 0;
 	int i, backwards;
@@ -2898,6 +2899,7 @@ semaphore_waits_for(struct intel_engine_cs *engine, u32 *seqno)
 	 */
 	head = I915_READ_HEAD(engine) & HEAD_ADDR;
 	backwards = (INTEL_GEN(dev_priv) >= 8) ? 5 : 4;
+	vaddr = (void __iomem *) engine->buffer->virtual_start;
 
 	for (i = backwards; i; --i) {
 		/*
@@ -2908,7 +2910,7 @@ semaphore_waits_for(struct intel_engine_cs *engine, u32 *seqno)
 		head &= engine->buffer->size - 1;
 
 		/* This here seems to blow up */
-		cmd = ioread32(engine->buffer->virtual_start + head);
+		cmd = ioread32(vaddr + head);
 		if (cmd == ipehr)
 			break;
 
@@ -2918,11 +2920,11 @@ semaphore_waits_for(struct intel_engine_cs *engine, u32 *seqno)
 	if (!i)
 		return NULL;
 
-	*seqno = ioread32(engine->buffer->virtual_start + head + 4) + 1;
+	*seqno = ioread32(vaddr + head + 4) + 1;
 	if (INTEL_GEN(dev_priv) >= 8) {
-		offset = ioread32(engine->buffer->virtual_start + head + 12);
+		offset = ioread32(vaddr + head + 12);
 		offset <<= 32;
-		offset = ioread32(engine->buffer->virtual_start + head + 8);
+		offset |= ioread32(vaddr + head + 8);
 	}
 	return semaphore_wait_to_signaller_ring(engine, ipehr, offset);
 }
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index b1074a2b2860..6344999d3d6e 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -189,7 +189,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	struct i915_vma *vma;
 	struct drm_i915_gem_object *obj;
 	bool prealloc = false;
-	void *vaddr;
+	void __iomem *vaddr;
 	int ret;
 
 	if (intel_fb &&
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 938e3eec06cf..aa3ac023aa73 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -81,8 +81,9 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req);
  */
 static inline void intel_logical_ring_advance(struct intel_ringbuffer *ringbuf)
 {
-	ringbuf->tail &= ringbuf->size - 1;
+	__intel_ringbuffer_advance(ringbuf);
 }
+
 /**
  * intel_logical_ring_emit() - write a DWORD to the ringbuffer.
  * @ringbuf: Ringbuffer to write to.
@@ -91,8 +92,7 @@ static inline void intel_logical_ring_advance(struct intel_ringbuffer *ringbuf)
 static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
 					   u32 data)
 {
-	iowrite32(data, ringbuf->virtual_start + ringbuf->tail);
-	ringbuf->tail += 4;
+	__intel_ringbuffer_emit(ringbuf, data);
 }
 static inline void intel_logical_ring_emit_reg(struct intel_ringbuffer *ringbuf,
 					       i915_reg_t reg)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9c0a0b47628a..afed24abb596 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2021,7 +2021,8 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_i915_private *dev_priv,
 		/* Access through the GTT requires the device to be awake. */
 		assert_rpm_wakelock_held(dev_priv);
 
-		addr = i915_vma_pin_iomap(i915_gem_obj_to_ggtt(obj));
+		addr = (void __force *)
+			i915_vma_pin_iomap(i915_gem_obj_to_ggtt(obj));
 		if (IS_ERR(addr)) {
 			ret = PTR_ERR(addr);
 			goto err_unpin;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 5cbafc00bfd5..d1b2d9bd89eb 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -84,7 +84,7 @@ struct intel_ring_hangcheck {
 
 struct intel_ringbuffer {
 	struct drm_i915_gem_object *obj;
-	void __iomem *virtual_start;
+	void *virtual_start;
 	struct i915_vma *vma;
 
 	struct intel_engine_cs *engine;
@@ -453,23 +453,35 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request);
 
 int __must_check intel_ring_begin(struct drm_i915_gem_request *req, int n);
 int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req);
-static inline void intel_ring_emit(struct intel_engine_cs *engine,
-				   u32 data)
+
+static inline void __intel_ringbuffer_emit(struct intel_ringbuffer *rb,
+					   u32 data)
+{
+	*(uint32_t *)(rb->virtual_start + rb->tail) = data;
+	rb->tail += 4;
+}
+
+static inline void __intel_ringbuffer_advance(struct intel_ringbuffer *rb)
+{
+	rb->tail &= rb->size - 1;
+}
+
+static inline void intel_ring_emit(struct intel_engine_cs *engine, u32 data)
 {
-	struct intel_ringbuffer *ringbuf = engine->buffer;
-	iowrite32(data, ringbuf->virtual_start + ringbuf->tail);
-	ringbuf->tail += 4;
+	__intel_ringbuffer_emit(engine->buffer, data);
 }
+
 static inline void intel_ring_emit_reg(struct intel_engine_cs *engine,
 				       i915_reg_t reg)
 {
 	intel_ring_emit(engine, i915_mmio_reg_offset(reg));
 }
+
 static inline void intel_ring_advance(struct intel_engine_cs *engine)
 {
-	struct intel_ringbuffer *ringbuf = engine->buffer;
-	ringbuf->tail &= ringbuf->size - 1;
+	__intel_ringbuffer_advance(engine->buffer);
 }
+
 int __intel_ring_space(int head, int tail, int size);
 void intel_ring_update_space(struct intel_ringbuffer *ringbuf);
 
-- 
2.8.1

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

[PATCH 8/9] drm/i915: Rename ring->virtual_start as ring->vaddr

[Chris Wilson]

Just a different colour to better match virtual addresses elsewhere.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c         |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +++++------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  4 ++--
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 38d9ae90b613..0fef5cda4aca 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2899,7 +2899,7 @@ semaphore_waits_for(struct intel_engine_cs *engine, u32 *seqno)
 	 */
 	head = I915_READ_HEAD(engine) & HEAD_ADDR;
 	backwards = (INTEL_GEN(dev_priv) >= 8) ? 5 : 4;
-	vaddr = (void __iomem *) engine->buffer->virtual_start;
+	vaddr = (void __iomem *) engine->buffer->vaddr;
 
 	for (i = backwards; i; --i) {
 		/*
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index afed24abb596..b9638e19d304 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1972,14 +1972,14 @@ static int init_phys_status_page(struct intel_engine_cs *engine)
 
 void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
 {
-	GEM_BUG_ON(ringbuf->vma == NULL);
-	GEM_BUG_ON(ringbuf->virtual_start == NULL);
+	GEM_BUG_ON(!ringbuf->vma);
+	GEM_BUG_ON(!ringbuf->vaddr);
 
 	if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen)
 		i915_gem_object_unpin_map(ringbuf->obj);
 	else
 		i915_vma_unpin_iomap(ringbuf->vma);
-	ringbuf->virtual_start = NULL;
+	ringbuf->vaddr = NULL;
 
 	i915_gem_object_ggtt_unpin(ringbuf->obj);
 	ringbuf->vma = NULL;
@@ -2029,7 +2029,7 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_i915_private *dev_priv,
 		}
 	}
 
-	ringbuf->virtual_start = addr;
+	ringbuf->vaddr = addr;
 	ringbuf->vma = i915_gem_obj_to_ggtt(obj);
 	return 0;
 
@@ -2391,8 +2391,7 @@ int intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
 		GEM_BUG_ON(ringbuf->tail + remain_actual > ringbuf->size);
 
 		/* Fill the tail with MI_NOOP */
-		memset(ringbuf->virtual_start + ringbuf->tail,
-		       0, remain_actual);
+		memset(ringbuf->vaddr + ringbuf->tail, 0, remain_actual);
 		ringbuf->tail = 0;
 		ringbuf->space -= remain_actual;
 	}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d1b2d9bd89eb..05bab8bda63d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -84,7 +84,7 @@ struct intel_ring_hangcheck {
 
 struct intel_ringbuffer {
 	struct drm_i915_gem_object *obj;
-	void *virtual_start;
+	void *vaddr;
 	struct i915_vma *vma;
 
 	struct intel_engine_cs *engine;
@@ -457,7 +457,7 @@ int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req);
 static inline void __intel_ringbuffer_emit(struct intel_ringbuffer *rb,
 					   u32 data)
 {
-	*(uint32_t *)(rb->virtual_start + rb->tail) = data;
+	*(uint32_t *)(rb->vaddr + rb->tail) = data;
 	rb->tail += 4;
 }
 
-- 
2.8.1

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

[PATCH 9/9] drm/i915: Convert i915_semaphores_is_enabled over to early sanitize

[Chris Wilson]

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_drv.c         | 25 +++----------------------
 drivers/gpu/drm/i915/i915_drv.h         |  3 ++-
 drivers/gpu/drm/i915/i915_gem.c         | 23 ++++++++++++++++++++++-
 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 |  6 +++---
 7 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 618f8cf210fc..9aa62c5b5f65 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3220,7 +3220,7 @@ static int i915_semaphore_status(struct seq_file *m, void *unused)
 	enum intel_engine_id id;
 	int j, ret;
 
-	if (!i915_semaphore_is_enabled(dev_priv)) {
+	if (!i915.semaphores) {
 		seq_puts(m, "Semaphores are disabled\n");
 		return 0;
 	}
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c5b7b8e0678a..f4110bdb7b3e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -228,27 +228,6 @@ static void intel_detect_pch(struct drm_device *dev)
 	pci_dev_put(pch);
 }
 
-bool i915_semaphore_is_enabled(struct drm_i915_private *dev_priv)
-{
-	if (INTEL_GEN(dev_priv) < 6)
-		return false;
-
-	if (i915.semaphores >= 0)
-		return i915.semaphores;
-
-	/* TODO: make semaphores and Execlists play nicely together */
-	if (i915.enable_execlists)
-		return false;
-
-#ifdef CONFIG_INTEL_IOMMU
-	/* Enable semaphores on SNB when IO remapping is off */
-	if (IS_GEN6(dev_priv) && intel_iommu_gfx_mapped)
-		return false;
-#endif
-
-	return true;
-}
-
 static int i915_getparam(struct drm_device *dev, void *data,
 			 struct drm_file *file_priv)
 {
@@ -324,7 +303,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_priv);
+		value = i915.semaphores;
 		break;
 	case I915_PARAM_HAS_PRIME_VMAP_FLUSH:
 		value = 1;
@@ -999,6 +978,8 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
 	i915.enable_ppgtt =
 		intel_sanitize_enable_ppgtt(dev_priv, i915.enable_ppgtt);
 	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915.enable_ppgtt);
+
+	i915.semaphores = intel_sanitize_semaphores(dev_priv, i915.semaphores);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3ca1acec1522..0f408ada1c65 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2813,6 +2813,8 @@ extern int i915_resume_switcheroo(struct drm_device *dev);
 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,
@@ -3641,7 +3643,6 @@ extern void intel_set_rps(struct drm_i915_private *dev_priv, u8 val);
 extern void intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
 				  bool enable);
 
-extern bool i915_semaphore_is_enabled(struct drm_i915_private *dev_priv);
 int i915_reg_read_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 536acd527359..40047eb48826 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2856,7 +2856,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(to_i915(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,
@@ -4537,6 +4537,27 @@ out:
 	return ret;
 }
 
+bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
+{
+	if (INTEL_INFO(dev_priv)->gen < 6)
+		return false;
+
+	/* TODO: make semaphores and Execlists play nicely together */
+	if (i915.enable_execlists)
+		return false;
+
+	if (value >= 0)
+		return value;
+
+#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 = to_i915(dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 2600939fc2ee..bd13d084e19c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -556,7 +556,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(dev_priv) ?
+		i915.semaphores ?
 		hweight32(INTEL_INFO(dev_priv)->ring_mask) - 1 :
 		0;
 	int len, ret;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 6daaf4ecd2da..4d39c7284605 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -863,7 +863,7 @@ static void gen8_record_semaphore_state(struct drm_i915_private *dev_priv,
 	struct intel_engine_cs *to;
 	enum intel_engine_id id;
 
-	if (!i915_semaphore_is_enabled(dev_priv))
+	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 b9638e19d304..b844e6984ae7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2679,7 +2679,7 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 	struct drm_i915_gem_object *obj;
 	int ret, i;
 
-	if (!i915_semaphore_is_enabled(dev_priv))
+	if (!i915.semaphores)
 		return;
 
 	if (INTEL_GEN(dev_priv) >= 8 && !dev_priv->semaphore_obj) {
@@ -2700,7 +2700,7 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 		}
 	}
 
-	if (!i915_semaphore_is_enabled(dev_priv))
+	if (!i915.semaphores)
 		return;
 
 	if (INTEL_GEN(dev_priv) >= 8) {
@@ -2838,7 +2838,7 @@ int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
 		engine->init_context = intel_rcs_ctx_init;
 		engine->add_request = gen8_render_add_request;
 		engine->flush = gen8_render_ring_flush;
-		if (i915_semaphore_is_enabled(dev_priv))
+		if (i915.semaphores)
 			engine->semaphore.signal = gen8_rcs_signal;
 	} else if (INTEL_GEN(dev_priv) >= 6) {
 		engine->init_context = intel_rcs_ctx_init;
-- 
2.8.1

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

✗ Ro.CI.BAT: failure for series starting with [1/9] drm/i915: Rename request reference/unreference to get/put

[Patchwork]

== Series Details ==

Series: series starting with [1/9] drm/i915: Rename request reference/unreference to get/put
URL   : https://patchwork.freedesktop.org/series/10081/
State : failure

== Summary ==

Series 10081v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/10081/revisions/1/mbox

Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> FAIL       (ro-byt-n2820)

fi-hsw-i7-4770k  total:244  pass:216  dwarn:0   dfail:0   fail:8   skip:20 
fi-kbl-qkkr      total:244  pass:179  dwarn:28  dfail:1   fail:8   skip:28 
fi-skl-i5-6260u  total:244  pass:224  dwarn:0   dfail:0   fail:8   skip:12 
fi-skl-i7-6700k  total:107  pass:84   dwarn:0   dfail:0   fail:0   skip:22 
fi-snb-i7-2600   total:244  pass:196  dwarn:0   dfail:0   fail:8   skip:40 
ro-bdw-i5-5250u  total:244  pass:219  dwarn:4   dfail:0   fail:8   skip:13 
ro-bdw-i7-5600u  total:244  pass:204  dwarn:0   dfail:0   fail:8   skip:32 
ro-bsw-n3050     total:218  pass:173  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:244  pass:196  dwarn:0   dfail:0   fail:10  skip:38 
ro-hsw-i3-4010u  total:244  pass:212  dwarn:0   dfail:0   fail:8   skip:24 
ro-hsw-i7-4770r  total:244  pass:212  dwarn:0   dfail:0   fail:8   skip:24 
ro-ilk-i7-620lm  total:244  pass:172  dwarn:0   dfail:0   fail:9   skip:63 
ro-ilk1-i5-650   total:239  pass:172  dwarn:0   dfail:0   fail:9   skip:58 
ro-ivb-i7-3770   total:244  pass:203  dwarn:0   dfail:0   fail:8   skip:33 
ro-skl3-i5-6260u total:244  pass:224  dwarn:0   dfail:0   fail:8   skip:12 
ro-snb-i7-2620M  total:244  pass:193  dwarn:0   dfail:0   fail:9   skip:42 
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1539/

cafe8a2 drm-intel-nightly: 2016y-07m-20d-08h-45m-03s UTC integration manifest
571af5e drm/i915: Convert i915_semaphores_is_enabled over to early sanitize
ddfb139 drm/i915: Rename ring->virtual_start as ring->vaddr
6d0ded7 drm/i915: Treat ringbuffer writes as write to normal memory
14bc45f drm/i915: Rename drm_gem_object_unreference_unlocked in preparation for lockless free
60ff8d1 drm/i915: Rename drm_gem_object_unreference in preparation for lockless free
e870428 drm/i915: Wrap drm_gem_object_reference in i915_gem_object_get
58a9f19 drm/i915: Wrap drm_gem_object_lookup in i915_gem_object_lookup
d7e1904 drm/i915: Rename i915_gem_context_reference/unreference()
ffd6224 drm/i915: Rename request reference/unreference to get/put

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

Re: [PATCH 3/9] drm/i915: Wrap drm_gem_object_lookup in i915_gem_object_lookup

[Dave Gordon]

On 20/07/16 09:59, Chris Wilson wrote:
> For symmetry with a forthcoming i915_gem_object_get() and
> i915_gem_object_pu().
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.h        | 20 +++++++++++-
>   drivers/gpu/drm/i915/i915_gem.c        | 58 +++++++++++++++++-----------------
>   drivers/gpu/drm/i915/i915_gem_tiling.c |  8 ++---
>   drivers/gpu/drm/i915/intel_display.c   |  4 +--
>   drivers/gpu/drm/i915/intel_overlay.c   |  5 ++-
>   5 files changed, 56 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2131e7f1e57a..47f244f9c64e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2286,7 +2286,25 @@ struct drm_i915_gem_object {
>   		} userptr;
>   	};
>   };
> -#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
> +
> +static inline struct drm_i915_gem_object *
> +to_intel_bo(struct drm_gem_object *gem)
> +{
> +	/* Assert that to_intel_bo(NULL) == NULL */
> +	BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base));
> +
> +	return container_of(gem, struct drm_i915_gem_object, base);
> +}

Yes, I think this is a much better way of doing the conversion, 'cos 
it's type-safe and we get that all-important check that 'base' is still 
at offset 0.

In my variant on this, I also introduced the reverse mapping, 
to_gem_object(), so that we could completely abstract away the 
conversion back-and-forth between the two classes. Do you think that 
would be worth adding here too?

> +static inline struct drm_i915_gem_object *
> +i915_gem_object_lookup(struct drm_file *file, u32 handle)
> +{
> +	return to_intel_bo(drm_gem_object_lookup(file, handle));
> +}
> +
> +__deprecated
> +extern struct drm_gem_object *
> +drm_gem_object_lookup(struct drm_file *file, u32 handle);
>
>   static inline bool
>   i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d825db7b763d..970496419098 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -864,8 +864,8 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {

Yes, this construct was not only bizarre, but could actually cause 
problems on some compilers. See
https://lists.freedesktop.org/archives/mesa-dev/2012-April/020514.html

Just writing 'obj' is much nicer :)

> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>   		ret = -ENOENT;
>   		goto unlock;
>   	}
> @@ -1280,8 +1280,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		goto put_rpm;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>   		ret = -ENOENT;
>   		goto unlock;
>   	}
> @@ -1497,8 +1497,8 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>   		ret = -ENOENT;
>   		goto unlock;
>   	}
> @@ -1546,8 +1546,8 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>   		ret = -ENOENT;
>   		goto unlock;
>   	}
> @@ -1587,7 +1587,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>   		    struct drm_file *file)
>   {
>   	struct drm_i915_gem_mmap *args = data;
> -	struct drm_gem_object *obj;
> +	struct drm_i915_gem_object *obj;
>   	unsigned long addr;
>
>   	if (args->flags & ~(I915_MMAP_WC))
> @@ -1596,19 +1596,19 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>   	if (args->flags & I915_MMAP_WC && !boot_cpu_has(X86_FEATURE_PAT))
>   		return -ENODEV;
>
> -	obj = drm_gem_object_lookup(file, args->handle);
> -	if (obj == NULL)
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj)
>   		return -ENOENT;
>
>   	/* prime objects have no backing filp to GEM mmap
>   	 * pages from.
>   	 */
> -	if (!obj->filp) {
> -		drm_gem_object_unreference_unlocked(obj);
> +	if (!obj->base.filp) {
> +		drm_gem_object_unreference_unlocked(&obj->base);
>   		return -EINVAL;
>   	}
>
> -	addr = vm_mmap(obj->filp, 0, args->size,
> +	addr = vm_mmap(obj->base.filp, 0, args->size,
>   		       PROT_READ | PROT_WRITE, MAP_SHARED,
>   		       args->offset);
>   	if (args->flags & I915_MMAP_WC) {
> @@ -1616,7 +1616,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>   		struct vm_area_struct *vma;
>
>   		if (down_write_killable(&mm->mmap_sem)) {
> -			drm_gem_object_unreference_unlocked(obj);
> +			drm_gem_object_unreference_unlocked(&obj->base);
>   			return -EINTR;
>   		}
>   		vma = find_vma(mm, addr);
> @@ -1628,9 +1628,9 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>   		up_write(&mm->mmap_sem);
>
>   		/* This may race, but that's ok, it only gets set */
> -		WRITE_ONCE(to_intel_bo(obj)->has_wc_mmap, true);
> +		WRITE_ONCE(obj->has_wc_mmap, true);
>   	}
> -	drm_gem_object_unreference_unlocked(obj);
> +	drm_gem_object_unreference_unlocked(&obj->base);
>   	if (IS_ERR((void *)addr))
>   		return addr;
>
> @@ -1968,8 +1968,8 @@ i915_gem_mmap_gtt(struct drm_file *file,
>   	if (ret)
>   		return ret;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, handle);
> +	if (!obj) {
>   		ret = -ENOENT;
>   		goto unlock;
>   	}
> @@ -2792,8 +2792,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>   	if (ret)
>   		return ret;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->bo_handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->bo_handle);
> +	if (!obj) {
>   		mutex_unlock(&dev->struct_mutex);
>   		return -ENOENT;
>   	}
> @@ -3596,8 +3596,8 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, void *data,
>   	struct drm_i915_gem_caching *args = data;
>   	struct drm_i915_gem_object *obj;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL)
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj)
>   		return -ENOENT;
>
>   	switch (obj->cache_level) {
> @@ -3657,8 +3657,8 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		goto rpm_put;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>   		ret = -ENOENT;
>   		goto unlock;
>   	}
> @@ -4026,8 +4026,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>   		ret = -ENOENT;
>   		goto unlock;
>   	}
> @@ -4091,8 +4091,8 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		return ret;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file_priv, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file_priv, args->handle);
> +	if (!obj) {
>   		ret = -ENOENT;
>   		goto unlock;
>   	}
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index 8030199731db..46e80f30afcd 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -166,8 +166,8 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
>   	struct drm_i915_gem_object *obj;
>   	int ret = 0;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL)
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj)
>   		return -ENOENT;
>
>   	if (!i915_tiling_ok(dev,
> @@ -297,8 +297,8 @@ i915_gem_get_tiling(struct drm_device *dev, void *data,
>   	struct drm_i915_private *dev_priv = to_i915(dev);
>   	struct drm_i915_gem_object *obj;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL)
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj)
>   		return -ENOENT;
>
>   	mutex_lock(&dev->struct_mutex);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2f7af855a585..77d320584478 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15089,8 +15089,8 @@ intel_user_framebuffer_create(struct drm_device *dev,
>   	struct drm_i915_gem_object *obj;
>   	struct drm_mode_fb_cmd2 mode_cmd = *user_mode_cmd;
>
> -	obj = to_intel_bo(drm_gem_object_lookup(filp, mode_cmd.handles[0]));
> -	if (&obj->base == NULL)
> +	obj = i915_gem_object_lookup(filp, mode_cmd.handles[0]);
> +	if (!obj)
>   		return ERR_PTR(-ENOENT);
>
>   	fb = intel_framebuffer_create(dev, &mode_cmd, obj);
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 3212d8806b5a..5ca797b01ccb 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -1122,9 +1122,8 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>   	}
>   	crtc = to_intel_crtc(drmmode_crtc);
>
> -	new_bo = to_intel_bo(drm_gem_object_lookup(file_priv,
> -						   put_image_rec->bo_handle));
> -	if (&new_bo->base == NULL) {
> +	new_bo = i915_gem_object_lookup(file_priv, put_image_rec->bo_handle);
> +	if (!new_bo) {
>   		ret = -ENOENT;
>   		goto out_free;
>   	}

LGTM.

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

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

Re: [PATCH 3/9] drm/i915: Wrap drm_gem_object_lookup in i915_gem_object_lookup

[Joonas Lahtinen]

On ke, 2016-07-20 at 09:59 +0100, Chris Wilson wrote:
> For symmetry with a forthcoming i915_gem_object_get() and
> i915_gem_object_pu().
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h        | 20 +++++++++++-
>  drivers/gpu/drm/i915/i915_gem.c        | 58 +++++++++++++++++-----------------
>  drivers/gpu/drm/i915/i915_gem_tiling.c |  8 ++---
>  drivers/gpu/drm/i915/intel_display.c   |  4 +--
>  drivers/gpu/drm/i915/intel_overlay.c   |  5 ++-
>  5 files changed, 56 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2131e7f1e57a..47f244f9c64e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2286,7 +2286,25 @@ struct drm_i915_gem_object {
>  		} userptr;
>  	};
>  };
> -#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
> +
> +static inline struct drm_i915_gem_object *
> +to_intel_bo(struct drm_gem_object *gem)
> +{
> +	/* Assert that to_intel_bo(NULL) == NULL */
> +	BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base));
> +
> +	return container_of(gem, struct drm_i915_gem_object, base);
> +}
> +
> +static inline struct drm_i915_gem_object *
> +i915_gem_object_lookup(struct drm_file *file, u32 handle)
> +{
> +	return to_intel_bo(drm_gem_object_lookup(file, handle));
> +}
> +
> +__deprecated
> +extern struct drm_gem_object *

_deprecated seems to be placed just before the function name elsewhere,
for easier spotting, I guess. So lets stick to it.

Also I'm little wary about doing this deprecate another function when
we introduce a new one, especially header level, but I guess there is
no scenario when somebody would like to include i915_drv.h and use
drm_gem_object_lookup...

> +drm_gem_object_lookup(struct drm_file *file, u32 handle);
>  
>  static inline bool
>  i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d825db7b763d..970496419098 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -864,8 +864,8 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
>  	if (ret)
>  		return ret;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>  		ret = -ENOENT;
>  		goto unlock;
>  	}
> @@ -1280,8 +1280,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>  	if (ret)
>  		goto put_rpm;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>  		ret = -ENOENT;
>  		goto unlock;
>  	}
> @@ -1497,8 +1497,8 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>  	if (ret)
>  		return ret;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>  		ret = -ENOENT;
>  		goto unlock;
>  	}
> @@ -1546,8 +1546,8 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
>  	if (ret)
>  		return ret;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>  		ret = -ENOENT;
>  		goto unlock;
>  	}
> @@ -1587,7 +1587,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  		    struct drm_file *file)
>  {
>  	struct drm_i915_gem_mmap *args = data;
> -	struct drm_gem_object *obj;
> +	struct drm_i915_gem_object *obj;
>  	unsigned long addr;
>  
>  	if (args->flags & ~(I915_MMAP_WC))
> @@ -1596,19 +1596,19 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  	if (args->flags & I915_MMAP_WC && !boot_cpu_has(X86_FEATURE_PAT))
>  		return -ENODEV;
>  
> -	obj = drm_gem_object_lookup(file, args->handle);
> -	if (obj == NULL)
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj)
>  		return -ENOENT;
>  
>  	/* prime objects have no backing filp to GEM mmap
>  	 * pages from.
>  	 */
> -	if (!obj->filp) {
> -		drm_gem_object_unreference_unlocked(obj);
> +	if (!obj->base.filp) {
> +		drm_gem_object_unreference_unlocked(&obj->base);
>  		return -EINVAL;
>  	}
>  
> -	addr = vm_mmap(obj->filp, 0, args->size,
> +	addr = vm_mmap(obj->base.filp, 0, args->size,
>  		       PROT_READ | PROT_WRITE, MAP_SHARED,
>  		       args->offset);
>  	if (args->flags & I915_MMAP_WC) {
> @@ -1616,7 +1616,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  		struct vm_area_struct *vma;
>  
>  		if (down_write_killable(&mm->mmap_sem)) {
> -			drm_gem_object_unreference_unlocked(obj);
> +			drm_gem_object_unreference_unlocked(&obj->base);
>  			return -EINTR;
>  		}
>  		vma = find_vma(mm, addr);
> @@ -1628,9 +1628,9 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  		up_write(&mm->mmap_sem);
>  
>  		/* This may race, but that's ok, it only gets set */
> -		WRITE_ONCE(to_intel_bo(obj)->has_wc_mmap, true);
> +		WRITE_ONCE(obj->has_wc_mmap, true);
>  	}
> -	drm_gem_object_unreference_unlocked(obj);
> +	drm_gem_object_unreference_unlocked(&obj->base);
>  	if (IS_ERR((void *)addr))
>  		return addr;
>  
> @@ -1968,8 +1968,8 @@ i915_gem_mmap_gtt(struct drm_file *file,
>  	if (ret)
>  		return ret;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, handle);
> +	if (!obj) {
>  		ret = -ENOENT;
>  		goto unlock;
>  	}
> @@ -2792,8 +2792,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>  	if (ret)
>  		return ret;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->bo_handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->bo_handle);
> +	if (!obj) {
>  		mutex_unlock(&dev->struct_mutex);
>  		return -ENOENT;
>  	}
> @@ -3596,8 +3596,8 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, void *data,
>  	struct drm_i915_gem_caching *args = data;
>  	struct drm_i915_gem_object *obj;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL)
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj)
>  		return -ENOENT;
>  
>  	switch (obj->cache_level) {
> @@ -3657,8 +3657,8 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
>  	if (ret)
>  		goto rpm_put;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>  		ret = -ENOENT;
>  		goto unlock;
>  	}
> @@ -4026,8 +4026,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>  	if (ret)
>  		return ret;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj) {
>  		ret = -ENOENT;
>  		goto unlock;
>  	}
> @@ -4091,8 +4091,8 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
>  	if (ret)
>  		return ret;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file_priv, args->handle));
> -	if (&obj->base == NULL) {
> +	obj = i915_gem_object_lookup(file_priv, args->handle);
> +	if (!obj) {
>  		ret = -ENOENT;
>  		goto unlock;
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index 8030199731db..46e80f30afcd 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -166,8 +166,8 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
>  	struct drm_i915_gem_object *obj;
>  	int ret = 0;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL)
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj)
>  		return -ENOENT;
>  
>  	if (!i915_tiling_ok(dev,
> @@ -297,8 +297,8 @@ i915_gem_get_tiling(struct drm_device *dev, void *data,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct drm_i915_gem_object *obj;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
> -	if (&obj->base == NULL)
> +	obj = i915_gem_object_lookup(file, args->handle);
> +	if (!obj)
>  		return -ENOENT;
>  
>  	mutex_lock(&dev->struct_mutex);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2f7af855a585..77d320584478 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15089,8 +15089,8 @@ intel_user_framebuffer_create(struct drm_device *dev,
>  	struct drm_i915_gem_object *obj;
>  	struct drm_mode_fb_cmd2 mode_cmd = *user_mode_cmd;
>  
> -	obj = to_intel_bo(drm_gem_object_lookup(filp, mode_cmd.handles[0]));
> -	if (&obj->base == NULL)
> +	obj = i915_gem_object_lookup(filp, mode_cmd.handles[0]);
> +	if (!obj)
>  		return ERR_PTR(-ENOENT);
>  
>  	fb = intel_framebuffer_create(dev, &mode_cmd, obj);
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 3212d8806b5a..5ca797b01ccb 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -1122,9 +1122,8 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>  	}
>  	crtc = to_intel_crtc(drmmode_crtc);
>  
> -	new_bo = to_intel_bo(drm_gem_object_lookup(file_priv,
> -						   put_image_rec->bo_handle));
> -	if (&new_bo->base == NULL) {
> +	new_bo = i915_gem_object_lookup(file_priv, put_image_rec->bo_handle);
> +	if (!new_bo) {
>  		ret = -ENOENT;
>  		goto out_free;
>  	}

LGTM, unless I missed some non-mechanic changes.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/9] drm/i915: Wrap drm_gem_object_lookup in i915_gem_object_lookup

[Chris Wilson]

On Wed, Jul 20, 2016 at 12:28:16PM +0100, Dave Gordon wrote:
> On 20/07/16 09:59, Chris Wilson wrote:
> >For symmetry with a forthcoming i915_gem_object_get() and
> >i915_gem_object_pu().
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >---
> >  drivers/gpu/drm/i915/i915_drv.h        | 20 +++++++++++-
> >  drivers/gpu/drm/i915/i915_gem.c        | 58 +++++++++++++++++-----------------
> >  drivers/gpu/drm/i915/i915_gem_tiling.c |  8 ++---
> >  drivers/gpu/drm/i915/intel_display.c   |  4 +--
> >  drivers/gpu/drm/i915/intel_overlay.c   |  5 ++-
> >  5 files changed, 56 insertions(+), 39 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >index 2131e7f1e57a..47f244f9c64e 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -2286,7 +2286,25 @@ struct drm_i915_gem_object {
> >  		} userptr;
> >  	};
> >  };
> >-#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
> >+
> >+static inline struct drm_i915_gem_object *
> >+to_intel_bo(struct drm_gem_object *gem)
> >+{
> >+	/* Assert that to_intel_bo(NULL) == NULL */
> >+	BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base));
> >+
> >+	return container_of(gem, struct drm_i915_gem_object, base);
> >+}
> 
> Yes, I think this is a much better way of doing the conversion, 'cos
> it's type-safe and we get that all-important check that 'base' is
> still at offset 0.
> 
> In my variant on this, I also introduced the reverse mapping,
> to_gem_object(), so that we could completely abstract away the
> conversion back-and-forth between the two classes. Do you think that
> would be worth adding here too?

The convention is that the downcast is tidied away, but as the upcast is
just &foo->bar that is easy enough on the eyes. There is also the
problem of a limited namespace, so picking who should be to_foo() and
to_bar() is tricky. (Without delving into huge polymorphic macros.)
-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

Re: [PATCH 4/9] drm/i915: Wrap drm_gem_object_reference in i915_gem_object_get

[Dave Gordon]

On 20/07/16 09:59, Chris Wilson wrote:
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.h            | 11 +++++++++++
>   drivers/gpu/drm/i915/i915_gem.c            |  4 ++--
>   drivers/gpu/drm/i915/i915_gem_dmabuf.c     |  3 +--
>   drivers/gpu/drm/i915/i915_gem_evict.c      |  2 +-
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++--
>   drivers/gpu/drm/i915/i915_gem_shrinker.c   |  2 +-
>   drivers/gpu/drm/i915/i915_gem_userptr.c    |  3 +--
>   drivers/gpu/drm/i915/intel_display.c       |  3 +--
>   8 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 47f244f9c64e..163d28e8e92c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2306,6 +2306,17 @@ __deprecated
>   extern struct drm_gem_object *
>   drm_gem_object_lookup(struct drm_file *file, u32 handle);
>
> +__attribute__((nonnull))
> +static inline struct drm_i915_gem_object *
> +i915_gem_object_get(struct drm_i915_gem_object *obj)
> +{
> +	drm_gem_object_reference(&obj->base);
> +	return obj;
> +}
> +
> +__deprecated
> +extern void drm_gem_object_reference(struct drm_gem_object *);
> +
>   static inline bool
>   i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
>   {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 970496419098..f886c0bb24ca 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -289,7 +289,7 @@ drop_pages(struct drm_i915_gem_object *obj)
>   	struct i915_vma *vma, *next;
>   	int ret;
>
> -	drm_gem_object_reference(&obj->base);
> +	i915_gem_object_get(obj);
>   	list_for_each_entry_safe(vma, next, &obj->vma_list, obj_link)
>   		if (i915_vma_unbind(vma))
>   			break;
> @@ -2361,7 +2361,7 @@ void i915_vma_move_to_active(struct i915_vma *vma,
>
>   	/* Add a reference if we're newly entering the active list. */
>   	if (obj->active == 0)
> -		drm_gem_object_reference(&obj->base);
> +		i915_gem_object_get(obj);
>   	obj->active |= intel_engine_flag(engine);
>
>   	list_move_tail(&obj->engine_list[engine->id], &engine->active_list);
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index 7d08ac08c401..3a00ab3ad06e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -278,8 +278,7 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
>   			 * Importing dmabuf exported from out own gem increases
>   			 * refcount on gem itself instead of f_count of dmabuf.
>   			 */
> -			drm_gem_object_reference(&obj->base);
> -			return &obj->base;
> +			return &i915_gem_object_get(obj)->base;

This would be a good place to write

	return to_gem_object(i915_gem_object_get(obj));

if you had a to_gem_object() function :)

>   		}
>   	}
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index b1194c7c0a48..56103948ee16 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -183,7 +183,7 @@ found:
>   				       exec_list);
>   		if (drm_mm_scan_remove_block(&vma->node)) {
>   			list_move(&vma->exec_list, &eviction_list);
> -			drm_gem_object_reference(&vma->obj->base);
> +			i915_gem_object_get(vma->obj);
>   			continue;
>   		}
>   		list_del_init(&vma->exec_list);
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 551dd4f549fa..28bcc99a0cc1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -123,7 +123,7 @@ eb_lookup_vmas(struct eb_vmas *eb,
>   			goto err;
>   		}
>
> -		drm_gem_object_reference(&obj->base);
> +		i915_gem_object_get(obj);
>   		list_add_tail(&obj->obj_exec_link, &objects);
>   	}
>   	spin_unlock(&file->table_lock);
> @@ -1236,7 +1236,7 @@ i915_gem_execbuffer_parse(struct intel_engine_cs *engine,
>   	vma = i915_gem_obj_to_ggtt(shadow_batch_obj);
>   	vma->exec_entry = shadow_exec_entry;
>   	vma->exec_entry->flags = __EXEC_OBJECT_HAS_PIN;
> -	drm_gem_object_reference(&shadow_batch_obj->base);
> +	i915_gem_object_get(shadow_batch_obj);
>   	list_add_tail(&vma->exec_list, &eb->vmas);
>
>   	shadow_batch_obj->base.pending_read_domains = I915_GEM_DOMAIN_COMMAND;
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index 6f10b421487b..3aa76d3608ea 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -190,7 +190,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
>   			if (!can_release_pages(obj))
>   				continue;
>
> -			drm_gem_object_reference(&obj->base);
> +			i915_gem_object_get(obj);
>
>   			/* For the unbound phase, this should be a no-op! */
>   			list_for_each_entry_safe(vma, v,
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index ba16e044fac6..c41bf74f926e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -622,8 +622,7 @@ __i915_gem_userptr_get_pages_schedule(struct drm_i915_gem_object *obj,
>   	obj->userptr.work = &work->work;
>   	obj->userptr.workers++;
>
> -	work->obj = obj;
> -	drm_gem_object_reference(&obj->base);
> +	work->obj = i915_gem_object_get(obj);
>
>   	work->task = current;
>   	get_task_struct(work->task);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 77d320584478..84904a2d3fcd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11649,7 +11649,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>
>   	/* Reference the objects for the scheduled work. */
>   	drm_framebuffer_reference(work->old_fb);
> -	drm_gem_object_reference(&obj->base);
>
>   	crtc->primary->fb = fb;
>   	update_state_fb(crtc->primary);
> @@ -11657,7 +11656,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>   	intel_fbc_pre_update(intel_crtc, intel_crtc->config,
>   			     to_intel_plane_state(primary->state));
>
> -	work->pending_flip_obj = obj;
> +	work->pending_flip_obj = i915_gem_object_get(obj);
>
>   	ret = i915_mutex_lock_interruptible(dev);
>   	if (ret)

This last one moves the point at which the reference is taken past 
several function calls; is that intentional, or at least harmless?

With that answered, then

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

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

Re: [PATCH 5/9] drm/i915: Rename drm_gem_object_unreference in preparation for lockless free

[Joonas Lahtinen]

On ke, 2016-07-20 at 09:59 +0100, Chris Wilson wrote:

> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 605c69658d2c..75a1496ceb6f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -737,13 +737,15 @@ void intel_guc_fini(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
>  
> +	if (!guc_fw->guc_fw_obj)
> +		return;
> +

This is definitely not a mechanical change, should be split to separate
patch (maybe outside of this series too).

With that split out,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

>  	mutex_lock(&dev->struct_mutex);
>  	direct_interrupts_to_host(dev_priv);
>  	i915_guc_submission_disable(dev_priv);
>  	i915_guc_submission_fini(dev_priv);
>  
> -	if (guc_fw->guc_fw_obj)
> -		drm_gem_object_unreference(&guc_fw->guc_fw_obj->base);
> +	i915_gem_object_put(guc_fw->guc_fw_obj);
>  	guc_fw->guc_fw_obj = NULL;
>  	mutex_unlock(&dev->struct_mutex);
>  

-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/9] drm/i915: Wrap drm_gem_object_reference in i915_gem_object_get

[Joonas Lahtinen]

On ke, 2016-07-20 at 09:59 +0100, Chris Wilson wrote:
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Mechanical stuff,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h            | 11 +++++++++++
>  drivers/gpu/drm/i915/i915_gem.c            |  4 ++--
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c     |  3 +--
>  drivers/gpu/drm/i915/i915_gem_evict.c      |  2 +-
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++--
>  drivers/gpu/drm/i915/i915_gem_shrinker.c   |  2 +-
>  drivers/gpu/drm/i915/i915_gem_userptr.c    |  3 +--
>  drivers/gpu/drm/i915/intel_display.c       |  3 +--
>  8 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 47f244f9c64e..163d28e8e92c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2306,6 +2306,17 @@ __deprecated
>  extern struct drm_gem_object *
>  drm_gem_object_lookup(struct drm_file *file, u32 handle);
>  
> +__attribute__((nonnull))
> +static inline struct drm_i915_gem_object *
> +i915_gem_object_get(struct drm_i915_gem_object *obj)
> +{
> +	drm_gem_object_reference(&obj->base);
> +	return obj;
> +}
> +
> +__deprecated
> +extern void drm_gem_object_reference(struct drm_gem_object *);
> +
>  static inline bool
>  i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 970496419098..f886c0bb24ca 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -289,7 +289,7 @@ drop_pages(struct drm_i915_gem_object *obj)
>  	struct i915_vma *vma, *next;
>  	int ret;
>  
> -	drm_gem_object_reference(&obj->base);
> +	i915_gem_object_get(obj);
>  	list_for_each_entry_safe(vma, next, &obj->vma_list, obj_link)
>  		if (i915_vma_unbind(vma))
>  			break;
> @@ -2361,7 +2361,7 @@ void i915_vma_move_to_active(struct i915_vma *vma,
>  
>  	/* Add a reference if we're newly entering the active list. */
>  	if (obj->active == 0)
> -		drm_gem_object_reference(&obj->base);
> +		i915_gem_object_get(obj);
>  	obj->active |= intel_engine_flag(engine);
>  
>  	list_move_tail(&obj->engine_list[engine->id], &engine->active_list);
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index 7d08ac08c401..3a00ab3ad06e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -278,8 +278,7 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
>  			 * Importing dmabuf exported from out own gem increases
>  			 * refcount on gem itself instead of f_count of dmabuf.
>  			 */
> -			drm_gem_object_reference(&obj->base);
> -			return &obj->base;
> +			return &i915_gem_object_get(obj)->base;
>  		}
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index b1194c7c0a48..56103948ee16 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -183,7 +183,7 @@ found:
>  				       exec_list);
>  		if (drm_mm_scan_remove_block(&vma->node)) {
>  			list_move(&vma->exec_list, &eviction_list);
> -			drm_gem_object_reference(&vma->obj->base);
> +			i915_gem_object_get(vma->obj);
>  			continue;
>  		}
>  		list_del_init(&vma->exec_list);
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 551dd4f549fa..28bcc99a0cc1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -123,7 +123,7 @@ eb_lookup_vmas(struct eb_vmas *eb,
>  			goto err;
>  		}
>  
> -		drm_gem_object_reference(&obj->base);
> +		i915_gem_object_get(obj);
>  		list_add_tail(&obj->obj_exec_link, &objects);
>  	}
>  	spin_unlock(&file->table_lock);
> @@ -1236,7 +1236,7 @@ i915_gem_execbuffer_parse(struct intel_engine_cs *engine,
>  	vma = i915_gem_obj_to_ggtt(shadow_batch_obj);
>  	vma->exec_entry = shadow_exec_entry;
>  	vma->exec_entry->flags = __EXEC_OBJECT_HAS_PIN;
> -	drm_gem_object_reference(&shadow_batch_obj->base);
> +	i915_gem_object_get(shadow_batch_obj);
>  	list_add_tail(&vma->exec_list, &eb->vmas);
>  
>  	shadow_batch_obj->base.pending_read_domains = I915_GEM_DOMAIN_COMMAND;
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index 6f10b421487b..3aa76d3608ea 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -190,7 +190,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
>  			if (!can_release_pages(obj))
>  				continue;
>  
> -			drm_gem_object_reference(&obj->base);
> +			i915_gem_object_get(obj);
>  
>  			/* For the unbound phase, this should be a no-op! */
>  			list_for_each_entry_safe(vma, v,
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index ba16e044fac6..c41bf74f926e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -622,8 +622,7 @@ __i915_gem_userptr_get_pages_schedule(struct drm_i915_gem_object *obj,
>  	obj->userptr.work = &work->work;
>  	obj->userptr.workers++;
>  
> -	work->obj = obj;
> -	drm_gem_object_reference(&obj->base);
> +	work->obj = i915_gem_object_get(obj);
>  
>  	work->task = current;
>  	get_task_struct(work->task);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 77d320584478..84904a2d3fcd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11649,7 +11649,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  
>  	/* Reference the objects for the scheduled work. */
>  	drm_framebuffer_reference(work->old_fb);
> -	drm_gem_object_reference(&obj->base);
>  
>  	crtc->primary->fb = fb;
>  	update_state_fb(crtc->primary);
> @@ -11657,7 +11656,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	intel_fbc_pre_update(intel_crtc, intel_crtc->config,
>  			     to_intel_plane_state(primary->state));
>  
> -	work->pending_flip_obj = obj;
> +	work->pending_flip_obj = i915_gem_object_get(obj);
>  
>  	ret = i915_mutex_lock_interruptible(dev);
>  	if (ret)
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/9] drm/i915: Wrap drm_gem_object_reference in i915_gem_object_get

[Chris Wilson]

On Wed, Jul 20, 2016 at 12:38:24PM +0100, Dave Gordon wrote:
> On 20/07/16 09:59, Chris Wilson wrote:
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index 77d320584478..84904a2d3fcd 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -11649,7 +11649,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> >
> >  	/* Reference the objects for the scheduled work. */
> >  	drm_framebuffer_reference(work->old_fb);
> >-	drm_gem_object_reference(&obj->base);
> >
> >  	crtc->primary->fb = fb;
> >  	update_state_fb(crtc->primary);
> >@@ -11657,7 +11656,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> >  	intel_fbc_pre_update(intel_crtc, intel_crtc->config,
> >  			     to_intel_plane_state(primary->state));
> >
> >-	work->pending_flip_obj = obj;
> >+	work->pending_flip_obj = i915_gem_object_get(obj);
> >
> >  	ret = i915_mutex_lock_interruptible(dev);
> >  	if (ret)
> 
> This last one moves the point at which the reference is taken past
> several function calls; is that intentional, or at least harmless?

It is harmless. The function owns a reference to the framebuffer (which
owns the obj), what is required is an extra reference for the deferred
work item.
-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

Re: [PATCH 5/9] drm/i915: Rename drm_gem_object_unreference in preparation for lockless free

[Dave Gordon]

On 20/07/16 12:43, Joonas Lahtinen wrote:
> On ke, 2016-07-20 at 09:59 +0100, Chris Wilson wrote:
>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index 605c69658d2c..75a1496ceb6f 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -737,13 +737,15 @@ void intel_guc_fini(struct drm_device *dev)
>>   	struct drm_i915_private *dev_priv = to_i915(dev);
>>   	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
>>
>> +	if (!guc_fw->guc_fw_obj)
>> +		return;
>> +
>
> This is definitely not a mechanical change, should be split to separate
> patch (maybe outside of this series too).
>
> With that split out,
>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> Regards, Joonas
>
>>   	mutex_lock(&dev->struct_mutex);
>>   	direct_interrupts_to_host(dev_priv);
>>   	i915_guc_submission_disable(dev_priv);
>>   	i915_guc_submission_fini(dev_priv);
>>
>> -	if (guc_fw->guc_fw_obj)
>> -		drm_gem_object_unreference(&guc_fw->guc_fw_obj->base);
>> +	i915_gem_object_put(guc_fw->guc_fw_obj);
>>   	guc_fw->guc_fw_obj = NULL;
>>   	mutex_unlock(&dev->struct_mutex);

In my version of this set of transformations, I allowed the 
reference-releasing function (equivalent to i915_gem_object_put() here) 
to be called with NULL as a parameter; drm_gem_object_unreference() can 
already handle NULL, and since we now guarantee gem-obj == NULL <=> 
i915-obj == NULL, it's safe to pass it though. That allows quite a few 
simplifications in various bits of cleanup code.

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

Re: [PATCH 8/9] drm/i915: Rename ring->virtual_start as ring->vaddr

[Dave Gordon]

On 20/07/16 10:00, Chris Wilson wrote:
> Just a different colour to better match virtual addresses elsewhere.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_irq.c         |  2 +-
>   drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +++++------
>   drivers/gpu/drm/i915/intel_ringbuffer.h |  4 ++--
>   3 files changed, 8 insertions(+), 9 deletions(-)

Just mechanical transformations with no issues that I can see, so

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

> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 38d9ae90b613..0fef5cda4aca 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2899,7 +2899,7 @@ semaphore_waits_for(struct intel_engine_cs *engine, u32 *seqno)
>   	 */
>   	head = I915_READ_HEAD(engine) & HEAD_ADDR;
>   	backwards = (INTEL_GEN(dev_priv) >= 8) ? 5 : 4;
> -	vaddr = (void __iomem *) engine->buffer->virtual_start;
> +	vaddr = (void __iomem *) engine->buffer->vaddr;
>
>   	for (i = backwards; i; --i) {
>   		/*
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index afed24abb596..b9638e19d304 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1972,14 +1972,14 @@ static int init_phys_status_page(struct intel_engine_cs *engine)
>
>   void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
>   {
> -	GEM_BUG_ON(ringbuf->vma == NULL);
> -	GEM_BUG_ON(ringbuf->virtual_start == NULL);
> +	GEM_BUG_ON(!ringbuf->vma);
> +	GEM_BUG_ON(!ringbuf->vaddr);
>
>   	if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen)
>   		i915_gem_object_unpin_map(ringbuf->obj);
>   	else
>   		i915_vma_unpin_iomap(ringbuf->vma);
> -	ringbuf->virtual_start = NULL;
> +	ringbuf->vaddr = NULL;
>
>   	i915_gem_object_ggtt_unpin(ringbuf->obj);
>   	ringbuf->vma = NULL;
> @@ -2029,7 +2029,7 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_i915_private *dev_priv,
>   		}
>   	}
>
> -	ringbuf->virtual_start = addr;
> +	ringbuf->vaddr = addr;
>   	ringbuf->vma = i915_gem_obj_to_ggtt(obj);
>   	return 0;
>
> @@ -2391,8 +2391,7 @@ int intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
>   		GEM_BUG_ON(ringbuf->tail + remain_actual > ringbuf->size);
>
>   		/* Fill the tail with MI_NOOP */
> -		memset(ringbuf->virtual_start + ringbuf->tail,
> -		       0, remain_actual);
> +		memset(ringbuf->vaddr + ringbuf->tail, 0, remain_actual);
>   		ringbuf->tail = 0;
>   		ringbuf->space -= remain_actual;
>   	}
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index d1b2d9bd89eb..05bab8bda63d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -84,7 +84,7 @@ struct intel_ring_hangcheck {
>
>   struct intel_ringbuffer {
>   	struct drm_i915_gem_object *obj;
> -	void *virtual_start;
> +	void *vaddr;
>   	struct i915_vma *vma;
>
>   	struct intel_engine_cs *engine;
> @@ -457,7 +457,7 @@ int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req);
>   static inline void __intel_ringbuffer_emit(struct intel_ringbuffer *rb,
>   					   u32 data)
>   {
> -	*(uint32_t *)(rb->virtual_start + rb->tail) = data;
> +	*(uint32_t *)(rb->vaddr + rb->tail) = data;
>   	rb->tail += 4;
>   }
>
>

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

Re: [PATCH 3/9] drm/i915: Wrap drm_gem_object_lookup in i915_gem_object_lookup

[Chris Wilson]

On Wed, Jul 20, 2016 at 02:38:19PM +0300, Joonas Lahtinen wrote:
> On ke, 2016-07-20 at 09:59 +0100, Chris Wilson wrote:
> > For symmetry with a forthcoming i915_gem_object_get() and
> > i915_gem_object_pu().
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h        | 20 +++++++++++-
> >  drivers/gpu/drm/i915/i915_gem.c        | 58 +++++++++++++++++-----------------
> >  drivers/gpu/drm/i915/i915_gem_tiling.c |  8 ++---
> >  drivers/gpu/drm/i915/intel_display.c   |  4 +--
> >  drivers/gpu/drm/i915/intel_overlay.c   |  5 ++-
> >  5 files changed, 56 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 2131e7f1e57a..47f244f9c64e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2286,7 +2286,25 @@ struct drm_i915_gem_object {
> >  		} userptr;
> >  	};
> >  };
> > -#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
> > +
> > +static inline struct drm_i915_gem_object *
> > +to_intel_bo(struct drm_gem_object *gem)
> > +{
> > +	/* Assert that to_intel_bo(NULL) == NULL */
> > +	BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base));
> > +
> > +	return container_of(gem, struct drm_i915_gem_object, base);
> > +}
> > +
> > +static inline struct drm_i915_gem_object *
> > +i915_gem_object_lookup(struct drm_file *file, u32 handle)
> > +{
> > +	return to_intel_bo(drm_gem_object_lookup(file, handle));
> > +}
> > +
> > +__deprecated
> > +extern struct drm_gem_object *
> 
> _deprecated seems to be placed just before the function name elsewhere,
> for easier spotting, I guess. So lets stick to it.

I was trying to make this look exactly like the extern it was
deprecating, so I settled on putting it on the line before. I wanted it
tight against the previous function, so that I had

static inline
new_function()
{
)
__deprecates
extern old_function()

But it is not the verb and checkpatch moans about the lack of newline.
 
> Also I'm little wary about doing this deprecate another function when
> we introduce a new one, especially header level, but I guess there is
> no scenario when somebody would like to include i915_drv.h and use
> drm_gem_object_lookup...

Exactly. That would be bad and needs to be caught.
-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

Re: [PATCH 6/9] drm/i915: Rename drm_gem_object_unreference_unlocked in preparation for lockless free

[Joonas Lahtinen]

On ke, 2016-07-20 at 09:59 +0100, Chris Wilson wrote:
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

No hidden treasures in this one,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

> ---
>  drivers/gpu/drm/i915/i915_drv.h         | 10 ++++++++++
>  drivers/gpu/drm/i915/i915_gem.c         | 10 +++++-----
>  drivers/gpu/drm/i915/i915_gem_tiling.c  |  2 +-
>  drivers/gpu/drm/i915/i915_gem_userptr.c |  2 +-
>  drivers/gpu/drm/i915/intel_display.c    |  6 +++---
>  drivers/gpu/drm/i915/intel_overlay.c    |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>  7 files changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ffc97c5f9d2e..3ca1acec1522 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2327,6 +2327,16 @@ i915_gem_object_put(struct drm_i915_gem_object *obj)
>  __deprecated
>  extern void drm_gem_object_unreference(struct drm_gem_object *);
>  
> +__attribute__((nonnull))
> +static inline void
> +i915_gem_object_put_unlocked(struct drm_i915_gem_object *obj)
> +{
> +	drm_gem_object_unreference_unlocked(&obj->base);
> +}
> +
> +__deprecated
> +extern void drm_gem_object_unreference_unlocked(struct drm_gem_object *);
> +
>  static inline bool
>  i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index f23670fbc8a9..536acd527359 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -410,7 +410,7 @@ i915_gem_create(struct drm_file *file,
>  
>  	ret = drm_gem_handle_create(file, &obj->base, &handle);
>  	/* drop reference from allocate - handle holds it now */
> -	drm_gem_object_unreference_unlocked(&obj->base);
> +	i915_gem_object_put_unlocked(obj);
>  	if (ret)
>  		return ret;
>  
> @@ -1604,7 +1604,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  	 * pages from.
>  	 */
>  	if (!obj->base.filp) {
> -		drm_gem_object_unreference_unlocked(&obj->base);
> +		i915_gem_object_put_unlocked(obj);
>  		return -EINVAL;
>  	}
>  
> @@ -1616,7 +1616,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  		struct vm_area_struct *vma;
>  
>  		if (down_write_killable(&mm->mmap_sem)) {
> -			drm_gem_object_unreference_unlocked(&obj->base);
> +			i915_gem_object_put_unlocked(obj);
>  			return -EINTR;
>  		}
>  		vma = find_vma(mm, addr);
> @@ -1630,7 +1630,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  		/* This may race, but that's ok, it only gets set */
>  		WRITE_ONCE(obj->has_wc_mmap, true);
>  	}
> -	drm_gem_object_unreference_unlocked(&obj->base);
> +	i915_gem_object_put_unlocked(obj);
>  	if (IS_ERR((void *)addr))
>  		return addr;
>  
> @@ -3615,7 +3615,7 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, void *data,
>  		break;
>  	}
>  
> -	drm_gem_object_unreference_unlocked(&obj->base);
> +	i915_gem_object_put_unlocked(obj);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index 2d9d190bd6ef..e83fc2d0433c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -172,7 +172,7 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
>  
>  	if (!i915_tiling_ok(dev,
>  			    args->stride, obj->base.size, args->tiling_mode)) {
> -		drm_gem_object_unreference_unlocked(&obj->base);
> +		i915_gem_object_put_unlocked(obj);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index cd4af22b8c59..ca8b82ab93d6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -845,7 +845,7 @@ i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file
>  		ret = drm_gem_handle_create(file, &obj->base, &handle);
>  
>  	/* drop reference from allocate - handle holds it now */
> -	drm_gem_object_unreference_unlocked(&obj->base);
> +	i915_gem_object_put_unlocked(obj);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a18a79c5db1c..78beb7e9d384 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10442,7 +10442,7 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
>  
>  	fb = intel_framebuffer_create(dev, &mode_cmd, obj);
>  	if (IS_ERR(fb))
> -		drm_gem_object_unreference_unlocked(&obj->base);
> +		i915_gem_object_put_unlocked(obj);
>  
>  	return fb;
>  }
> @@ -11756,7 +11756,7 @@ cleanup:
>  	crtc->primary->fb = old_fb;
>  	update_state_fb(crtc->primary);
>  
> -	drm_gem_object_unreference_unlocked(&obj->base);
> +	i915_gem_object_put_unlocked(obj);
>  	drm_framebuffer_unreference(work->old_fb);
>  
>  	spin_lock_irq(&dev->event_lock);
> @@ -15094,7 +15094,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
>  
>  	fb = intel_framebuffer_create(dev, &mode_cmd, obj);
>  	if (IS_ERR(fb))
> -		drm_gem_object_unreference_unlocked(&obj->base);
> +		i915_gem_object_put_unlocked(obj);
>  
>  	return fb;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 30d2f5db7d87..8654a323722e 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -1460,7 +1460,7 @@ void intel_cleanup_overlay(struct drm_i915_private *dev_priv)
>  	 * hardware should be off already */
>  	WARN_ON(dev_priv->overlay->active);
>  
> -	drm_gem_object_unreference_unlocked(&dev_priv->overlay->reg_bo->base);
> +	i915_gem_object_put_unlocked(dev_priv->overlay->reg_bo);
>  	kfree(dev_priv->overlay);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4e9846e70b48..64d628c915a3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5709,7 +5709,7 @@ static void valleyview_cleanup_pctx(struct drm_i915_private *dev_priv)
>  	if (WARN_ON(!dev_priv->vlv_pctx))
>  		return;
>  
> -	drm_gem_object_unreference_unlocked(&dev_priv->vlv_pctx->base);
> +	i915_gem_object_put_unlocked(dev_priv->vlv_pctx);
>  	dev_priv->vlv_pctx = NULL;
>  }
>  
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/9] drm/i915: Rename ring->virtual_start as ring->vaddr

[Joonas Lahtinen]

On ke, 2016-07-20 at 10:00 +0100, Chris Wilson wrote:
> Just a different colour to better match virtual addresses elsewhere.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c         |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +++++------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  4 ++--
>  3 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 38d9ae90b613..0fef5cda4aca 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2899,7 +2899,7 @@ semaphore_waits_for(struct intel_engine_cs *engine, u32 *seqno)
>  	 */
>  	head = I915_READ_HEAD(engine) & HEAD_ADDR;
>  	backwards = (INTEL_GEN(dev_priv) >= 8) ? 5 : 4;
> -	vaddr = (void __iomem *) engine->buffer->virtual_start;
> +	vaddr = (void __iomem *) engine->buffer->vaddr;
>  
>  	for (i = backwards; i; --i) {
>  		/*
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index afed24abb596..b9638e19d304 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1972,14 +1972,14 @@ static int init_phys_status_page(struct intel_engine_cs *engine)
>  
>  void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
>  {
> -	GEM_BUG_ON(ringbuf->vma == NULL);
> -	GEM_BUG_ON(ringbuf->virtual_start == NULL);
> +	GEM_BUG_ON(!ringbuf->vma);
> +	GEM_BUG_ON(!ringbuf->vaddr);
>  
>  	if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen)
>  		i915_gem_object_unpin_map(ringbuf->obj);
>  	else
>  		i915_vma_unpin_iomap(ringbuf->vma);
> -	ringbuf->virtual_start = NULL;
> +	ringbuf->vaddr = NULL;
>  
>  	i915_gem_object_ggtt_unpin(ringbuf->obj);
>  	ringbuf->vma = NULL;
> @@ -2029,7 +2029,7 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_i915_private *dev_priv,
>  		}
>  	}
>  
> -	ringbuf->virtual_start = addr;
> +	ringbuf->vaddr = addr;
>  	ringbuf->vma = i915_gem_obj_to_ggtt(obj);
>  	return 0;
>  
> @@ -2391,8 +2391,7 @@ int intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
>  		GEM_BUG_ON(ringbuf->tail + remain_actual > ringbuf->size);
>  
>  		/* Fill the tail with MI_NOOP */
> -		memset(ringbuf->virtual_start + ringbuf->tail,
> -		       0, remain_actual);
> +		memset(ringbuf->vaddr + ringbuf->tail, 0, remain_actual);
>  		ringbuf->tail = 0;
>  		ringbuf->space -= remain_actual;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index d1b2d9bd89eb..05bab8bda63d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -84,7 +84,7 @@ struct intel_ring_hangcheck {
>  
>  struct intel_ringbuffer {
>  	struct drm_i915_gem_object *obj;
> -	void *virtual_start;
> +	void *vaddr;
>  	struct i915_vma *vma;
>  
>  	struct intel_engine_cs *engine;
> @@ -457,7 +457,7 @@ int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req);
>  static inline void __intel_ringbuffer_emit(struct intel_ringbuffer *rb,
>  					   u32 data)
>  {
> -	*(uint32_t *)(rb->virtual_start + rb->tail) = data;
> +	*(uint32_t *)(rb->vaddr + rb->tail) = data;
>  	rb->tail += 4;
>  }
>  
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/9] drm/i915: Rename drm_gem_object_unreference in preparation for lockless free

[Chris Wilson]

On Wed, Jul 20, 2016 at 12:51:31PM +0100, Dave Gordon wrote:
> On 20/07/16 12:43, Joonas Lahtinen wrote:
> >On ke, 2016-07-20 at 09:59 +0100, Chris Wilson wrote:
> >
> >>diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> >>index 605c69658d2c..75a1496ceb6f 100644
> >>--- a/drivers/gpu/drm/i915/intel_guc_loader.c
> >>+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> >>@@ -737,13 +737,15 @@ void intel_guc_fini(struct drm_device *dev)
> >>  	struct drm_i915_private *dev_priv = to_i915(dev);
> >>  	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> >>
> >>+	if (!guc_fw->guc_fw_obj)
> >>+		return;
> >>+
> >
> >This is definitely not a mechanical change, should be split to separate
> >patch (maybe outside of this series too).
> >
> >With that split out,
> >
> >Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >
> >Regards, Joonas
> >
> >>  	mutex_lock(&dev->struct_mutex);
> >>  	direct_interrupts_to_host(dev_priv);
> >>  	i915_guc_submission_disable(dev_priv);
> >>  	i915_guc_submission_fini(dev_priv);
> >>
> >>-	if (guc_fw->guc_fw_obj)
> >>-		drm_gem_object_unreference(&guc_fw->guc_fw_obj->base);
> >>+	i915_gem_object_put(guc_fw->guc_fw_obj);
> >>  	guc_fw->guc_fw_obj = NULL;
> >>  	mutex_unlock(&dev->struct_mutex);
> 
> In my version of this set of transformations, I allowed the
> reference-releasing function (equivalent to i915_gem_object_put()
> here) to be called with NULL as a parameter;
> drm_gem_object_unreference() can already handle NULL, and since we
> now guarantee gem-obj == NULL <=> i915-obj == NULL, it's safe to
> pass it though. That allows quite a few simplifications in various
> bits of cleanup code.

There are 3 places where this applies, 2 of which are inside the guc. It
is the guc that is where the coventions breakdown.
-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

Re: [PATCH 9/9] drm/i915: Convert i915_semaphores_is_enabled over to early sanitize

[Joonas Lahtinen]

On ke, 2016-07-20 at 10:00 +0100, 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.

Has been since 2014 at least.

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c     |  2 +-
>  drivers/gpu/drm/i915/i915_drv.c         | 25 +++----------------------
>  drivers/gpu/drm/i915/i915_drv.h         |  3 ++-
>  drivers/gpu/drm/i915/i915_gem.c         | 23 ++++++++++++++++++++++-
>  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 |  6 +++---
>  7 files changed, 33 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 618f8cf210fc..9aa62c5b5f65 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3220,7 +3220,7 @@ static int i915_semaphore_status(struct seq_file *m, void *unused)
>  	enum intel_engine_id id;
>  	int j, ret;
>  
> -	if (!i915_semaphore_is_enabled(dev_priv)) {
> +	if (!i915.semaphores) {
>  		seq_puts(m, "Semaphores are disabled\n");
>  		return 0;
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index c5b7b8e0678a..f4110bdb7b3e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -228,27 +228,6 @@ static void intel_detect_pch(struct drm_device *dev)
>  	pci_dev_put(pch);
>  }
>  
> -bool i915_semaphore_is_enabled(struct drm_i915_private *dev_priv)
> -{
> -	if (INTEL_GEN(dev_priv) < 6)
> -		return false;
> -
> -	if (i915.semaphores >= 0)
> -		return i915.semaphores;
> -
> -	/* TODO: make semaphores and Execlists play nicely together */
> -	if (i915.enable_execlists)
> -		return false;
> -
> -#ifdef CONFIG_INTEL_IOMMU
> -	/* Enable semaphores on SNB when IO remapping is off */
> -	if (IS_GEN6(dev_priv) && intel_iommu_gfx_mapped)
> -		return false;
> -#endif
> -
> -	return true;
> -}
> -
>  static int i915_getparam(struct drm_device *dev, void *data,
>  			 struct drm_file *file_priv)
>  {
> @@ -324,7 +303,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_priv);
> +		value = i915.semaphores;
>  		break;
>  	case I915_PARAM_HAS_PRIME_VMAP_FLUSH:
>  		value = 1;
> @@ -999,6 +978,8 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
>  	i915.enable_ppgtt =
>  		intel_sanitize_enable_ppgtt(dev_priv, i915.enable_ppgtt);
>  	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915.enable_ppgtt);
> +
> +	i915.semaphores = intel_sanitize_semaphores(dev_priv, i915.semaphores);

All the same to DRM_DEBUG_DRIVER too?

Otherwise looks sane/mechanical to me,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3ca1acec1522..0f408ada1c65 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2813,6 +2813,8 @@ extern int i915_resume_switcheroo(struct drm_device *dev);
>  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,
> @@ -3641,7 +3643,6 @@ extern void intel_set_rps(struct drm_i915_private *dev_priv, u8 val);
>  extern void intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
>  				  bool enable);
>  
> -extern bool i915_semaphore_is_enabled(struct drm_i915_private *dev_priv);
>  int i915_reg_read_ioctl(struct drm_device *dev, void *data,
>  			struct drm_file *file);
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 536acd527359..40047eb48826 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2856,7 +2856,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(to_i915(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,
> @@ -4537,6 +4537,27 @@ out:
>  	return ret;
>  }
>  
> +bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
> +{
> +	if (INTEL_INFO(dev_priv)->gen < 6)
> +		return false;
> +
> +	/* TODO: make semaphores and Execlists play nicely together */
> +	if (i915.enable_execlists)
> +		return false;
> +
> +	if (value >= 0)
> +		return value;
> +
> +#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 = to_i915(dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 2600939fc2ee..bd13d084e19c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -556,7 +556,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(dev_priv) ?
> +		i915.semaphores ?
>  		hweight32(INTEL_INFO(dev_priv)->ring_mask) - 1 :
>  		0;
>  	int len, ret;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 6daaf4ecd2da..4d39c7284605 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -863,7 +863,7 @@ static void gen8_record_semaphore_state(struct drm_i915_private *dev_priv,
>  	struct intel_engine_cs *to;
>  	enum intel_engine_id id;
>  
> -	if (!i915_semaphore_is_enabled(dev_priv))
> +	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 b9638e19d304..b844e6984ae7 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2679,7 +2679,7 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
>  	struct drm_i915_gem_object *obj;
>  	int ret, i;
>  
> -	if (!i915_semaphore_is_enabled(dev_priv))
> +	if (!i915.semaphores)
>  		return;
>  
>  	if (INTEL_GEN(dev_priv) >= 8 && !dev_priv->semaphore_obj) {
> @@ -2700,7 +2700,7 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
>  		}
>  	}
>  
> -	if (!i915_semaphore_is_enabled(dev_priv))
> +	if (!i915.semaphores)
>  		return;
>  
>  	if (INTEL_GEN(dev_priv) >= 8) {
> @@ -2838,7 +2838,7 @@ int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
>  		engine->init_context = intel_rcs_ctx_init;
>  		engine->add_request = gen8_render_add_request;
>  		engine->flush = gen8_render_ring_flush;
> -		if (i915_semaphore_is_enabled(dev_priv))
> +		if (i915.semaphores)
>  			engine->semaphore.signal = gen8_rcs_signal;
>  	} else if (INTEL_GEN(dev_priv) >= 6) {
>  		engine->init_context = intel_rcs_ctx_init;
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: use i915_gem_object_put_unlocked() after releasing mutex

[Dave Gordon]

The exit path in intel_overlay_put_image_ioctl() first unlocks the
struct_mutex, then drops its reference to 'new_bo' by calling
i915_gem_object_put(). As it isn't holding the mutex at this point,
this should be i915_gem_object_put_unlocked().

This was previously correct but got splatted in the recent
s/drm_gem_object_unreference/i915_gem_object_put/
where the _unlocked suffix was lost in this one case.

Also fixes a whitespace glitch introduced in the same commit.

Fixes: f8c417cd drm/i915: Rename drm_gem_object_unreference in preparation ...
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_evict.c | 3 +--
 drivers/gpu/drm/i915/intel_overlay.c  | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 81f7b43..6e09e11 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -196,8 +196,7 @@
 		vma = list_first_entry(&eviction_list,
 				       struct i915_vma,
 				       exec_list);
-
-		obj =  vma->obj;
+		obj = vma->obj;
 		list_del_init(&vma->exec_list);
 		if (ret == 0)
 			ret = i915_vma_unbind(vma);
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 8654a32..c10ce36 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1219,7 +1219,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 out_unlock:
 	mutex_unlock(&dev->struct_mutex);
 	drm_modeset_unlock_all(dev);
-	i915_gem_object_put(new_bo);
+	i915_gem_object_put_unlocked(new_bo);
 out_free:
 	kfree(params);
 
-- 
1.9.1

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

Re: [PATCH] drm/i915: use i915_gem_object_put_unlocked() after releasing mutex

[Chris Wilson]

On Thu, Jul 21, 2016 at 05:58:03PM +0100, Dave Gordon wrote:
> The exit path in intel_overlay_put_image_ioctl() first unlocks the
> struct_mutex, then drops its reference to 'new_bo' by calling
> i915_gem_object_put(). As it isn't holding the mutex at this point,
> this should be i915_gem_object_put_unlocked().
> 
> This was previously correct but got splatted in the recent
> s/drm_gem_object_unreference/i915_gem_object_put/
> where the _unlocked suffix was lost in this one case.
> 
> Also fixes a whitespace glitch introduced in the same commit.

The glitch was already there.
 
> Fixes: f8c417cd drm/i915: Rename drm_gem_object_unreference in preparation ...
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_evict.c | 3 +--
>  drivers/gpu/drm/i915/intel_overlay.c  | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index 81f7b43..6e09e11 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -196,8 +196,7 @@
>  		vma = list_first_entry(&eviction_list,
>  				       struct i915_vma,
>  				       exec_list);
> -

Don't remove this line.

> -		obj =  vma->obj;
> +		obj = vma->obj;
>  		list_del_init(&vma->exec_list);
>  		if (ret == 0)
>  			ret = i915_vma_unbind(vma);
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 8654a32..c10ce36 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -1219,7 +1219,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
>  out_unlock:
>  	mutex_unlock(&dev->struct_mutex);
>  	drm_modeset_unlock_all(dev);
> -	i915_gem_object_put(new_bo);
> +	i915_gem_object_put_unlocked(new_bo);

Otherwise,
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

✗ Ro.CI.BAT: failure for series starting with [1/9] drm/i915: Rename request reference/unreference to get/put (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/9] drm/i915: Rename request reference/unreference to get/put (rev2)
URL   : https://patchwork.freedesktop.org/series/10081/
State : failure

== Summary ==

Applying: drm/i915: Rename request reference/unreference to get/put
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_gem.c
M	drivers/gpu/drm/i915/i915_gem_request.c
M	drivers/gpu/drm/i915/i915_gem_request.h
M	drivers/gpu/drm/i915/i915_gem_userptr.c
M	drivers/gpu/drm/i915/intel_breadcrumbs.c
M	drivers/gpu/drm/i915/intel_display.c
M	drivers/gpu/drm/i915/intel_lrc.c
M	drivers/gpu/drm/i915/intel_pm.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_display.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_display.c
Auto-merging drivers/gpu/drm/i915/i915_gem_request.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_gem_request.c
error: Failed to merge in the changes.
Patch failed at 0001 drm/i915: Rename request reference/unreference to get/put
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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