| Message ID | 20171207222434.17686-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 07, 2017 at 10:24:34PM +0000, Chris Wilson wrote: > Currently on every submission, we recalculate the ELSP register offset > for the engine, after chasing the pointers to find the iomem base. Since > this is fixed for the lifetime of the driver record the offset in the > execlists struct. > > In practice the difference is negligible, it just happens to remove 27 > bytes of eyesore pointer dancing from next to the hottest instruction > (which is itself due to stalling for a cache miss) in perf profiles of > the execlists_submission_tasklet(). > > v2: Trim off one more elsp local. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Reviewed-by: Michel Thierry <michel.thierry@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> would this be useful somehow on error state? > --- > drivers/gpu/drm/i915/intel_lrc.c | 13 ++++++------- > drivers/gpu/drm/i915/intel_ringbuffer.h | 5 +++++ > 2 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 2a8160f603ab..2e38fbfdf08f 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -431,8 +431,6 @@ static inline void elsp_write(u64 desc, u32 __iomem *elsp) > static void execlists_submit_ports(struct intel_engine_cs *engine) > { > struct execlist_port *port = engine->execlists.port; > - u32 __iomem *elsp = > - engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine)); > unsigned int n; > > for (n = execlists_num_ports(&engine->execlists); n--; ) { > @@ -458,7 +456,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) > desc = 0; > } > > - elsp_write(desc, elsp); > + elsp_write(desc, engine->execlists.elsp); > } > execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK); > } > @@ -496,8 +494,6 @@ static void inject_preempt_context(struct intel_engine_cs *engine) > { > struct intel_context *ce = > &engine->i915->preempt_context->engine[engine->id]; > - u32 __iomem *elsp = > - engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine)); > unsigned int n; > > GEM_BUG_ON(engine->i915->preempt_context->hw_id != PREEMPT_ID); > @@ -510,9 +506,9 @@ static void inject_preempt_context(struct intel_engine_cs *engine) > > GEM_TRACE("\n"); > for (n = execlists_num_ports(&engine->execlists); --n; ) > - elsp_write(0, elsp); > + elsp_write(0, engine->execlists.elsp); > > - elsp_write(ce->lrc_desc, elsp); > + elsp_write(ce->lrc_desc, engine->execlists.elsp); > execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK); > } > > @@ -1509,6 +1505,9 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine) > execlists->csb_head = -1; > execlists->active = 0; > > + execlists->elsp = > + dev_priv->regs + i915_mmio_reg_offset(RING_ELSP(engine)); > + > /* After a GPU reset, we may have requests to replay */ > if (execlists->first) > tasklet_schedule(&execlists->tasklet); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index c68ab3ead83c..183165b9b3fb 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -199,6 +199,11 @@ struct intel_engine_execlists { > */ > bool no_priolist; > > + /** > + * @elsp: the ExecList Submission Port register > + */ > + u32 __iomem *elsp; > + > /** > * @port: execlist port states > * > -- > 2.15.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Rodrigo Vivi (2017-12-07 23:04:13) > On Thu, Dec 07, 2017 at 10:24:34PM +0000, Chris Wilson wrote: > > Currently on every submission, we recalculate the ELSP register offset > > for the engine, after chasing the pointers to find the iomem base. Since > > this is fixed for the lifetime of the driver record the offset in the > > execlists struct. > > > > In practice the difference is negligible, it just happens to remove 27 > > bytes of eyesore pointer dancing from next to the hottest instruction > > (which is itself due to stalling for a cache miss) in perf profiles of > > the execlists_submission_tasklet(). > > > > v2: Trim off one more elsp local. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Reviewed-by: Michel Thierry <michel.thierry@intel.com> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > would this be useful somehow on error state? Nope, write-only register. We do how what we think is in ELSP, i.e. what we last wrote to ELSP. -Chris
Quoting Rodrigo Vivi (2017-12-07 23:04:13) > On Thu, Dec 07, 2017 at 10:24:34PM +0000, Chris Wilson wrote: > > Currently on every submission, we recalculate the ELSP register offset > > for the engine, after chasing the pointers to find the iomem base. Since > > this is fixed for the lifetime of the driver record the offset in the > > execlists struct. > > > > In practice the difference is negligible, it just happens to remove 27 > > bytes of eyesore pointer dancing from next to the hottest instruction > > (which is itself due to stalling for a cache miss) in perf profiles of > > the execlists_submission_tasklet(). > > > > v2: Trim off one more elsp local. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Reviewed-by: Michel Thierry <michel.thierry@intel.com> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Thanks for the reviews, applied. -Chris
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 2a8160f603ab..2e38fbfdf08f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -431,8 +431,6 @@ static inline void elsp_write(u64 desc, u32 __iomem *elsp) static void execlists_submit_ports(struct intel_engine_cs *engine) { struct execlist_port *port = engine->execlists.port; - u32 __iomem *elsp = - engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine)); unsigned int n; for (n = execlists_num_ports(&engine->execlists); n--; ) { @@ -458,7 +456,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) desc = 0; } - elsp_write(desc, elsp); + elsp_write(desc, engine->execlists.elsp); } execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK); } @@ -496,8 +494,6 @@ static void inject_preempt_context(struct intel_engine_cs *engine) { struct intel_context *ce = &engine->i915->preempt_context->engine[engine->id]; - u32 __iomem *elsp = - engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine)); unsigned int n; GEM_BUG_ON(engine->i915->preempt_context->hw_id != PREEMPT_ID); @@ -510,9 +506,9 @@ static void inject_preempt_context(struct intel_engine_cs *engine) GEM_TRACE("\n"); for (n = execlists_num_ports(&engine->execlists); --n; ) - elsp_write(0, elsp); + elsp_write(0, engine->execlists.elsp); - elsp_write(ce->lrc_desc, elsp); + elsp_write(ce->lrc_desc, engine->execlists.elsp); execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK); } @@ -1509,6 +1505,9 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine) execlists->csb_head = -1; execlists->active = 0; + execlists->elsp = + dev_priv->regs + i915_mmio_reg_offset(RING_ELSP(engine)); + /* After a GPU reset, we may have requests to replay */ if (execlists->first) tasklet_schedule(&execlists->tasklet); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index c68ab3ead83c..183165b9b3fb 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -199,6 +199,11 @@ struct intel_engine_execlists { */ bool no_priolist; + /** + * @elsp: the ExecList Submission Port register + */ + u32 __iomem *elsp; + /** * @port: execlist port states *