Driver needs to ensure that it doesn't mask the PM interrupts, which are unmasked/needed by GuC firmware. For that, Driver maintains a bitmask of interrupts to be kept unmasked, pm_intr_keep. pm_intr_keep was determined across GuC load. GuC gets loaded in different scenarios and it is not going to change the pm_intr_keep so this patch moves its setup to intel_irq_init. This patch fixes incorrect RPS masking and solves bunch of SKL GT3 performance regressions as Host RPS was taking frequency down when GuC is enabled. This was happening due to signed last_adj that becomes negative after 32 UP interrupts and makes cur_freq=min starting the ramp again. Next patch in the series avoids such frequency transitions. Cc: Szwichtenberg, Radoslaw <radoslaw.szwichtenberg@xxxxxxxxx> Cc: Tomi Sarvela <tomi.p.sarvela@xxxxxxxxx> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97017 Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/i915_irq.c | 24 ++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 3 ++- drivers/gpu/drm/i915/intel_guc_loader.c | 26 -------------------------- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index ce5663d..95b4047 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -4193,6 +4193,30 @@ void intel_irq_init(struct drm_i915_private *dev_priv) if (INTEL_INFO(dev_priv)->gen >= 8) dev_priv->rps.pm_intr_keep |= GEN8_PMINTR_REDIRECT_TO_GUC; + /* + * The REDIRECT_TO_GUC bit of the PMINTRMSK register directs all + * (unmasked) PM interrupts to the GuC. All other bits of this + * register *disable* generation of a specific interrupt. + * + * 'pm_intr_keep' indicates bits that are NOT to be set when + * writing to the PM interrupt mask register, i.e. interrupts + * that must not be disabled. + * + * If the GuC is handling these interrupts, then we must not let + * the PM code disable ANY interrupt that the GuC is expecting. + * So for each ENABLED (0) bit in this register, we must SET the + * bit in pm_intr_keep so that it's left enabled for the GuC. + * GuC needs ARAT expired interrupt unmasked hence it is set in + * pm_intr_keep. + * + * Here we CLEAR REDIRECT_TO_GUC bit in pm_intr_keep, which will + * result in the register bit being left SET! + */ + if (HAS_GUC_SCHED(dev_priv)) { + dev_priv->rps.pm_intr_keep |= ARAT_EXPIRED_INTRMSK; + dev_priv->rps.pm_intr_keep &= ~GEN8_PMINTR_REDIRECT_TO_GUC; + } + if (IS_GEN2(dev_priv)) { /* Gen2 doesn't have a hardware frame counter */ dev->max_vblank_count = 0; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 72f9f36..822c147 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7395,7 +7395,8 @@ enum { #define VLV_RCEDATA _MMIO(0xA0BC) #define GEN6_RC6pp_THRESHOLD _MMIO(0xA0C0) #define GEN6_PMINTRMSK _MMIO(0xA168) -#define GEN8_PMINTR_REDIRECT_TO_GUC (1<<31) +#define GEN8_PMINTR_REDIRECT_TO_GUC (1<<31) +#define ARAT_EXPIRED_INTRMSK (1<<9) #define GEN8_MISC_CTRL0 _MMIO(0xA180) #define VLV_PWRDWNUPCTL _MMIO(0xA294) #define GEN9_MEDIA_PG_IDLE_HYSTERESIS _MMIO(0xA0C4) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 2f1cf9a..0eaa8e4 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -114,7 +114,6 @@ static void guc_interrupts_capture(struct drm_i915_private *dev_priv) struct intel_engine_cs *engine; enum intel_engine_id id; int irqs; - u32 tmp; /* tell all command streamers to forward interrupts (but not vblank) to GuC */ irqs = _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING); @@ -128,31 +127,6 @@ static void guc_interrupts_capture(struct drm_i915_private *dev_priv) I915_WRITE(GUC_BCS_RCS_IER, ~irqs); I915_WRITE(GUC_VCS2_VCS1_IER, ~irqs); I915_WRITE(GUC_WD_VECS_IER, ~irqs); - - /* - * The REDIRECT_TO_GUC bit of the PMINTRMSK register directs all - * (unmasked) PM interrupts to the GuC. All other bits of this - * register *disable* generation of a specific interrupt. - * - * 'pm_intr_keep' indicates bits that are NOT to be set when - * writing to the PM interrupt mask register, i.e. interrupts - * that must not be disabled. - * - * If the GuC is handling these interrupts, then we must not let - * the PM code disable ANY interrupt that the GuC is expecting. - * So for each ENABLED (0) bit in this register, we must SET the - * bit in pm_intr_keep so that it's left enabled for the GuC. - * - * OTOH the REDIRECT_TO_GUC bit is initially SET in pm_intr_keep - * (so interrupts go to the DISPLAY unit at first); but here we - * need to CLEAR that bit, which will result in the register bit - * being left SET! - */ - tmp = I915_READ(GEN6_PMINTRMSK); - if (tmp & GEN8_PMINTR_REDIRECT_TO_GUC) { - dev_priv->rps.pm_intr_keep |= ~tmp; - dev_priv->rps.pm_intr_keep &= ~GEN8_PMINTR_REDIRECT_TO_GUC; - } } static u32 get_gttype(struct drm_i915_private *dev_priv) -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
