[PATCH 0/3] Restore pipe interrupts registers after power well enabling

[PATCH 0/3] Restore pipe interrupts registers after power well enabling

[Damien Lespiau]

Just like BDW, those registers are on the respective pipe powell, and if we
don't restore them bad things happen! like not flipping anymore because the
primary plane flip done interrupt is disabled.

With that, the torture test that is kms_flip seems to be fairly happy, it's
been running (and passing tests) for a good 20 minutes and is still going.

-- 
Damien

Damien Lespiau (3):
  drm/i915/skl: Make gen8_irq_power_well_post_enable() take a pipe mask
  drm/i915/skl: Restore pipe interrupt registers after power well
    enabling
  drm/i915: Remove unused condition in hsw_power_well_post_enable()

 drivers/gpu/drm/i915/i915_irq.c         | 19 ++++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h        |  3 ++-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 36 +++++++++++++++++++++++++++++++--
 3 files changed, 50 insertions(+), 8 deletions(-)

-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915/skl: Make gen8_irq_power_well_post_enable() take a pipe mask

[Damien Lespiau]

While we only need to restore pipe B/C interrupt registers on BDW when
enabling the power well, skylake a bit more flexible and we'll also need
to restore the pipe A registers as it has its own power well that can be
toggled.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c         | 15 ++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h        |  3 ++-
 drivers/gpu/drm/i915/intel_runtime_pm.c |  3 ++-
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e71d2b2..65fe9e7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3229,15 +3229,20 @@ static void gen8_irq_reset(struct drm_device *dev)
 	ibx_irq_reset(dev);
 }
 
-void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv)
+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;
 
 	spin_lock_irq(&dev_priv->irq_lock);
-	GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_B, dev_priv->de_irq_mask[PIPE_B],
-			  ~dev_priv->de_irq_mask[PIPE_B] | extra_ier);
-	GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_C, dev_priv->de_irq_mask[PIPE_C],
-			  ~dev_priv->de_irq_mask[PIPE_C] | 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);
 	spin_unlock_irq(&dev_priv->irq_lock);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1de8e20..f9a96bd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -818,7 +818,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);
+void gen8_irq_power_well_post_enable(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 6d8e29a..35e0cb6 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -195,7 +195,8 @@ static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv)
 	vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
 
 	if (IS_BROADWELL(dev) || (INTEL_INFO(dev)->gen >= 9))
-		gen8_irq_power_well_post_enable(dev_priv);
+		gen8_irq_power_well_post_enable(dev_priv,
+						1 << PIPE_C | 1 << PIPE_B);
 }
 
 static void hsw_set_power_well(struct drm_i915_private *dev_priv,
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/skl: Make gen8_irq_power_well_post_enable() take a pipe mask

[Paulo Zanoni]

2015-02-13 17:37 GMT-02:00 Damien Lespiau <damien.lespiau@intel.com>:
> While we only need to restore pipe B/C interrupt registers on BDW when
> enabling the power well, skylake a bit more flexible and we'll also need
> to restore the pipe A registers as it has its own power well that can be
> toggled.

- I'll admit I had to open the gcc operator precedence table to check
for those "a & b << c" without parens :)
- Reading "PIPE_C | PIPE_B" increases my anxiety levels.
- Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c         | 15 ++++++++++-----
>  drivers/gpu/drm/i915/intel_drv.h        |  3 ++-
>  drivers/gpu/drm/i915/intel_runtime_pm.c |  3 ++-
>  3 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index e71d2b2..65fe9e7 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3229,15 +3229,20 @@ static void gen8_irq_reset(struct drm_device *dev)
>         ibx_irq_reset(dev);
>  }
>
> -void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv)
> +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;
>
>         spin_lock_irq(&dev_priv->irq_lock);
> -       GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_B, dev_priv->de_irq_mask[PIPE_B],
> -                         ~dev_priv->de_irq_mask[PIPE_B] | extra_ier);
> -       GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_C, dev_priv->de_irq_mask[PIPE_C],
> -                         ~dev_priv->de_irq_mask[PIPE_C] | 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);
>         spin_unlock_irq(&dev_priv->irq_lock);
>  }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1de8e20..f9a96bd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -818,7 +818,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);
> +void gen8_irq_power_well_post_enable(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 6d8e29a..35e0cb6 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -195,7 +195,8 @@ static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv)
>         vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
>
>         if (IS_BROADWELL(dev) || (INTEL_INFO(dev)->gen >= 9))
> -               gen8_irq_power_well_post_enable(dev_priv);
> +               gen8_irq_power_well_post_enable(dev_priv,
> +                                               1 << PIPE_C | 1 << PIPE_B);
>  }
>
>  static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915/skl: Restore pipe interrupt registers after power well enabling

[Damien Lespiau]

The pipe interrupt registers are in the actual pipe power well, so we
need to restore them when re-enable the corresponding power well.

I've also copied what we do on HSW/BDW for VGA, even if the we haven't
enabled unclaimed registers just yet.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c         |  4 ++++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 65fe9e7..292ba89 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3235,6 +3235,10 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
 	uint32_t extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN;
 
 	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],
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 35e0cb6..8989747 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -199,6 +199,34 @@ static void hsw_power_well_post_enable(struct drm_i915_private *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)
+{
+	struct drm_device *dev = dev_priv->dev;
+
+	/*
+	 * After we re-enable the power well, if we touch VGA register 0x3d5
+	 * we'll get unclaimed register interrupts. This stops after we write
+	 * anything to the VGA MSR register. The vgacon module uses this
+	 * register all the time, so if we unbind our driver and, as a
+	 * consequence, bind vgacon, we'll get stuck in an infinite loop at
+	 * console_unlock(). So make here we touch the VGA MSR register, making
+	 * sure vgacon can keep working normally without triggering interrupts
+	 * and error messages.
+	 */
+	if (power_well->data == SKL_DISP_PW_2) {
+		vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
+		outb(inb(VGA_MSR_READ), VGA_MSR_WRITE);
+		vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
+
+		gen8_irq_power_well_post_enable(dev_priv,
+						1 << PIPE_C | 1 << PIPE_B);
+	}
+
+	if (power_well->data == SKL_DISP_PW_1)
+		gen8_irq_power_well_post_enable(dev_priv, 1 << PIPE_A);
+}
+
 static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 			       struct i915_power_well *power_well, bool enable)
 {
@@ -359,6 +387,9 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
 				DRM_ERROR("PG2 distributing status timeout\n");
 		}
 	}
+
+	if (enable)
+		skl_power_well_post_enable(dev_priv, power_well);
 }
 
 static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv,
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/skl: Restore pipe interrupt registers after power well enabling

[Paulo Zanoni]

2015-02-13 17:37 GMT-02:00 Damien Lespiau <damien.lespiau@intel.com>:
> The pipe interrupt registers are in the actual pipe power well, so we
> need to restore them when re-enable the corresponding power well.
>
> I've also copied what we do on HSW/BDW for VGA, even if the we haven't
> enabled unclaimed registers just yet.
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c         |  4 ++++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 65fe9e7..292ba89 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3235,6 +3235,10 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
>         uint32_t extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN;
>
>         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],
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 35e0cb6..8989747 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -199,6 +199,34 @@ static void hsw_power_well_post_enable(struct drm_i915_private *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)
> +{
> +       struct drm_device *dev = dev_priv->dev;
> +
> +       /*
> +        * After we re-enable the power well, if we touch VGA register 0x3d5
> +        * we'll get unclaimed register interrupts. This stops after we write
> +        * anything to the VGA MSR register. The vgacon module uses this
> +        * register all the time, so if we unbind our driver and, as a
> +        * consequence, bind vgacon, we'll get stuck in an infinite loop at
> +        * console_unlock(). So make here we touch the VGA MSR register, making
> +        * sure vgacon can keep working normally without triggering interrupts
> +        * and error messages.
> +        */
> +       if (power_well->data == SKL_DISP_PW_2) {
> +               vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
> +               outb(inb(VGA_MSR_READ), VGA_MSR_WRITE);
> +               vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
> +
> +               gen8_irq_power_well_post_enable(dev_priv,
> +                                               1 << PIPE_C | 1 << PIPE_B);
> +       }
> +
> +       if (power_well->data == SKL_DISP_PW_1)
> +               gen8_irq_power_well_post_enable(dev_priv, 1 << PIPE_A);
> +}
> +
>  static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>                                struct i915_power_well *power_well, bool enable)
>  {
> @@ -359,6 +387,9 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>                                 DRM_ERROR("PG2 distributing status timeout\n");
>                 }
>         }
> +
> +       if (enable)
> +               skl_power_well_post_enable(dev_priv, power_well);

Please take a look at "drm/i915: only run hsw_power_well_post_enable
when really needed": http://patchwork.freedesktop.org/patch/34764/ and
please do the equivalent change here. I also won't complain if you
create "is_enabled" and "enable_requested" variables on
skl_set_power_well, just like we have for hsw_set_power_well.


>  }
>
>  static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv,
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/skl: Restore pipe interrupt registers after power well enabling

[Daniel Vetter]

On Mon, Mar 02, 2015 at 03:37:47PM -0300, Paulo Zanoni wrote:
> 2015-02-13 17:37 GMT-02:00 Damien Lespiau <damien.lespiau@intel.com>:
> > The pipe interrupt registers are in the actual pipe power well, so we
> > need to restore them when re-enable the corresponding power well.
> >
> > I've also copied what we do on HSW/BDW for VGA, even if the we haven't
> > enabled unclaimed registers just yet.
> >
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c         |  4 ++++
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 31 +++++++++++++++++++++++++++++++
> >  2 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 65fe9e7..292ba89 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3235,6 +3235,10 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> >         uint32_t extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN;
> >
> >         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],
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 35e0cb6..8989747 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -199,6 +199,34 @@ static void hsw_power_well_post_enable(struct drm_i915_private *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)
> > +{
> > +       struct drm_device *dev = dev_priv->dev;
> > +
> > +       /*
> > +        * After we re-enable the power well, if we touch VGA register 0x3d5
> > +        * we'll get unclaimed register interrupts. This stops after we write
> > +        * anything to the VGA MSR register. The vgacon module uses this
> > +        * register all the time, so if we unbind our driver and, as a
> > +        * consequence, bind vgacon, we'll get stuck in an infinite loop at
> > +        * console_unlock(). So make here we touch the VGA MSR register, making
> > +        * sure vgacon can keep working normally without triggering interrupts
> > +        * and error messages.
> > +        */
> > +       if (power_well->data == SKL_DISP_PW_2) {
> > +               vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
> > +               outb(inb(VGA_MSR_READ), VGA_MSR_WRITE);
> > +               vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
> > +
> > +               gen8_irq_power_well_post_enable(dev_priv,
> > +                                               1 << PIPE_C | 1 << PIPE_B);
> > +       }
> > +
> > +       if (power_well->data == SKL_DISP_PW_1)
> > +               gen8_irq_power_well_post_enable(dev_priv, 1 << PIPE_A);
> > +}
> > +
> >  static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> >                                struct i915_power_well *power_well, bool enable)
> >  {
> > @@ -359,6 +387,9 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
> >                                 DRM_ERROR("PG2 distributing status timeout\n");
> >                 }
> >         }
> > +
> > +       if (enable)
> > +               skl_power_well_post_enable(dev_priv, power_well);
> 
> Please take a look at "drm/i915: only run hsw_power_well_post_enable
> when really needed": http://patchwork.freedesktop.org/patch/34764/ and
> please do the equivalent change here. I also won't complain if you
> create "is_enabled" and "enable_requested" variables on
> skl_set_power_well, just like we have for hsw_set_power_well.

tbh I wonder whether we should move the irq enable/disable into the crtc
enable/disable functions on gen8+. That would have avoided the "have we
enabled this already" logic, which tends to be fragile in general. Otoh
this works too.
-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

[PATCH 3/3] drm/i915: Remove unused condition in hsw_power_well_post_enable()

[Damien Lespiau]

We don't use this function on gen9, no need for that test here.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8989747..0898550 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -194,7 +194,7 @@ static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv)
 	outb(inb(VGA_MSR_READ), VGA_MSR_WRITE);
 	vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
 
-	if (IS_BROADWELL(dev) || (INTEL_INFO(dev)->gen >= 9))
+	if (IS_BROADWELL(dev))
 		gen8_irq_power_well_post_enable(dev_priv,
 						1 << PIPE_C | 1 << PIPE_B);
 }
-- 
1.8.3.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Remove unused condition in hsw_power_well_post_enable()

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5776
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              281/281              279/281
ILK                                  313/313              313/313
SNB                                  309/309              309/309
IVB                                  382/382              382/382
BYT                                  296/296              296/296
HSW                                  426/426              426/426
BDW                 -1              318/318              317/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gen3_render_mixed_blits      PASS(1)      NRUN(1)
*PNV  igt_gen3_render_tiledx_blits      PASS(1)      CRASH(1)
*BDW  igt_gem_gtt_hog      PASS(1)      DMESG_WARN(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Remove unused condition in hsw_power_well_post_enable()

[Paulo Zanoni]

2015-02-13 17:37 GMT-02:00 Damien Lespiau <damien.lespiau@intel.com>:
> We don't use this function on gen9, no need for that test here.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 8989747..0898550 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -194,7 +194,7 @@ static void hsw_power_well_post_enable(struct drm_i915_private *dev_priv)
>         outb(inb(VGA_MSR_READ), VGA_MSR_WRITE);
>         vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
>
> -       if (IS_BROADWELL(dev) || (INTEL_INFO(dev)->gen >= 9))
> +       if (IS_BROADWELL(dev))
>                 gen8_irq_power_well_post_enable(dev_priv,
>                                                 1 << PIPE_C | 1 << PIPE_B);
>  }
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx