| Message ID | 20180316121456.11577-8-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 3/16/2018 5:14 AM, Mika Kuoppala wrote: > Interrupt identity register we already read from hardware > contains engine class and instance fields. Leverage > these fields to find correct engine to handle the interrupt. > > v3: rebase on top of rps intr > use correct class / instance limits (Michel) > > Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 80 +++++++++++++++-------------------------- > drivers/gpu/drm/i915/i915_reg.h | 4 ++- > 2 files changed, 31 insertions(+), 53 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 8c4510ffe625..dd2fb2d0457f 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -413,8 +413,8 @@ static void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 disable_m > } > > static u32 > -gen11_gt_engine_intr(struct drm_i915_private * const i915, > - const unsigned int bank, const unsigned int bit); > +gen11_gt_engine_identity(struct drm_i915_private * const i915, > + const unsigned int bank, const unsigned int bit); > > void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv) > { > @@ -428,7 +428,7 @@ void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv) > */ > dw = I915_READ_FW(GEN11_GT_INTR_DW0); > while (dw & BIT(GEN11_GTPM)) { > - gen11_gt_engine_intr(dev_priv, 0, GEN11_GTPM); > + gen11_gt_engine_identity(dev_priv, 0, GEN11_GTPM); > I915_WRITE_FW(GEN11_GT_INTR_DW0, BIT(GEN11_GTPM)); > dw = I915_READ_FW(GEN11_GT_INTR_DW0); > } > @@ -2771,50 +2771,9 @@ static void __fini_wedge(struct wedge_me *w) > (W)->i915; \ > __fini_wedge((W))) > > -static void > -gen11_gt_engine_irq_handler(struct drm_i915_private * const i915, > - const unsigned int bank, > - const unsigned int engine_n, > - const u16 iir) > -{ > - struct intel_engine_cs ** const engine = i915->engine; > - > - switch (bank) { > - case 0: > - switch (engine_n) { > - > - case GEN11_RCS0: > - return gen8_cs_irq_handler(engine[RCS], iir); > - > - case GEN11_BCS: > - return gen8_cs_irq_handler(engine[BCS], iir); > - > - case GEN11_GTPM: > - return gen6_rps_irq_handler(i915, iir); > - } > - case 1: > - switch (engine_n) { > - > - case GEN11_VCS(0): > - return gen8_cs_irq_handler(engine[_VCS(0)], iir); > - case GEN11_VCS(1): > - return gen8_cs_irq_handler(engine[_VCS(1)], iir); > - case GEN11_VCS(2): > - return gen8_cs_irq_handler(engine[_VCS(2)], iir); > - case GEN11_VCS(3): > - return gen8_cs_irq_handler(engine[_VCS(3)], iir); > - > - case GEN11_VECS(0): > - return gen8_cs_irq_handler(engine[_VECS(0)], iir); > - case GEN11_VECS(1): > - return gen8_cs_irq_handler(engine[_VECS(1)], iir); > - } > - } > -} > - > static u32 > -gen11_gt_engine_intr(struct drm_i915_private * const i915, > - const unsigned int bank, const unsigned int bit) > +gen11_gt_engine_identity(struct drm_i915_private * const i915, > + const unsigned int bank, const unsigned int bit) > { > void __iomem * const regs = i915->regs; > u32 timeout_ts; > @@ -2841,7 +2800,26 @@ gen11_gt_engine_intr(struct drm_i915_private * const i915, > raw_reg_write(regs, GEN11_INTR_IDENTITY_REG(bank), > GEN11_INTR_DATA_VALID); > > - return ident & GEN11_INTR_ENGINE_MASK; > + return ident; > +} > + > +static void > +gen11_gt_identity_handler(struct drm_i915_private * const i915, > + const u32 identity) > +{ > + const u8 class = GEN11_INTR_ENGINE_CLASS(identity); > + const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity); > + const u16 iir = GEN11_INTR_ENGINE_MASK(identity); > + > + if (unlikely(!iir)) > + return; > + > + if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) > + return gen8_cs_irq_handler(i915->engine_class[class][instance], > + iir); > + > + if (class == GEN11_GTPM) > + return gen6_rps_irq_handler(i915, iir); Hi, GEN11_GTPM should be [Class ID] == 'OTHER_CLASS (4)', and [Engine Id] == 1. (I'm looking at bspec 20944) So not only the GPTM check needs to be before the if (class <= MAX_ENGINE_CLASS), but it can't use GEN11_GTPM either. -Michel > } > > static void > @@ -2866,12 +2844,10 @@ gen11_gt_irq_handler(struct drm_i915_private * const i915, > } > > for_each_set_bit(bit, &intr_dw, 32) { > - const u16 iir = gen11_gt_engine_intr(i915, bank, bit); > - > - if (unlikely(!iir)) > - continue; > + const u32 ident = gen11_gt_engine_identity(i915, > + bank, bit); > > - gen11_gt_engine_irq_handler(i915, bank, bit, iir); > + gen11_gt_identity_handler(i915, ident); > } > > /* Clear must be after shared has been served for engine */ > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index f3cc77690124..74a8f454e8a0 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6826,7 +6826,9 @@ enum { > #define GEN11_INTR_IDENTITY_REG0 _MMIO(0x190060) > #define GEN11_INTR_IDENTITY_REG1 _MMIO(0x190064) > #define GEN11_INTR_DATA_VALID (1 << 31) > -#define GEN11_INTR_ENGINE_MASK (0xffff) > +#define GEN11_INTR_ENGINE_MASK(x) ((x) & 0xffff) > +#define GEN11_INTR_ENGINE_CLASS(x) (((x) & GENMASK(18, 16)) >> 16) > +#define GEN11_INTR_ENGINE_INSTANCE(x) (((x) & GENMASK(25, 20)) >> 20) > > #define GEN11_INTR_IDENTITY_REG(x) _MMIO(0x190060 + (x * 4)) > >
On 16/03/18 11:28, Michel Thierry wrote: > On 3/16/2018 5:14 AM, Mika Kuoppala wrote: >> Interrupt identity register we already read from hardware >> contains engine class and instance fields. Leverage >> these fields to find correct engine to handle the interrupt. >> >> v3: rebase on top of rps intr >> use correct class / instance limits (Michel) >> >> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Michel Thierry <michel.thierry@intel.com> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 80 >> +++++++++++++++-------------------------- >> drivers/gpu/drm/i915/i915_reg.h | 4 ++- >> 2 files changed, 31 insertions(+), 53 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c >> b/drivers/gpu/drm/i915/i915_irq.c >> index 8c4510ffe625..dd2fb2d0457f 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -413,8 +413,8 @@ static void gen6_disable_pm_irq(struct >> drm_i915_private *dev_priv, u32 disable_m >> } >> static u32 >> -gen11_gt_engine_intr(struct drm_i915_private * const i915, >> - const unsigned int bank, const unsigned int bit); >> +gen11_gt_engine_identity(struct drm_i915_private * const i915, >> + const unsigned int bank, const unsigned int bit); >> void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv) >> { >> @@ -428,7 +428,7 @@ void gen11_reset_rps_interrupts(struct >> drm_i915_private *dev_priv) >> */ >> dw = I915_READ_FW(GEN11_GT_INTR_DW0); >> while (dw & BIT(GEN11_GTPM)) { >> - gen11_gt_engine_intr(dev_priv, 0, GEN11_GTPM); >> + gen11_gt_engine_identity(dev_priv, 0, GEN11_GTPM); >> I915_WRITE_FW(GEN11_GT_INTR_DW0, BIT(GEN11_GTPM)); >> dw = I915_READ_FW(GEN11_GT_INTR_DW0); >> } >> @@ -2771,50 +2771,9 @@ static void __fini_wedge(struct wedge_me *w) >> (W)->i915; \ >> __fini_wedge((W))) >> -static void >> -gen11_gt_engine_irq_handler(struct drm_i915_private * const i915, >> - const unsigned int bank, >> - const unsigned int engine_n, >> - const u16 iir) >> -{ >> - struct intel_engine_cs ** const engine = i915->engine; >> - >> - switch (bank) { >> - case 0: >> - switch (engine_n) { >> - >> - case GEN11_RCS0: >> - return gen8_cs_irq_handler(engine[RCS], iir); >> - >> - case GEN11_BCS: >> - return gen8_cs_irq_handler(engine[BCS], iir); >> - >> - case GEN11_GTPM: >> - return gen6_rps_irq_handler(i915, iir); >> - } >> - case 1: >> - switch (engine_n) { >> - >> - case GEN11_VCS(0): >> - return gen8_cs_irq_handler(engine[_VCS(0)], iir); >> - case GEN11_VCS(1): >> - return gen8_cs_irq_handler(engine[_VCS(1)], iir); >> - case GEN11_VCS(2): >> - return gen8_cs_irq_handler(engine[_VCS(2)], iir); >> - case GEN11_VCS(3): >> - return gen8_cs_irq_handler(engine[_VCS(3)], iir); >> - >> - case GEN11_VECS(0): >> - return gen8_cs_irq_handler(engine[_VECS(0)], iir); >> - case GEN11_VECS(1): >> - return gen8_cs_irq_handler(engine[_VECS(1)], iir); >> - } >> - } >> -} >> - >> static u32 >> -gen11_gt_engine_intr(struct drm_i915_private * const i915, >> - const unsigned int bank, const unsigned int bit) >> +gen11_gt_engine_identity(struct drm_i915_private * const i915, >> + const unsigned int bank, const unsigned int bit) >> { >> void __iomem * const regs = i915->regs; >> u32 timeout_ts; >> @@ -2841,7 +2800,26 @@ gen11_gt_engine_intr(struct drm_i915_private * >> const i915, >> raw_reg_write(regs, GEN11_INTR_IDENTITY_REG(bank), >> GEN11_INTR_DATA_VALID); >> - return ident & GEN11_INTR_ENGINE_MASK; >> + return ident; >> +} >> + >> +static void >> +gen11_gt_identity_handler(struct drm_i915_private * const i915, >> + const u32 identity) >> +{ >> + const u8 class = GEN11_INTR_ENGINE_CLASS(identity); >> + const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity); >> + const u16 iir = GEN11_INTR_ENGINE_MASK(identity); >> + >> + if (unlikely(!iir)) >> + return; >> + >> + if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) >> + return gen8_cs_irq_handler(i915->engine_class[class][instance], >> + iir); >> + >> + if (class == GEN11_GTPM) >> + return gen6_rps_irq_handler(i915, iir); > > Hi, > > GEN11_GTPM should be > [Class ID] == 'OTHER_CLASS (4)', and > [Engine Id] == 1. > (I'm looking at bspec 20944) > > So not only the GPTM check needs to be before the > if (class <= MAX_ENGINE_CLASS), > > but it can't use GEN11_GTPM either. > > -Michel > I'm not fully convinced about checking against MAX_ENGINE_CLASS or MAX_ENGINE_INSTANCE, since we do have cases that are within those values but are still invalid (e.g. VCS1). Maybe we can just do something simpler (that should still be safe) like: /* OTHER_CLASS is for interrupts not comings from an engine */ if (class != OTHER_CLASS) { engine = i915->engine_class[class][instance]; if (likely(engine)) return gen8_cs_irq_handler(...); } else { if (instance == 1) return gen6_rps_irq_handler(...); /* more cases incoming (e.g. GuC) */ } > >> } >> static void >> @@ -2866,12 +2844,10 @@ gen11_gt_irq_handler(struct drm_i915_private * >> const i915, >> } >> for_each_set_bit(bit, &intr_dw, 32) { >> - const u16 iir = gen11_gt_engine_intr(i915, bank, bit); >> - >> - if (unlikely(!iir)) >> - continue; >> + const u32 ident = gen11_gt_engine_identity(i915, >> + bank, bit); >> - gen11_gt_engine_irq_handler(i915, bank, bit, iir); >> + gen11_gt_identity_handler(i915, ident); >> } >> /* Clear must be after shared has been served for engine */ >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index f3cc77690124..74a8f454e8a0 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -6826,7 +6826,9 @@ enum { >> #define GEN11_INTR_IDENTITY_REG0 _MMIO(0x190060) >> #define GEN11_INTR_IDENTITY_REG1 _MMIO(0x190064) >> #define GEN11_INTR_DATA_VALID (1 << 31) >> -#define GEN11_INTR_ENGINE_MASK (0xffff) >> +#define GEN11_INTR_ENGINE_MASK(x) ((x) & 0xffff) >> +#define GEN11_INTR_ENGINE_CLASS(x) (((x) & GENMASK(18, 16)) >> 16) >> +#define GEN11_INTR_ENGINE_INSTANCE(x) (((x) & GENMASK(25, 20)) >> >> 20) >> #define GEN11_INTR_IDENTITY_REG(x) _MMIO(0x190060 + (x * 4)) >>
On 16/03/18 12:37, Daniele Ceraolo Spurio wrote: > > > On 16/03/18 11:28, Michel Thierry wrote: >> On 3/16/2018 5:14 AM, Mika Kuoppala wrote: >>> Interrupt identity register we already read from hardware >>> contains engine class and instance fields. Leverage >>> these fields to find correct engine to handle the interrupt. >>> >>> v3: rebase on top of rps intr >>> use correct class / instance limits (Michel) >>> >>> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> Cc: Michel Thierry <michel.thierry@intel.com> >>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> >>> --- >>> drivers/gpu/drm/i915/i915_irq.c | 80 >>> +++++++++++++++-------------------------- >>> drivers/gpu/drm/i915/i915_reg.h | 4 ++- >>> 2 files changed, 31 insertions(+), 53 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_irq.c >>> b/drivers/gpu/drm/i915/i915_irq.c >>> index 8c4510ffe625..dd2fb2d0457f 100644 >>> --- a/drivers/gpu/drm/i915/i915_irq.c >>> +++ b/drivers/gpu/drm/i915/i915_irq.c >>> @@ -413,8 +413,8 @@ static void gen6_disable_pm_irq(struct >>> drm_i915_private *dev_priv, u32 disable_m >>> } >>> static u32 >>> -gen11_gt_engine_intr(struct drm_i915_private * const i915, >>> - const unsigned int bank, const unsigned int bit); >>> +gen11_gt_engine_identity(struct drm_i915_private * const i915, >>> + const unsigned int bank, const unsigned int bit); >>> void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv) >>> { >>> @@ -428,7 +428,7 @@ void gen11_reset_rps_interrupts(struct >>> drm_i915_private *dev_priv) >>> */ >>> dw = I915_READ_FW(GEN11_GT_INTR_DW0); >>> while (dw & BIT(GEN11_GTPM)) { >>> - gen11_gt_engine_intr(dev_priv, 0, GEN11_GTPM); >>> + gen11_gt_engine_identity(dev_priv, 0, GEN11_GTPM); >>> I915_WRITE_FW(GEN11_GT_INTR_DW0, BIT(GEN11_GTPM)); >>> dw = I915_READ_FW(GEN11_GT_INTR_DW0); >>> } >>> @@ -2771,50 +2771,9 @@ static void __fini_wedge(struct wedge_me *w) >>> (W)->i915; \ >>> __fini_wedge((W))) >>> -static void >>> -gen11_gt_engine_irq_handler(struct drm_i915_private * const i915, >>> - const unsigned int bank, >>> - const unsigned int engine_n, >>> - const u16 iir) >>> -{ >>> - struct intel_engine_cs ** const engine = i915->engine; >>> - >>> - switch (bank) { >>> - case 0: >>> - switch (engine_n) { >>> - >>> - case GEN11_RCS0: >>> - return gen8_cs_irq_handler(engine[RCS], iir); >>> - >>> - case GEN11_BCS: >>> - return gen8_cs_irq_handler(engine[BCS], iir); >>> - >>> - case GEN11_GTPM: >>> - return gen6_rps_irq_handler(i915, iir); >>> - } >>> - case 1: >>> - switch (engine_n) { >>> - >>> - case GEN11_VCS(0): >>> - return gen8_cs_irq_handler(engine[_VCS(0)], iir); >>> - case GEN11_VCS(1): >>> - return gen8_cs_irq_handler(engine[_VCS(1)], iir); >>> - case GEN11_VCS(2): >>> - return gen8_cs_irq_handler(engine[_VCS(2)], iir); >>> - case GEN11_VCS(3): >>> - return gen8_cs_irq_handler(engine[_VCS(3)], iir); >>> - >>> - case GEN11_VECS(0): >>> - return gen8_cs_irq_handler(engine[_VECS(0)], iir); >>> - case GEN11_VECS(1): >>> - return gen8_cs_irq_handler(engine[_VECS(1)], iir); >>> - } >>> - } >>> -} >>> - >>> static u32 >>> -gen11_gt_engine_intr(struct drm_i915_private * const i915, >>> - const unsigned int bank, const unsigned int bit) >>> +gen11_gt_engine_identity(struct drm_i915_private * const i915, >>> + const unsigned int bank, const unsigned int bit) >>> { >>> void __iomem * const regs = i915->regs; >>> u32 timeout_ts; >>> @@ -2841,7 +2800,26 @@ gen11_gt_engine_intr(struct drm_i915_private * >>> const i915, >>> raw_reg_write(regs, GEN11_INTR_IDENTITY_REG(bank), >>> GEN11_INTR_DATA_VALID); >>> - return ident & GEN11_INTR_ENGINE_MASK; >>> + return ident; >>> +} >>> + >>> +static void >>> +gen11_gt_identity_handler(struct drm_i915_private * const i915, >>> + const u32 identity) >>> +{ >>> + const u8 class = GEN11_INTR_ENGINE_CLASS(identity); >>> + const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity); >>> + const u16 iir = GEN11_INTR_ENGINE_MASK(identity); >>> + >>> + if (unlikely(!iir)) >>> + return; >>> + >>> + if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) >>> + return gen8_cs_irq_handler(i915->engine_class[class][instance], >>> + iir); >>> + >>> + if (class == GEN11_GTPM) >>> + return gen6_rps_irq_handler(i915, iir); >> >> Hi, >> >> GEN11_GTPM should be >> [Class ID] == 'OTHER_CLASS (4)', and >> [Engine Id] == 1. >> (I'm looking at bspec 20944) >> >> So not only the GPTM check needs to be before the >> if (class <= MAX_ENGINE_CLASS), >> >> but it can't use GEN11_GTPM either. >> >> -Michel >> > > I'm not fully convinced about checking against MAX_ENGINE_CLASS or > MAX_ENGINE_INSTANCE, since we do have cases that are within those values > but are still invalid (e.g. VCS1). Maybe we can just do something > simpler (that should still be safe) like: > Thinking about this again you were right, we do need a check for MAX_ENGINE_CLASS and MAX_ENGINE_INSTANCE to avoid overflowing the engine_class array. However, while the MAX_ENGINE_CLASS check applies to all cases, the MAX_ENGINE_INSTANCE one only applies for class != OTHER_CLASS, so we may need to split them accordingly. Daniele > /* OTHER_CLASS is for interrupts not comings from an engine */ > if (class != OTHER_CLASS) { > engine = i915->engine_class[class][instance]; > if (likely(engine)) > return gen8_cs_irq_handler(...); > } else { > if (instance == 1) > return gen6_rps_irq_handler(...); > > /* more cases incoming (e.g. GuC) */ > } > >> >>> } >>> static void >>> @@ -2866,12 +2844,10 @@ gen11_gt_irq_handler(struct drm_i915_private >>> * const i915, >>> } >>> for_each_set_bit(bit, &intr_dw, 32) { >>> - const u16 iir = gen11_gt_engine_intr(i915, bank, bit); >>> - >>> - if (unlikely(!iir)) >>> - continue; >>> + const u32 ident = gen11_gt_engine_identity(i915, >>> + bank, bit); >>> - gen11_gt_engine_irq_handler(i915, bank, bit, iir); >>> + gen11_gt_identity_handler(i915, ident); >>> } >>> /* Clear must be after shared has been served for engine */ >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h >>> b/drivers/gpu/drm/i915/i915_reg.h >>> index f3cc77690124..74a8f454e8a0 100644 >>> --- a/drivers/gpu/drm/i915/i915_reg.h >>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>> @@ -6826,7 +6826,9 @@ enum { >>> #define GEN11_INTR_IDENTITY_REG0 _MMIO(0x190060) >>> #define GEN11_INTR_IDENTITY_REG1 _MMIO(0x190064) >>> #define GEN11_INTR_DATA_VALID (1 << 31) >>> -#define GEN11_INTR_ENGINE_MASK (0xffff) >>> +#define GEN11_INTR_ENGINE_MASK(x) ((x) & 0xffff) >>> +#define GEN11_INTR_ENGINE_CLASS(x) (((x) & GENMASK(18, 16)) >> 16) >>> +#define GEN11_INTR_ENGINE_INSTANCE(x) (((x) & GENMASK(25, 20)) >>> >> 20) >>> #define GEN11_INTR_IDENTITY_REG(x) _MMIO(0x190060 + (x * 4)) >>> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 8c4510ffe625..dd2fb2d0457f 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -413,8 +413,8 @@ static void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 disable_m } static u32 -gen11_gt_engine_intr(struct drm_i915_private * const i915, - const unsigned int bank, const unsigned int bit); +gen11_gt_engine_identity(struct drm_i915_private * const i915, + const unsigned int bank, const unsigned int bit); void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv) { @@ -428,7 +428,7 @@ void gen11_reset_rps_interrupts(struct drm_i915_private *dev_priv) */ dw = I915_READ_FW(GEN11_GT_INTR_DW0); while (dw & BIT(GEN11_GTPM)) { - gen11_gt_engine_intr(dev_priv, 0, GEN11_GTPM); + gen11_gt_engine_identity(dev_priv, 0, GEN11_GTPM); I915_WRITE_FW(GEN11_GT_INTR_DW0, BIT(GEN11_GTPM)); dw = I915_READ_FW(GEN11_GT_INTR_DW0); } @@ -2771,50 +2771,9 @@ static void __fini_wedge(struct wedge_me *w) (W)->i915; \ __fini_wedge((W))) -static void -gen11_gt_engine_irq_handler(struct drm_i915_private * const i915, - const unsigned int bank, - const unsigned int engine_n, - const u16 iir) -{ - struct intel_engine_cs ** const engine = i915->engine; - - switch (bank) { - case 0: - switch (engine_n) { - - case GEN11_RCS0: - return gen8_cs_irq_handler(engine[RCS], iir); - - case GEN11_BCS: - return gen8_cs_irq_handler(engine[BCS], iir); - - case GEN11_GTPM: - return gen6_rps_irq_handler(i915, iir); - } - case 1: - switch (engine_n) { - - case GEN11_VCS(0): - return gen8_cs_irq_handler(engine[_VCS(0)], iir); - case GEN11_VCS(1): - return gen8_cs_irq_handler(engine[_VCS(1)], iir); - case GEN11_VCS(2): - return gen8_cs_irq_handler(engine[_VCS(2)], iir); - case GEN11_VCS(3): - return gen8_cs_irq_handler(engine[_VCS(3)], iir); - - case GEN11_VECS(0): - return gen8_cs_irq_handler(engine[_VECS(0)], iir); - case GEN11_VECS(1): - return gen8_cs_irq_handler(engine[_VECS(1)], iir); - } - } -} - static u32 -gen11_gt_engine_intr(struct drm_i915_private * const i915, - const unsigned int bank, const unsigned int bit) +gen11_gt_engine_identity(struct drm_i915_private * const i915, + const unsigned int bank, const unsigned int bit) { void __iomem * const regs = i915->regs; u32 timeout_ts; @@ -2841,7 +2800,26 @@ gen11_gt_engine_intr(struct drm_i915_private * const i915, raw_reg_write(regs, GEN11_INTR_IDENTITY_REG(bank), GEN11_INTR_DATA_VALID); - return ident & GEN11_INTR_ENGINE_MASK; + return ident; +} + +static void +gen11_gt_identity_handler(struct drm_i915_private * const i915, + const u32 identity) +{ + const u8 class = GEN11_INTR_ENGINE_CLASS(identity); + const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity); + const u16 iir = GEN11_INTR_ENGINE_MASK(identity); + + if (unlikely(!iir)) + return; + + if (class <= MAX_ENGINE_CLASS && instance <= MAX_ENGINE_INSTANCE) + return gen8_cs_irq_handler(i915->engine_class[class][instance], + iir); + + if (class == GEN11_GTPM) + return gen6_rps_irq_handler(i915, iir); } static void @@ -2866,12 +2844,10 @@ gen11_gt_irq_handler(struct drm_i915_private * const i915, } for_each_set_bit(bit, &intr_dw, 32) { - const u16 iir = gen11_gt_engine_intr(i915, bank, bit); - - if (unlikely(!iir)) - continue; + const u32 ident = gen11_gt_engine_identity(i915, + bank, bit); - gen11_gt_engine_irq_handler(i915, bank, bit, iir); + gen11_gt_identity_handler(i915, ident); } /* Clear must be after shared has been served for engine */ diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f3cc77690124..74a8f454e8a0 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6826,7 +6826,9 @@ enum { #define GEN11_INTR_IDENTITY_REG0 _MMIO(0x190060) #define GEN11_INTR_IDENTITY_REG1 _MMIO(0x190064) #define GEN11_INTR_DATA_VALID (1 << 31) -#define GEN11_INTR_ENGINE_MASK (0xffff) +#define GEN11_INTR_ENGINE_MASK(x) ((x) & 0xffff) +#define GEN11_INTR_ENGINE_CLASS(x) (((x) & GENMASK(18, 16)) >> 16) +#define GEN11_INTR_ENGINE_INSTANCE(x) (((x) & GENMASK(25, 20)) >> 20) #define GEN11_INTR_IDENTITY_REG(x) _MMIO(0x190060 + (x * 4))
Interrupt identity register we already read from hardware contains engine class and instance fields. Leverage these fields to find correct engine to handle the interrupt. v3: rebase on top of rps intr use correct class / instance limits (Michel) Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 80 +++++++++++++++-------------------------- drivers/gpu/drm/i915/i915_reg.h | 4 ++- 2 files changed, 31 insertions(+), 53 deletions(-)