Re: [Intel-gfx] [PATCH 3/8] drm/i915: Wrap port cancellation into a function

2017-09-21 Thread Mika Kuoppala 
Michał Winiarski  writes:

> On Wed, Sep 20, 2017 at 05:37:00PM +0300, Mika Kuoppala wrote:
>> On reset and wedged path, we want to release the requests
>> that are tied to ports and then mark the ports to be unset.
>> Introduce a function for this.
>> 
>> v2: rebase
>> 
>> Cc: Chris Wilson 
>> Signed-off-by: Mika Kuoppala 
>> ---
>>  drivers/gpu/drm/i915/intel_lrc.c | 21 -
>>  1 file changed, 12 insertions(+), 9 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
>> b/drivers/gpu/drm/i915/intel_lrc.c
>> index a4ece4c4f291..ffb9c900328b 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -568,6 +568,16 @@ static void execlists_dequeue(struct intel_engine_cs 
>> *engine)
>>  execlists_submit_ports(engine);
>>  }
>>  
>> +static void execlist_cancel_port_requests(struct intel_engine_execlist *el)
>> +{
>> +unsigned int i;
>> +
>> +for (i = 0; i < ARRAY_SIZE(el->port); i++)
>> +i915_gem_request_put(port_request(&el->port[i]));
>> +
>> +memset(el->port, 0, sizeof(el->port));
>> +}
>> +
>>  static void execlists_cancel_requests(struct intel_engine_cs *engine)
>>  {
>>  struct intel_engine_execlist * const el = &engine->execlist;
>> @@ -575,14 +585,11 @@ static void execlists_cancel_requests(struct 
>> intel_engine_cs *engine)
>>  struct drm_i915_gem_request *rq, *rn;
>>  struct rb_node *rb;
>>  unsigned long flags;
>> -unsigned long n;
>>  
>>  spin_lock_irqsave(&engine->timeline->lock, flags);
>>  
>>  /* Cancel the requests on the HW and clear the ELSP tracker. */
>> -for (n = 0; n < ARRAY_SIZE(el->port); n++)
>> -i915_gem_request_put(port_request(&port[n]));
>
> We could also drop the local variable for port.

Dropped.

> It's only used in GEM_BUG_ON(port_isset(&port[0])).
> Do we even need this assert when we're starting to treat ports in a more
> ring-like fashion?
>

The memset is, still, so close there in this version that it indeed
begs the question.

But it is there to ensure that we really did the port parts
properly.

-Mika

> Reviewed-by: Michał Winiarski 
>
> -Michał
>
>> -memset(el->port, 0, sizeof(el->port));
>> +execlist_cancel_port_requests(el);
>>  
>>  /* Mark all executing requests as skipped. */
>>  list_for_each_entry(rq, &engine->timeline->requests, link) {
>> @@ -1372,11 +1379,9 @@ static void reset_common_ring(struct intel_engine_cs 
>> *engine,
>>struct drm_i915_gem_request *request)
>>  {
>>  struct intel_engine_execlist * const el = &engine->execlist;
>> -struct execlist_port *port = el->port;
>>  struct drm_i915_gem_request *rq, *rn;
>>  struct intel_context *ce;
>>  unsigned long flags;
>> -unsigned int n;
>>  
>>  spin_lock_irqsave(&engine->timeline->lock, flags);
>>  
>> @@ -1389,9 +1394,7 @@ static void reset_common_ring(struct intel_engine_cs 
>> *engine,
>>   * guessing the missed context-switch events by looking at what
>>   * requests were completed.
>>   */
>> -for (n = 0; n < ARRAY_SIZE(el->port); n++)
>> -i915_gem_request_put(port_request(&port[n]));
>> -memset(el->port, 0, sizeof(el->port));
>> +execlist_cancel_port_requests(el);
>>  
>>  /* Push back any incomplete requests for replay after the reset. */
>>  list_for_each_entry_safe_reverse(rq, rn,
>> -- 
>> 2.11.0
>> 
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/8] drm/i915: Wrap port cancellation into a function

2017-09-21 Thread Chris Wilson 
Quoting Mika Kuoppala (2017-09-20 15:37:00)
> On reset and wedged path, we want to release the requests
> that are tied to ports and then mark the ports to be unset.
> Introduce a function for this.
> 
> v2: rebase
> 
> Cc: Chris Wilson 
> Signed-off-by: Mika Kuoppala 
Reviewed-by: Chris Wilson 
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/8] drm/i915: Wrap port cancellation into a function

2017-09-21 Thread Michał Winiarski 
On Wed, Sep 20, 2017 at 05:37:00PM +0300, Mika Kuoppala wrote:
> On reset and wedged path, we want to release the requests
> that are tied to ports and then mark the ports to be unset.
> Introduce a function for this.
> 
> v2: rebase
> 
> Cc: Chris Wilson 
> Signed-off-by: Mika Kuoppala 
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index a4ece4c4f291..ffb9c900328b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -568,6 +568,16 @@ static void execlists_dequeue(struct intel_engine_cs 
> *engine)
>   execlists_submit_ports(engine);
>  }
>  
> +static void execlist_cancel_port_requests(struct intel_engine_execlist *el)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(el->port); i++)
> + i915_gem_request_put(port_request(&el->port[i]));
> +
> + memset(el->port, 0, sizeof(el->port));
> +}
> +
>  static void execlists_cancel_requests(struct intel_engine_cs *engine)
>  {
>   struct intel_engine_execlist * const el = &engine->execlist;
> @@ -575,14 +585,11 @@ static void execlists_cancel_requests(struct 
> intel_engine_cs *engine)
>   struct drm_i915_gem_request *rq, *rn;
>   struct rb_node *rb;
>   unsigned long flags;
> - unsigned long n;
>  
>   spin_lock_irqsave(&engine->timeline->lock, flags);
>  
>   /* Cancel the requests on the HW and clear the ELSP tracker. */
> - for (n = 0; n < ARRAY_SIZE(el->port); n++)
> - i915_gem_request_put(port_request(&port[n]));

We could also drop the local variable for port.
It's only used in GEM_BUG_ON(port_isset(&port[0])).
Do we even need this assert when we're starting to treat ports in a more
ring-like fashion?

Reviewed-by: Michał Winiarski 

-Michał

> - memset(el->port, 0, sizeof(el->port));
> + execlist_cancel_port_requests(el);
>  
>   /* Mark all executing requests as skipped. */
>   list_for_each_entry(rq, &engine->timeline->requests, link) {
> @@ -1372,11 +1379,9 @@ static void reset_common_ring(struct intel_engine_cs 
> *engine,
> struct drm_i915_gem_request *request)
>  {
>   struct intel_engine_execlist * const el = &engine->execlist;
> - struct execlist_port *port = el->port;
>   struct drm_i915_gem_request *rq, *rn;
>   struct intel_context *ce;
>   unsigned long flags;
> - unsigned int n;
>  
>   spin_lock_irqsave(&engine->timeline->lock, flags);
>  
> @@ -1389,9 +1394,7 @@ static void reset_common_ring(struct intel_engine_cs 
> *engine,
>* guessing the missed context-switch events by looking at what
>* requests were completed.
>*/
> - for (n = 0; n < ARRAY_SIZE(el->port); n++)
> - i915_gem_request_put(port_request(&port[n]));
> - memset(el->port, 0, sizeof(el->port));
> + execlist_cancel_port_requests(el);
>  
>   /* Push back any incomplete requests for replay after the reset. */
>   list_for_each_entry_safe_reverse(rq, rn,
> -- 
> 2.11.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/8] drm/i915: Wrap port cancellation into a function

2017-09-20 Thread Mika Kuoppala 
On reset and wedged path, we want to release the requests
that are tied to ports and then mark the ports to be unset.
Introduce a function for this.

v2: rebase

Cc: Chris Wilson 
Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_lrc.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index a4ece4c4f291..ffb9c900328b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -568,6 +568,16 @@ static void execlists_dequeue(struct intel_engine_cs 
*engine)
execlists_submit_ports(engine);
 }
 
+static void execlist_cancel_port_requests(struct intel_engine_execlist *el)
+{
+   unsigned int i;
+
+   for (i = 0; i < ARRAY_SIZE(el->port); i++)
+   i915_gem_request_put(port_request(&el->port[i]));
+
+   memset(el->port, 0, sizeof(el->port));
+}
+
 static void execlists_cancel_requests(struct intel_engine_cs *engine)
 {
struct intel_engine_execlist * const el = &engine->execlist;
@@ -575,14 +585,11 @@ static void execlists_cancel_requests(struct 
intel_engine_cs *engine)
struct drm_i915_gem_request *rq, *rn;
struct rb_node *rb;
unsigned long flags;
-   unsigned long n;
 
spin_lock_irqsave(&engine->timeline->lock, flags);
 
/* Cancel the requests on the HW and clear the ELSP tracker. */
-   for (n = 0; n < ARRAY_SIZE(el->port); n++)
-   i915_gem_request_put(port_request(&port[n]));
-   memset(el->port, 0, sizeof(el->port));
+   execlist_cancel_port_requests(el);
 
/* Mark all executing requests as skipped. */
list_for_each_entry(rq, &engine->timeline->requests, link) {
@@ -1372,11 +1379,9 @@ static void reset_common_ring(struct intel_engine_cs 
*engine,
  struct drm_i915_gem_request *request)
 {
struct intel_engine_execlist * const el = &engine->execlist;
-   struct execlist_port *port = el->port;
struct drm_i915_gem_request *rq, *rn;
struct intel_context *ce;
unsigned long flags;
-   unsigned int n;
 
spin_lock_irqsave(&engine->timeline->lock, flags);
 
@@ -1389,9 +1394,7 @@ static void reset_common_ring(struct intel_engine_cs 
*engine,
 * guessing the missed context-switch events by looking at what
 * requests were completed.
 */
-   for (n = 0; n < ARRAY_SIZE(el->port); n++)
-   i915_gem_request_put(port_request(&port[n]));
-   memset(el->port, 0, sizeof(el->port));
+   execlist_cancel_port_requests(el);
 
/* Push back any incomplete requests for replay after the reset. */
list_for_each_entry_safe_reverse(rq, rn,
-- 
2.11.0

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