Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Implement a basic PM interrupt handler
On 4/16/2014 2:11 PM, Ville Syrjälä wrote:
On Tue, Apr 15, 2014 at 07:43:07PM -0700, Ben Widawsky wrote:
On Mon, Apr 14, 2014 at 10:55:53PM +0300, Ville Syrjälä wrote:
On Mon, Apr 14, 2014 at 10:41:14PM +0530, deepa...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.
The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.
This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.
v2: Move the commit about not touching the ringbuffer interrupt to the
snb_* function where it belongs (Rodrigo)
v3: Rebased on Paulo's runtime PM changes
v4: Not well validated, but rebase on
commit 730488b2eddded4497f63f70867b1256cd9e117c
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Fri Mar 7 20:12:32 2014 -0300
drm/i915: kill dev_priv-pm.regsave
v5: Rebased on latest code base. (Deepak)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Conflicts:
drivers/gpu/drm/i915/i915_irq.c
IIRC Daniel doesn't like these conflict markers. So should be dropped.
I like the conflict markers generally. Daniel can kill it if he likes,
but thanks for the input. I've killed it this time around, but I don't
plan on it for the future.
---
drivers/gpu/drm/i915/i915_irq.c | 81 +---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pm.c | 38 ++-
4 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..96c459a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -248,6 +248,50 @@ static bool ivb_can_enable_err_int(struct drm_device *dev)
return true;
}
+/**
+ * bdw_update_pm_irq - update GT interrupt 2
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ *
+ * Copied from the snb function, updated with relevant register offsets
+ */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+
+ assert_spin_locked(dev_priv-irq_lock);
+
+ if (dev_priv-pm.irqs_disabled) {
+ WARN(1, IRQs disabled\n);
+ return;
+ }
+
+ new_val = dev_priv-pm_irq_mask;
+ new_val = ~interrupt_mask;
+ new_val |= (~enabled_irq_mask interrupt_mask);
+
+ if (new_val != dev_priv-pm_irq_mask) {
+ dev_priv-pm_irq_mask = new_val;
+ I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+ dev_priv-pm_irq_mask);
+ POSTING_READ(GEN8_GT_IMR(2));
+ }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
Unnecessary empty line.
Got it, thanks.
static bool cpt_can_enable_serr_int(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -1126,13 +1170,17 @@ static void gen6_pm_rps_work(struct work_struct *work)
spin_lock_irq(dev_priv-irq_lock);
pm_iir = dev_priv-rps.pm_iir;
dev_priv-rps.pm_iir = 0;
- /* Make sure not to corrupt PMIMR state used by ringbuffer code */
- snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ if (IS_BROADWELL(dev_priv-dev))
+ bdw_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ else {
+ /* Make sure not to corrupt PMIMR state used by ringbuffer */
+ snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ /* Make sure we didn't queue anything we're not going to
+* process. */
+ WARN_ON(pm_iir ~dev_priv-pm_rps_events);
+ }
spin_unlock_irq(dev_priv-irq_lock);
- /* Make sure we didn't queue anything we're not going to process. */
- WARN_ON(pm_iir ~dev_priv-pm_rps_events);
Isn't this WARN equally valid for bdw?
So first, let me just mention, this has moved slightly in my latest
version of this patch, but it's still a valid question.
The answer is ambiguous actually. The
Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Implement a basic PM interrupt handler
On 4/16/2014 2:11 PM, Ville Syrjälä wrote:
On Tue, Apr 15, 2014 at 07:43:07PM -0700, Ben Widawsky wrote:
On Mon, Apr 14, 2014 at 10:55:53PM +0300, Ville Syrjälä wrote:
On Mon, Apr 14, 2014 at 10:41:14PM +0530, deepa...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.
The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.
This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.
v2: Move the commit about not touching the ringbuffer interrupt to the
snb_* function where it belongs (Rodrigo)
v3: Rebased on Paulo's runtime PM changes
v4: Not well validated, but rebase on
commit 730488b2eddded4497f63f70867b1256cd9e117c
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Fri Mar 7 20:12:32 2014 -0300
drm/i915: kill dev_priv-pm.regsave
v5: Rebased on latest code base. (Deepak)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Conflicts:
drivers/gpu/drm/i915/i915_irq.c
IIRC Daniel doesn't like these conflict markers. So should be dropped.
I like the conflict markers generally. Daniel can kill it if he likes,
but thanks for the input. I've killed it this time around, but I don't
plan on it for the future.
---
drivers/gpu/drm/i915/i915_irq.c | 81 +---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pm.c | 38 ++-
4 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..96c459a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -248,6 +248,50 @@ static bool ivb_can_enable_err_int(struct drm_device *dev)
return true;
}
+/**
+ * bdw_update_pm_irq - update GT interrupt 2
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ *
+ * Copied from the snb function, updated with relevant register offsets
+ */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+
+ assert_spin_locked(dev_priv-irq_lock);
+
+ if (dev_priv-pm.irqs_disabled) {
+ WARN(1, IRQs disabled\n);
+ return;
+ }
+
+ new_val = dev_priv-pm_irq_mask;
+ new_val = ~interrupt_mask;
+ new_val |= (~enabled_irq_mask interrupt_mask);
+
+ if (new_val != dev_priv-pm_irq_mask) {
+ dev_priv-pm_irq_mask = new_val;
+ I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+ dev_priv-pm_irq_mask);
+ POSTING_READ(GEN8_GT_IMR(2));
+ }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
Unnecessary empty line.
Got it, thanks.
static bool cpt_can_enable_serr_int(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -1126,13 +1170,17 @@ static void gen6_pm_rps_work(struct work_struct *work)
spin_lock_irq(dev_priv-irq_lock);
pm_iir = dev_priv-rps.pm_iir;
dev_priv-rps.pm_iir = 0;
- /* Make sure not to corrupt PMIMR state used by ringbuffer code */
- snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ if (IS_BROADWELL(dev_priv-dev))
+ bdw_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ else {
+ /* Make sure not to corrupt PMIMR state used by ringbuffer */
+ snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ /* Make sure we didn't queue anything we're not going to
+* process. */
+ WARN_ON(pm_iir ~dev_priv-pm_rps_events);
+ }
spin_unlock_irq(dev_priv-irq_lock);
- /* Make sure we didn't queue anything we're not going to process. */
- WARN_ON(pm_iir ~dev_priv-pm_rps_events);
Isn't this WARN equally valid for bdw?
So first, let me just mention, this has moved slightly in my latest
version of this patch, but it's still a valid question.
The answer is ambiguous actually. The
Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Implement a basic PM interrupt handler
On Tue, Apr 15, 2014 at 07:43:07PM -0700, Ben Widawsky wrote: On Mon, Apr 14, 2014 at 10:55:53PM +0300, Ville Syrjälä wrote: On Mon, Apr 14, 2014 at 10:41:14PM +0530, deepa...@intel.com wrote: From: Ben Widawsky benjamin.widaw...@intel.com Almost all of it is reusable from the existing code. The primary difference is we need to do even less in the interrupt handler, since interrupts are not shared in the same way. The patch is mostly a copy-paste of the existing snb+ code, with updates to the relevant parts requiring changes to the interrupt handling. As such it /should/ be relatively trivial. It's highly likely that I missed some places where I need a gen8 version of the PM interrupts, but it has become invisible to me by now. This patch could probably be split into adding the new functions, followed by actually handling the interrupts. Since the code is currently disabled (and broken) I think the patch stands better by itself. v2: Move the commit about not touching the ringbuffer interrupt to the snb_* function where it belongs (Rodrigo) v3: Rebased on Paulo's runtime PM changes v4: Not well validated, but rebase on commit 730488b2eddded4497f63f70867b1256cd9e117c Author: Paulo Zanoni paulo.r.zan...@intel.com Date: Fri Mar 7 20:12:32 2014 -0300 drm/i915: kill dev_priv-pm.regsave v5: Rebased on latest code base. (Deepak) Signed-off-by: Ben Widawsky b...@bwidawsk.net Conflicts: drivers/gpu/drm/i915/i915_irq.c IIRC Daniel doesn't like these conflict markers. So should be dropped. I like the conflict markers generally. Daniel can kill it if he likes, but thanks for the input. I've killed it this time around, but I don't plan on it for the future. Imo leaving them in is just lazy. Either there was some real conflict that required real work, and then you should rev the patch revision and take note about the changes in the in-patch changelog. Or the conflict was trivial, in which case the left-behind marker is meaningless and doesn't add anything useful to the commit message. So please remove them and if it makes sense augment the patch revision log. See e.g. the rebase notes (where I mention the upstream changes that required the rebase) I add for -internal patches. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Implement a basic PM interrupt handler
On Tue, Apr 15, 2014 at 07:43:07PM -0700, Ben Widawsky wrote:
On Mon, Apr 14, 2014 at 10:55:53PM +0300, Ville Syrjälä wrote:
On Mon, Apr 14, 2014 at 10:41:14PM +0530, deepa...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.
The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.
This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.
v2: Move the commit about not touching the ringbuffer interrupt to the
snb_* function where it belongs (Rodrigo)
v3: Rebased on Paulo's runtime PM changes
v4: Not well validated, but rebase on
commit 730488b2eddded4497f63f70867b1256cd9e117c
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Fri Mar 7 20:12:32 2014 -0300
drm/i915: kill dev_priv-pm.regsave
v5: Rebased on latest code base. (Deepak)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Conflicts:
drivers/gpu/drm/i915/i915_irq.c
IIRC Daniel doesn't like these conflict markers. So should be dropped.
I like the conflict markers generally. Daniel can kill it if he likes,
but thanks for the input. I've killed it this time around, but I don't
plan on it for the future.
---
drivers/gpu/drm/i915/i915_irq.c | 81
+---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pm.c | 38 ++-
4 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..96c459a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -248,6 +248,50 @@ static bool ivb_can_enable_err_int(struct drm_device
*dev)
return true;
}
+/**
+ * bdw_update_pm_irq - update GT interrupt 2
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ *
+ * Copied from the snb function, updated with relevant register offsets
+ */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+
+ assert_spin_locked(dev_priv-irq_lock);
+
+ if (dev_priv-pm.irqs_disabled) {
+ WARN(1, IRQs disabled\n);
+ return;
+ }
+
+ new_val = dev_priv-pm_irq_mask;
+ new_val = ~interrupt_mask;
+ new_val |= (~enabled_irq_mask interrupt_mask);
+
+ if (new_val != dev_priv-pm_irq_mask) {
+ dev_priv-pm_irq_mask = new_val;
+ I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+dev_priv-pm_irq_mask);
+ POSTING_READ(GEN8_GT_IMR(2));
+ }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
Unnecessary empty line.
Got it, thanks.
static bool cpt_can_enable_serr_int(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -1126,13 +1170,17 @@ static void gen6_pm_rps_work(struct work_struct
*work)
spin_lock_irq(dev_priv-irq_lock);
pm_iir = dev_priv-rps.pm_iir;
dev_priv-rps.pm_iir = 0;
- /* Make sure not to corrupt PMIMR state used by ringbuffer code */
- snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ if (IS_BROADWELL(dev_priv-dev))
+ bdw_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ else {
+ /* Make sure not to corrupt PMIMR state used by ringbuffer */
+ snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ /* Make sure we didn't queue anything we're not going to
+ * process. */
+ WARN_ON(pm_iir ~dev_priv-pm_rps_events);
+ }
spin_unlock_irq(dev_priv-irq_lock);
- /* Make sure we didn't queue anything we're not going to process. */
- WARN_ON(pm_iir ~dev_priv-pm_rps_events);
Isn't this WARN equally valid for bdw?
So first, let me just mention, this has moved slightly in my latest
Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Implement a basic PM interrupt handler
On Mon, Apr 14, 2014 at 10:55:53PM +0300, Ville Syrjälä wrote:
On Mon, Apr 14, 2014 at 10:41:14PM +0530, deepa...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.
The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.
This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.
v2: Move the commit about not touching the ringbuffer interrupt to the
snb_* function where it belongs (Rodrigo)
v3: Rebased on Paulo's runtime PM changes
v4: Not well validated, but rebase on
commit 730488b2eddded4497f63f70867b1256cd9e117c
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Fri Mar 7 20:12:32 2014 -0300
drm/i915: kill dev_priv-pm.regsave
v5: Rebased on latest code base. (Deepak)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Conflicts:
drivers/gpu/drm/i915/i915_irq.c
IIRC Daniel doesn't like these conflict markers. So should be dropped.
I like the conflict markers generally. Daniel can kill it if he likes,
but thanks for the input. I've killed it this time around, but I don't
plan on it for the future.
---
drivers/gpu/drm/i915/i915_irq.c | 81
+---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pm.c | 38 ++-
4 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..96c459a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -248,6 +248,50 @@ static bool ivb_can_enable_err_int(struct drm_device
*dev)
return true;
}
+/**
+ * bdw_update_pm_irq - update GT interrupt 2
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ *
+ * Copied from the snb function, updated with relevant register offsets
+ */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+
+ assert_spin_locked(dev_priv-irq_lock);
+
+ if (dev_priv-pm.irqs_disabled) {
+ WARN(1, IRQs disabled\n);
+ return;
+ }
+
+ new_val = dev_priv-pm_irq_mask;
+ new_val = ~interrupt_mask;
+ new_val |= (~enabled_irq_mask interrupt_mask);
+
+ if (new_val != dev_priv-pm_irq_mask) {
+ dev_priv-pm_irq_mask = new_val;
+ I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+ dev_priv-pm_irq_mask);
+ POSTING_READ(GEN8_GT_IMR(2));
+ }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
Unnecessary empty line.
Got it, thanks.
static bool cpt_can_enable_serr_int(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -1126,13 +1170,17 @@ static void gen6_pm_rps_work(struct work_struct
*work)
spin_lock_irq(dev_priv-irq_lock);
pm_iir = dev_priv-rps.pm_iir;
dev_priv-rps.pm_iir = 0;
- /* Make sure not to corrupt PMIMR state used by ringbuffer code */
- snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ if (IS_BROADWELL(dev_priv-dev))
+ bdw_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ else {
+ /* Make sure not to corrupt PMIMR state used by ringbuffer */
+ snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ /* Make sure we didn't queue anything we're not going to
+* process. */
+ WARN_ON(pm_iir ~dev_priv-pm_rps_events);
+ }
spin_unlock_irq(dev_priv-irq_lock);
- /* Make sure we didn't queue anything we're not going to process. */
- WARN_ON(pm_iir ~dev_priv-pm_rps_events);
Isn't this WARN equally valid for bdw?
So first, let me just mention, this has moved slightly in my latest
version of this patch, but it's still a valid question.
The answer is ambiguous actually. The WARN is always valid
[Intel-gfx] [PATCH 1/2] drm/i915/bdw: Implement a basic PM interrupt handler
From: Ben Widawsky benjamin.widaw...@intel.com
Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.
The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.
This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.
v2: Move the commit about not touching the ringbuffer interrupt to the
snb_* function where it belongs (Rodrigo)
v3: Rebased on Paulo's runtime PM changes
v4: Not well validated, but rebase on
commit 730488b2eddded4497f63f70867b1256cd9e117c
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Fri Mar 7 20:12:32 2014 -0300
drm/i915: kill dev_priv-pm.regsave
v5: Rebased on latest code base. (Deepak)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Conflicts:
drivers/gpu/drm/i915/i915_irq.c
---
drivers/gpu/drm/i915/i915_irq.c | 81 +---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pm.c | 38 ++-
4 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..96c459a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -248,6 +248,50 @@ static bool ivb_can_enable_err_int(struct drm_device *dev)
return true;
}
+/**
+ * bdw_update_pm_irq - update GT interrupt 2
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ *
+ * Copied from the snb function, updated with relevant register offsets
+ */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+
+ assert_spin_locked(dev_priv-irq_lock);
+
+ if (dev_priv-pm.irqs_disabled) {
+ WARN(1, IRQs disabled\n);
+ return;
+ }
+
+ new_val = dev_priv-pm_irq_mask;
+ new_val = ~interrupt_mask;
+ new_val |= (~enabled_irq_mask interrupt_mask);
+
+ if (new_val != dev_priv-pm_irq_mask) {
+ dev_priv-pm_irq_mask = new_val;
+ I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+ dev_priv-pm_irq_mask);
+ POSTING_READ(GEN8_GT_IMR(2));
+ }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
static bool cpt_can_enable_serr_int(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -1126,13 +1170,17 @@ static void gen6_pm_rps_work(struct work_struct *work)
spin_lock_irq(dev_priv-irq_lock);
pm_iir = dev_priv-rps.pm_iir;
dev_priv-rps.pm_iir = 0;
- /* Make sure not to corrupt PMIMR state used by ringbuffer code */
- snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ if (IS_BROADWELL(dev_priv-dev))
+ bdw_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ else {
+ /* Make sure not to corrupt PMIMR state used by ringbuffer */
+ snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ /* Make sure we didn't queue anything we're not going to
+* process. */
+ WARN_ON(pm_iir ~dev_priv-pm_rps_events);
+ }
spin_unlock_irq(dev_priv-irq_lock);
- /* Make sure we didn't queue anything we're not going to process. */
- WARN_ON(pm_iir ~dev_priv-pm_rps_events);
-
if ((pm_iir dev_priv-pm_rps_events) == 0)
return;
@@ -1324,6 +1372,19 @@ static void snb_gt_irq_handler(struct drm_device *dev,
ivybridge_parity_error_irq_handler(dev, gt_iir);
}
+static void gen8_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
+{
+ if ((pm_iir dev_priv-pm_rps_events) == 0)
+ return;
+
+ spin_lock(dev_priv-irq_lock);
+ dev_priv-rps.pm_iir |= pm_iir dev_priv-pm_rps_events;
+ bdw_disable_pm_irq(dev_priv, pm_iir dev_priv-pm_rps_events);
+ spin_unlock(dev_priv-irq_lock);
+
+ queue_work(dev_priv-wq, dev_priv-rps.work);
+}
+
static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev,
Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Implement a basic PM interrupt handler
On Mon, Apr 14, 2014 at 10:41:14PM +0530, deepa...@intel.com wrote:
From: Ben Widawsky benjamin.widaw...@intel.com
Almost all of it is reusable from the existing code. The primary
difference is we need to do even less in the interrupt handler, since
interrupts are not shared in the same way.
The patch is mostly a copy-paste of the existing snb+ code, with updates
to the relevant parts requiring changes to the interrupt handling. As
such it /should/ be relatively trivial. It's highly likely that I missed
some places where I need a gen8 version of the PM interrupts, but it has
become invisible to me by now.
This patch could probably be split into adding the new functions,
followed by actually handling the interrupts. Since the code is
currently disabled (and broken) I think the patch stands better by
itself.
v2: Move the commit about not touching the ringbuffer interrupt to the
snb_* function where it belongs (Rodrigo)
v3: Rebased on Paulo's runtime PM changes
v4: Not well validated, but rebase on
commit 730488b2eddded4497f63f70867b1256cd9e117c
Author: Paulo Zanoni paulo.r.zan...@intel.com
Date: Fri Mar 7 20:12:32 2014 -0300
drm/i915: kill dev_priv-pm.regsave
v5: Rebased on latest code base. (Deepak)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Conflicts:
drivers/gpu/drm/i915/i915_irq.c
IIRC Daniel doesn't like these conflict markers. So should be dropped.
---
drivers/gpu/drm/i915/i915_irq.c | 81
+---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_pm.c | 38 ++-
4 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7a4d3ae..96c459a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -248,6 +248,50 @@ static bool ivb_can_enable_err_int(struct drm_device
*dev)
return true;
}
+/**
+ * bdw_update_pm_irq - update GT interrupt 2
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ *
+ * Copied from the snb function, updated with relevant register offsets
+ */
+static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+
+ assert_spin_locked(dev_priv-irq_lock);
+
+ if (dev_priv-pm.irqs_disabled) {
+ WARN(1, IRQs disabled\n);
+ return;
+ }
+
+ new_val = dev_priv-pm_irq_mask;
+ new_val = ~interrupt_mask;
+ new_val |= (~enabled_irq_mask interrupt_mask);
+
+ if (new_val != dev_priv-pm_irq_mask) {
+ dev_priv-pm_irq_mask = new_val;
+ I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) |
+dev_priv-pm_irq_mask);
+ POSTING_READ(GEN8_GT_IMR(2));
+ }
+}
+
+void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, mask);
+}
+
+void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+{
+ bdw_update_pm_irq(dev_priv, mask, 0);
+}
+
+
Unnecessary empty line.
static bool cpt_can_enable_serr_int(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -1126,13 +1170,17 @@ static void gen6_pm_rps_work(struct work_struct *work)
spin_lock_irq(dev_priv-irq_lock);
pm_iir = dev_priv-rps.pm_iir;
dev_priv-rps.pm_iir = 0;
- /* Make sure not to corrupt PMIMR state used by ringbuffer code */
- snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ if (IS_BROADWELL(dev_priv-dev))
+ bdw_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ else {
+ /* Make sure not to corrupt PMIMR state used by ringbuffer */
+ snb_enable_pm_irq(dev_priv, dev_priv-pm_rps_events);
+ /* Make sure we didn't queue anything we're not going to
+ * process. */
+ WARN_ON(pm_iir ~dev_priv-pm_rps_events);
+ }
spin_unlock_irq(dev_priv-irq_lock);
- /* Make sure we didn't queue anything we're not going to process. */
- WARN_ON(pm_iir ~dev_priv-pm_rps_events);
Isn't this WARN equally valid for bdw?
-
if ((pm_iir dev_priv-pm_rps_events) == 0)
return;
@@ -1324,6 +1372,19 @@ static void snb_gt_irq_handler(struct drm_device *dev,
ivybridge_parity_error_irq_handler(dev, gt_iir);
}
+static void gen8_rps_irq_handler(struct drm_i915_private *dev_priv, u32
pm_iir)
+{
+ if ((pm_iir dev_priv-pm_rps_events) == 0)
+ return;
+
+ spin_lock(dev_priv-irq_lock);
+ dev_priv-rps.pm_iir |= pm_iir dev_priv-pm_rps_events;
+
