| Message ID | 20180213163738.9055-7-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/02/18 08:37, Mika Kuoppala wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Since it is not possible to mask individual engine instances > and they are all permanently unmasked we do not need to do > anything for engine interrupt management. > > v2: Rebase. > v3: Remove gen 11 extra check in logical_render_ring_init. > v4: Rebase fixes. > v5: Rebase/refactor. > v6: Rebase. > v7: Rebase. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > --- > drivers/gpu/drm/i915/intel_breadcrumbs.c | 20 ++++++++++++-------- > drivers/gpu/drm/i915/intel_lrc.c | 11 +++++++++-- > drivers/gpu/drm/i915/intel_ringbuffer.h | 5 +++++ > 3 files changed, 26 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c > index b955f7d7bd0f..69c727f97eb5 100644 > --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c > +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c > @@ -167,18 +167,22 @@ static void irq_enable(struct intel_engine_cs *engine) > */ > set_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted); > > - /* Caller disables interrupts */ > - spin_lock(&engine->i915->irq_lock); > - engine->irq_enable(engine); > - spin_unlock(&engine->i915->irq_lock); > + if (engine->irq_enable) { > + /* Caller disables interrupts */ > + spin_lock(&engine->i915->irq_lock); > + engine->irq_enable(engine); > + spin_unlock(&engine->i915->irq_lock); > + } > } > > static void irq_disable(struct intel_engine_cs *engine) > { > - /* Caller disables interrupts */ > - spin_lock(&engine->i915->irq_lock); > - engine->irq_disable(engine); > - spin_unlock(&engine->i915->irq_lock); > + if (engine->irq_disable) { > + /* Caller disables interrupts */ > + spin_lock(&engine->i915->irq_lock); > + engine->irq_disable(engine); > + spin_unlock(&engine->i915->irq_lock); > + } > } > > void __intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine) > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index c2c8380a0121..da396c46b345 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1989,8 +1989,15 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine) > > engine->set_default_submission = execlists_set_default_submission; > > - engine->irq_enable = gen8_logical_ring_enable_irq; > - engine->irq_disable = gen8_logical_ring_disable_irq; > + if (INTEL_GEN(engine->i915) < 11) { > + engine->irq_enable = gen8_logical_ring_enable_irq; > + engine->irq_disable = gen8_logical_ring_disable_irq; > + } else { > + /* > + * On Gen11 interrupts are permanently unmasked and there > + * are no per-engine instance mask bits. > + */ As mentioned on the previous review, the masks exist but we can't use them easily, because according to the docs we need to clear them to allow C6 entry. My guess is that it is actually the possible pending interrupts behind the mask that can keep the device awake more than the masks value themselves, but the docs just say to clear the registers. Maybe a possible solution would be to track idleness per-engine and unmask the interrupts once all requests have completed on the specific engine? Anyway, while we're still in early enabling I'd just update this comment and the commit message to reflect that we don't use the mask to allow C6 entry and we can eventually come back to refine this at a later point if we see that we're generating too many interrupts. With the updated comment and commit message: Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele > + } > engine->emit_bb_start = gen8_emit_bb_start; > } > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index f743351c441f..caed64bb02da 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -393,6 +393,11 @@ struct intel_engine_cs { > > u32 irq_keep_mask; /* always keep these interrupts */ > u32 irq_enable_mask; /* bitmask to enable ring interrupt */ > + > + /* > + * irq_enable and irq_disable do not have to be provided for > + * an engine. In other words they can be NULL. > + */ > void (*irq_enable)(struct intel_engine_cs *engine); > void (*irq_disable)(struct intel_engine_cs *engine); > >
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index b955f7d7bd0f..69c727f97eb5 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -167,18 +167,22 @@ static void irq_enable(struct intel_engine_cs *engine) */ set_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted); - /* Caller disables interrupts */ - spin_lock(&engine->i915->irq_lock); - engine->irq_enable(engine); - spin_unlock(&engine->i915->irq_lock); + if (engine->irq_enable) { + /* Caller disables interrupts */ + spin_lock(&engine->i915->irq_lock); + engine->irq_enable(engine); + spin_unlock(&engine->i915->irq_lock); + } } static void irq_disable(struct intel_engine_cs *engine) { - /* Caller disables interrupts */ - spin_lock(&engine->i915->irq_lock); - engine->irq_disable(engine); - spin_unlock(&engine->i915->irq_lock); + if (engine->irq_disable) { + /* Caller disables interrupts */ + spin_lock(&engine->i915->irq_lock); + engine->irq_disable(engine); + spin_unlock(&engine->i915->irq_lock); + } } void __intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index c2c8380a0121..da396c46b345 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1989,8 +1989,15 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine) engine->set_default_submission = execlists_set_default_submission; - engine->irq_enable = gen8_logical_ring_enable_irq; - engine->irq_disable = gen8_logical_ring_disable_irq; + if (INTEL_GEN(engine->i915) < 11) { + engine->irq_enable = gen8_logical_ring_enable_irq; + engine->irq_disable = gen8_logical_ring_disable_irq; + } else { + /* + * On Gen11 interrupts are permanently unmasked and there + * are no per-engine instance mask bits. + */ + } engine->emit_bb_start = gen8_emit_bb_start; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index f743351c441f..caed64bb02da 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -393,6 +393,11 @@ struct intel_engine_cs { u32 irq_keep_mask; /* always keep these interrupts */ u32 irq_enable_mask; /* bitmask to enable ring interrupt */ + + /* + * irq_enable and irq_disable do not have to be provided for + * an engine. In other words they can be NULL. + */ void (*irq_enable)(struct intel_engine_cs *engine); void (*irq_disable)(struct intel_engine_cs *engine);