| Message ID | 20230417223443.1284617-1-andi.shyti@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] drm/i915: Make IRQ reset and postinstall multi-gt aware | expand |
On Tue, Apr 18, 2023 at 12:34:43AM +0200, Andi Shyti wrote: > In multi-gt systems IRQs need to be reset and enabled per GT. > > This might add some redundancy when handling interrupts for > engines that might not exist in every tile, but helps to keep the > code cleaner and more understandable. > > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > Hi, > > The rsults of this patch are more than promising as we are able > to have MTL booting and executing basic tests.(*) > > Thank you Daniele and Matt for the valuable exchange of opinions. > > Amdo > > (*) https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115465v5/index.html? > > Changelog > ========= > v3 -> v4 > - do not change the initial gt and uncore initialization in > order to gain a better understanding at a glance of the > purpose of all the local variables. I think I may not have explained myself well on the previous feedback here. What I meant was that rather than doing struct intel_uncore *uncore = to_gt(dev_priv)->uncore; as you were in the previous rev, you can simply do struct intel_uncore *uncore = dev_priv->uncore; because gt0's uncore pointer is always the same as dev_priv's. Since we're using the uncore variable to access display-specific gunit stuff I figured that was slightly more clear to the reader to take the device-level pointer rather than grabbing it from any of the GTs. That said, using "uncore = gt->uncore" as you have in this version doesn't cause any real problems since the actual registers being accessed are sgunit registers and thus don't get translated by GSI offset. You still wind up at the same sgunit register offsets on MTL no matter which GT you grab an uncore from, and display/gunit isn't something that PVC even needs to worry about. So Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > v2 -> v3 > - keep GUnit irq initialization out of the for_each_gt() loop as > the media GT doesn't have a GUnit. > v1 -> v2 > - improve description in the commit log. > > drivers/gpu/drm/i915/i915_irq.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index dea1a117f3fa1..c027fd5189b85 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2858,10 +2858,13 @@ static void dg1_irq_reset(struct drm_i915_private *dev_priv) > { > struct intel_gt *gt = to_gt(dev_priv); > struct intel_uncore *uncore = gt->uncore; > + unsigned int i; > > dg1_master_intr_disable(dev_priv->uncore.regs); > > - gen11_gt_irq_reset(gt); > + for_each_gt(gt, dev_priv, i) > + gen11_gt_irq_reset(gt); > + > gen11_display_irq_reset(dev_priv); > > GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_); > @@ -3646,8 +3649,10 @@ static void dg1_irq_postinstall(struct drm_i915_private *dev_priv) > struct intel_gt *gt = to_gt(dev_priv); > struct intel_uncore *uncore = gt->uncore; > u32 gu_misc_masked = GEN11_GU_MISC_GSE; > + unsigned int i; > > - gen11_gt_irq_postinstall(gt); > + for_each_gt(gt, dev_priv, i) > + gen11_gt_irq_postinstall(gt); > > GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked); > > -- > 2.39.2 >
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index dea1a117f3fa1..c027fd5189b85 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2858,10 +2858,13 @@ static void dg1_irq_reset(struct drm_i915_private *dev_priv) { struct intel_gt *gt = to_gt(dev_priv); struct intel_uncore *uncore = gt->uncore; + unsigned int i; dg1_master_intr_disable(dev_priv->uncore.regs); - gen11_gt_irq_reset(gt); + for_each_gt(gt, dev_priv, i) + gen11_gt_irq_reset(gt); + gen11_display_irq_reset(dev_priv); GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_); @@ -3646,8 +3649,10 @@ static void dg1_irq_postinstall(struct drm_i915_private *dev_priv) struct intel_gt *gt = to_gt(dev_priv); struct intel_uncore *uncore = gt->uncore; u32 gu_misc_masked = GEN11_GU_MISC_GSE; + unsigned int i; - gen11_gt_irq_postinstall(gt); + for_each_gt(gt, dev_priv, i) + gen11_gt_irq_postinstall(gt); GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
In multi-gt systems IRQs need to be reset and enabled per GT. This might add some redundancy when handling interrupts for engines that might not exist in every tile, but helps to keep the code cleaner and more understandable. Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- Hi, The rsults of this patch are more than promising as we are able to have MTL booting and executing basic tests.(*) Thank you Daniele and Matt for the valuable exchange of opinions. Amdo (*) https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115465v5/index.html? Changelog ========= v3 -> v4 - do not change the initial gt and uncore initialization in order to gain a better understanding at a glance of the purpose of all the local variables. v2 -> v3 - keep GUnit irq initialization out of the for_each_gt() loop as the media GT doesn't have a GUnit. v1 -> v2 - improve description in the commit log. drivers/gpu/drm/i915/i915_irq.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)