| Message ID | 1464683307-19475-1-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 31, 2016 at 01:58:27PM +0530, Sagar Arun Kamble wrote: > On Loading, GuC sets PM interrupts routing (bit 31) and clears ARAT > expired interrupt (bit 9). Host turbo also updates this register > in RPS flows. This patch ensures bit 31 and bit 9 setup by GuC persists. > ARAT timer interrupt is needed in GuC for various features. It also > facilitates halting GuC and hence achieving RC6. PM interrupt routing > will not impact RPS interrupt reception by host as GuC will redirect > them. > This patch fixes igt test pm_rc6_residency that was failing with guc > load/submission enabled. Tested with SKL GuC v6.1 and BXT GuC v5.1 and v8.7. > > v2: i915_irq/i915_pm decoupling from intel_guc. (ChrisW) > > v3: restructuring the mask update and rebase w.r.t Ville's patch. (ChrisW) > > v4: Updating the pm_intr_keep during direct_interrupts_to_guc. (Sagar) > > Cc: Chris Harris <chris.harris@intel.com> > Cc: Zhe Wang <zhe1.wang@intel.com> > Cc: Deepak S <deepak.s@intel.com> > Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com> > Cc: Akash Goel <akash.goel@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> I can understand what you mean by this patch, perfect! Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Tue, May 31, 2016 at 09:51:53AM +0100, Chris Wilson wrote: > On Tue, May 31, 2016 at 01:58:27PM +0530, Sagar Arun Kamble wrote: > > On Loading, GuC sets PM interrupts routing (bit 31) and clears ARAT > > expired interrupt (bit 9). Host turbo also updates this register > > in RPS flows. This patch ensures bit 31 and bit 9 setup by GuC persists. > > ARAT timer interrupt is needed in GuC for various features. It also > > facilitates halting GuC and hence achieving RC6. PM interrupt routing > > will not impact RPS interrupt reception by host as GuC will redirect > > them. > > This patch fixes igt test pm_rc6_residency that was failing with guc > > load/submission enabled. Tested with SKL GuC v6.1 and BXT GuC v5.1 and v8.7. > > > > v2: i915_irq/i915_pm decoupling from intel_guc. (ChrisW) > > > > v3: restructuring the mask update and rebase w.r.t Ville's patch. (ChrisW) > > > > v4: Updating the pm_intr_keep during direct_interrupts_to_guc. (Sagar) > > > > Cc: Chris Harris <chris.harris@intel.com> > > Cc: Zhe Wang <zhe1.wang@intel.com> > > Cc: Deepak S <deepak.s@intel.com> > > Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com> > > Cc: Akash Goel <akash.goel@intel.com> > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > I can understand what you mean by this patch, perfect! > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > -Chris Testcase: igt/pm_rc6_residency Tested-by: Matt Roper <matthew.d.roper@intel.com> Merged to dinq. Thanks for the patch and review. Matt > > -- > Chris Wilson, Intel Open Source Technology Centre > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, May 31, 2016 at 04:18:34PM -0700, Matt Roper wrote: > On Tue, May 31, 2016 at 09:51:53AM +0100, Chris Wilson wrote: > > On Tue, May 31, 2016 at 01:58:27PM +0530, Sagar Arun Kamble wrote: > > > On Loading, GuC sets PM interrupts routing (bit 31) and clears ARAT > > > expired interrupt (bit 9). Host turbo also updates this register > > > in RPS flows. This patch ensures bit 31 and bit 9 setup by GuC persists. > > > ARAT timer interrupt is needed in GuC for various features. It also > > > facilitates halting GuC and hence achieving RC6. PM interrupt routing > > > will not impact RPS interrupt reception by host as GuC will redirect > > > them. > > > This patch fixes igt test pm_rc6_residency that was failing with guc > > > load/submission enabled. Tested with SKL GuC v6.1 and BXT GuC v5.1 and v8.7. > > > > > > v2: i915_irq/i915_pm decoupling from intel_guc. (ChrisW) > > > > > > v3: restructuring the mask update and rebase w.r.t Ville's patch. (ChrisW) > > > > > > v4: Updating the pm_intr_keep during direct_interrupts_to_guc. (Sagar) > > > > > > Cc: Chris Harris <chris.harris@intel.com> > > > Cc: Zhe Wang <zhe1.wang@intel.com> > > > Cc: Deepak S <deepak.s@intel.com> > > > Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com> > > > Cc: Akash Goel <akash.goel@intel.com> > > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > > > I can understand what you mean by this patch, perfect! > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > -Chris > > Testcase: igt/pm_rc6_residency > Tested-by: Matt Roper <matthew.d.roper@intel.com> > > Merged to dinq. Thanks for the patch and review. This was only the second patch, it also wants the first patch to always use gen6_sanitize_pm_mask otherwise we loose the interrupt bypass from gen6_rps_idle(). That should have been caught by the testcase... -Chris
On 6/1/2016 12:24 PM, Chris Wilson wrote: > On Tue, May 31, 2016 at 04:18:34PM -0700, Matt Roper wrote: >> On Tue, May 31, 2016 at 09:51:53AM +0100, Chris Wilson wrote: >>> On Tue, May 31, 2016 at 01:58:27PM +0530, Sagar Arun Kamble wrote: >>>> On Loading, GuC sets PM interrupts routing (bit 31) and clears ARAT >>>> expired interrupt (bit 9). Host turbo also updates this register >>>> in RPS flows. This patch ensures bit 31 and bit 9 setup by GuC persists. >>>> ARAT timer interrupt is needed in GuC for various features. It also >>>> facilitates halting GuC and hence achieving RC6. PM interrupt routing >>>> will not impact RPS interrupt reception by host as GuC will redirect >>>> them. >>>> This patch fixes igt test pm_rc6_residency that was failing with guc >>>> load/submission enabled. Tested with SKL GuC v6.1 and BXT GuC v5.1 and v8.7. >>>> >>>> v2: i915_irq/i915_pm decoupling from intel_guc. (ChrisW) >>>> >>>> v3: restructuring the mask update and rebase w.r.t Ville's patch. (ChrisW) >>>> >>>> v4: Updating the pm_intr_keep during direct_interrupts_to_guc. (Sagar) >>>> >>>> Cc: Chris Harris <chris.harris@intel.com> >>>> Cc: Zhe Wang <zhe1.wang@intel.com> >>>> Cc: Deepak S <deepak.s@intel.com> >>>> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com> >>>> Cc: Akash Goel <akash.goel@intel.com> >>>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >>> I can understand what you mean by this patch, perfect! >>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> >>> -Chris >> Testcase: igt/pm_rc6_residency >> Tested-by: Matt Roper <matthew.d.roper@intel.com> >> >> Merged to dinq. Thanks for the patch and review. > This was only the second patch, it also wants the first patch to always > use gen6_sanitize_pm_mask otherwise we loose the interrupt bypass from > gen6_rps_idle(). That should have been caught by the testcase... > -Chris BAT picked up these patches separately. I had submitted them in sequence although I updated current patch twice posting them together. How is this supposed to work? >
On Wed, Jun 01, 2016 at 07:54:42AM +0100, Chris Wilson wrote: > On Tue, May 31, 2016 at 04:18:34PM -0700, Matt Roper wrote: > > On Tue, May 31, 2016 at 09:51:53AM +0100, Chris Wilson wrote: > > > On Tue, May 31, 2016 at 01:58:27PM +0530, Sagar Arun Kamble wrote: > > > > On Loading, GuC sets PM interrupts routing (bit 31) and clears ARAT > > > > expired interrupt (bit 9). Host turbo also updates this register > > > > in RPS flows. This patch ensures bit 31 and bit 9 setup by GuC persists. > > > > ARAT timer interrupt is needed in GuC for various features. It also > > > > facilitates halting GuC and hence achieving RC6. PM interrupt routing > > > > will not impact RPS interrupt reception by host as GuC will redirect > > > > them. > > > > This patch fixes igt test pm_rc6_residency that was failing with guc > > > > load/submission enabled. Tested with SKL GuC v6.1 and BXT GuC v5.1 and v8.7. > > > > > > > > v2: i915_irq/i915_pm decoupling from intel_guc. (ChrisW) > > > > > > > > v3: restructuring the mask update and rebase w.r.t Ville's patch. (ChrisW) > > > > > > > > v4: Updating the pm_intr_keep during direct_interrupts_to_guc. (Sagar) > > > > > > > > Cc: Chris Harris <chris.harris@intel.com> > > > > Cc: Zhe Wang <zhe1.wang@intel.com> > > > > Cc: Deepak S <deepak.s@intel.com> > > > > Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com> > > > > Cc: Akash Goel <akash.goel@intel.com> > > > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > > > > > I can understand what you mean by this patch, perfect! > > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > > -Chris > > > > Testcase: igt/pm_rc6_residency > > Tested-by: Matt Roper <matthew.d.roper@intel.com> > > > > Merged to dinq. Thanks for the patch and review. > > This was only the second patch, it also wants the first patch to always > use gen6_sanitize_pm_mask otherwise we loose the interrupt bypass from > gen6_rps_idle(). That should have been caught by the testcase... > -Chris Hmm, I guess is misunderstood the message thread flow here and didn't realize there was another patch necessary as well. I did find that just this one patch caused the IGT to start passing where it had failed before (on BXT), so not sure why I didn't run into problems. I did merge in a couple un-related TSC patches (required to keep my BXT stable in general) before testing, but I don't think that would have changed the behavior here. Matt > > -- > Chris Wilson, Intel Open Source Technology Centre
On 6/1/2016 7:59 PM, Matt Roper wrote: > On Wed, Jun 01, 2016 at 07:54:42AM +0100, Chris Wilson wrote: >> On Tue, May 31, 2016 at 04:18:34PM -0700, Matt Roper wrote: >>> On Tue, May 31, 2016 at 09:51:53AM +0100, Chris Wilson wrote: >>>> On Tue, May 31, 2016 at 01:58:27PM +0530, Sagar Arun Kamble wrote: >>>>> On Loading, GuC sets PM interrupts routing (bit 31) and clears ARAT >>>>> expired interrupt (bit 9). Host turbo also updates this register >>>>> in RPS flows. This patch ensures bit 31 and bit 9 setup by GuC persists. >>>>> ARAT timer interrupt is needed in GuC for various features. It also >>>>> facilitates halting GuC and hence achieving RC6. PM interrupt routing >>>>> will not impact RPS interrupt reception by host as GuC will redirect >>>>> them. >>>>> This patch fixes igt test pm_rc6_residency that was failing with guc >>>>> load/submission enabled. Tested with SKL GuC v6.1 and BXT GuC v5.1 and v8.7. >>>>> >>>>> v2: i915_irq/i915_pm decoupling from intel_guc. (ChrisW) >>>>> >>>>> v3: restructuring the mask update and rebase w.r.t Ville's patch. (ChrisW) >>>>> >>>>> v4: Updating the pm_intr_keep during direct_interrupts_to_guc. (Sagar) >>>>> >>>>> Cc: Chris Harris <chris.harris@intel.com> >>>>> Cc: Zhe Wang <zhe1.wang@intel.com> >>>>> Cc: Deepak S <deepak.s@intel.com> >>>>> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com> >>>>> Cc: Akash Goel <akash.goel@intel.com> >>>>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >>>> I can understand what you mean by this patch, perfect! >>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> >>>> -Chris >>> Testcase: igt/pm_rc6_residency >>> Tested-by: Matt Roper <matthew.d.roper@intel.com> >>> >>> Merged to dinq. Thanks for the patch and review. >> This was only the second patch, it also wants the first patch to always >> use gen6_sanitize_pm_mask otherwise we loose the interrupt bypass from >> gen6_rps_idle(). That should have been caught by the testcase... >> -Chris > Hmm, I guess is misunderstood the message thread flow here and didn't > realize there was another patch necessary as well. I did find that just > this one patch caused the IGT to start passing where it had failed > before (on BXT), so not sure why I didn't run into problems. I did > merge in a couple un-related TSC patches (required to keep my BXT stable > in general) before testing, but I don't think that would have changed > the behavior here. Testcase did not catch it because guc loading/submission is disabled by default. I have submitted BAT request with guc loading/submission enabled on trybot. Waiting for the results. > > > Matt > >> -- >> Chris Wilson, Intel Open Source Technology Centre
On Thu, Jun 02, 2016 at 05:42:08PM +0530, Kamble, Sagar A wrote: > On 6/1/2016 7:59 PM, Matt Roper wrote: > >Hmm, I guess is misunderstood the message thread flow here and didn't > >realize there was another patch necessary as well. I did find that just > >this one patch caused the IGT to start passing where it had failed > >before (on BXT), so not sure why I didn't run into problems. I did > >merge in a couple un-related TSC patches (required to keep my BXT stable > >in general) before testing, but I don't think that would have changed > >the behavior here. > Testcase did not catch it because guc loading/submission is disabled > by default. That's the unspoken question, to make sure that the testcase is sufficient (in case we needed a new test). > I have submitted BAT request with guc loading/submission enabled on trybot. > Waiting for the results. Ah, you need to send the patches as a single series. Pull them into a local branch, then git send-email -4 --to trybot. Fwiw, this is the script I use: #!/bin/bash COMMIT=$1 INTEL=intel DIN=${INTEL}/drm-intel-nightly git fetch ${INTEL} git merge-base --is-ancestor ${DIN} ${COMMIT:-HEAD} || { echo Tree is out of date exit 1 } git send-email --to intel-gfx-trybot@lists.freedesktop.org --suppress-cc=all ${DIN}..${COMMIT} Definitely not fool^Wmeproof! -Chris
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 24f4105..4733b80 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1281,6 +1281,7 @@ static int i915_frequency_info(struct seq_file *m, void *unused) } seq_printf(m, "PM IER=0x%08x IMR=0x%08x ISR=0x%08x IIR=0x%08x, MASK=0x%08x\n", pm_ier, pm_imr, pm_isr, pm_iir, pm_mask); + seq_printf(m, "pm_intr_keep: 0x%08x\n", dev_priv->rps.pm_intr_keep); seq_printf(m, "GT_PERF_STATUS: 0x%08x\n", gt_perf_status); seq_printf(m, "Render p-state ratio: %d\n", (gt_perf_status & (IS_GEN9(dev) ? 0x1ff00 : 0xff00)) >> 8); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 72f0b02..6a69ed9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1132,6 +1132,8 @@ struct intel_gen6_power_mgmt { bool interrupts_enabled; u32 pm_iir; + u32 pm_intr_keep; + /* Frequencies are stored in potentially platform dependent multiples. * In other words, *_freq needs to be multiplied by X to be interesting. * Soft limits are those which are used for the dynamic reclocking done diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index f0d9414..65b5aadc 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -364,19 +364,7 @@ void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv) u32 gen6_sanitize_rps_pm_mask(struct drm_i915_private *dev_priv, u32 mask) { - /* - * SNB,IVB can while VLV,CHV may hard hang on looping batchbuffer - * if GEN6_PM_UP_EI_EXPIRED is masked. - * - * TODO: verify if this can be reproduced on VLV,CHV. - */ - if (INTEL_INFO(dev_priv)->gen <= 7 && !IS_HASWELL(dev_priv)) - mask &= ~GEN6_PM_RP_UP_EI_EXPIRED; - - if (INTEL_INFO(dev_priv)->gen >= 8) - mask &= ~GEN8_PMINTR_REDIRECT_TO_NON_DISP; - - return mask; + return (mask & ~dev_priv->rps.pm_intr_keep); } void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv) @@ -4580,6 +4568,20 @@ void intel_irq_init(struct drm_i915_private *dev_priv) else dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS; + dev_priv->rps.pm_intr_keep = 0; + + /* + * SNB,IVB can while VLV,CHV may hard hang on looping batchbuffer + * if GEN6_PM_UP_EI_EXPIRED is masked. + * + * TODO: verify if this can be reproduced on VLV,CHV. + */ + if (INTEL_INFO(dev_priv)->gen <= 7 && !IS_HASWELL(dev_priv)) + dev_priv->rps.pm_intr_keep |= GEN6_PM_RP_UP_EI_EXPIRED; + + if (INTEL_INFO(dev_priv)->gen >= 8) + dev_priv->rps.pm_intr_keep |= GEN8_PMINTR_REDIRECT_TO_NON_DISP; + INIT_DELAYED_WORK(&dev_priv->gpu_error.hangcheck_work, i915_hangcheck_elapsed); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 86fbf72..ba9d9f3 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7030,7 +7030,7 @@ enum skl_disp_power_wells { #define VLV_RCEDATA _MMIO(0xA0BC) #define GEN6_RC6pp_THRESHOLD _MMIO(0xA0C0) #define GEN6_PMINTRMSK _MMIO(0xA168) -#define GEN8_PMINTR_REDIRECT_TO_NON_DISP (1<<31) +#define GEN8_PMINTR_REDIRECT_TO_NON_DISP (1<<31) #define VLV_PWRDWNUPCTL _MMIO(0xA294) #define GEN9_MEDIA_PG_IDLE_HYSTERESIS _MMIO(0xA0C4) #define GEN9_RENDER_PG_IDLE_HYSTERESIS _MMIO(0xA0C8) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 23345e1..79fb6db 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -103,6 +103,7 @@ static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv) { struct intel_engine_cs *engine; int irqs; + u32 tmp; /* tell all command streamers to forward interrupts and vblank to GuC */ irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS); @@ -117,6 +118,16 @@ static void direct_interrupts_to_guc(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); + + /* + * If GuC has routed PM interrupts to itself, don't keep it. + * and keep other interrupts those are unmasked by GuC. + */ + tmp = I915_READ(GEN6_PMINTRMSK); + if (tmp & GEN8_PMINTR_REDIRECT_TO_NON_DISP) { + dev_priv->rps.pm_intr_keep |= ~(tmp & ~GEN8_PMINTR_REDIRECT_TO_NON_DISP); + dev_priv->rps.pm_intr_keep &= ~GEN8_PMINTR_REDIRECT_TO_NON_DISP; + } } static u32 get_gttype(struct drm_i915_private *dev_priv)
On Loading, GuC sets PM interrupts routing (bit 31) and clears ARAT expired interrupt (bit 9). Host turbo also updates this register in RPS flows. This patch ensures bit 31 and bit 9 setup by GuC persists. ARAT timer interrupt is needed in GuC for various features. It also facilitates halting GuC and hence achieving RC6. PM interrupt routing will not impact RPS interrupt reception by host as GuC will redirect them. This patch fixes igt test pm_rc6_residency that was failing with guc load/submission enabled. Tested with SKL GuC v6.1 and BXT GuC v5.1 and v8.7. v2: i915_irq/i915_pm decoupling from intel_guc. (ChrisW) v3: restructuring the mask update and rebase w.r.t Ville's patch. (ChrisW) v4: Updating the pm_intr_keep during direct_interrupts_to_guc. (Sagar) Cc: Chris Harris <chris.harris@intel.com> Cc: Zhe Wang <zhe1.wang@intel.com> Cc: Deepak S <deepak.s@intel.com> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com> Cc: Akash Goel <akash.goel@intel.com> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_irq.c | 28 +++++++++++++++------------- drivers/gpu/drm/i915/i915_reg.h | 2 +- drivers/gpu/drm/i915/intel_guc_loader.c | 11 +++++++++++ 5 files changed, 30 insertions(+), 14 deletions(-)