Re: [Intel-gfx] [PATCH v2] drm/i915/gt: Move CS interrupt handler to the backend

2021-02-02 Thread Tvrtko Ursulin



On 02/02/2021 16:15, 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.

v2: An overabundance of caution is always justified; put a barrier on
updating the irq handler so that we know that the next interrupt will
be redirected towards ourselves.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
---
  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| 22 +
  .../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, 118 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 c7d17f8767a1..e06ae4ae1710 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 = _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..05846f97f1af 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;
+   }

[Intel-gfx] [PATCH v2] drm/i915/gt: Move CS interrupt handler to the backend

2021-02-02 Thread Chris Wilson
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.

v2: An overabundance of caution is always justified; put a barrier on
updating the irq handler so that we know that the next interrupt will
be redirected towards ourselves.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
---
 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| 22 +
 .../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, 118 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 c7d17f8767a1..e06ae4ae1710 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 = _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..05846f97f1af 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 &