diff mbox

[56/59] drm/i915: Remove 'faked' request from LRC submission

Message ID 1426768264-16996-57-git-send-email-John.C.Harrison@Intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

John Harrison March 19, 2015, 12:31 p.m. UTC
From: John Harrison <John.C.Harrison@Intel.com>

The LRC submission code requires a request for tracking purposes. It does not
actually require that request to 'complete' it simply uses it for keeping hold
of reference counts on contexts and such like.

Previously, the fall back path of polling for space in the ring would start by
submitting any outstanding work that was sat in the buffer. This submission was
not done as part of the request that that work was owned by because that would
lead to complications with the request being submitted twice. Instead, a null
request structure was passed in to the submit call and a fake one was created.

That fall back path has long since been obsoleted and has now been removed. Thus
there is never any need to fake up a request structure. This patch removes that
code. A couple of sanity check warnings are added as well, just in case.

For: VIZ-5115
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c |   22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

Comments

Thomas Daniel March 19, 2015, 3:02 p.m. UTC | #1
> -----Original Message-----

> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of

> John.C.Harrison@Intel.com

> Sent: Thursday, March 19, 2015 12:31 PM

> To: Intel-GFX@Lists.FreeDesktop.Org

> Subject: [Intel-gfx] [PATCH 56/59] drm/i915: Remove 'faked' request from LRC

> submission

> 

> From: John Harrison <John.C.Harrison@Intel.com>

> 

> The LRC submission code requires a request for tracking purposes. It does not

> actually require that request to 'complete' it simply uses it for keeping hold

> of reference counts on contexts and such like.

> 

> Previously, the fall back path of polling for space in the ring would start by

> submitting any outstanding work that was sat in the buffer. This submission was

> not done as part of the request that that work was owned by because that

> would

> lead to complications with the request being submitted twice. Instead, a null

> request structure was passed in to the submit call and a fake one was created.

> 

> That fall back path has long since been obsoleted and has now been removed.

> Thus

> there is never any need to fake up a request structure. This patch removes that

> code. A couple of sanity check warnings are added as well, just in case.

> 

> For: VIZ-5115

> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>

> ---

>  drivers/gpu/drm/i915/intel_lrc.c |   22 +++++-----------------

>  1 file changed, 5 insertions(+), 17 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c

> index f21f449..82190ad 100644

> --- a/drivers/gpu/drm/i915/intel_lrc.c

> +++ b/drivers/gpu/drm/i915/intel_lrc.c

> @@ -507,23 +507,11 @@ static int execlists_context_queue(struct

> intel_engine_cs *ring,

>  	if (to != ring->default_context)

>  		intel_lr_context_pin(ring, to);

> 

> -	if (!request) {

> -		/*

> -		 * If there isn't a request associated with this submission,

> -		 * create one as a temporary holder.

> -		 */

> -		request = kzalloc(sizeof(*request), GFP_KERNEL);

> -		if (request == NULL)

> -			return -ENOMEM;

> -		request->ring = ring;

> -		request->ctx = to;

> -		kref_init(&request->ref);

> -		request->uniq = dev_priv->request_uniq++;

> -		i915_gem_context_reference(request->ctx);

> -	} else {

> -		i915_gem_request_reference(request);

> -		WARN_ON(to != request->ctx);

> -	}

> +	WARN_ON(!request);

> +	WARN_ON(to != request->ctx);

> +

> +	i915_gem_request_reference(request);

> +

>  	request->tail = tail;

> 

>  	intel_runtime_pm_get(dev_priv);

> --

> 1.7.9.5

I need this to fix a leak in a forthcoming patch I have.  Thanks!
Reviewed-by: Thomas Daniel <thomas.daniel@intel.com>
Tomas Elf March 31, 2015, 6:14 p.m. UTC | #2
On 19/03/2015 12:31, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> The LRC submission code requires a request for tracking purposes. It does not
> actually require that request to 'complete' it simply uses it for keeping hold
> of reference counts on contexts and such like.
>
> Previously, the fall back path of polling for space in the ring would start by
> submitting any outstanding work that was sat in the buffer. This submission was
> not done as part of the request that that work was owned by because that would
> lead to complications with the request being submitted twice. Instead, a null
> request structure was passed in to the submit call and a fake one was created.
>
> That fall back path has long since been obsoleted and has now been removed. Thus
> there is never any need to fake up a request structure. This patch removes that
> code. A couple of sanity check warnings are added as well, just in case.
>
> For: VIZ-5115
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c |   22 +++++-----------------
>   1 file changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index f21f449..82190ad 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -507,23 +507,11 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
>   	if (to != ring->default_context)
>   		intel_lr_context_pin(ring, to);
>
> -	if (!request) {
> -		/*
> -		 * If there isn't a request associated with this submission,
> -		 * create one as a temporary holder.
> -		 */
> -		request = kzalloc(sizeof(*request), GFP_KERNEL);
> -		if (request == NULL)
> -			return -ENOMEM;
> -		request->ring = ring;
> -		request->ctx = to;
> -		kref_init(&request->ref);
> -		request->uniq = dev_priv->request_uniq++;
> -		i915_gem_context_reference(request->ctx);
> -	} else {
> -		i915_gem_request_reference(request);
> -		WARN_ON(to != request->ctx);
> -	}
> +	WARN_ON(!request);
> +	WARN_ON(to != request->ctx);
> +
> +	i915_gem_request_reference(request);
> +
>   	request->tail = tail;
>
>   	intel_runtime_pm_get(dev_priv);
>

Reviewed-by: Tomas Elf <tomas.elf@intel.com>

Thanks,
Tomas
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f21f449..82190ad 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -507,23 +507,11 @@  static int execlists_context_queue(struct intel_engine_cs *ring,
 	if (to != ring->default_context)
 		intel_lr_context_pin(ring, to);
 
-	if (!request) {
-		/*
-		 * If there isn't a request associated with this submission,
-		 * create one as a temporary holder.
-		 */
-		request = kzalloc(sizeof(*request), GFP_KERNEL);
-		if (request == NULL)
-			return -ENOMEM;
-		request->ring = ring;
-		request->ctx = to;
-		kref_init(&request->ref);
-		request->uniq = dev_priv->request_uniq++;
-		i915_gem_context_reference(request->ctx);
-	} else {
-		i915_gem_request_reference(request);
-		WARN_ON(to != request->ctx);
-	}
+	WARN_ON(!request);
+	WARN_ON(to != request->ctx);
+
+	i915_gem_request_reference(request);
+
 	request->tail = tail;
 
 	intel_runtime_pm_get(dev_priv);