Re: [Intel-gfx] [PATCH 03/11] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

2016-06-28 Thread Goel, Akash



On 6/28/2016 2:05 PM, Tvrtko Ursulin wrote:


On 27/06/16 17:35, Goel, Akash wrote:

On 6/27/2016 9:16 PM, Tvrtko Ursulin wrote:


On 27/06/16 13:16, akash.g...@intel.com wrote:

From: Akash Goel 

So far PM IER/IIR/IMR registers were being used only for Turbo related
interrupts. But interrupts coming from GuC also use the same set.
As a precursor to supporting GuC interrupts, added new low level
routines
so as to allow sharing the programming of PM IER/IIR/IMR registers
between
Turbo & GuC.
Also similar to PM IMR, maintaining a bitmask for PM IER register, to
allow
easy sharing of it between Turbo & GuC without involving a rmw
operation.

Suggested-by: Chris Wilson 
Signed-off-by: Akash Goel 
---
  drivers/gpu/drm/i915/i915_drv.h  |  1 +
  drivers/gpu/drm/i915/i915_irq.c  | 55

  drivers/gpu/drm/i915/intel_drv.h |  6 +
  3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 9ef4919..85a7103 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1806,6 +1806,7 @@ struct drm_i915_private {
  };
  u32 gt_irq_mask;
  u32 pm_irq_mask;
+u32 pm_ier_mask;
  u32 pm_rps_events;
  u32 pipestat_irq_mask[I915_MAX_PIPES];

diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 4378a65..7316ab4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -336,14 +336,52 @@ void gen6_disable_pm_irq(struct drm_i915_private
*dev_priv, uint32_t mask)
  __gen6_disable_pm_irq(dev_priv, mask);
  }

-void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+void gen6_reset_pm_interrupts(struct drm_i915_private *dev_priv,
+   uint32_t reset_mask)


Kernel prefers u32. It is not that overall i915 is clean in that
respect, but every time maintainers merge patches checkpatch shouts
about it, and more noise tougher it is to spot more important issues. I
would appreciate if u32 was used throughout.


Fine, will use u32.


Thanks!


  {
  i915_reg_t reg = gen6_pm_iir(dev_priv);

-spin_lock_irq(&dev_priv->irq_lock);
-I915_WRITE(reg, dev_priv->pm_rps_events);
-I915_WRITE(reg, dev_priv->pm_rps_events);
+assert_spin_locked(&dev_priv->irq_lock);
+
+I915_WRITE(reg, reset_mask);
+I915_WRITE(reg, reset_mask);
  POSTING_READ(reg);
+}
+
+void gen6_enable_pm_interrupts(struct drm_i915_private *dev_priv,
+   uint32_t enable_mask)
+{
+uint32_t new_val;
+
+assert_spin_locked(&dev_priv->irq_lock);
+
+new_val = dev_priv->pm_ier_mask;
+new_val |= enable_mask;
+
+dev_priv->pm_ier_mask = new_val;
+I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+gen6_enable_pm_irq(dev_priv, enable_mask);


Hm, will this be confusing that we will have gen6_enable_pm_interrupts
and gen6_enable_pm_irq, so extremely similar names and same parameters,
but for different use?

Sorry for using confusing, ambiguous names.


Maybe rename the old one to gen6_unmask_pm_irq and name this one
gen6_enable_pm_irq ? If there is really need to have both. Or add some
kerneldoc explaining which one is used for what?


Can I do like this, keep gen6_enable_pm_interrupts as is and rename
gen6_enable_pm_irq to gen6_unmask_pm_irq.
Similarly also rename gen6_disable_pm_irq to gen6_mask_pm_irq.


Yes for mask/unmask, but I think the suffix really needs to be the same
since it is the same functional family.

Fine, so will rename gen6_enable_pm_interrupts to gen6_enable_pm_irq,
and gen6_enable_pm_irq to gen6_unmask_pm_irq

Best regards
Akash


Regards,

Tvrtko

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


Re: [Intel-gfx] [PATCH 03/11] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

2016-06-28 Thread Tvrtko Ursulin


On 27/06/16 17:35, Goel, Akash wrote:

On 6/27/2016 9:16 PM, Tvrtko Ursulin wrote:


On 27/06/16 13:16, akash.g...@intel.com wrote:

From: Akash Goel 

So far PM IER/IIR/IMR registers were being used only for Turbo related
interrupts. But interrupts coming from GuC also use the same set.
As a precursor to supporting GuC interrupts, added new low level
routines
so as to allow sharing the programming of PM IER/IIR/IMR registers
between
Turbo & GuC.
Also similar to PM IMR, maintaining a bitmask for PM IER register, to
allow
easy sharing of it between Turbo & GuC without involving a rmw
operation.

Suggested-by: Chris Wilson 
Signed-off-by: Akash Goel 
---
  drivers/gpu/drm/i915/i915_drv.h  |  1 +
  drivers/gpu/drm/i915/i915_irq.c  | 55

  drivers/gpu/drm/i915/intel_drv.h |  6 +
  3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 9ef4919..85a7103 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1806,6 +1806,7 @@ struct drm_i915_private {
  };
  u32 gt_irq_mask;
  u32 pm_irq_mask;
+u32 pm_ier_mask;
  u32 pm_rps_events;
  u32 pipestat_irq_mask[I915_MAX_PIPES];

diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 4378a65..7316ab4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -336,14 +336,52 @@ void gen6_disable_pm_irq(struct drm_i915_private
*dev_priv, uint32_t mask)
  __gen6_disable_pm_irq(dev_priv, mask);
  }

-void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+void gen6_reset_pm_interrupts(struct drm_i915_private *dev_priv,
+   uint32_t reset_mask)


Kernel prefers u32. It is not that overall i915 is clean in that
respect, but every time maintainers merge patches checkpatch shouts
about it, and more noise tougher it is to spot more important issues. I
would appreciate if u32 was used throughout.


Fine, will use u32.


Thanks!


  {
  i915_reg_t reg = gen6_pm_iir(dev_priv);

-spin_lock_irq(&dev_priv->irq_lock);
-I915_WRITE(reg, dev_priv->pm_rps_events);
-I915_WRITE(reg, dev_priv->pm_rps_events);
+assert_spin_locked(&dev_priv->irq_lock);
+
+I915_WRITE(reg, reset_mask);
+I915_WRITE(reg, reset_mask);
  POSTING_READ(reg);
+}
+
+void gen6_enable_pm_interrupts(struct drm_i915_private *dev_priv,
+   uint32_t enable_mask)
+{
+uint32_t new_val;
+
+assert_spin_locked(&dev_priv->irq_lock);
+
+new_val = dev_priv->pm_ier_mask;
+new_val |= enable_mask;
+
+dev_priv->pm_ier_mask = new_val;
+I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+gen6_enable_pm_irq(dev_priv, enable_mask);


Hm, will this be confusing that we will have gen6_enable_pm_interrupts
and gen6_enable_pm_irq, so extremely similar names and same parameters,
but for different use?

Sorry for using confusing, ambiguous names.


Maybe rename the old one to gen6_unmask_pm_irq and name this one
gen6_enable_pm_irq ? If there is really need to have both. Or add some
kerneldoc explaining which one is used for what?


Can I do like this, keep gen6_enable_pm_interrupts as is and rename
gen6_enable_pm_irq to gen6_unmask_pm_irq.
Similarly also rename gen6_disable_pm_irq to gen6_mask_pm_irq.


Yes for mask/unmask, but I think the suffix really needs to be the same 
since it is the same functional family.


Regards,

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


Re: [Intel-gfx] [PATCH 03/11] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

2016-06-27 Thread Goel, Akash



On 6/27/2016 9:16 PM, Tvrtko Ursulin wrote:


On 27/06/16 13:16, akash.g...@intel.com wrote:

From: Akash Goel 

So far PM IER/IIR/IMR registers were being used only for Turbo related
interrupts. But interrupts coming from GuC also use the same set.
As a precursor to supporting GuC interrupts, added new low level routines
so as to allow sharing the programming of PM IER/IIR/IMR registers
between
Turbo & GuC.
Also similar to PM IMR, maintaining a bitmask for PM IER register, to
allow
easy sharing of it between Turbo & GuC without involving a rmw operation.

Suggested-by: Chris Wilson 
Signed-off-by: Akash Goel 
---
  drivers/gpu/drm/i915/i915_drv.h  |  1 +
  drivers/gpu/drm/i915/i915_irq.c  | 55

  drivers/gpu/drm/i915/intel_drv.h |  6 +
  3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 9ef4919..85a7103 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1806,6 +1806,7 @@ struct drm_i915_private {
  };
  u32 gt_irq_mask;
  u32 pm_irq_mask;
+u32 pm_ier_mask;
  u32 pm_rps_events;
  u32 pipestat_irq_mask[I915_MAX_PIPES];

diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 4378a65..7316ab4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -336,14 +336,52 @@ void gen6_disable_pm_irq(struct drm_i915_private
*dev_priv, uint32_t mask)
  __gen6_disable_pm_irq(dev_priv, mask);
  }

-void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+void gen6_reset_pm_interrupts(struct drm_i915_private *dev_priv,
+   uint32_t reset_mask)


Kernel prefers u32. It is not that overall i915 is clean in that
respect, but every time maintainers merge patches checkpatch shouts
about it, and more noise tougher it is to spot more important issues. I
would appreciate if u32 was used throughout.


Fine, will use u32.




  {
  i915_reg_t reg = gen6_pm_iir(dev_priv);

-spin_lock_irq(&dev_priv->irq_lock);
-I915_WRITE(reg, dev_priv->pm_rps_events);
-I915_WRITE(reg, dev_priv->pm_rps_events);
+assert_spin_locked(&dev_priv->irq_lock);
+
+I915_WRITE(reg, reset_mask);
+I915_WRITE(reg, reset_mask);
  POSTING_READ(reg);
+}
+
+void gen6_enable_pm_interrupts(struct drm_i915_private *dev_priv,
+   uint32_t enable_mask)
+{
+uint32_t new_val;
+
+assert_spin_locked(&dev_priv->irq_lock);
+
+new_val = dev_priv->pm_ier_mask;
+new_val |= enable_mask;
+
+dev_priv->pm_ier_mask = new_val;
+I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+gen6_enable_pm_irq(dev_priv, enable_mask);


Hm, will this be confusing that we will have gen6_enable_pm_interrupts
and gen6_enable_pm_irq, so extremely similar names and same parameters,
but for different use?

Sorry for using confusing, ambiguous names.


Maybe rename the old one to gen6_unmask_pm_irq and name this one
gen6_enable_pm_irq ? If there is really need to have both. Or add some
kerneldoc explaining which one is used for what?


Can I do like this, keep gen6_enable_pm_interrupts as is and rename 
gen6_enable_pm_irq to gen6_unmask_pm_irq.

Similarly also rename gen6_disable_pm_irq to gen6_mask_pm_irq.

Best regards
Akash



+}
+
+void gen6_disable_pm_interrupts(struct drm_i915_private *dev_priv,
+uint32_t disable_mask)
+{
+uint32_t new_val;
+
+assert_spin_locked(&dev_priv->irq_lock);
+
+new_val = dev_priv->pm_ier_mask;
+new_val &= ~disable_mask;
+
+dev_priv->pm_ier_mask = new_val;
+__gen6_disable_pm_irq(dev_priv, disable_mask);
+I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+}
+
+void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+{
+spin_lock_irq(&dev_priv->irq_lock);
+gen6_reset_pm_interrupts(dev_priv, dev_priv->pm_rps_events);
  dev_priv->rps.pm_iir = 0;
  spin_unlock_irq(&dev_priv->irq_lock);
  }
@@ -355,9 +393,7 @@ void gen6_enable_rps_interrupts(struct
drm_i915_private *dev_priv)
  WARN_ON(dev_priv->rps.pm_iir);
  WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) &
dev_priv->pm_rps_events);
  dev_priv->rps.interrupts_enabled = true;
-I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) |
-dev_priv->pm_rps_events);
-gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
+gen6_enable_pm_interrupts(dev_priv, dev_priv->pm_rps_events);

  spin_unlock_irq(&dev_priv->irq_lock);
  }
@@ -379,9 +415,7 @@ void gen6_disable_rps_interrupts(struct
drm_i915_private *dev_priv)

  I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv,
~0));

-__gen6_disable_pm_irq(dev_priv, dev_priv->pm_rps_events);
-I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) &
-~dev_priv->pm_rps_events);
+gen6_disable_pm_interrupts(dev_priv, dev_priv->pm_rps_events);

  spin_unlock_irq(

Re: [Intel-gfx] [PATCH 03/11] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

2016-06-27 Thread Tvrtko Ursulin


On 27/06/16 13:16, akash.g...@intel.com wrote:

From: Akash Goel 

So far PM IER/IIR/IMR registers were being used only for Turbo related
interrupts. But interrupts coming from GuC also use the same set.
As a precursor to supporting GuC interrupts, added new low level routines
so as to allow sharing the programming of PM IER/IIR/IMR registers between
Turbo & GuC.
Also similar to PM IMR, maintaining a bitmask for PM IER register, to allow
easy sharing of it between Turbo & GuC without involving a rmw operation.

Suggested-by: Chris Wilson 
Signed-off-by: Akash Goel 
---
  drivers/gpu/drm/i915/i915_drv.h  |  1 +
  drivers/gpu/drm/i915/i915_irq.c  | 55 
  drivers/gpu/drm/i915/intel_drv.h |  6 +
  3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9ef4919..85a7103 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1806,6 +1806,7 @@ struct drm_i915_private {
};
u32 gt_irq_mask;
u32 pm_irq_mask;
+   u32 pm_ier_mask;
u32 pm_rps_events;
u32 pipestat_irq_mask[I915_MAX_PIPES];

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4378a65..7316ab4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -336,14 +336,52 @@ void gen6_disable_pm_irq(struct drm_i915_private 
*dev_priv, uint32_t mask)
__gen6_disable_pm_irq(dev_priv, mask);
  }

-void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+void gen6_reset_pm_interrupts(struct drm_i915_private *dev_priv,
+  uint32_t reset_mask)


Kernel prefers u32. It is not that overall i915 is clean in that 
respect, but every time maintainers merge patches checkpatch shouts 
about it, and more noise tougher it is to spot more important issues. I 
would appreciate if u32 was used throughout.



  {
i915_reg_t reg = gen6_pm_iir(dev_priv);

-   spin_lock_irq(&dev_priv->irq_lock);
-   I915_WRITE(reg, dev_priv->pm_rps_events);
-   I915_WRITE(reg, dev_priv->pm_rps_events);
+   assert_spin_locked(&dev_priv->irq_lock);
+
+   I915_WRITE(reg, reset_mask);
+   I915_WRITE(reg, reset_mask);
POSTING_READ(reg);
+}
+
+void gen6_enable_pm_interrupts(struct drm_i915_private *dev_priv,
+  uint32_t enable_mask)
+{
+   uint32_t new_val;
+
+   assert_spin_locked(&dev_priv->irq_lock);
+
+   new_val = dev_priv->pm_ier_mask;
+   new_val |= enable_mask;
+
+   dev_priv->pm_ier_mask = new_val;
+   I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+   gen6_enable_pm_irq(dev_priv, enable_mask);


Hm, will this be confusing that we will have gen6_enable_pm_interrupts 
and gen6_enable_pm_irq, so extremely similar names and same parameters, 
but for different use?


Maybe rename the old one to gen6_unmask_pm_irq and name this one 
gen6_enable_pm_irq ? If there is really need to have both. Or add some 
kerneldoc explaining which one is used for what?



+}
+
+void gen6_disable_pm_interrupts(struct drm_i915_private *dev_priv,
+   uint32_t disable_mask)
+{
+   uint32_t new_val;
+
+   assert_spin_locked(&dev_priv->irq_lock);
+
+   new_val = dev_priv->pm_ier_mask;
+   new_val &= ~disable_mask;
+
+   dev_priv->pm_ier_mask = new_val;
+   __gen6_disable_pm_irq(dev_priv, disable_mask);
+   I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+}
+
+void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+{
+   spin_lock_irq(&dev_priv->irq_lock);
+   gen6_reset_pm_interrupts(dev_priv, dev_priv->pm_rps_events);
dev_priv->rps.pm_iir = 0;
spin_unlock_irq(&dev_priv->irq_lock);
  }
@@ -355,9 +393,7 @@ void gen6_enable_rps_interrupts(struct drm_i915_private 
*dev_priv)
WARN_ON(dev_priv->rps.pm_iir);
WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) & dev_priv->pm_rps_events);
dev_priv->rps.interrupts_enabled = true;
-   I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) |
-   dev_priv->pm_rps_events);
-   gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
+   gen6_enable_pm_interrupts(dev_priv, dev_priv->pm_rps_events);

spin_unlock_irq(&dev_priv->irq_lock);
  }
@@ -379,9 +415,7 @@ void gen6_disable_rps_interrupts(struct drm_i915_private 
*dev_priv)

I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv, ~0));

-   __gen6_disable_pm_irq(dev_priv, dev_priv->pm_rps_events);
-   I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) &
-   ~dev_priv->pm_rps_events);
+   gen6_disable_pm_interrupts(dev_priv, dev_priv->pm_rps_events);

spin_unlock_irq(&dev_priv->irq_lock);

@@ -3770,6 +3804,7 @@ static void gen8_gt_irq_postinstall(struct 
drm_i915_private *dev_priv)

[Intel-gfx] [PATCH 03/11] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

2016-06-27 Thread akash . goel
From: Akash Goel 

So far PM IER/IIR/IMR registers were being used only for Turbo related
interrupts. But interrupts coming from GuC also use the same set.
As a precursor to supporting GuC interrupts, added new low level routines
so as to allow sharing the programming of PM IER/IIR/IMR registers between
Turbo & GuC.
Also similar to PM IMR, maintaining a bitmask for PM IER register, to allow
easy sharing of it between Turbo & GuC without involving a rmw operation.

Suggested-by: Chris Wilson 
Signed-off-by: Akash Goel 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/i915_irq.c  | 55 
 drivers/gpu/drm/i915/intel_drv.h |  6 +
 3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9ef4919..85a7103 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1806,6 +1806,7 @@ struct drm_i915_private {
};
u32 gt_irq_mask;
u32 pm_irq_mask;
+   u32 pm_ier_mask;
u32 pm_rps_events;
u32 pipestat_irq_mask[I915_MAX_PIPES];
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4378a65..7316ab4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -336,14 +336,52 @@ void gen6_disable_pm_irq(struct drm_i915_private 
*dev_priv, uint32_t mask)
__gen6_disable_pm_irq(dev_priv, mask);
 }
 
-void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+void gen6_reset_pm_interrupts(struct drm_i915_private *dev_priv,
+  uint32_t reset_mask)
 {
i915_reg_t reg = gen6_pm_iir(dev_priv);
 
-   spin_lock_irq(&dev_priv->irq_lock);
-   I915_WRITE(reg, dev_priv->pm_rps_events);
-   I915_WRITE(reg, dev_priv->pm_rps_events);
+   assert_spin_locked(&dev_priv->irq_lock);
+
+   I915_WRITE(reg, reset_mask);
+   I915_WRITE(reg, reset_mask);
POSTING_READ(reg);
+}
+
+void gen6_enable_pm_interrupts(struct drm_i915_private *dev_priv,
+  uint32_t enable_mask)
+{
+   uint32_t new_val;
+
+   assert_spin_locked(&dev_priv->irq_lock);
+
+   new_val = dev_priv->pm_ier_mask;
+   new_val |= enable_mask;
+
+   dev_priv->pm_ier_mask = new_val;
+   I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+   gen6_enable_pm_irq(dev_priv, enable_mask);
+}
+
+void gen6_disable_pm_interrupts(struct drm_i915_private *dev_priv,
+   uint32_t disable_mask)
+{
+   uint32_t new_val;
+
+   assert_spin_locked(&dev_priv->irq_lock);
+
+   new_val = dev_priv->pm_ier_mask;
+   new_val &= ~disable_mask;
+
+   dev_priv->pm_ier_mask = new_val;
+   __gen6_disable_pm_irq(dev_priv, disable_mask);
+   I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+}
+
+void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+{
+   spin_lock_irq(&dev_priv->irq_lock);
+   gen6_reset_pm_interrupts(dev_priv, dev_priv->pm_rps_events);
dev_priv->rps.pm_iir = 0;
spin_unlock_irq(&dev_priv->irq_lock);
 }
@@ -355,9 +393,7 @@ void gen6_enable_rps_interrupts(struct drm_i915_private 
*dev_priv)
WARN_ON(dev_priv->rps.pm_iir);
WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) & dev_priv->pm_rps_events);
dev_priv->rps.interrupts_enabled = true;
-   I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) |
-   dev_priv->pm_rps_events);
-   gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
+   gen6_enable_pm_interrupts(dev_priv, dev_priv->pm_rps_events);
 
spin_unlock_irq(&dev_priv->irq_lock);
 }
@@ -379,9 +415,7 @@ void gen6_disable_rps_interrupts(struct drm_i915_private 
*dev_priv)
 
I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
-   __gen6_disable_pm_irq(dev_priv, dev_priv->pm_rps_events);
-   I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) &
-   ~dev_priv->pm_rps_events);
+   gen6_disable_pm_interrupts(dev_priv, dev_priv->pm_rps_events);
 
spin_unlock_irq(&dev_priv->irq_lock);
 
@@ -3770,6 +3804,7 @@ static void gen8_gt_irq_postinstall(struct 
drm_i915_private *dev_priv)
gt_interrupts[0] |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
 
dev_priv->pm_irq_mask = 0x;
+   dev_priv->pm_ier_mask = 0x0;
GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3156d8d..2a013fc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1060,6 +1060,12 @@ void gen6_disable_pm_irq(struct drm_i915_private 
*dev_priv, uint32_t mask);
 void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv);
 vo

[Intel-gfx] [PATCH 03/11] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

2016-06-10 Thread akash . goel
From: Akash Goel 

So far PM IER/IIR/IMR registers were being used only for Turbo related
interrupts. But interrupts coming from GuC also use the same set.
As a precursor to supporting GuC interrupts, added new low level routines
so as to allow sharing the programming of PM IER/IIR/IMR registers between
Turbo & GuC.
Also similar to PM IMR, maintaining a bitmask for PM IER register, to allow
easy sharing of it between Turbo & GuC without involving a rmw operation.

Suggested-by: Chris Wilson 
Signed-off-by: Akash Goel 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/i915_irq.c  | 55 
 drivers/gpu/drm/i915/intel_drv.h |  6 +
 3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2a88a46..021dbe5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1789,6 +1789,7 @@ struct drm_i915_private {
};
u32 gt_irq_mask;
u32 pm_irq_mask;
+   u32 pm_ier_mask;
u32 pm_rps_events;
u32 pipestat_irq_mask[I915_MAX_PIPES];
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index caaf1e2..330d87b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -336,14 +336,52 @@ void gen6_disable_pm_irq(struct drm_i915_private 
*dev_priv, uint32_t mask)
__gen6_disable_pm_irq(dev_priv, mask);
 }
 
-void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+void gen6_reset_pm_interrupts(struct drm_i915_private *dev_priv,
+  uint32_t reset_mask)
 {
i915_reg_t reg = gen6_pm_iir(dev_priv);
 
-   spin_lock_irq(&dev_priv->irq_lock);
-   I915_WRITE(reg, dev_priv->pm_rps_events);
-   I915_WRITE(reg, dev_priv->pm_rps_events);
+   assert_spin_locked(&dev_priv->irq_lock);
+
+   I915_WRITE(reg, reset_mask);
+   I915_WRITE(reg, reset_mask);
POSTING_READ(reg);
+}
+
+void gen6_enable_pm_interrupts(struct drm_i915_private *dev_priv,
+  uint32_t enable_mask)
+{
+   uint32_t new_val;
+
+   assert_spin_locked(&dev_priv->irq_lock);
+
+   new_val = dev_priv->pm_ier_mask;
+   new_val |= enable_mask;
+
+   dev_priv->pm_ier_mask = new_val;
+   I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+   gen6_enable_pm_irq(dev_priv, enable_mask);
+}
+
+void gen6_disable_pm_interrupts(struct drm_i915_private *dev_priv,
+   uint32_t disable_mask)
+{
+   uint32_t new_val;
+
+   assert_spin_locked(&dev_priv->irq_lock);
+
+   new_val = dev_priv->pm_ier_mask;
+   new_val &= ~disable_mask;
+
+   dev_priv->pm_ier_mask = new_val;
+   __gen6_disable_pm_irq(dev_priv, disable_mask);
+   I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+}
+
+void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+{
+   spin_lock_irq(&dev_priv->irq_lock);
+   gen6_reset_pm_interrupts(dev_priv, dev_priv->pm_rps_events);
dev_priv->rps.pm_iir = 0;
spin_unlock_irq(&dev_priv->irq_lock);
 }
@@ -355,9 +393,7 @@ void gen6_enable_rps_interrupts(struct drm_i915_private 
*dev_priv)
WARN_ON(dev_priv->rps.pm_iir);
WARN_ON(I915_READ(gen6_pm_iir(dev_priv)) & dev_priv->pm_rps_events);
dev_priv->rps.interrupts_enabled = true;
-   I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) |
-   dev_priv->pm_rps_events);
-   gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
+   gen6_enable_pm_interrupts(dev_priv, dev_priv->pm_rps_events);
 
spin_unlock_irq(&dev_priv->irq_lock);
 }
@@ -391,9 +427,7 @@ void gen6_disable_rps_interrupts(struct drm_i915_private 
*dev_priv)
 
I915_WRITE(GEN6_PMINTRMSK, gen6_sanitize_rps_pm_mask(dev_priv, ~0));
 
-   __gen6_disable_pm_irq(dev_priv, dev_priv->pm_rps_events);
-   I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) &
-   ~dev_priv->pm_rps_events);
+   gen6_disable_pm_interrupts(dev_priv, dev_priv->pm_rps_events);
 
spin_unlock_irq(&dev_priv->irq_lock);
 
@@ -3781,6 +3815,7 @@ static void gen8_gt_irq_postinstall(struct 
drm_i915_private *dev_priv)
gt_interrupts[0] |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
 
dev_priv->pm_irq_mask = 0x;
+   dev_priv->pm_ier_mask = 0x0;
GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9b5f663..2dae2e5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1058,6 +1058,12 @@ void gen6_disable_pm_irq(struct drm_i915_private 
*dev_priv, uint32_t mask);
 void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv);
 vo