Re: [PATCH 12/16] drm/i915: Swap if(enable_execlists) in i915_gem_request_alloc for a vfunc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 09/12/2016 15:25, Chris Wilson wrote:
On Fri, Dec 09, 2016 at 03:16:33PM +0000, Tvrtko Ursulin wrote:

On 07/12/2016 13:58, Chris Wilson wrote:
A fairly trivial move of a matching pair of routines (for preparing a
request for construction) onto an engine vfunc. The ulterior motive is
to be able to create a mock request implementation.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem_request.c | 5 +----
drivers/gpu/drm/i915/intel_lrc.c        | 4 +++-
drivers/gpu/drm/i915/intel_lrc.h        | 2 --
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +++-
drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +--
5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 06e9a607d934..881bed5347fb 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -622,10 +622,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
	req->reserved_space = MIN_SPACE_FOR_ADD_REQUEST;
	GEM_BUG_ON(req->reserved_space < engine->emit_breadcrumb_sz);

-	if (i915.enable_execlists)
-		ret = intel_logical_ring_alloc_request_extras(req);
-	else
-		ret = intel_ring_alloc_request_extras(req);
+	ret = engine->request_alloc(req);
	if (ret)
		goto err_ctx;

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 22ded92d0346..3f536ef37968 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -845,7 +845,7 @@ static void execlists_context_unpin(struct intel_engine_cs *engine,
	i915_gem_context_put(ctx);
}

-int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request)
+static int execlists_request_alloc(struct drm_i915_gem_request *request)
{
	struct intel_engine_cs *engine = request->engine;
	struct intel_context *ce = &request->ctx->engine[engine->id];
@@ -1816,6 +1816,8 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
	engine->context_pin = execlists_context_pin;
	engine->context_unpin = execlists_context_unpin;

+	engine->request_alloc = execlists_request_alloc;
+
	engine->emit_flush = gen8_emit_flush;
	engine->emit_breadcrumb = gen8_emit_breadcrumb;
	engine->emit_breadcrumb_sz = gen8_emit_breadcrumb_sz;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index b5630331086a..01ba36ea125e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -63,8 +63,6 @@ enum {
};

/* Logical Rings */
-int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request);
-int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
void intel_logical_ring_stop(struct intel_engine_cs *engine);
void intel_logical_ring_cleanup(struct intel_engine_cs *engine);
int logical_render_ring_init(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a57eb5dec991..a42b9bf45b42 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2102,7 +2102,7 @@ void intel_legacy_submission_resume(struct drm_i915_private *dev_priv)
	}
}

-int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
+static int ring_request_alloc(struct drm_i915_gem_request *request)
{
	int ret;

@@ -2597,6 +2597,8 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
	engine->context_pin = intel_ring_context_pin;
	engine->context_unpin = intel_ring_context_unpin;

+	engine->request_alloc = ring_request_alloc;
+
	engine->emit_breadcrumb = i9xx_emit_breadcrumb;
	engine->emit_breadcrumb_sz = i9xx_emit_breadcrumb_sz;
	if (i915.semaphores) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 0b69a50ab833..12f4dd1cbf9c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -272,6 +272,7 @@ struct intel_engine_cs {
				       struct i915_gem_context *ctx);
	void		(*context_unpin)(struct intel_engine_cs *engine,
					 struct i915_gem_context *ctx);
+	int		(*request_alloc)(struct drm_i915_gem_request *req);
	int		(*init_context)(struct drm_i915_gem_request *req);

	int		(*emit_flush)(struct drm_i915_gem_request *request,
@@ -476,8 +477,6 @@ void intel_engine_cleanup(struct intel_engine_cs *engine);

void intel_legacy_submission_resume(struct drm_i915_private *dev_priv);

-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);



In dev_priv->gt and not per engine would be better. I think we
talked about this in some other context as well.

Blast, the same then applies to engine->context_(un)pin. :)

I've toyed with the idea of splitting context_pin for RCS/xCS on legacy
since only RCS has a context, and before Ironlake we have no contexts at
all.

->request_alloc() could specialise itself per engine, in theory at
least, don't know if we ever would want to do so in practice.

What do you think?

Quite happy to pencil the idea in for later under sorting out a new split
for GT vs engine [vs scheduler] ;)

Yeah OK, fine by me.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux