Re: [Intel-gfx] [CI 03/14] drm/i915/gt: Move CS interrupt handler to the backend
Quoting Chris Wilson (2021-02-02 15:53:41)
> Quoting Tvrtko Ursulin (2021-02-02 15:49:59)
> >
> > On 02/02/2021 15:14, Chris Wilson wrote:
> > > The different submission backends each have their own preferred
> > > behaviour and interrupt setup. Let each handle their own interrupts.
> > >
> > > This becomes more useful later as we to extract the use of auxiliary
> > > state in the interrupt handler that is backend specific.
> > >
> > > Signed-off-by: Chris Wilson
> > > ---
> > > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++
> > > drivers/gpu/drm/i915/gt/intel_engine_types.h | 14 +---
> > > .../drm/i915/gt/intel_execlists_submission.c | 40 +
> > > drivers/gpu/drm/i915/gt/intel_gt_irq.c| 82 ++-
> > > drivers/gpu/drm/i915/gt/intel_gt_irq.h| 7 ++
> > > .../gpu/drm/i915/gt/intel_ring_submission.c | 7 ++
> > > drivers/gpu/drm/i915/gt/intel_rps.c | 2 +-
> > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 10 ++-
> > > drivers/gpu/drm/i915/i915_irq.c | 8 +-
> > > 9 files changed, 103 insertions(+), 74 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > index dab8d734e272..2a453ba5f25a 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > @@ -255,6 +255,11 @@ static void intel_engine_sanitize_mmio(struct
> > > intel_engine_cs *engine)
> > > intel_engine_set_hwsp_writemask(engine, ~0u);
> > > }
> > >
> > > +static void nop_irq_handler(struct intel_engine_cs *engine, u32 iir)
> > > +{
> > > + GEM_DEBUG_WARN_ON(iir);
> > > +}
> > > +
> > > static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id
> > > id)
> > > {
> > > const struct engine_info *info = &intel_engines[id];
> > > @@ -292,6 +297,8 @@ static int intel_engine_setup(struct intel_gt *gt,
> > > enum intel_engine_id id)
> > > engine->hw_id = info->hw_id;
> > > engine->guc_id = MAKE_GUC_ID(info->class, info->instance);
> > >
> > > + engine->irq_handler = nop_irq_handler;
> > > +
> > > engine->class = info->class;
> > > engine->instance = info->instance;
> > > __sprint_engine_name(engine);
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > index 9d59de5c559a..7fd035d45263 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > @@ -402,6 +402,7 @@ struct intel_engine_cs {
> > > u32 irq_enable_mask; /* bitmask to enable ring
> > > interrupt */
> > > void(*irq_enable)(struct intel_engine_cs *engine);
> > > void(*irq_disable)(struct intel_engine_cs *engine);
> > > + void(*irq_handler)(struct intel_engine_cs *engine, u32
> > > iir);
> > >
> > > void(*sanitize)(struct intel_engine_cs *engine);
> > > int (*resume)(struct intel_engine_cs *engine);
> > > @@ -481,10 +482,9 @@ struct intel_engine_cs {
> > > #define I915_ENGINE_HAS_PREEMPTION BIT(2)
> > > #define I915_ENGINE_HAS_SEMAPHORES BIT(3)
> > > #define I915_ENGINE_HAS_TIMESLICES BIT(4)
> > > -#define I915_ENGINE_NEEDS_BREADCRUMB_TASKLET BIT(5)
> > > -#define I915_ENGINE_IS_VIRTUAL BIT(6)
> > > -#define I915_ENGINE_HAS_RELATIVE_MMIO BIT(7)
> > > -#define I915_ENGINE_REQUIRES_CMD_PARSER BIT(8)
> > > +#define I915_ENGINE_IS_VIRTUAL BIT(5)
> > > +#define I915_ENGINE_HAS_RELATIVE_MMIO BIT(6)
> > > +#define I915_ENGINE_REQUIRES_CMD_PARSER BIT(7)
> > > unsigned int flags;
> > >
> > > /*
> > > @@ -588,12 +588,6 @@ intel_engine_has_timeslices(const struct
> > > intel_engine_cs *engine)
> > > return engine->flags & I915_ENGINE_HAS_TIMESLICES;
> > > }
> > >
> > > -static inline bool
> > > -intel_engine_needs_breadcrumb_tasklet(const struct intel_engine_cs
> > > *engine)
> > > -{
> > > - return engine->flags & I915_ENGINE_NEEDS_BREADCRUMB_TASKLET;
> > > -}
> > > -
> > > static inline bool
> > > intel_engine_is_virtual(const struct intel_engine_cs *engine)
> > > {
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > > b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > > index 4ddd2099a931..ed62e4b549d2 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > > @@ -2394,6 +2394,45 @@ static void execlists_submission_tasklet(struct
> > > tasklet_struct *t)
> > > rcu_read_unlock();
> > > }
> > >
> > > +static void execlists_irq_handler(struct intel_engine_cs *engine, u32
> > > iir)
> > > +{
> > > + bool tasklet = false;
> > > +
> > > + if (unlikely(iir & GT_CS_MASTER_ERROR_INTERRUPT)) {
> > > + u32 eir;
> > > +
> > > + /* Upper 16b
Re: [Intel-gfx] [CI 03/14] drm/i915/gt: Move CS interrupt handler to the backend
Quoting Tvrtko Ursulin (2021-02-02 15:49:59)
>
> On 02/02/2021 15:14, Chris Wilson wrote:
> > The different submission backends each have their own preferred
> > behaviour and interrupt setup. Let each handle their own interrupts.
> >
> > This becomes more useful later as we to extract the use of auxiliary
> > state in the interrupt handler that is backend specific.
> >
> > Signed-off-by: Chris Wilson
> > ---
> > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++
> > drivers/gpu/drm/i915/gt/intel_engine_types.h | 14 +---
> > .../drm/i915/gt/intel_execlists_submission.c | 40 +
> > drivers/gpu/drm/i915/gt/intel_gt_irq.c| 82 ++-
> > drivers/gpu/drm/i915/gt/intel_gt_irq.h| 7 ++
> > .../gpu/drm/i915/gt/intel_ring_submission.c | 7 ++
> > drivers/gpu/drm/i915/gt/intel_rps.c | 2 +-
> > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 10 ++-
> > drivers/gpu/drm/i915/i915_irq.c | 8 +-
> > 9 files changed, 103 insertions(+), 74 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > index dab8d734e272..2a453ba5f25a 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > @@ -255,6 +255,11 @@ static void intel_engine_sanitize_mmio(struct
> > intel_engine_cs *engine)
> > intel_engine_set_hwsp_writemask(engine, ~0u);
> > }
> >
> > +static void nop_irq_handler(struct intel_engine_cs *engine, u32 iir)
> > +{
> > + GEM_DEBUG_WARN_ON(iir);
> > +}
> > +
> > static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id
> > id)
> > {
> > const struct engine_info *info = &intel_engines[id];
> > @@ -292,6 +297,8 @@ static int intel_engine_setup(struct intel_gt *gt, enum
> > intel_engine_id id)
> > engine->hw_id = info->hw_id;
> > engine->guc_id = MAKE_GUC_ID(info->class, info->instance);
> >
> > + engine->irq_handler = nop_irq_handler;
> > +
> > engine->class = info->class;
> > engine->instance = info->instance;
> > __sprint_engine_name(engine);
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > index 9d59de5c559a..7fd035d45263 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > @@ -402,6 +402,7 @@ struct intel_engine_cs {
> > u32 irq_enable_mask; /* bitmask to enable ring interrupt
> > */
> > void(*irq_enable)(struct intel_engine_cs *engine);
> > void(*irq_disable)(struct intel_engine_cs *engine);
> > + void(*irq_handler)(struct intel_engine_cs *engine, u32
> > iir);
> >
> > void(*sanitize)(struct intel_engine_cs *engine);
> > int (*resume)(struct intel_engine_cs *engine);
> > @@ -481,10 +482,9 @@ struct intel_engine_cs {
> > #define I915_ENGINE_HAS_PREEMPTION BIT(2)
> > #define I915_ENGINE_HAS_SEMAPHORES BIT(3)
> > #define I915_ENGINE_HAS_TIMESLICES BIT(4)
> > -#define I915_ENGINE_NEEDS_BREADCRUMB_TASKLET BIT(5)
> > -#define I915_ENGINE_IS_VIRTUAL BIT(6)
> > -#define I915_ENGINE_HAS_RELATIVE_MMIO BIT(7)
> > -#define I915_ENGINE_REQUIRES_CMD_PARSER BIT(8)
> > +#define I915_ENGINE_IS_VIRTUAL BIT(5)
> > +#define I915_ENGINE_HAS_RELATIVE_MMIO BIT(6)
> > +#define I915_ENGINE_REQUIRES_CMD_PARSER BIT(7)
> > unsigned int flags;
> >
> > /*
> > @@ -588,12 +588,6 @@ intel_engine_has_timeslices(const struct
> > intel_engine_cs *engine)
> > return engine->flags & I915_ENGINE_HAS_TIMESLICES;
> > }
> >
> > -static inline bool
> > -intel_engine_needs_breadcrumb_tasklet(const struct intel_engine_cs *engine)
> > -{
> > - return engine->flags & I915_ENGINE_NEEDS_BREADCRUMB_TASKLET;
> > -}
> > -
> > static inline bool
> > intel_engine_is_virtual(const struct intel_engine_cs *engine)
> > {
> > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > index 4ddd2099a931..ed62e4b549d2 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > @@ -2394,6 +2394,45 @@ static void execlists_submission_tasklet(struct
> > tasklet_struct *t)
> > rcu_read_unlock();
> > }
> >
> > +static void execlists_irq_handler(struct intel_engine_cs *engine, u32 iir)
> > +{
> > + bool tasklet = false;
> > +
> > + if (unlikely(iir & GT_CS_MASTER_ERROR_INTERRUPT)) {
> > + u32 eir;
> > +
> > + /* Upper 16b are the enabling mask, rsvd for internal errors
> > */
> > + eir = ENGINE_READ(engine, RING_EIR) & GENMASK(15, 0);
> > + ENGINE_TRACE(engine, "CS error: %x\n", eir);
> > +
> > + /* Disable the error interrupt until after the reset */
> > +
Re: [Intel-gfx] [CI 03/14] drm/i915/gt: Move CS interrupt handler to the backend
On 02/02/2021 15:14, Chris Wilson wrote:
The different submission backends each have their own preferred
behaviour and interrupt setup. Let each handle their own interrupts.
This becomes more useful later as we to extract the use of auxiliary
state in the interrupt handler that is backend specific.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++
drivers/gpu/drm/i915/gt/intel_engine_types.h | 14 +---
.../drm/i915/gt/intel_execlists_submission.c | 40 +
drivers/gpu/drm/i915/gt/intel_gt_irq.c| 82 ++-
drivers/gpu/drm/i915/gt/intel_gt_irq.h| 7 ++
.../gpu/drm/i915/gt/intel_ring_submission.c | 7 ++
drivers/gpu/drm/i915/gt/intel_rps.c | 2 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 10 ++-
drivers/gpu/drm/i915/i915_irq.c | 8 +-
9 files changed, 103 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index dab8d734e272..2a453ba5f25a 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -255,6 +255,11 @@ static void intel_engine_sanitize_mmio(struct
intel_engine_cs *engine)
intel_engine_set_hwsp_writemask(engine, ~0u);
}
+static void nop_irq_handler(struct intel_engine_cs *engine, u32 iir)
+{
+ GEM_DEBUG_WARN_ON(iir);
+}
+
static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
{
const struct engine_info *info = &intel_engines[id];
@@ -292,6 +297,8 @@ static int intel_engine_setup(struct intel_gt *gt, enum
intel_engine_id id)
engine->hw_id = info->hw_id;
engine->guc_id = MAKE_GUC_ID(info->class, info->instance);
+ engine->irq_handler = nop_irq_handler;
+
engine->class = info->class;
engine->instance = info->instance;
__sprint_engine_name(engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 9d59de5c559a..7fd035d45263 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -402,6 +402,7 @@ struct intel_engine_cs {
u32 irq_enable_mask; /* bitmask to enable ring interrupt */
void(*irq_enable)(struct intel_engine_cs *engine);
void(*irq_disable)(struct intel_engine_cs *engine);
+ void(*irq_handler)(struct intel_engine_cs *engine, u32 iir);
void (*sanitize)(struct intel_engine_cs *engine);
int (*resume)(struct intel_engine_cs *engine);
@@ -481,10 +482,9 @@ struct intel_engine_cs {
#define I915_ENGINE_HAS_PREEMPTION BIT(2)
#define I915_ENGINE_HAS_SEMAPHORES BIT(3)
#define I915_ENGINE_HAS_TIMESLICES BIT(4)
-#define I915_ENGINE_NEEDS_BREADCRUMB_TASKLET BIT(5)
-#define I915_ENGINE_IS_VIRTUAL BIT(6)
-#define I915_ENGINE_HAS_RELATIVE_MMIO BIT(7)
-#define I915_ENGINE_REQUIRES_CMD_PARSER BIT(8)
+#define I915_ENGINE_IS_VIRTUAL BIT(5)
+#define I915_ENGINE_HAS_RELATIVE_MMIO BIT(6)
+#define I915_ENGINE_REQUIRES_CMD_PARSER BIT(7)
unsigned int flags;
/*
@@ -588,12 +588,6 @@ intel_engine_has_timeslices(const struct intel_engine_cs
*engine)
return engine->flags & I915_ENGINE_HAS_TIMESLICES;
}
-static inline bool
-intel_engine_needs_breadcrumb_tasklet(const struct intel_engine_cs *engine)
-{
- return engine->flags & I915_ENGINE_NEEDS_BREADCRUMB_TASKLET;
-}
-
static inline bool
intel_engine_is_virtual(const struct intel_engine_cs *engine)
{
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 4ddd2099a931..ed62e4b549d2 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -2394,6 +2394,45 @@ static void execlists_submission_tasklet(struct
tasklet_struct *t)
rcu_read_unlock();
}
+static void execlists_irq_handler(struct intel_engine_cs *engine, u32 iir)
+{
+ bool tasklet = false;
+
+ if (unlikely(iir & GT_CS_MASTER_ERROR_INTERRUPT)) {
+ u32 eir;
+
+ /* Upper 16b are the enabling mask, rsvd for internal errors */
+ eir = ENGINE_READ(engine, RING_EIR) & GENMASK(15, 0);
+ ENGINE_TRACE(engine, "CS error: %x\n", eir);
+
+ /* Disable the error interrupt until after the reset */
+ if (likely(eir)) {
+ ENGINE_WRITE(engine, RING_EMR, ~0u);
+ ENGINE_WRITE(engine, RING_EIR, eir);
+ WRITE_ONCE(engine->execlists.error_interrupt, eir);
+ tasklet = true;
+ }
+ }
+
+ if (iir & GT_WAIT_SEMAPHORE_INTERRUPT) {
+ WRITE_ONCE(engine->execlists.yield,
+ ENGINE_READ_FW(engine, RING_EXECLIST_STATUS_HI));
+
