All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: disable shared panel fitter for pipe
@ 2013-02-08 14:35 Mika Kuoppala
  2013-02-08 14:35 ` [PATCH 2/2] drm/i915: clean up panel fitter handling in lvds Mika Kuoppala
  2013-02-09 12:13 ` [Intel-gfx] [PATCH 1/2] drm/i915: disable shared panel fitter for pipe Daniel Vetter
  0 siblings, 2 replies; 5+ messages in thread
From: Mika Kuoppala @ 2013-02-08 14:35 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala, stable

If encoder is switched off by BIOS, but the panel fitter is left on,
we never try to turn off the panel fitter and leave it still attached
to the pipe - which can cause blurry output elsewhere.

Based on work by Chris Wilson <chris@chris-wilson.co.uk>

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=58867
Cc: stable@vger.kernel.org
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d75c6a0..9b5f0fb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3656,6 +3656,15 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 
 	intel_disable_plane(dev_priv, plane, pipe);
 	intel_disable_pipe(dev_priv, pipe);
+
+	if (!HAS_PCH_SPLIT(dev)) {
+		const u32 pctl = I915_READ(PFIT_CONTROL);
+
+		if ((pctl & PFIT_ENABLE) &&
+		    ((pctl & PFIT_PIPE_MASK) >> PFIT_PIPE_SHIFT) == pipe)
+			I915_WRITE(PFIT_CONTROL, 0);
+	}
+
 	intel_disable_pll(dev_priv, pipe);
 
 	intel_crtc->active = false;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/i915: clean up panel fitter handling in lvds
  2013-02-08 14:35 [PATCH 1/2] drm/i915: disable shared panel fitter for pipe Mika Kuoppala
@ 2013-02-08 14:35 ` Mika Kuoppala
  2013-02-09 12:19   ` Daniel Vetter
  2013-02-12 14:18   ` Daniel Vetter
  2013-02-09 12:13 ` [Intel-gfx] [PATCH 1/2] drm/i915: disable shared panel fitter for pipe Daniel Vetter
  1 sibling, 2 replies; 5+ messages in thread
From: Mika Kuoppala @ 2013-02-08 14:35 UTC (permalink / raw)
  To: intel-gfx

commit c1d1f5aeda2033d96e872f416388653f05d4c16d
Author: Mika Kuoppala <mika.kuoppala@intel.com>
Date:   Tue Feb 5 17:26:52 2013 +0200

    drm/i915: disable shared panel fitter for pipe

moved panel fit disabling to be crtc property. Thus
the need to explicitly turn pfit off in encoder side
became obsolete. Take advantage of that and get rid of
pfit_dirty state handling in lvds.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    5 ++++
 drivers/gpu/drm/i915/intel_lvds.c    |   47 +++++++++++++++++-----------------
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9b5f0fb..175538d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3615,6 +3615,11 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	intel_update_watermarks(dev);
 
 	intel_enable_pll(dev_priv, pipe);
+
+	for_each_encoder_on_crtc(dev, crtc, encoder)
+		if (encoder->pre_enable)
+			encoder->pre_enable(encoder);
+
 	intel_enable_pipe(dev_priv, pipe, false);
 	intel_enable_plane(dev_priv, plane, pipe);
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 7e4ec63..f93058a 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -51,7 +51,6 @@ struct intel_lvds_encoder {
 
 	u32 pfit_control;
 	u32 pfit_pgm_ratios;
-	bool pfit_dirty;
 	bool is_dual_link;
 	u32 reg;
 
@@ -151,6 +150,29 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
 	I915_WRITE(lvds_encoder->reg, temp);
 }
 
+static void intel_pre_enable_lvds(struct intel_encoder *encoder)
+{
+	struct drm_device *dev = encoder->base.dev;
+	struct intel_lvds_encoder *enc = to_lvds_encoder(&encoder->base);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (HAS_PCH_SPLIT(dev) || !enc->pfit_control)
+		return;
+
+	/*
+	 * Enable automatic panel scaling so that non-native modes
+	 * fill the screen.  The panel fitter should only be
+	 * adjusted whilst the pipe is disabled, according to
+	 * register description and PRM.
+	 */
+	DRM_DEBUG_KMS("applying panel-fitter: %x, %x\n",
+		      enc->pfit_control,
+		      enc->pfit_pgm_ratios);
+
+	I915_WRITE(PFIT_PGM_RATIOS, enc->pfit_pgm_ratios);
+	I915_WRITE(PFIT_CONTROL, enc->pfit_control);
+}
+
 /**
  * Sets the power state for the panel.
  */
@@ -172,22 +194,6 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
 
 	I915_WRITE(lvds_encoder->reg, I915_READ(lvds_encoder->reg) | LVDS_PORT_EN);
 
-	if (lvds_encoder->pfit_dirty) {
-		/*
-		 * Enable automatic panel scaling so that non-native modes
-		 * fill the screen.  The panel fitter should only be
-		 * adjusted whilst the pipe is disabled, according to
-		 * register description and PRM.
-		 */
-		DRM_DEBUG_KMS("applying panel-fitter: %x, %x\n",
-			      lvds_encoder->pfit_control,
-			      lvds_encoder->pfit_pgm_ratios);
-
-		I915_WRITE(PFIT_PGM_RATIOS, lvds_encoder->pfit_pgm_ratios);
-		I915_WRITE(PFIT_CONTROL, lvds_encoder->pfit_control);
-		lvds_encoder->pfit_dirty = false;
-	}
-
 	I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
 	POSTING_READ(lvds_encoder->reg);
 	if (wait_for((I915_READ(stat_reg) & PP_ON) != 0, 1000))
@@ -217,11 +223,6 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
 	if (wait_for((I915_READ(stat_reg) & PP_ON) == 0, 1000))
 		DRM_ERROR("timed out waiting for panel to power off\n");
 
-	if (lvds_encoder->pfit_control) {
-		I915_WRITE(PFIT_CONTROL, 0);
-		lvds_encoder->pfit_dirty = true;
-	}
-
 	I915_WRITE(lvds_encoder->reg, I915_READ(lvds_encoder->reg) & ~LVDS_PORT_EN);
 	POSTING_READ(lvds_encoder->reg);
 }
@@ -461,7 +462,6 @@ out:
 	    pfit_pgm_ratios != lvds_encoder->pfit_pgm_ratios) {
 		lvds_encoder->pfit_control = pfit_control;
 		lvds_encoder->pfit_pgm_ratios = pfit_pgm_ratios;
-		lvds_encoder->pfit_dirty = true;
 	}
 	dev_priv->lvds_border_bits = border;
 
@@ -1109,6 +1109,7 @@ bool intel_lvds_init(struct drm_device *dev)
 			 DRM_MODE_ENCODER_LVDS);
 
 	intel_encoder->enable = intel_enable_lvds;
+	intel_encoder->pre_enable = intel_pre_enable_lvds;
 	intel_encoder->pre_pll_enable = intel_pre_pll_enable_lvds;
 	intel_encoder->disable = intel_disable_lvds;
 	intel_encoder->get_hw_state = intel_lvds_get_hw_state;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: disable shared panel fitter for pipe
  2013-02-08 14:35 [PATCH 1/2] drm/i915: disable shared panel fitter for pipe Mika Kuoppala
  2013-02-08 14:35 ` [PATCH 2/2] drm/i915: clean up panel fitter handling in lvds Mika Kuoppala
@ 2013-02-09 12:13 ` Daniel Vetter
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2013-02-09 12:13 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx, stable

On Fri, Feb 8, 2013 at 3:35 PM, Mika Kuoppala
<mika.kuoppala@linux.intel.com> wrote:
> If encoder is switched off by BIOS, but the panel fitter is left on,
> we never try to turn off the panel fitter and leave it still attached
> to the pipe - which can cause blurry output elsewhere.
>
> Based on work by Chris Wilson <chris@chris-wilson.co.uk>
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=58867

Tested-by from the reporter is missing.

> Cc: stable@vger.kernel.org
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d75c6a0..9b5f0fb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3656,6 +3656,15 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>
>         intel_disable_plane(dev_priv, plane, pipe);
>         intel_disable_pipe(dev_priv, pipe);
> +
> +       if (!HAS_PCH_SPLIT(dev)) {

This is always true in i9xx_crtc_disable, so you can remove this indentation.
-Daniel

> +               const u32 pctl = I915_READ(PFIT_CONTROL);
> +
> +               if ((pctl & PFIT_ENABLE) &&
> +                   ((pctl & PFIT_PIPE_MASK) >> PFIT_PIPE_SHIFT) == pipe)
> +                       I915_WRITE(PFIT_CONTROL, 0);
> +       }
> +
>         intel_disable_pll(dev_priv, pipe);
>
>         intel_crtc->active = false;
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: clean up panel fitter handling in lvds
  2013-02-08 14:35 ` [PATCH 2/2] drm/i915: clean up panel fitter handling in lvds Mika Kuoppala
@ 2013-02-09 12:19   ` Daniel Vetter
  2013-02-12 14:18   ` Daniel Vetter
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2013-02-09 12:19 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Fri, Feb 8, 2013 at 3:35 PM, Mika Kuoppala
<mika.kuoppala@linux.intel.com> wrote:
> commit c1d1f5aeda2033d96e872f416388653f05d4c16d
> Author: Mika Kuoppala <mika.kuoppala@intel.com>
> Date:   Tue Feb 5 17:26:52 2013 +0200
>
>     drm/i915: disable shared panel fitter for pipe

You can't cite the sha1 of a patch which isn't merged yet, since the
sha1 you have in your git tree will be different than the one this
patch will get when I pull it in. If you want to cite the commit just
say "... previous patch titled 'drm/i915: blabla' in this series ...".

Note that even when a patch is merged into dinq the sha1 might still
change when I do a rebase. Commits are only truly frozen once they've
landed in Dave's tree.

> moved panel fit disabling to be crtc property. Thus
> the need to explicitly turn pfit off in encoder side
> became obsolete. Take advantage of that and get rid of
> pfit_dirty state handling in lvds.

I think you should add that this simplification is only possible due
to the modeset rework - before the crtc helpers could call the pfit
setup code a few times too often.

But imo the important thing is that we now enable/disable the pfit at
the right time in the modeset sequence (when planes are already
disabled). The old code you're removing here didn't do that correctly.

Can you please update the commit message?

Thanks, Daniel
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    5 ++++
>  drivers/gpu/drm/i915/intel_lvds.c    |   47 +++++++++++++++++-----------------
>  2 files changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9b5f0fb..175538d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3615,6 +3615,11 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>         intel_update_watermarks(dev);
>
>         intel_enable_pll(dev_priv, pipe);
> +
> +       for_each_encoder_on_crtc(dev, crtc, encoder)
> +               if (encoder->pre_enable)
> +                       encoder->pre_enable(encoder);
> +
>         intel_enable_pipe(dev_priv, pipe, false);
>         intel_enable_plane(dev_priv, plane, pipe);
>
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 7e4ec63..f93058a 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -51,7 +51,6 @@ struct intel_lvds_encoder {
>
>         u32 pfit_control;
>         u32 pfit_pgm_ratios;
> -       bool pfit_dirty;
>         bool is_dual_link;
>         u32 reg;
>
> @@ -151,6 +150,29 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
>         I915_WRITE(lvds_encoder->reg, temp);
>  }
>
> +static void intel_pre_enable_lvds(struct intel_encoder *encoder)
> +{
> +       struct drm_device *dev = encoder->base.dev;
> +       struct intel_lvds_encoder *enc = to_lvds_encoder(&encoder->base);
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +       if (HAS_PCH_SPLIT(dev) || !enc->pfit_control)
> +               return;
> +
> +       /*
> +        * Enable automatic panel scaling so that non-native modes
> +        * fill the screen.  The panel fitter should only be
> +        * adjusted whilst the pipe is disabled, according to
> +        * register description and PRM.
> +        */
> +       DRM_DEBUG_KMS("applying panel-fitter: %x, %x\n",
> +                     enc->pfit_control,
> +                     enc->pfit_pgm_ratios);
> +
> +       I915_WRITE(PFIT_PGM_RATIOS, enc->pfit_pgm_ratios);
> +       I915_WRITE(PFIT_CONTROL, enc->pfit_control);
> +}
> +
>  /**
>   * Sets the power state for the panel.
>   */
> @@ -172,22 +194,6 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
>
>         I915_WRITE(lvds_encoder->reg, I915_READ(lvds_encoder->reg) | LVDS_PORT_EN);
>
> -       if (lvds_encoder->pfit_dirty) {
> -               /*
> -                * Enable automatic panel scaling so that non-native modes
> -                * fill the screen.  The panel fitter should only be
> -                * adjusted whilst the pipe is disabled, according to
> -                * register description and PRM.
> -                */
> -               DRM_DEBUG_KMS("applying panel-fitter: %x, %x\n",
> -                             lvds_encoder->pfit_control,
> -                             lvds_encoder->pfit_pgm_ratios);
> -
> -               I915_WRITE(PFIT_PGM_RATIOS, lvds_encoder->pfit_pgm_ratios);
> -               I915_WRITE(PFIT_CONTROL, lvds_encoder->pfit_control);
> -               lvds_encoder->pfit_dirty = false;
> -       }
> -
>         I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
>         POSTING_READ(lvds_encoder->reg);
>         if (wait_for((I915_READ(stat_reg) & PP_ON) != 0, 1000))
> @@ -217,11 +223,6 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
>         if (wait_for((I915_READ(stat_reg) & PP_ON) == 0, 1000))
>                 DRM_ERROR("timed out waiting for panel to power off\n");
>
> -       if (lvds_encoder->pfit_control) {
> -               I915_WRITE(PFIT_CONTROL, 0);
> -               lvds_encoder->pfit_dirty = true;
> -       }
> -
>         I915_WRITE(lvds_encoder->reg, I915_READ(lvds_encoder->reg) & ~LVDS_PORT_EN);
>         POSTING_READ(lvds_encoder->reg);
>  }
> @@ -461,7 +462,6 @@ out:
>             pfit_pgm_ratios != lvds_encoder->pfit_pgm_ratios) {
>                 lvds_encoder->pfit_control = pfit_control;
>                 lvds_encoder->pfit_pgm_ratios = pfit_pgm_ratios;
> -               lvds_encoder->pfit_dirty = true;
>         }
>         dev_priv->lvds_border_bits = border;
>
> @@ -1109,6 +1109,7 @@ bool intel_lvds_init(struct drm_device *dev)
>                          DRM_MODE_ENCODER_LVDS);
>
>         intel_encoder->enable = intel_enable_lvds;
> +       intel_encoder->pre_enable = intel_pre_enable_lvds;
>         intel_encoder->pre_pll_enable = intel_pre_pll_enable_lvds;
>         intel_encoder->disable = intel_disable_lvds;
>         intel_encoder->get_hw_state = intel_lvds_get_hw_state;
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: clean up panel fitter handling in lvds
  2013-02-08 14:35 ` [PATCH 2/2] drm/i915: clean up panel fitter handling in lvds Mika Kuoppala
  2013-02-09 12:19   ` Daniel Vetter
@ 2013-02-12 14:18   ` Daniel Vetter
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2013-02-12 14:18 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Fri, Feb 08, 2013 at 04:35:38PM +0200, Mika Kuoppala wrote:
> commit c1d1f5aeda2033d96e872f416388653f05d4c16d
> Author: Mika Kuoppala <mika.kuoppala@intel.com>
> Date:   Tue Feb 5 17:26:52 2013 +0200
> 
>     drm/i915: disable shared panel fitter for pipe
> 
> moved panel fit disabling to be crtc property. Thus
> the need to explicitly turn pfit off in encoder side
> became obsolete. Take advantage of that and get rid of
> pfit_dirty state handling in lvds.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Merged both patches, with the bit of bikeshed applied as discussed on irc.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-02-12 14:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-08 14:35 [PATCH 1/2] drm/i915: disable shared panel fitter for pipe Mika Kuoppala
2013-02-08 14:35 ` [PATCH 2/2] drm/i915: clean up panel fitter handling in lvds Mika Kuoppala
2013-02-09 12:19   ` Daniel Vetter
2013-02-12 14:18   ` Daniel Vetter
2013-02-09 12:13 ` [Intel-gfx] [PATCH 1/2] drm/i915: disable shared panel fitter for pipe Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.