diff mbox series

drm/i915: extract common ce->pin_count check

Message ID 20181001204656.34804-1-daniele.ceraolospurio@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: extract common ce->pin_count check | expand

Commit Message

Daniele Ceraolo Spurio Oct. 1, 2018, 8:46 p.m. UTC 
We already have it coded 3 times and a 4th one is coming for the GuC
path in an upcoming patch, so let's move it to a common place.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.h      |  8 +++++++-
 drivers/gpu/drm/i915/intel_lrc.c             | 10 ++--------
 drivers/gpu/drm/i915/intel_ringbuffer.c      | 10 ++--------
 drivers/gpu/drm/i915/intel_ringbuffer.h      |  3 ++-
 drivers/gpu/drm/i915/selftests/mock_engine.c | 13 +++++--------
 5 files changed, 18 insertions(+), 26 deletions(-)

Comments

Chris Wilson Oct. 1, 2018, 9:09 p.m. UTC | #1 
Quoting Daniele Ceraolo Spurio (2018-10-01 21:46:56)
> We already have it coded 3 times and a 4th one is coming for the GuC
> path in an upcoming patch, so let's move it to a common place.

Nope. It is separate as virtual engine does not follow the same pattern.
-Chris
Daniele Ceraolo Spurio Oct. 1, 2018, 9:34 p.m. UTC | #2 
On 01/10/18 14:09, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2018-10-01 21:46:56)
>> We already have it coded 3 times and a 4th one is coming for the GuC
>> path in an upcoming patch, so let's move it to a common place.
> 
> Nope. It is separate as virtual engine does not follow the same pattern.
> -Chris
> 

Would it be worth adding virtual engine knowledge to to_intel_context()?

e.g.:

static inline struct intel_context *
to_intel_context(struct i915_gem_context *ctx,
		 const struct intel_engine_cs *engine)
{
	if (intel_engine_is_virtual(engine)) {
		struct virtual_engine *ve = to_virtual_engine(engine);
		return &ve->context;
	} else {
		return &ctx->__engine[engine->id];
	}
}

Maybe a bit overkill, but having the same check repeated in 5 versions 
of *_context_pin() really bugs me :P

Daniele
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 08165f6a0a84..5309004f8f8a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -302,7 +302,13 @@  to_intel_context(struct i915_gem_context *ctx,
 static inline struct intel_context *
 intel_context_pin(struct i915_gem_context *ctx, struct intel_engine_cs *engine)
 {
-	return engine->context_pin(engine, ctx);
+	struct intel_context *ce = to_intel_context(ctx, engine);
+
+	if (likely(ce->pin_count++))
+		return ce;
+	GEM_BUG_ON(!ce->pin_count); /* no overflow please! */
+
+	return engine->context_pin(engine, ctx, ce);
 }
 
 static inline void __intel_context_pin(struct intel_context *ce)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4ee00f531153..a97f455361f6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1422,19 +1422,13 @@  static const struct intel_context_ops execlists_context_ops = {
 
 static struct intel_context *
 execlists_context_pin(struct intel_engine_cs *engine,
-		      struct i915_gem_context *ctx)
+		      struct i915_gem_context *ctx,
+		      struct intel_context *ce)
 {
-	struct intel_context *ce = to_intel_context(ctx, engine);
-
 	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
 	GEM_BUG_ON(!ctx->ppgtt);
 
-	if (likely(ce->pin_count++))
-		return ce;
-	GEM_BUG_ON(!ce->pin_count); /* no overflow please! */
-
 	ce->ops = &execlists_context_ops;
-
 	return __execlists_context_pin(engine, ctx, ce);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c092d5099ebf..a82564c50496 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1386,18 +1386,12 @@  static const struct intel_context_ops ring_context_ops = {
 
 static struct intel_context *
 intel_ring_context_pin(struct intel_engine_cs *engine,
-		       struct i915_gem_context *ctx)
+		       struct i915_gem_context *ctx,
+		       struct intel_context *ce)
 {
-	struct intel_context *ce = to_intel_context(ctx, engine);
-
 	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
 
-	if (likely(ce->pin_count++))
-		return ce;
-	GEM_BUG_ON(!ce->pin_count); /* no overflow please! */
-
 	ce->ops = &ring_context_ops;
-
 	return __ring_context_pin(engine, ctx, ce);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 1534de5bb852..91973bd9003c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -473,7 +473,8 @@  struct intel_engine_cs {
 	void		(*set_default_submission)(struct intel_engine_cs *engine);
 
 	struct intel_context *(*context_pin)(struct intel_engine_cs *engine,
-					     struct i915_gem_context *ctx);
+					     struct i915_gem_context *ctx,
+					     struct intel_context *ce);
 
 	int		(*request_alloc)(struct i915_request *rq);
 	int		(*init_context)(struct i915_request *rq);
diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
index 22a73da45ad5..4d5103634a2b 100644
--- a/drivers/gpu/drm/i915/selftests/mock_engine.c
+++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
@@ -89,15 +89,12 @@  static const struct intel_context_ops mock_context_ops = {
 
 static struct intel_context *
 mock_context_pin(struct intel_engine_cs *engine,
-		 struct i915_gem_context *ctx)
+		 struct i915_gem_context *ctx,
+		 struct intel_context *ce)
 {
-	struct intel_context *ce = to_intel_context(ctx, engine);
-
-	if (!ce->pin_count++) {
-		i915_gem_context_get(ctx);
-		ce->ring = engine->buffer;
-		ce->ops = &mock_context_ops;
-	}
+	i915_gem_context_get(ctx);
+	ce->ring = engine->buffer;
+	ce->ops = &mock_context_ops;
 
 	return ce;
 }