Re: [PATCH 19/22] drm/i915/execlists: Refactor out can_merge_rq()

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 04/02/2019 13:22, Chris Wilson wrote:
> In the next patch, we add another user that wants to check whether
> requests can be merge into a single HW execution, and in the future we
> want to add more conditions under which requests from the same context
> cannot be merge. In preparation, extract out can_merge_rq().
> 
> v2: Reorder tests to decide if we can continue filling ELSP and bonus
> comments.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 35 ++++++++++++++++++++++----------
>   1 file changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index e37f207afb5a..66d465708bc6 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -285,12 +285,11 @@ static inline bool need_preempt(const struct intel_engine_cs *engine,
>   }
>   
>   __maybe_unused static inline bool
> -assert_priority_queue(const struct intel_engine_execlists *execlists,
> -		      const struct i915_request *prev,
> +assert_priority_queue(const struct i915_request *prev,
>   		      const struct i915_request *next)
>   {
> -	if (!prev)
> -		return true;
> +	const struct intel_engine_execlists *execlists =
> +		&prev->engine->execlists;
>   
>   	/*
>   	 * Without preemption, the prev may refer to the still active element
> @@ -601,6 +600,17 @@ static bool can_merge_ctx(const struct intel_context *prev,
>   	return true;
>   }
>   
> +static bool can_merge_rq(const struct i915_request *prev,
> +			 const struct i915_request *next)
> +{
> +	GEM_BUG_ON(!assert_priority_queue(prev, next));
> +
> +	if (!can_merge_ctx(prev->hw_context, next->hw_context))
> +		return false;
> +
> +	return true;
> +}
> +
>   static void port_assign(struct execlist_port *port, struct i915_request *rq)
>   {
>   	GEM_BUG_ON(rq == port_request(port));
> @@ -753,8 +763,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>   		int i;
>   
>   		priolist_for_each_request_consume(rq, rn, p, i) {
> -			GEM_BUG_ON(!assert_priority_queue(execlists, last, rq));
> -
>   			/*
>   			 * Can we combine this request with the current port?
>   			 * It has to be the same context/ringbuffer and not
> @@ -766,8 +774,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>   			 * second request, and so we never need to tell the
>   			 * hardware about the first.
>   			 */
> -			if (last &&
> -			    !can_merge_ctx(rq->hw_context, last->hw_context)) {
> +			if (last && !can_merge_rq(last, rq)) {
>   				/*
>   				 * If we are on the second port and cannot
>   				 * combine this request with the last, then we
> @@ -776,6 +783,14 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>   				if (port == last_port)
>   					goto done;
>   
> +				/*
> +				 * We must not populate both ELSP[] with the
> +				 * same LRCA, i.e. we must submit 2 different
> +				 * contexts if we submit 2 ELSP.
> +				 */
> +				if (last->hw_context == rq->hw_context)
> +					goto done;
> +
>   				/*
>   				 * If GVT overrides us we only ever submit
>   				 * port[0], leaving port[1] empty. Note that we
> @@ -787,7 +802,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>   				    ctx_single_port_submission(rq->hw_context))
>   					goto done;
>   
> -				GEM_BUG_ON(last->hw_context == rq->hw_context);
>   
>   				if (submit)
>   					port_assign(port, last);
> @@ -826,8 +840,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>   	 * request triggering preemption on the next dequeue (or subsequent
>   	 * interrupt for secondary ports).
>   	 */
> -	execlists->queue_priority_hint =
> -		port != execlists->port ? rq_prio(last) : INT_MIN;
> +	execlists->queue_priority_hint = queue_prio(execlists);
>   
>   	if (submit) {
>   		port_assign(port, last);
> 

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

Regards,

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