Re: [Intel-gfx] [PATCH v3 1/1] drm/i915: Update GEN6_PMINTRMSK setup with GuC enabled

2016-05-30 Thread Kamble, Sagar A



On 5/31/2016 1:48 AM, Chris Wilson wrote:

On Tue, May 31, 2016 at 12:16:11AM +0530, Sagar Arun Kamble wrote:

  void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
@@ -4580,6 +4568,28 @@ 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 GuC submission is enabled keep PM interrupts routed to GuC
+* and unmask ARAT Expired interrupt as it is needed by GuC.
+   */
+   if (INTEL_INFO(dev_priv)->gen >= 8) {
+   if (i915.enable_guc_submission)
+   dev_priv->rps.pm_intr_keep |= 
GEN8_ARAT_EXPIRED_INT_MASK;
+   else
+   dev_priv->rps.pm_intr_keep |= 
GEN8_PMINTR_REDIRECT_TO_NON_DISP;

The logic doesn't match the comment. Don't you want

if (i915.enable_guc_submission) {
dev_priv->rps.pm_intr_keep |= GEN8_ARAT_EXPIRED_INT_MASK;
dev_priv->rps.pm_intr_keep |= GEN8_PMINTR_REDIRECT_TO_NON_DISP;
}
We don't want to keep GEN8_PMINTR_REDIRECT_TO_NON_DISP if GuC submission 
is used.

I added WARN_ON for bits of GEN6_PMINTRMASK if GuC has set expected.
With GuC loaded bit 31 will be set and bit 9 is reset. GuC might reset 
other bits if it needs
those interrupts. I am extending the logic to other interrupts to make 
it extensible.

Certainly this patch is looking more neat now :) ... Thank you.


as implied by the comment? Although I'm guessing the
i915.enable_guc_submission will be refined in future, it should do for
now.


+   WARN_ON(!(I915_READ(GEN6_PMINTRMSK) |
+   GEN8_PMINTR_REDIRECT_TO_NON_DISP));

Always false;


+   WARN_ON(I915_READ(GEN6_PMINTRMSK) |
+   GEN8_ARAT_EXPIRED_INT_MASK);

Always true.

I don't this was quite what you meant :)

But the patch is a lot neater now!
-Chris



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 1/1] drm/i915: Update GEN6_PMINTRMSK setup with GuC enabled

2016-05-30 Thread Chris Wilson
On Tue, May 31, 2016 at 12:16:11AM +0530, Sagar Arun Kamble wrote:
>  void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
> @@ -4580,6 +4568,28 @@ 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 GuC submission is enabled keep PM interrupts routed to GuC
> +  * and unmask ARAT Expired interrupt as it is needed by GuC.
> + */
> + if (INTEL_INFO(dev_priv)->gen >= 8) {
> + if (i915.enable_guc_submission)
> + dev_priv->rps.pm_intr_keep |= 
> GEN8_ARAT_EXPIRED_INT_MASK;
> + else
> + dev_priv->rps.pm_intr_keep |= 
> GEN8_PMINTR_REDIRECT_TO_NON_DISP;

The logic doesn't match the comment. Don't you want

if (i915.enable_guc_submission) {
dev_priv->rps.pm_intr_keep |= GEN8_ARAT_EXPIRED_INT_MASK;
dev_priv->rps.pm_intr_keep |= GEN8_PMINTR_REDIRECT_TO_NON_DISP;
}

as implied by the comment? Although I'm guessing the
i915.enable_guc_submission will be refined in future, it should do for
now.

> + WARN_ON(!(I915_READ(GEN6_PMINTRMSK) |
> + GEN8_PMINTR_REDIRECT_TO_NON_DISP));

Always false;

> + WARN_ON(I915_READ(GEN6_PMINTRMSK) |
> + GEN8_ARAT_EXPIRED_INT_MASK);

Always true.

I don't this was quite what you meant :)

But the patch is a lot neater now!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v3 1/1] drm/i915: Update GEN6_PMINTRMSK setup with GuC enabled

2016-05-30 Thread Sagar Arun Kamble
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)

Cc: Chris Harris 
Cc: Zhe Wang 
Cc: Deepak S 
Cc: Satyanantha, Rama Gopal M 
Cc: Akash Goel 
Signed-off-by: Sagar Arun Kamble 
---
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_irq.c | 36 +
 drivers/gpu/drm/i915/i915_reg.h |  3 ++-
 drivers/gpu/drm/i915/intel_guc_loader.c |  5 +
 4 files changed, 32 insertions(+), 14 deletions(-)

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..96849f2 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,28 @@ 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 GuC submission is enabled keep PM interrupts routed to GuC
+* and unmask ARAT Expired interrupt as it is needed by GuC.
+   */
+   if (INTEL_INFO(dev_priv)->gen >= 8) {
+   if (i915.enable_guc_submission)
+   dev_priv->rps.pm_intr_keep |= 
GEN8_ARAT_EXPIRED_INT_MASK;
+   else
+   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..98c20d7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7030,7 +7030,8 @@ 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   GEN8_ARAT_EXPIRED_INT_MASK   (1<<9)
 #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..51a3939 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -476,6 +476,11 @@ int intel_guc_ucode_load(struct drm_device *dev)
/* The execbuf_client will be recreated. Release it first. */
i915_guc_submission_disable(dev);
 
+   WARN_ON(!(I915_READ(GEN6