Re: [Intel-gfx] [PATCH 7/9] drm/i915/execlists: Reduce lock context between schedule/submit_request

2017-05-05 Thread Chris Wilson 
On Fri, May 05, 2017 at 02:30:08PM +0100, Tvrtko Ursulin wrote:
> 
> On 03/05/2017 12:37, Chris Wilson wrote:
> >If we do not require to perform priority bumping, and we haven't yet
> >submitted the request, we can update its priority in situ and skip
> >acquiring the engine locks -- thus avoiding any contention between us
> >and submit/execute.
> >
> >Signed-off-by: Chris Wilson 
> >---
> > drivers/gpu/drm/i915/intel_lrc.c | 11 +++
> > 1 file changed, 11 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> >b/drivers/gpu/drm/i915/intel_lrc.c
> >index fb0025627676..ca7f28795e2d 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -767,6 +767,17 @@ static void execlists_schedule(struct 
> >drm_i915_gem_request *request, int prio)
> > list_safe_reset_next(dep, p, dfs_link);
> > }
> >
> >+/* If we didn't need to bump any existing priorites, and we haven't
> 
> priorities
> 
> >+ * yet submitted this request (i..e there is no porential race with
> 
> potential
> 
> >+ * execlists_submit_request()), we can set our own priority and skip
> >+ * acquiring the engine locks.
> >+ */
> >+if (request->priotree.priority == INT_MIN) {
> >+request->priotree.priority = prio;
> >+if (stack.dfs_link.next == stack.dfs_link.prev)
> >+return;
> 
> Move the assignment of the priority under the if?

The assignment always work. I just liked the look of this code more :)
The skip of the assignment is minor benefit. For bonus points, could do
a list_del_entry(_link) after the return.

Sold.
-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: [Intel-gfx] [PATCH 7/9] drm/i915/execlists: Reduce lock context between schedule/submit_request

2017-05-05 Thread Tvrtko Ursulin 


On 03/05/2017 12:37, Chris Wilson wrote:

If we do not require to perform priority bumping, and we haven't yet
submitted the request, we can update its priority in situ and skip
acquiring the engine locks -- thus avoiding any contention between us
and submit/execute.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_lrc.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index fb0025627676..ca7f28795e2d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -767,6 +767,17 @@ static void execlists_schedule(struct drm_i915_gem_request 
*request, int prio)
list_safe_reset_next(dep, p, dfs_link);
}

+   /* If we didn't need to bump any existing priorites, and we haven't


priorities


+* yet submitted this request (i..e there is no porential race with


potential


+* execlists_submit_request()), we can set our own priority and skip
+* acquiring the engine locks.
+*/
+   if (request->priotree.priority == INT_MIN) {
+   request->priotree.priority = prio;
+   if (stack.dfs_link.next == stack.dfs_link.prev)
+   return;


Move the assignment of the priority under the if?


+   }
+
engine = request->engine;
spin_lock_irq(>timeline->lock);




Regards,

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

Re: [Intel-gfx] [PATCH 7/9] drm/i915/execlists: Reduce lock context between schedule/submit_request

2017-05-05 Thread Chris Wilson 
s/context/contention/ in subject

On Wed, May 03, 2017 at 12:37:57PM +0100, Chris Wilson wrote:
> If we do not require to perform priority bumping, and we haven't yet
> submitted the request, we can update its priority in situ and skip
> acquiring the engine locks -- thus avoiding any contention between us
> and submit/execute.
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index fb0025627676..ca7f28795e2d 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -767,6 +767,17 @@ static void execlists_schedule(struct 
> drm_i915_gem_request *request, int prio)
>   list_safe_reset_next(dep, p, dfs_link);
>   }
>  
> + /* If we didn't need to bump any existing priorites, and we haven't
> +  * yet submitted this request (i..e there is no porential race with
> +  * execlists_submit_request()), we can set our own priority and skip
> +  * acquiring the engine locks.
> +  */
> + if (request->priotree.priority == INT_MIN) {
> + request->priotree.priority = prio;
> + if (stack.dfs_link.next == stack.dfs_link.prev)
> + return;
> + }
> +
>   engine = request->engine;
>   spin_lock_irq(>timeline->lock);
>  
> -- 
> 2.11.0
> 

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 7/9] drm/i915/execlists: Reduce lock context between schedule/submit_request

2017-05-03 Thread Chris Wilson 
If we do not require to perform priority bumping, and we haven't yet
submitted the request, we can update its priority in situ and skip
acquiring the engine locks -- thus avoiding any contention between us
and submit/execute.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_lrc.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index fb0025627676..ca7f28795e2d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -767,6 +767,17 @@ static void execlists_schedule(struct drm_i915_gem_request 
*request, int prio)
list_safe_reset_next(dep, p, dfs_link);
}
 
+   /* If we didn't need to bump any existing priorites, and we haven't
+* yet submitted this request (i..e there is no porential race with
+* execlists_submit_request()), we can set our own priority and skip
+* acquiring the engine locks.
+*/
+   if (request->priotree.priority == INT_MIN) {
+   request->priotree.priority = prio;
+   if (stack.dfs_link.next == stack.dfs_link.prev)
+   return;
+   }
+
engine = request->engine;
spin_lock_irq(>timeline->lock);
 
-- 
2.11.0

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