* [PATCH 1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+
@ 2016-02-19 18:47 ville.syrjala
2016-02-19 18:47 ` [PATCH 2/2] drm/i915: Add for_each_pipe_masked() ville.syrjala
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: ville.syrjala @ 2016-02-19 18:47 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Starting from BDW the DE_PIPE interrupts for pipe B and C belong to the
relevant display power well. So we should make sure we've finished
processing them before turning off the power well.
The pipe interrupts shouldn't really happen at this point anymore since
we've already shut down the planes/pipes/whatnot, but being a bit
paranoid shouldn't hurt.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 16 ++++++++++++++++
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++++++++++++++++++
3 files changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d56c261ad867..a9048e1b96e5 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3366,6 +3366,22 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
spin_unlock_irq(&dev_priv->irq_lock);
}
+void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
+ unsigned int pipe_mask)
+{
+ spin_lock_irq(&dev_priv->irq_lock);
+ if (pipe_mask & 1 << PIPE_A)
+ GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_A);
+ if (pipe_mask & 1 << PIPE_B)
+ GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_B);
+ if (pipe_mask & 1 << PIPE_C)
+ GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_C);
+ spin_unlock_irq(&dev_priv->irq_lock);
+
+ /* make sure we're done processing display irqs */
+ synchronize_irq(dev_priv->dev->irq);
+}
+
static void cherryview_irq_preinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 285b0570be9c..2618e3026e8b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -993,6 +993,8 @@ static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
int intel_get_crtc_scanline(struct intel_crtc *crtc);
void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
unsigned int pipe_mask);
+void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
+ unsigned int pipe_mask);
/* intel_crt.c */
void intel_crt_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 59e9222223c9..1afa00855ed8 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -284,6 +284,13 @@ static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv)
1 << PIPE_C | 1 << PIPE_B);
}
+static void hsw_power_well_pre_disable(struct drm_i915_private *dev_priv)
+{
+ if (IS_BROADWELL(dev_priv))
+ gen8_irq_power_well_pre_disable(dev_priv,
+ 1 << PIPE_C | 1 << PIPE_B);
+}
+
static void skl_power_well_post_enable(struct drm_i915_private *dev_priv,
struct i915_power_well *power_well)
{
@@ -309,6 +316,14 @@ static void skl_power_well_post_enable(struct drm_i915_private *dev_priv,
}
}
+static void skl_power_well_pre_disable(struct drm_i915_private *dev_priv,
+ struct i915_power_well *power_well)
+{
+ if (power_well->data == SKL_DISP_PW_2)
+ gen8_irq_power_well_pre_disable(dev_priv,
+ 1 << PIPE_C | 1 << PIPE_B);
+}
+
static void hsw_set_power_well(struct drm_i915_private *dev_priv,
struct i915_power_well *power_well, bool enable)
{
@@ -334,6 +349,7 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
} else {
if (enable_requested) {
+ hsw_power_well_pre_disable(dev_priv);
I915_WRITE(HSW_PWR_WELL_DRIVER, 0);
POSTING_READ(HSW_PWR_WELL_DRIVER);
DRM_DEBUG_KMS("Requesting to disable the power well\n");
@@ -663,6 +679,9 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
state_mask = SKL_POWER_WELL_STATE(power_well->data);
is_enabled = tmp & state_mask;
+ if (!enable && enable_requested)
+ skl_power_well_pre_disable(dev_priv, power_well);
+
if (enable) {
if (!enable_requested) {
WARN((tmp & state_mask) &&
--
2.4.10
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/2] drm/i915: Add for_each_pipe_masked()
2016-02-19 18:47 [PATCH 1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+ ville.syrjala
@ 2016-02-19 18:47 ` ville.syrjala
2016-02-22 14:18 ` Imre Deak
2016-02-22 11:18 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+ Patchwork
2016-02-22 13:59 ` [PATCH 1/2] " Imre Deak
2 siblings, 1 reply; 8+ messages in thread
From: ville.syrjala @ 2016-02-19 18:47 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
for_each_pipe_masked() can be used to iterate over the pipes
included in the user provided pipe mask. Removes a few lines of
duplicated code.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/i915_irq.c | 27 +++++++++------------------
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6644c2e354c1..00ab9c0e18e0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -261,6 +261,9 @@ struct i915_hotplug {
#define for_each_pipe(__dev_priv, __p) \
for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes; (__p)++)
+#define for_each_pipe_masked(__dev_priv, __p, __mask) \
+ for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes; (__p)++) \
+ for_each_if ((__mask) & (1 << (__p)))
#define for_each_plane(__dev_priv, __pipe, __p) \
for ((__p) = 0; \
(__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] + 1; \
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a9048e1b96e5..d1a46ef5ab3f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3349,33 +3349,24 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
unsigned int pipe_mask)
{
uint32_t extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN;
+ enum pipe pipe;
spin_lock_irq(&dev_priv->irq_lock);
- if (pipe_mask & 1 << PIPE_A)
- GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_A,
- dev_priv->de_irq_mask[PIPE_A],
- ~dev_priv->de_irq_mask[PIPE_A] | extra_ier);
- if (pipe_mask & 1 << PIPE_B)
- GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_B,
- dev_priv->de_irq_mask[PIPE_B],
- ~dev_priv->de_irq_mask[PIPE_B] | extra_ier);
- if (pipe_mask & 1 << PIPE_C)
- GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_C,
- dev_priv->de_irq_mask[PIPE_C],
- ~dev_priv->de_irq_mask[PIPE_C] | extra_ier);
+ for_each_pipe_masked(dev_priv, pipe, pipe_mask)
+ GEN8_IRQ_INIT_NDX(DE_PIPE, pipe,
+ dev_priv->de_irq_mask[pipe],
+ ~dev_priv->de_irq_mask[pipe] | extra_ier);
spin_unlock_irq(&dev_priv->irq_lock);
}
void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
unsigned int pipe_mask)
{
+ enum pipe pipe;
+
spin_lock_irq(&dev_priv->irq_lock);
- if (pipe_mask & 1 << PIPE_A)
- GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_A);
- if (pipe_mask & 1 << PIPE_B)
- GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_B);
- if (pipe_mask & 1 << PIPE_C)
- GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_C);
+ for_each_pipe_masked(dev_priv, pipe, pipe_mask)
+ GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
spin_unlock_irq(&dev_priv->irq_lock);
/* make sure we're done processing display irqs */
--
2.4.10
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] drm/i915: Add for_each_pipe_masked()
2016-02-19 18:47 ` [PATCH 2/2] drm/i915: Add for_each_pipe_masked() ville.syrjala
@ 2016-02-22 14:18 ` Imre Deak
2016-02-22 17:47 ` Ville Syrjälä
0 siblings, 1 reply; 8+ messages in thread
From: Imre Deak @ 2016-02-22 14:18 UTC (permalink / raw)
To: ville.syrjala, intel-gfx
On pe, 2016-02-19 at 20:47 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> for_each_pipe_masked() can be used to iterate over the pipes
> included in the user provided pipe mask. Removes a few lines of
> duplicated code.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/gpu/drm/i915/i915_irq.c | 27 +++++++++------------------
> 2 files changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 6644c2e354c1..00ab9c0e18e0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -261,6 +261,9 @@ struct i915_hotplug {
>
> #define for_each_pipe(__dev_priv, __p) \
> for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes;
> (__p)++)
> +#define for_each_pipe_masked(__dev_priv, __p, __mask) \
> + for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes;
> (__p)++) \
> + for_each_if ((__mask) & (1 << (__p)))
> #define for_each_plane(__dev_priv, __pipe, __p)
> \
> for ((__p) = 0;
> \
> (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] +
> 1; \
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index a9048e1b96e5..d1a46ef5ab3f 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3349,33 +3349,24 @@ void gen8_irq_power_well_post_enable(struct
> drm_i915_private *dev_priv,
> unsigned int pipe_mask)
> {
> uint32_t extra_ier = GEN8_PIPE_VBLANK |
> GEN8_PIPE_FIFO_UNDERRUN;
> + enum pipe pipe;
>
> spin_lock_irq(&dev_priv->irq_lock);
> - if (pipe_mask & 1 << PIPE_A)
> - GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_A,
> - dev_priv->de_irq_mask[PIPE_A],
> - ~dev_priv->de_irq_mask[PIPE_A] |
> extra_ier);
> - if (pipe_mask & 1 << PIPE_B)
> - GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_B,
> - dev_priv->de_irq_mask[PIPE_B],
> - ~dev_priv->de_irq_mask[PIPE_B] |
> extra_ier);
> - if (pipe_mask & 1 << PIPE_C)
> - GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_C,
> - dev_priv->de_irq_mask[PIPE_C],
> - ~dev_priv->de_irq_mask[PIPE_C] |
> extra_ier);
> + for_each_pipe_masked(dev_priv, pipe, pipe_mask)
> + GEN8_IRQ_INIT_NDX(DE_PIPE, pipe,
> + dev_priv->de_irq_mask[pipe],
> + ~dev_priv->de_irq_mask[pipe] |
> extra_ier);
> spin_unlock_irq(&dev_priv->irq_lock);
> }
>
> void gen8_irq_power_well_pre_disable(struct drm_i915_private
> *dev_priv,
> unsigned int pipe_mask)
> {
> + enum pipe pipe;
> +
> spin_lock_irq(&dev_priv->irq_lock);
> - if (pipe_mask & 1 << PIPE_A)
> - GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_A);
> - if (pipe_mask & 1 << PIPE_B)
> - GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_B);
> - if (pipe_mask & 1 << PIPE_C)
> - GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_C);
> + for_each_pipe_masked(dev_priv, pipe, pipe_mask)
> + GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
> spin_unlock_irq(&dev_priv->irq_lock);
>
> /* make sure we're done processing display irqs */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] drm/i915: Add for_each_pipe_masked()
2016-02-22 14:18 ` Imre Deak
@ 2016-02-22 17:47 ` Ville Syrjälä
0 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2016-02-22 17:47 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
On Mon, Feb 22, 2016 at 04:18:33PM +0200, Imre Deak wrote:
> On pe, 2016-02-19 at 20:47 +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > for_each_pipe_masked() can be used to iterate over the pipes
> > included in the user provided pipe mask. Removes a few lines of
> > duplicated code.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Reviewed-by: Imre Deak <imre.deak@intel.com>
Series pushed to dinq. Thanks for the review.
>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 3 +++
> > drivers/gpu/drm/i915/i915_irq.c | 27 +++++++++------------------
> > 2 files changed, 12 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 6644c2e354c1..00ab9c0e18e0 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -261,6 +261,9 @@ struct i915_hotplug {
> >
> > #define for_each_pipe(__dev_priv, __p) \
> > for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes;
> > (__p)++)
> > +#define for_each_pipe_masked(__dev_priv, __p, __mask) \
> > + for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes;
> > (__p)++) \
> > + for_each_if ((__mask) & (1 << (__p)))
> > #define for_each_plane(__dev_priv, __pipe, __p)
> > \
> > for ((__p) = 0;
> > \
> > (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] +
> > 1; \
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index a9048e1b96e5..d1a46ef5ab3f 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3349,33 +3349,24 @@ void gen8_irq_power_well_post_enable(struct
> > drm_i915_private *dev_priv,
> > unsigned int pipe_mask)
> > {
> > uint32_t extra_ier = GEN8_PIPE_VBLANK |
> > GEN8_PIPE_FIFO_UNDERRUN;
> > + enum pipe pipe;
> >
> > spin_lock_irq(&dev_priv->irq_lock);
> > - if (pipe_mask & 1 << PIPE_A)
> > - GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_A,
> > - dev_priv->de_irq_mask[PIPE_A],
> > - ~dev_priv->de_irq_mask[PIPE_A] |
> > extra_ier);
> > - if (pipe_mask & 1 << PIPE_B)
> > - GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_B,
> > - dev_priv->de_irq_mask[PIPE_B],
> > - ~dev_priv->de_irq_mask[PIPE_B] |
> > extra_ier);
> > - if (pipe_mask & 1 << PIPE_C)
> > - GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_C,
> > - dev_priv->de_irq_mask[PIPE_C],
> > - ~dev_priv->de_irq_mask[PIPE_C] |
> > extra_ier);
> > + for_each_pipe_masked(dev_priv, pipe, pipe_mask)
> > + GEN8_IRQ_INIT_NDX(DE_PIPE, pipe,
> > + dev_priv->de_irq_mask[pipe],
> > + ~dev_priv->de_irq_mask[pipe] |
> > extra_ier);
> > spin_unlock_irq(&dev_priv->irq_lock);
> > }
> >
> > void gen8_irq_power_well_pre_disable(struct drm_i915_private
> > *dev_priv,
> > unsigned int pipe_mask)
> > {
> > + enum pipe pipe;
> > +
> > spin_lock_irq(&dev_priv->irq_lock);
> > - if (pipe_mask & 1 << PIPE_A)
> > - GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_A);
> > - if (pipe_mask & 1 << PIPE_B)
> > - GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_B);
> > - if (pipe_mask & 1 << PIPE_C)
> > - GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_C);
> > + for_each_pipe_masked(dev_priv, pipe, pipe_mask)
> > + GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
> > spin_unlock_irq(&dev_priv->irq_lock);
> >
> > /* make sure we're done processing display irqs */
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+
2016-02-19 18:47 [PATCH 1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+ ville.syrjala
2016-02-19 18:47 ` [PATCH 2/2] drm/i915: Add for_each_pipe_masked() ville.syrjala
@ 2016-02-22 11:18 ` Patchwork
2016-02-22 14:59 ` Ville Syrjälä
2016-02-22 13:59 ` [PATCH 1/2] " Imre Deak
2 siblings, 1 reply; 8+ messages in thread
From: Patchwork @ 2016-02-22 11:18 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Summary ==
Series 3639v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/3639/revisions/1/mbox/
Test kms_force_connector_basic:
Subgroup force-load-detect:
dmesg-fail -> FAIL (snb-x220t)
fail -> DMESG-FAIL (snb-dellxps)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (skl-i5k-2) UNSTABLE
Subgroup suspend-read-crc-pipe-c:
pass -> DMESG-WARN (skl-i5k-2) UNSTABLE
bdw-nuci7 total:165 pass:154 dwarn:0 dfail:0 fail:0 skip:11
bdw-ultra total:168 pass:154 dwarn:0 dfail:0 fail:0 skip:14
bsw-nuc-2 total:168 pass:136 dwarn:1 dfail:0 fail:1 skip:30
byt-nuc total:168 pass:143 dwarn:0 dfail:0 fail:0 skip:25
hsw-gt2 total:168 pass:157 dwarn:0 dfail:1 fail:0 skip:10
ivb-t430s total:168 pass:153 dwarn:0 dfail:0 fail:1 skip:14
skl-i5k-2 total:168 pass:151 dwarn:1 dfail:0 fail:0 skip:16
snb-dellxps total:168 pass:145 dwarn:0 dfail:1 fail:0 skip:22
snb-x220t total:168 pass:145 dwarn:0 dfail:0 fail:2 skip:21
Results at /archive/results/CI_IGT_test/Patchwork_1451/
c278ff791cc73f90079c86f343be16214a0038b7 drm-intel-nightly: 2016y-02m-22d-09h-17m-11s UTC integration manifest
e01691f3763e01e8b842c36a5ec6ac29c0993c21 drm/i915: Add for_each_pipe_masked()
c305df268afa66201880d8ff7aa8c56ce4b4046c drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+
2016-02-22 11:18 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+ Patchwork
@ 2016-02-22 14:59 ` Ville Syrjälä
0 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2016-02-22 14:59 UTC (permalink / raw)
To: intel-gfx
On Mon, Feb 22, 2016 at 11:18:21AM -0000, Patchwork wrote:
> == Summary ==
>
> Series 3639v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/3639/revisions/1/mbox/
>
> Test kms_force_connector_basic:
> Subgroup force-load-detect:
> dmesg-fail -> FAIL (snb-x220t)
> fail -> DMESG-FAIL (snb-dellxps)
Maarten broke something I think.
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-b:
> dmesg-warn -> PASS (skl-i5k-2) UNSTABLE
> Subgroup suspend-read-crc-pipe-c:
> pass -> DMESG-WARN (skl-i5k-2) UNSTABLE
The usual cpu_hotplug lockdep fail.
>
> bdw-nuci7 total:165 pass:154 dwarn:0 dfail:0 fail:0 skip:11
> bdw-ultra total:168 pass:154 dwarn:0 dfail:0 fail:0 skip:14
> bsw-nuc-2 total:168 pass:136 dwarn:1 dfail:0 fail:1 skip:30
> byt-nuc total:168 pass:143 dwarn:0 dfail:0 fail:0 skip:25
> hsw-gt2 total:168 pass:157 dwarn:0 dfail:1 fail:0 skip:10
> ivb-t430s total:168 pass:153 dwarn:0 dfail:0 fail:1 skip:14
> skl-i5k-2 total:168 pass:151 dwarn:1 dfail:0 fail:0 skip:16
> snb-dellxps total:168 pass:145 dwarn:0 dfail:1 fail:0 skip:22
> snb-x220t total:168 pass:145 dwarn:0 dfail:0 fail:2 skip:21
>
> Results at /archive/results/CI_IGT_test/Patchwork_1451/
>
> c278ff791cc73f90079c86f343be16214a0038b7 drm-intel-nightly: 2016y-02m-22d-09h-17m-11s UTC integration manifest
> e01691f3763e01e8b842c36a5ec6ac29c0993c21 drm/i915: Add for_each_pipe_masked()
> c305df268afa66201880d8ff7aa8c56ce4b4046c drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+
2016-02-19 18:47 [PATCH 1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+ ville.syrjala
2016-02-19 18:47 ` [PATCH 2/2] drm/i915: Add for_each_pipe_masked() ville.syrjala
2016-02-22 11:18 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+ Patchwork
@ 2016-02-22 13:59 ` Imre Deak
2016-02-22 14:16 ` Ville Syrjälä
2 siblings, 1 reply; 8+ messages in thread
From: Imre Deak @ 2016-02-22 13:59 UTC (permalink / raw)
To: ville.syrjala, intel-gfx
On pe, 2016-02-19 at 20:47 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Starting from BDW the DE_PIPE interrupts for pipe B and C belong to the
> relevant display power well. So we should make sure we've finished
> processing them before turning off the power well.
>
> The pipe interrupts shouldn't really happen at this point anymore since
> we've already shut down the planes/pipes/whatnot, but being a bit
> paranoid shouldn't hurt.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 16 ++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++++++++++++++++++
> 3 files changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d56c261ad867..a9048e1b96e5 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3366,6 +3366,22 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> spin_unlock_irq(&dev_priv->irq_lock);
> }
>
> +void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
> + unsigned int pipe_mask)
> +{
> + spin_lock_irq(&dev_priv->irq_lock);
> + if (pipe_mask & 1 << PIPE_A)
> + GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_A);
> + if (pipe_mask & 1 << PIPE_B)
> + GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_B);
> + if (pipe_mask & 1 << PIPE_C)
> + GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_C);
> + spin_unlock_irq(&dev_priv->irq_lock);
> +
> + /* make sure we're done processing display irqs */
> + synchronize_irq(dev_priv->dev->irq);
> +}
> +
> static void cherryview_irq_preinstall(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 285b0570be9c..2618e3026e8b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -993,6 +993,8 @@ static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
> int intel_get_crtc_scanline(struct intel_crtc *crtc);
> void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> unsigned int pipe_mask);
> +void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
> + unsigned int pipe_mask);
>
> /* intel_crt.c */
> void intel_crt_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 59e9222223c9..1afa00855ed8 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -284,6 +284,13 @@ static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv)
> 1 << PIPE_C | 1 << PIPE_B);
> }
>
> +static void hsw_power_well_pre_disable(struct drm_i915_private *dev_priv)
> +{
> + if (IS_BROADWELL(dev_priv))
> + gen8_irq_power_well_pre_disable(dev_priv,
> + 1 << PIPE_C | 1 << PIPE_B);
> +}
> +
> static void skl_power_well_post_enable(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> @@ -309,6 +316,14 @@ static void skl_power_well_post_enable(struct drm_i915_private *dev_priv,
> }
> }
>
> +static void skl_power_well_pre_disable(struct drm_i915_private *dev_priv,
> + struct i915_power_well *power_well)
> +{
> + if (power_well->data == SKL_DISP_PW_2)
> + gen8_irq_power_well_pre_disable(dev_priv,
> + 1 << PIPE_C | 1 << PIPE_B);
> +}
> +
> static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well, bool enable)
> {
> @@ -334,6 +349,7 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>
> } else {
> if (enable_requested) {
> + hsw_power_well_pre_disable(dev_priv);
> I915_WRITE(HSW_PWR_WELL_DRIVER, 0);
> POSTING_READ(HSW_PWR_WELL_DRIVER);
> DRM_DEBUG_KMS("Requesting to disable the power well\n");
> @@ -663,6 +679,9 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
> state_mask = SKL_POWER_WELL_STATE(power_well->data);
> is_enabled = tmp & state_mask;
>
> + if (!enable && enable_requested)
> + skl_power_well_pre_disable(dev_priv, power_well);
> +
Why not putting this in the existing if branch? Either way:
Reviewed-by: Imre Deak <imre.deak@intel.com>
> if (enable) {
> if (!enable_requested) {
> WARN((tmp & state_mask) &&
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+
2016-02-22 13:59 ` [PATCH 1/2] " Imre Deak
@ 2016-02-22 14:16 ` Ville Syrjälä
0 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2016-02-22 14:16 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
On Mon, Feb 22, 2016 at 03:59:28PM +0200, Imre Deak wrote:
> On pe, 2016-02-19 at 20:47 +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Starting from BDW the DE_PIPE interrupts for pipe B and C belong to the
> > relevant display power well. So we should make sure we've finished
> > processing them before turning off the power well.
> >
> > The pipe interrupts shouldn't really happen at this point anymore since
> > we've already shut down the planes/pipes/whatnot, but being a bit
> > paranoid shouldn't hurt.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 16 ++++++++++++++++
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++++++++++++++++++
> > 3 files changed, 37 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index d56c261ad867..a9048e1b96e5 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3366,6 +3366,22 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> > spin_unlock_irq(&dev_priv->irq_lock);
> > }
> >
> > +void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
> > + unsigned int pipe_mask)
> > +{
> > + spin_lock_irq(&dev_priv->irq_lock);
> > + if (pipe_mask & 1 << PIPE_A)
> > + GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_A);
> > + if (pipe_mask & 1 << PIPE_B)
> > + GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_B);
> > + if (pipe_mask & 1 << PIPE_C)
> > + GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_C);
> > + spin_unlock_irq(&dev_priv->irq_lock);
> > +
> > + /* make sure we're done processing display irqs */
> > + synchronize_irq(dev_priv->dev->irq);
> > +}
> > +
> > static void cherryview_irq_preinstall(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 285b0570be9c..2618e3026e8b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -993,6 +993,8 @@ static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
> > int intel_get_crtc_scanline(struct intel_crtc *crtc);
> > void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> > unsigned int pipe_mask);
> > +void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
> > + unsigned int pipe_mask);
> >
> > /* intel_crt.c */
> > void intel_crt_init(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 59e9222223c9..1afa00855ed8 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -284,6 +284,13 @@ static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv)
> > 1 << PIPE_C | 1 << PIPE_B);
> > }
> >
> > +static void hsw_power_well_pre_disable(struct drm_i915_private *dev_priv)
> > +{
> > + if (IS_BROADWELL(dev_priv))
> > + gen8_irq_power_well_pre_disable(dev_priv,
> > + 1 << PIPE_C | 1 << PIPE_B);
> > +}
> > +
> > static void skl_power_well_post_enable(struct drm_i915_private *dev_priv,
> > struct i915_power_well *power_well)
> > {
> > @@ -309,6 +316,14 @@ static void skl_power_well_post_enable(struct drm_i915_private *dev_priv,
> > }
> > }
> >
> > +static void skl_power_well_pre_disable(struct drm_i915_private *dev_priv,
> > + struct i915_power_well *power_well)
> > +{
> > + if (power_well->data == SKL_DISP_PW_2)
> > + gen8_irq_power_well_pre_disable(dev_priv,
> > + 1 << PIPE_C | 1 << PIPE_B);
> > +}
> > +
> > static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> > struct i915_power_well *power_well, bool enable)
> > {
> > @@ -334,6 +349,7 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> >
> > } else {
> > if (enable_requested) {
> > + hsw_power_well_pre_disable(dev_priv);
> > I915_WRITE(HSW_PWR_WELL_DRIVER, 0);
> > POSTING_READ(HSW_PWR_WELL_DRIVER);
> > DRM_DEBUG_KMS("Requesting to disable the power well\n");
> > @@ -663,6 +679,9 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
> > state_mask = SKL_POWER_WELL_STATE(power_well->data);
> > is_enabled = tmp & state_mask;
> >
> > + if (!enable && enable_requested)
> > + skl_power_well_pre_disable(dev_priv, power_well);
> > +
>
> Why not putting this in the existing if branch? Either way:
> Reviewed-by: Imre Deak <imre.deak@intel.com>
I figured it's better if it mirrors the way the post_enable is done.
>
> > if (enable) {
> > if (!enable_requested) {
> > WARN((tmp & state_mask) &&
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-02-22 17:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-19 18:47 [PATCH 1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+ ville.syrjala
2016-02-19 18:47 ` [PATCH 2/2] drm/i915: Add for_each_pipe_masked() ville.syrjala
2016-02-22 14:18 ` Imre Deak
2016-02-22 17:47 ` Ville Syrjälä
2016-02-22 11:18 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Make sure pipe interrupts are processed before turning off power well on BDW+ Patchwork
2016-02-22 14:59 ` Ville Syrjälä
2016-02-22 13:59 ` [PATCH 1/2] " Imre Deak
2016-02-22 14:16 ` Ville Syrjälä
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).