diff mbox

[2/2] drm/i915: clean up panel fitter handling in lvds

Message ID 1360334138-3876-2-git-send-email-mika.kuoppala@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Kuoppala Feb. 8, 2013, 2:35 p.m. UTC 
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(-)

Comments

Daniel Vetter Feb. 9, 2013, 12:19 p.m. UTC | #1 
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 Feb. 12, 2013, 2:18 p.m. UTC | #2 
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
diff mbox

Patch

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;