| Message ID | 20180517074055.14638-11-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 17/05/2018 08:40, Chris Wilson wrote: > As we are splitting processing the CSB events from submitting the ELSP, > we also need to duplicate the check that we hold a device wakeref for our > hardware access to the disjoint locations. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_lrc.c | 26 ++++++++++++++++---------- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 4928e9ad7826..6d3b03299b0c 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -449,6 +449,16 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) > struct execlist_port *port = execlists->port; > unsigned int n; > > + /* > + * We can skip acquiring intel_runtime_pm_get() here as it was taken > + * on our behalf by the request (see i915_gem_mark_busy()) and it will > + * not be relinquished until the device is idle (see > + * i915_gem_idle_work_handler()). As a precaution, we make sure > + * that all ELSP are drained i.e. we have processed the CSB, > + * before allowing ourselves to idle and calling intel_runtime_pm_put(). > + */ > + GEM_BUG_ON(!engine->i915->gt.awake); Hmm.. I think it would be better to leave it in the tasklet and just add another instance to process_csb. Having it only deep in execlists_submit_ports could miss confusions when upper layers want to submit and state say it is not possible yet. > + > /* > * ELSQ note: the submit queue is not cleared after being submitted > * to the HW so we need to make sure we always clean it up. This is > @@ -959,6 +969,12 @@ static void process_csb(struct intel_engine_cs *engine) > struct drm_i915_private *i915 = engine->i915; > bool fw = false; > > + /* > + * We must never release our device wakeref until after we have > + * finished processing all potential interrupts from the hardware. > + */ > + GEM_BUG_ON(!engine->i915->gt.awake); > + > do { > /* The HWSP contains a (cacheable) mirror of the CSB */ > const u32 *buf = > @@ -1139,16 +1155,6 @@ static void execlists_submission_tasklet(unsigned long data) > engine->execlists.active, > test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)); > > - /* > - * We can skip acquiring intel_runtime_pm_get() here as it was taken > - * on our behalf by the request (see i915_gem_mark_busy()) and it will > - * not be relinquished until the device is idle (see > - * i915_gem_idle_work_handler()). As a precaution, we make sure > - * that all ELSP are drained i.e. we have processed the CSB, > - * before allowing ourselves to idle and calling intel_runtime_pm_put(). > - */ > - GEM_BUG_ON(!engine->i915->gt.awake); > - > /* > * Prefer doing test_and_clear_bit() as a two stage operation to avoid > * imposing the cost of a locked atomic transaction when submitting a > Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 4928e9ad7826..6d3b03299b0c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -449,6 +449,16 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) struct execlist_port *port = execlists->port; unsigned int n; + /* + * We can skip acquiring intel_runtime_pm_get() here as it was taken + * on our behalf by the request (see i915_gem_mark_busy()) and it will + * not be relinquished until the device is idle (see + * i915_gem_idle_work_handler()). As a precaution, we make sure + * that all ELSP are drained i.e. we have processed the CSB, + * before allowing ourselves to idle and calling intel_runtime_pm_put(). + */ + GEM_BUG_ON(!engine->i915->gt.awake); + /* * ELSQ note: the submit queue is not cleared after being submitted * to the HW so we need to make sure we always clean it up. This is @@ -959,6 +969,12 @@ static void process_csb(struct intel_engine_cs *engine) struct drm_i915_private *i915 = engine->i915; bool fw = false; + /* + * We must never release our device wakeref until after we have + * finished processing all potential interrupts from the hardware. + */ + GEM_BUG_ON(!engine->i915->gt.awake); + do { /* The HWSP contains a (cacheable) mirror of the CSB */ const u32 *buf = @@ -1139,16 +1155,6 @@ static void execlists_submission_tasklet(unsigned long data) engine->execlists.active, test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)); - /* - * We can skip acquiring intel_runtime_pm_get() here as it was taken - * on our behalf by the request (see i915_gem_mark_busy()) and it will - * not be relinquished until the device is idle (see - * i915_gem_idle_work_handler()). As a precaution, we make sure - * that all ELSP are drained i.e. we have processed the CSB, - * before allowing ourselves to idle and calling intel_runtime_pm_put(). - */ - GEM_BUG_ON(!engine->i915->gt.awake); - /* * Prefer doing test_and_clear_bit() as a two stage operation to avoid * imposing the cost of a locked atomic transaction when submitting a
As we are splitting processing the CSB events from submitting the ELSP, we also need to duplicate the check that we hold a device wakeref for our hardware access to the disjoint locations. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_lrc.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-)