On 1/5/2021 4:15 PM, Chris Wilson wrote:
Quoting Daniele Ceraolo Spurio (2021-01-05 23:56:52)
On 1/5/2021 3:38 PM, Chris Wilson wrote:
Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:47)
In GuC submission mode the CS is owned by the GuC FW, so all CS status
interrupts are handled by it. We only need the user interrupt as that
signals request completion.
Since we're now starting the engines directly in GuC submission mode
when selected, we can stop switching back and forth between the
execlists and the GuC programming and select directly the correct
interrupt mask.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
Cc: John Harrison <john.c.harrison@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 18 ++++++-----
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 -------------------
2 files changed, 11 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index 9830342aa6f4..7b2b8cb2d2be 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -237,14 +237,18 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
void gen11_gt_irq_postinstall(struct intel_gt *gt)
{
- const u32 irqs =
- GT_CS_MASTER_ERROR_INTERRUPT |
- GT_RENDER_USER_INTERRUPT |
- GT_CONTEXT_SWITCH_INTERRUPT |
- GT_WAIT_SEMAPHORE_INTERRUPT;
struct intel_uncore *uncore = gt->uncore;
- const u32 dmask = irqs << 16 | irqs;
- const u32 smask = irqs << 16;
+ u32 irqs = GT_RENDER_USER_INTERRUPT;
+ u32 dmask;
+ u32 smask;
+
+ if (!intel_uc_wants_guc_submission(>->uc))
+ irqs |= GT_CS_MASTER_ERROR_INTERRUPT |
+ GT_CONTEXT_SWITCH_INTERRUPT |
+ GT_WAIT_SEMAPHORE_INTERRUPT;
Hmm, we should stop performing this by default then, and make the
execlists setup request the interrupt vector it desires.
That's certainly a bit more fiddly to untangle the packed iir across
multiple gen. :|
I had considered that, but this is a gt-level setup while the execlists
submission code is currently all engine-based, so I couldn't find a good
place to move this to and I didn't want to add a new function just for
it. Any preference?
I think we should definitely strive to avoid having too many backend-
conditional paths in the common routines, and we can certainly adjust
the interrupts as we enable each engine (it certainly would help when
debugging by removing engines by removing spurious iir).
I guess we may end up with something like
gen8_gt_unmask_engine_irq(struct intel_gt *gt,
struct intel_engine_cs *engine,
u16 iir)
{
if (INTEL_GEN(gt->i915) >= 11)
__gen11...
else
__gen8...
}
[Not sure if gen8_gt or intel_gt; I chose gen8_gt for the iir definition]
Then that would fit reasonably into logical_ring_default_irqs().
Hindsight says that if we had done that earlier, we could have avoided
maintaining the same engine-iir in two different sites. So maybe worth
a bit of effort.
-Chris
Looks reasonable, I'll give it a try.
Daniele
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
