Re: [Intel-gfx] [PATCH 7/8] drm/i915: Keep track of reserved execlist ports

[Chris Wilson]

Quoting Mika Kuoppala (2017-09-20 15:37:04)
> To further enchance port processing, keep track of
> reserved ports. This way we can iterate only the used subset
> of port space. Note that we lift the responsibility of
> execlists_submit_request() to inspect hw availability and
> always do dequeuing. This is to ensure that only the irq
> handler will be responsible for keeping track of available ports.
> 
> v2: rebase, comment fix, READ_ONCE only outside of irq handler (Chris)
> 
> Cc: Chris Wilson 
> Cc: Michał Winiarski 
> Signed-off-by: Mika Kuoppala 

Ok, doesn't look hideous. I need to look at it with a clear head, but for
now, could you check scripts/bloat-o-meter for my usual quick guide on
how much gcc likes it?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 7/8] drm/i915: Keep track of reserved execlist ports

[Mika Kuoppala]

Mika Kuoppala  writes:

> To further enchance port processing, keep track of
> reserved ports. This way we can iterate only the used subset
> of port space. Note that we lift the responsibility of
> execlists_submit_request() to inspect hw availability and

s/execlist_submit_request/insert_request.

-Mika

> always do dequeuing. This is to ensure that only the irq
> handler will be responsible for keeping track of available ports.
>
> v2: rebase, comment fix, READ_ONCE only outside of irq handler (Chris)
>
> Cc: Chris Wilson 
> Cc: Michał Winiarski 
> Signed-off-by: Mika Kuoppala 
> ---
>  drivers/gpu/drm/i915/i915_guc_submission.c | 51 +
>  drivers/gpu/drm/i915/i915_irq.c|  2 +-
>  drivers/gpu/drm/i915/intel_engine_cs.c |  7 ++-
>  drivers/gpu/drm/i915/intel_lrc.c   | 90 
> ++
>  drivers/gpu/drm/i915/intel_ringbuffer.h| 55 +-
>  5 files changed, 129 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 25c9bac94c39..359f57a59cba 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -487,7 +487,7 @@ static void guc_ring_doorbell(struct i915_guc_client 
> *client)
>   * @engine: engine associated with the commands
>   *
>   * The only error here arises if the doorbell hardware isn't functioning
> - * as expected, which really shouln't happen.
> + * as expected, which really shouldn't happen.
>   */
>  static void i915_guc_submit(struct intel_engine_cs *engine)
>  {
> @@ -495,17 +495,19 @@ static void i915_guc_submit(struct intel_engine_cs 
> *engine)
>   struct intel_guc *guc = _priv->guc;
>   struct i915_guc_client *client = guc->execbuf_client;
>   struct intel_engine_execlist * const el = >execlist;
> - struct execlist_port *port = el->port;
>   const unsigned int engine_id = engine->id;
>   unsigned int n;
>  
> - for (n = 0; n < ARRAY_SIZE(el->port); n++) {
> + for (n = 0; n < execlist_active_ports(el); n++) {
> + struct execlist_port *port;
>   struct drm_i915_gem_request *rq;
>   unsigned int count;
>  
> - rq = port_unpack([n], );
> + port = execlist_port_index(el, n);
> +
> + rq = port_unpack(port, );
>   if (rq && count == 0) {
> - port_set([n], port_pack(rq, ++count));
> + port_set(port, port_pack(rq, ++count));
>  
>   if (i915_vma_is_map_and_fenceable(rq->ring->vma))
>   POSTING_READ_FW(GUC_STATUS);
> @@ -560,25 +562,27 @@ static void port_assign(struct execlist_port *port,
>  static void i915_guc_dequeue(struct intel_engine_cs *engine)
>  {
>   struct intel_engine_execlist * const el = >execlist;
> - struct execlist_port *port = el->port;
> + struct execlist_port *port;
>   struct drm_i915_gem_request *last = NULL;
> - const struct execlist_port * const last_port = execlist_port_tail(el);
>   bool submit = false;
>   struct rb_node *rb;
>  
> - if (port_isset(port))
> - port++;
> -
>   spin_lock_irq(>timeline->lock);
>   rb = el->first;
>   GEM_BUG_ON(rb_first(>queue) != rb);
> - while (rb) {
> +
> + if (unlikely(!rb))
> + goto done;
> +
> + port = execlist_request_port(el);
> +
> + do {
>   struct i915_priolist *p = rb_entry(rb, typeof(*p), node);
>   struct drm_i915_gem_request *rq, *rn;
>  
>   list_for_each_entry_safe(rq, rn, >requests, priotree.link) {
>   if (last && rq->ctx != last->ctx) {
> - if (port == last_port) {
> + if (!execlist_inactive_ports(el)) {
>   __list_del_many(>requests,
>   >priotree.link);
>   goto done;
> @@ -587,7 +591,8 @@ static void i915_guc_dequeue(struct intel_engine_cs 
> *engine)
>   if (submit)
>   port_assign(port, last);
>  
> - port = execlist_port_next(el, port);
> + port = execlist_request_port(el);
> + GEM_BUG_ON(port_isset(port));
>   }
>  
>   INIT_LIST_HEAD(>priotree.link);
> @@ -604,7 +609,7 @@ static void i915_guc_dequeue(struct intel_engine_cs 
> *engine)
>   INIT_LIST_HEAD(>requests);
>   if (p->priority != I915_PRIORITY_NORMAL)
>   kmem_cache_free(engine->i915->priorities, p);
> - }
> + } while (rb);
>  done:
>   el->first = rb;
>   if 

[Intel-gfx] [PATCH 7/8] drm/i915: Keep track of reserved execlist ports

[Mika Kuoppala]

To further enchance port processing, keep track of
reserved ports. This way we can iterate only the used subset
of port space. Note that we lift the responsibility of
execlists_submit_request() to inspect hw availability and
always do dequeuing. This is to ensure that only the irq
handler will be responsible for keeping track of available ports.

v2: rebase, comment fix, READ_ONCE only outside of irq handler (Chris)

Cc: Chris Wilson 
Cc: Michał Winiarski 
Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 51 +
 drivers/gpu/drm/i915/i915_irq.c|  2 +-
 drivers/gpu/drm/i915/intel_engine_cs.c |  7 ++-
 drivers/gpu/drm/i915/intel_lrc.c   | 90 ++
 drivers/gpu/drm/i915/intel_ringbuffer.h| 55 +-
 5 files changed, 129 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 25c9bac94c39..359f57a59cba 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -487,7 +487,7 @@ static void guc_ring_doorbell(struct i915_guc_client 
*client)
  * @engine: engine associated with the commands
  *
  * The only error here arises if the doorbell hardware isn't functioning
- * as expected, which really shouln't happen.
+ * as expected, which really shouldn't happen.
  */
 static void i915_guc_submit(struct intel_engine_cs *engine)
 {
@@ -495,17 +495,19 @@ static void i915_guc_submit(struct intel_engine_cs 
*engine)
struct intel_guc *guc = _priv->guc;
struct i915_guc_client *client = guc->execbuf_client;
struct intel_engine_execlist * const el = >execlist;
-   struct execlist_port *port = el->port;
const unsigned int engine_id = engine->id;
unsigned int n;
 
-   for (n = 0; n < ARRAY_SIZE(el->port); n++) {
+   for (n = 0; n < execlist_active_ports(el); n++) {
+   struct execlist_port *port;
struct drm_i915_gem_request *rq;
unsigned int count;
 
-   rq = port_unpack([n], );
+   port = execlist_port_index(el, n);
+
+   rq = port_unpack(port, );
if (rq && count == 0) {
-   port_set([n], port_pack(rq, ++count));
+   port_set(port, port_pack(rq, ++count));
 
if (i915_vma_is_map_and_fenceable(rq->ring->vma))
POSTING_READ_FW(GUC_STATUS);
@@ -560,25 +562,27 @@ static void port_assign(struct execlist_port *port,
 static void i915_guc_dequeue(struct intel_engine_cs *engine)
 {
struct intel_engine_execlist * const el = >execlist;
-   struct execlist_port *port = el->port;
+   struct execlist_port *port;
struct drm_i915_gem_request *last = NULL;
-   const struct execlist_port * const last_port = execlist_port_tail(el);
bool submit = false;
struct rb_node *rb;
 
-   if (port_isset(port))
-   port++;
-
spin_lock_irq(>timeline->lock);
rb = el->first;
GEM_BUG_ON(rb_first(>queue) != rb);
-   while (rb) {
+
+   if (unlikely(!rb))
+   goto done;
+
+   port = execlist_request_port(el);
+
+   do {
struct i915_priolist *p = rb_entry(rb, typeof(*p), node);
struct drm_i915_gem_request *rq, *rn;
 
list_for_each_entry_safe(rq, rn, >requests, priotree.link) {
if (last && rq->ctx != last->ctx) {
-   if (port == last_port) {
+   if (!execlist_inactive_ports(el)) {
__list_del_many(>requests,
>priotree.link);
goto done;
@@ -587,7 +591,8 @@ static void i915_guc_dequeue(struct intel_engine_cs *engine)
if (submit)
port_assign(port, last);
 
-   port = execlist_port_next(el, port);
+   port = execlist_request_port(el);
+   GEM_BUG_ON(port_isset(port));
}
 
INIT_LIST_HEAD(>priotree.link);
@@ -604,7 +609,7 @@ static void i915_guc_dequeue(struct intel_engine_cs *engine)
INIT_LIST_HEAD(>requests);
if (p->priority != I915_PRIORITY_NORMAL)
kmem_cache_free(engine->i915->priorities, p);
-   }
+   } while (rb);
 done:
el->first = rb;
if (submit) {
@@ -618,21 +623,21 @@ static void i915_guc_irq_handler(unsigned long data)
 {
struct intel_engine_cs * const engine = (struct intel_engine_cs *)data;
struct intel_engine_execlist * const el =