| Message ID | 20171031152734.11016-2-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Mika Kuoppala (2017-10-31 15:27:34) > From: Mika Kuoppala <mika.kuoppala@intel.com> > > As all our access to execlist ports are through head and tail > helpers, we can now move the head instead of memmoving the array. > > v2: use memset (Chris) > > Cc: Michał Winiarski <michal.winiarski@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Still waiting for patch 3 to kill the memset ;) -Chris
Quoting Mika Kuoppala (2017-10-31 15:27:34) > From: Mika Kuoppala <mika.kuoppala@intel.com> > > As all our access to execlist ports are through head and tail > helpers, we can now move the head instead of memmoving the array. > > v2: use memset (Chris) > > Cc: Michał Winiarski <michal.winiarski@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_ringbuffer.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 387667fe50d3..011c4b8f1339 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -611,13 +611,13 @@ static inline struct execlist_port * > execlists_head_complete(struct intel_engine_execlists * const execlists, > struct execlist_port * const port) > { > - const unsigned int m = execlists->port_mask; > - > - GEM_BUG_ON(port_index(port, execlists) != 0); > + GEM_BUG_ON(port_index(port, execlists) != execlists->port_head); > + GEM_BUG_ON(!port_isset(port)); > GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_USER)); > > - memmove(port, port + 1, m * sizeof(struct execlist_port)); > - memset(port + m, 0, sizeof(struct execlist_port)); > + memset(port, 0, sizeof(*port)); > + > + execlists->port_head = port_head_add(execlists, 1); Ok, I would have gone for port = port_next(port); execlists->port_head = port - execlists->port; return port; That to me looks more natural advance of port as we complete the requests, and matches the loop in the irq handler. Care to crunch the numbers and see which gcc favours? -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2017-10-31 15:27:34) >> From: Mika Kuoppala <mika.kuoppala@intel.com> >> >> As all our access to execlist ports are through head and tail >> helpers, we can now move the head instead of memmoving the array. >> >> v2: use memset (Chris) >> >> Cc: Michał Winiarski <michal.winiarski@intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_ringbuffer.h | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h >> index 387667fe50d3..011c4b8f1339 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h >> @@ -611,13 +611,13 @@ static inline struct execlist_port * >> execlists_head_complete(struct intel_engine_execlists * const execlists, >> struct execlist_port * const port) >> { >> - const unsigned int m = execlists->port_mask; >> - >> - GEM_BUG_ON(port_index(port, execlists) != 0); >> + GEM_BUG_ON(port_index(port, execlists) != execlists->port_head); >> + GEM_BUG_ON(!port_isset(port)); >> GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_USER)); >> >> - memmove(port, port + 1, m * sizeof(struct execlist_port)); >> - memset(port + m, 0, sizeof(struct execlist_port)); >> + memset(port, 0, sizeof(*port)); >> + >> + execlists->port_head = port_head_add(execlists, 1); > > Ok, I would have gone for > > port = port_next(port); > execlists->port_head = port - execlists->port; > return port; > > That to me looks more natural advance of port as we complete the > requests, and matches the loop in the irq handler. > > Care to crunch the numbers and see which gcc favours? gcc (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406: add/remove: 0/0 grow/shrink: 3/0 up/down: 219/0 (219) function old new delta execlists_submission_tasklet 2405 2525 +120 execlists_cancel_port_requests 315 376 +61 guc_submission_tasklet 1643 1681 +38 Total: Before=1168854, After=1169073, chg +0.02% gcc (Ubuntu 7.2.0-8ubuntu3) 7.2.0: add/remove: 0/0 grow/shrink: 3/0 up/down: 158/0 (158) function old new delta execlists_submission_tasklet 2383 2460 +77 execlists_cancel_port_requests 344 401 +57 guc_submission_tasklet 1684 1708 +24 Total: Before=1164662, After=1164820, chg +0.01% where new: execlists_head_complete(struct intel_engine_execlists * const execlists, - struct execlist_port * const port) + struct execlist_port *port) { GEM_BUG_ON(port_index(port, execlists) != execlists->port_head); GEM_BUG_ON(!port_isset(port)); @@ -677,9 +677,10 @@ execlists_head_complete(struct intel_engine_execlists * const execlists, memset(port, 0, sizeof(*port)); - execlists->port_head = port_head_add(execlists, 1); + port = execlists_port_next(execlists, port); + execlists->port_head = port - execlists->port; - return execlists_port_head(execlists); + return port; -Mika
Quoting Mika Kuoppala (2017-11-22 13:52:09) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Quoting Mika Kuoppala (2017-10-31 15:27:34) > >> From: Mika Kuoppala <mika.kuoppala@intel.com> > >> > >> As all our access to execlist ports are through head and tail > >> helpers, we can now move the head instead of memmoving the array. > >> > >> v2: use memset (Chris) > >> > >> Cc: Michał Winiarski <michal.winiarski@intel.com> > >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > >> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_ringbuffer.h | 10 +++++----- > >> 1 file changed, 5 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > >> index 387667fe50d3..011c4b8f1339 100644 > >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > >> @@ -611,13 +611,13 @@ static inline struct execlist_port * > >> execlists_head_complete(struct intel_engine_execlists * const execlists, > >> struct execlist_port * const port) > >> { > >> - const unsigned int m = execlists->port_mask; > >> - > >> - GEM_BUG_ON(port_index(port, execlists) != 0); > >> + GEM_BUG_ON(port_index(port, execlists) != execlists->port_head); > >> + GEM_BUG_ON(!port_isset(port)); > >> GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_USER)); > >> > >> - memmove(port, port + 1, m * sizeof(struct execlist_port)); > >> - memset(port + m, 0, sizeof(struct execlist_port)); > >> + memset(port, 0, sizeof(*port)); > >> + > >> + execlists->port_head = port_head_add(execlists, 1); > > > > Ok, I would have gone for > > > > port = port_next(port); > > execlists->port_head = port - execlists->port; > > return port; > > > > That to me looks more natural advance of port as we complete the > > requests, and matches the loop in the irq handler. > > > > Care to crunch the numbers and see which gcc favours? > > gcc (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406: > > add/remove: 0/0 grow/shrink: 3/0 up/down: 219/0 (219) > function old new delta > execlists_submission_tasklet 2405 2525 +120 > execlists_cancel_port_requests 315 376 +61 > guc_submission_tasklet 1643 1681 +38 > Total: Before=1168854, After=1169073, chg +0.02% > > gcc (Ubuntu 7.2.0-8ubuntu3) 7.2.0: > > add/remove: 0/0 grow/shrink: 3/0 up/down: 158/0 (158) > function old new delta > execlists_submission_tasklet 2383 2460 +77 > execlists_cancel_port_requests 344 401 +57 > guc_submission_tasklet 1684 1708 +24 > Total: Before=1164662, After=1164820, chg +0.01% Ok, have a Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 387667fe50d3..011c4b8f1339 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -611,13 +611,13 @@ static inline struct execlist_port * execlists_head_complete(struct intel_engine_execlists * const execlists, struct execlist_port * const port) { - const unsigned int m = execlists->port_mask; - - GEM_BUG_ON(port_index(port, execlists) != 0); + GEM_BUG_ON(port_index(port, execlists) != execlists->port_head); + GEM_BUG_ON(!port_isset(port)); GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_USER)); - memmove(port, port + 1, m * sizeof(struct execlist_port)); - memset(port + m, 0, sizeof(struct execlist_port)); + memset(port, 0, sizeof(*port)); + + execlists->port_head = port_head_add(execlists, 1); return execlists_port_head(execlists); }