[PATCH 0/9] drm/i915: Unify some modeset sequences v2

[PATCH 0/9] drm/i915: Unify some modeset sequences v2

[ville.syrjala]

Reposting the whole set for clarity.

Changes from v1:
- Rebasing due to other changes
- Fixed sprite disable in ironlake_crtc_disable()
- Improved some commit messages
- More asserts

[PATCH v2 1/9] drm/i915: Always load the display palette before enabling the pipe

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Loading the palette after the planes are enabled can risk showing
incorrect colors. ILK+ already load the palette before even the pipe
is enabled. Just follow the same order for gen2-4 and VLV.

According to BSpec the requirements for palette access are
display core clock and display PLL running. In certain platforms
just the core clock may be enough. But we definitely should have both
running when this gets called during the modeset.

v2: Amend the commit message with some display PLL/core clock info

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index eda2262..c28b402 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3615,10 +3615,11 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	/* Enable panel fitting for eDP */
 	i9xx_pfit_enable(intel_crtc);
 
+	intel_crtc_load_lut(crtc);
+
 	intel_enable_pipe(dev_priv, pipe, false);
 	intel_enable_plane(dev_priv, plane, pipe);
 
-	intel_crtc_load_lut(crtc);
 	intel_update_fbc(dev);
 
 	/* Give the overlay scaler a chance to enable if it's on this pipe */
@@ -3654,12 +3655,13 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	/* Enable panel fitting for LVDS */
 	i9xx_pfit_enable(intel_crtc);
 
+	intel_crtc_load_lut(crtc);
+
 	intel_enable_pipe(dev_priv, pipe, false);
 	intel_enable_plane(dev_priv, plane, pipe);
 	if (IS_G4X(dev))
 		g4x_fixup_plane(dev_priv, pipe);
 
-	intel_crtc_load_lut(crtc);
 	intel_update_fbc(dev);
 
 	/* Give the overlay scaler a chance to enable if it's on this pipe */
-- 
1.8.1.5

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

Re: [PATCH v2 1/9] drm/i915: Always load the display palette before enabling the pipe

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Tue, Jun 4, 2013 at 7:48 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Loading the palette after the planes are enabled can risk showing
> incorrect colors. ILK+ already load the palette before even the pipe
> is enabled. Just follow the same order for gen2-4 and VLV.
>
> According to BSpec the requirements for palette access are
> display core clock and display PLL running. In certain platforms
> just the core clock may be enough. But we definitely should have both
> running when this gets called during the modeset.
>
> v2: Amend the commit message with some display PLL/core clock info
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index eda2262..c28b402 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3615,10 +3615,11 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>         /* Enable panel fitting for eDP */
>         i9xx_pfit_enable(intel_crtc);
>
> +       intel_crtc_load_lut(crtc);
> +
>         intel_enable_pipe(dev_priv, pipe, false);
>         intel_enable_plane(dev_priv, plane, pipe);
>
> -       intel_crtc_load_lut(crtc);
>         intel_update_fbc(dev);
>
>         /* Give the overlay scaler a chance to enable if it's on this pipe */
> @@ -3654,12 +3655,13 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>         /* Enable panel fitting for LVDS */
>         i9xx_pfit_enable(intel_crtc);
>
> +       intel_crtc_load_lut(crtc);
> +
>         intel_enable_pipe(dev_priv, pipe, false);
>         intel_enable_plane(dev_priv, plane, pipe);
>         if (IS_G4X(dev))
>                 g4x_fixup_plane(dev_priv, pipe);
>
> -       intel_crtc_load_lut(crtc);
>         intel_update_fbc(dev);
>
>         /* Give the overlay scaler a chance to enable if it's on this pipe */
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[PATCH 2/9] drm/i915: Always enable the cursor right after the primary plane

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Follow the same sequence when enabling the cursor plane during
modeset. No point in doing this stuff in different order on different
generations.

This should also avoid a needless wait for vblank for the g4x cursor
workaround when the cursor gets enabled anyway.

Acked-by: Egbert Eich <eich@suse.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c28b402..c2e6e91 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3216,6 +3216,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	intel_enable_pipe(dev_priv, pipe,
 			  intel_crtc->config.has_pch_encoder);
 	intel_enable_plane(dev_priv, plane, pipe);
+	intel_crtc_update_cursor(crtc, true);
 
 	if (intel_crtc->config.has_pch_encoder)
 		ironlake_pch_enable(crtc);
@@ -3224,8 +3225,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	intel_update_fbc(dev);
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_crtc_update_cursor(crtc, true);
-
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
 
@@ -3325,6 +3324,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_enable_pipe(dev_priv, pipe,
 			  intel_crtc->config.has_pch_encoder);
 	intel_enable_plane(dev_priv, plane, pipe);
+	intel_crtc_update_cursor(crtc, true);
 
 	hsw_enable_ips(intel_crtc);
 
@@ -3335,8 +3335,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_update_fbc(dev);
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_crtc_update_cursor(crtc, true);
-
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
 
@@ -3619,12 +3617,12 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	intel_enable_pipe(dev_priv, pipe, false);
 	intel_enable_plane(dev_priv, plane, pipe);
+	intel_crtc_update_cursor(crtc, true);
 
 	intel_update_fbc(dev);
 
 	/* Give the overlay scaler a chance to enable if it's on this pipe */
 	intel_crtc_dpms_overlay(intel_crtc, true);
-	intel_crtc_update_cursor(crtc, true);
 
 	mutex_unlock(&dev_priv->dpio_lock);
 }
@@ -3659,6 +3657,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	intel_enable_pipe(dev_priv, pipe, false);
 	intel_enable_plane(dev_priv, plane, pipe);
+	intel_crtc_update_cursor(crtc, true);
 	if (IS_G4X(dev))
 		g4x_fixup_plane(dev_priv, pipe);
 
@@ -3666,7 +3665,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	/* Give the overlay scaler a chance to enable if it's on this pipe */
 	intel_crtc_dpms_overlay(intel_crtc, true);
-	intel_crtc_update_cursor(crtc, true);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
-- 
1.8.1.5

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

Re: [PATCH 2/9] drm/i915: Always enable the cursor right after the primary plane

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Follow the same sequence when enabling the cursor plane during
> modeset. No point in doing this stuff in different order on different
> generations.
>
> This should also avoid a needless wait for vblank for the g4x cursor
> workaround when the cursor gets enabled anyway.
>
> Acked-by: Egbert Eich <eich@suse.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c28b402..c2e6e91 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3216,6 +3216,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>         intel_enable_pipe(dev_priv, pipe,
>                           intel_crtc->config.has_pch_encoder);
>         intel_enable_plane(dev_priv, plane, pipe);
> +       intel_crtc_update_cursor(crtc, true);
>
>         if (intel_crtc->config.has_pch_encoder)
>                 ironlake_pch_enable(crtc);
> @@ -3224,8 +3225,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>         intel_update_fbc(dev);
>         mutex_unlock(&dev->struct_mutex);
>
> -       intel_crtc_update_cursor(crtc, true);
> -
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 encoder->enable(encoder);
>
> @@ -3325,6 +3324,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>         intel_enable_pipe(dev_priv, pipe,
>                           intel_crtc->config.has_pch_encoder);
>         intel_enable_plane(dev_priv, plane, pipe);
> +       intel_crtc_update_cursor(crtc, true);
>
>         hsw_enable_ips(intel_crtc);
>
> @@ -3335,8 +3335,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>         intel_update_fbc(dev);
>         mutex_unlock(&dev->struct_mutex);
>
> -       intel_crtc_update_cursor(crtc, true);
> -
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 encoder->enable(encoder);
>
> @@ -3619,12 +3617,12 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>
>         intel_enable_pipe(dev_priv, pipe, false);
>         intel_enable_plane(dev_priv, plane, pipe);
> +       intel_crtc_update_cursor(crtc, true);
>
>         intel_update_fbc(dev);
>
>         /* Give the overlay scaler a chance to enable if it's on this pipe */
>         intel_crtc_dpms_overlay(intel_crtc, true);
> -       intel_crtc_update_cursor(crtc, true);
>
>         mutex_unlock(&dev_priv->dpio_lock);
>  }
> @@ -3659,6 +3657,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>
>         intel_enable_pipe(dev_priv, pipe, false);
>         intel_enable_plane(dev_priv, plane, pipe);
> +       intel_crtc_update_cursor(crtc, true);
>         if (IS_G4X(dev))
>                 g4x_fixup_plane(dev_priv, pipe);
>
> @@ -3666,7 +3665,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>
>         /* Give the overlay scaler a chance to enable if it's on this pipe */
>         intel_crtc_dpms_overlay(intel_crtc, true);
> -       intel_crtc_update_cursor(crtc, true);
>
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 encoder->enable(encoder);
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[PATCH 3/9] drm/i915: Enable the overlay right after primary and cursor planes

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Again follow the same sequence for all generations, because doing
otherwise just doesn't make sense.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c2e6e91..05510f5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3619,11 +3619,11 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	intel_enable_plane(dev_priv, plane, pipe);
 	intel_crtc_update_cursor(crtc, true);
 
-	intel_update_fbc(dev);
-
 	/* Give the overlay scaler a chance to enable if it's on this pipe */
 	intel_crtc_dpms_overlay(intel_crtc, true);
 
+	intel_update_fbc(dev);
+
 	mutex_unlock(&dev_priv->dpio_lock);
 }
 
@@ -3661,11 +3661,11 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	if (IS_G4X(dev))
 		g4x_fixup_plane(dev_priv, pipe);
 
-	intel_update_fbc(dev);
-
 	/* Give the overlay scaler a chance to enable if it's on this pipe */
 	intel_crtc_dpms_overlay(intel_crtc, true);
 
+	intel_update_fbc(dev);
+
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
 }
-- 
1.8.1.5

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

Re: [PATCH 3/9] drm/i915: Enable the overlay right after primary and cursor planes

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Again follow the same sequence for all generations, because doing
> otherwise just doesn't make sense.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c2e6e91..05510f5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3619,11 +3619,11 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>         intel_enable_plane(dev_priv, plane, pipe);
>         intel_crtc_update_cursor(crtc, true);
>
> -       intel_update_fbc(dev);
> -
>         /* Give the overlay scaler a chance to enable if it's on this pipe */
>         intel_crtc_dpms_overlay(intel_crtc, true);
>
> +       intel_update_fbc(dev);
> +
>         mutex_unlock(&dev_priv->dpio_lock);
>  }
>
> @@ -3661,11 +3661,11 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>         if (IS_G4X(dev))
>                 g4x_fixup_plane(dev_priv, pipe);
>
> -       intel_update_fbc(dev);
> -
>         /* Give the overlay scaler a chance to enable if it's on this pipe */
>         intel_crtc_dpms_overlay(intel_crtc, true);
>
> +       intel_update_fbc(dev);
> +
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 encoder->enable(encoder);
>  }
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[PATCH v2 4/9] drm/i915: Follow the same sequence when disabling planes

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

First disable FBC, then IPS, then disable all planes, and finally
disable the pipe.

v2: Mention IPS in the commit message

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 05510f5..dac2db7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3383,13 +3383,13 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 
 	intel_crtc_wait_for_pending_flips(crtc);
 	drm_vblank_off(dev, pipe);
-	intel_crtc_update_cursor(crtc, false);
-
-	intel_disable_plane(dev_priv, plane, pipe);
 
 	if (dev_priv->cfb_plane == plane)
 		intel_disable_fbc(dev);
 
+	intel_crtc_update_cursor(crtc, false);
+	intel_disable_plane(dev_priv, plane, pipe);
+
 	intel_set_pch_fifo_underrun_reporting(dev, pipe, false);
 	intel_disable_pipe(dev_priv, pipe);
 
@@ -3462,7 +3462,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 
 	intel_crtc_wait_for_pending_flips(crtc);
 	drm_vblank_off(dev, pipe);
-	intel_crtc_update_cursor(crtc, false);
 
 	/* FBC must be disabled before disabling the plane on HSW. */
 	if (dev_priv->cfb_plane == plane)
@@ -3470,6 +3469,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 
 	hsw_disable_ips(intel_crtc);
 
+	intel_crtc_update_cursor(crtc, false);
 	intel_disable_plane(dev_priv, plane, pipe);
 
 	if (intel_crtc->config.has_pch_encoder)
@@ -3703,13 +3703,14 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	/* Give the overlay scaler a chance to disable if it's on this pipe */
 	intel_crtc_wait_for_pending_flips(crtc);
 	drm_vblank_off(dev, pipe);
-	intel_crtc_dpms_overlay(intel_crtc, false);
-	intel_crtc_update_cursor(crtc, false);
 
 	if (dev_priv->cfb_plane == plane)
 		intel_disable_fbc(dev);
 
+	intel_crtc_dpms_overlay(intel_crtc, false);
+	intel_crtc_update_cursor(crtc, false);
 	intel_disable_plane(dev_priv, plane, pipe);
+
 	intel_disable_pipe(dev_priv, pipe);
 
 	i9xx_pfit_disable(intel_crtc);
-- 
1.8.1.5

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

Re: [PATCH v2 4/9] drm/i915: Follow the same sequence when disabling planes

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> First disable FBC, then IPS, then disable all planes, and finally
> disable the pipe.
>
> v2: Mention IPS in the commit message
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 05510f5..dac2db7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3383,13 +3383,13 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>
>         intel_crtc_wait_for_pending_flips(crtc);
>         drm_vblank_off(dev, pipe);
> -       intel_crtc_update_cursor(crtc, false);
> -
> -       intel_disable_plane(dev_priv, plane, pipe);
>
>         if (dev_priv->cfb_plane == plane)
>                 intel_disable_fbc(dev);
>
> +       intel_crtc_update_cursor(crtc, false);
> +       intel_disable_plane(dev_priv, plane, pipe);
> +
>         intel_set_pch_fifo_underrun_reporting(dev, pipe, false);
>         intel_disable_pipe(dev_priv, pipe);
>
> @@ -3462,7 +3462,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>
>         intel_crtc_wait_for_pending_flips(crtc);
>         drm_vblank_off(dev, pipe);
> -       intel_crtc_update_cursor(crtc, false);
>
>         /* FBC must be disabled before disabling the plane on HSW. */
>         if (dev_priv->cfb_plane == plane)
> @@ -3470,6 +3469,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>
>         hsw_disable_ips(intel_crtc);
>
> +       intel_crtc_update_cursor(crtc, false);
>         intel_disable_plane(dev_priv, plane, pipe);
>
>         if (intel_crtc->config.has_pch_encoder)
> @@ -3703,13 +3703,14 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>         /* Give the overlay scaler a chance to disable if it's on this pipe */
>         intel_crtc_wait_for_pending_flips(crtc);
>         drm_vblank_off(dev, pipe);
> -       intel_crtc_dpms_overlay(intel_crtc, false);
> -       intel_crtc_update_cursor(crtc, false);
>
>         if (dev_priv->cfb_plane == plane)
>                 intel_disable_fbc(dev);
>
> +       intel_crtc_dpms_overlay(intel_crtc, false);
> +       intel_crtc_update_cursor(crtc, false);
>         intel_disable_plane(dev_priv, plane, pipe);
> +
>         intel_disable_pipe(dev_priv, pipe);
>
>         i9xx_pfit_disable(intel_crtc);
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[PATCH 5/9] drm/i915: Drop overlay DPMS call from valleyview_crtc_enable

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

VLV doesn't have the old video overlay.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dac2db7..61bee12 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3619,9 +3619,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	intel_enable_plane(dev_priv, plane, pipe);
 	intel_crtc_update_cursor(crtc, true);
 
-	/* Give the overlay scaler a chance to enable if it's on this pipe */
-	intel_crtc_dpms_overlay(intel_crtc, true);
-
 	intel_update_fbc(dev);
 
 	mutex_unlock(&dev_priv->dpio_lock);
-- 
1.8.1.5

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

Re: [PATCH 5/9] drm/i915: Drop overlay DPMS call from valleyview_crtc_enable

[Rodrigo Vivi]

just curious/trying to learn more: how do you know it doesn't have?

but if it doesn't feel free to use:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> VLV doesn't have the old video overlay.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dac2db7..61bee12 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3619,9 +3619,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>         intel_enable_plane(dev_priv, plane, pipe);
>         intel_crtc_update_cursor(crtc, true);
>
> -       /* Give the overlay scaler a chance to enable if it's on this pipe */
> -       intel_crtc_dpms_overlay(intel_crtc, true);
> -
>         intel_update_fbc(dev);
>
>         mutex_unlock(&dev_priv->dpio_lock);
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH 5/9] drm/i915: Drop overlay DPMS call from valleyview_crtc_enable

[Ville Syrjälä]

On Wed, Jun 05, 2013 at 04:27:25PM -0300, Rodrigo Vivi wrote:
> just curious/trying to learn more: how do you know it doesn't have?

Because the hardware designers hate me and don't want to give me nice
toys :)

Seriously though, the spec doesn't have it so I have to assume it's
really gone. And we got the sprite planes in exchange. Not the best
trade in my opinion, but what can you do.

> 
> but if it doesn't feel free to use:
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> 
> On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > VLV doesn't have the old video overlay.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index dac2db7..61bee12 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3619,9 +3619,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
> >         intel_enable_plane(dev_priv, plane, pipe);
> >         intel_crtc_update_cursor(crtc, true);
> >
> > -       /* Give the overlay scaler a chance to enable if it's on this pipe */
> > -       intel_crtc_dpms_overlay(intel_crtc, true);
> > -
> >         intel_update_fbc(dev);
> >
> >         mutex_unlock(&dev_priv->dpio_lock);
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

-- 
Ville Syrjälä
Intel OTC

[PATCH v2 6/9] drm/i915: Disable/restore all sprite planes around modeset

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Disable/restore sprite planes around mode-set just like we do for the
primary and cursor planes. Now that we have working sprite clipping,
this actually works quite decently.

Previosuly we didn't even bother to disable sprites when changing mode,
which could lead to a corrupted sprite appearing on the screen after a
modeset (at least on my IVB). Not sure if all hardware generations would
be so forgiving when enabled sprites end up outside the pipe dimensons.

v2: Disable rather than enable sprites in ironlake_crtc_disable()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_sprite.c  |  8 ++++++++
 3 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 61bee12..e928a54 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3161,6 +3161,28 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
 	}
 }
 
+static void intel_enable_planes(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	struct intel_plane *intel_plane;
+
+	list_for_each_entry(intel_plane, &dev->mode_config.plane_list, base.head)
+		if (intel_plane->pipe == pipe)
+			intel_plane_restore(&intel_plane->base);
+}
+
+static void intel_disable_planes(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	struct intel_plane *intel_plane;
+
+	list_for_each_entry(intel_plane, &dev->mode_config.plane_list, base.head)
+		if (intel_plane->pipe == pipe)
+			intel_plane_disable(&intel_plane->base);
+}
+
 static void ironlake_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -3216,6 +3238,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	intel_enable_pipe(dev_priv, pipe,
 			  intel_crtc->config.has_pch_encoder);
 	intel_enable_plane(dev_priv, plane, pipe);
+	intel_enable_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
 
 	if (intel_crtc->config.has_pch_encoder)
@@ -3324,6 +3347,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_enable_pipe(dev_priv, pipe,
 			  intel_crtc->config.has_pch_encoder);
 	intel_enable_plane(dev_priv, plane, pipe);
+	intel_enable_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
 
 	hsw_enable_ips(intel_crtc);
@@ -3388,6 +3412,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 		intel_disable_fbc(dev);
 
 	intel_crtc_update_cursor(crtc, false);
+	intel_disable_planes(crtc);
 	intel_disable_plane(dev_priv, plane, pipe);
 
 	intel_set_pch_fifo_underrun_reporting(dev, pipe, false);
@@ -3470,6 +3495,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	hsw_disable_ips(intel_crtc);
 
 	intel_crtc_update_cursor(crtc, false);
+	intel_disable_planes(crtc);
 	intel_disable_plane(dev_priv, plane, pipe);
 
 	if (intel_crtc->config.has_pch_encoder)
@@ -3617,6 +3643,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	intel_enable_pipe(dev_priv, pipe, false);
 	intel_enable_plane(dev_priv, plane, pipe);
+	intel_enable_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
 
 	intel_update_fbc(dev);
@@ -3654,6 +3681,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	intel_enable_pipe(dev_priv, pipe, false);
 	intel_enable_plane(dev_priv, plane, pipe);
+	intel_enable_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
 	if (IS_G4X(dev))
 		g4x_fixup_plane(dev_priv, pipe);
@@ -3706,6 +3734,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 
 	intel_crtc_dpms_overlay(intel_crtc, false);
 	intel_crtc_update_cursor(crtc, false);
+	intel_disable_planes(crtc);
 	intel_disable_plane(dev_priv, plane, pipe);
 
 	intel_disable_pipe(dev_priv, pipe);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fdf6303..0355315 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -633,6 +633,7 @@ extern void intel_connector_dpms(struct drm_connector *, int mode);
 extern bool intel_connector_get_hw_state(struct intel_connector *connector);
 extern void intel_modeset_check_state(struct drm_device *dev);
 extern void intel_plane_restore(struct drm_plane *plane);
+extern void intel_plane_disable(struct drm_plane *plane);
 
 
 static inline struct intel_encoder *intel_attached_encoder(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 04d38d4..1fa5612 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -957,6 +957,14 @@ void intel_plane_restore(struct drm_plane *plane)
 			   intel_plane->src_w, intel_plane->src_h);
 }
 
+void intel_plane_disable(struct drm_plane *plane)
+{
+	if (!plane->crtc || !plane->fb)
+		return;
+
+	intel_disable_plane(plane);
+}
+
 static const struct drm_plane_funcs intel_plane_funcs = {
 	.update_plane = intel_update_plane,
 	.disable_plane = intel_disable_plane,
-- 
1.8.1.5

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

Re: [PATCH v2 6/9] drm/i915: Disable/restore all sprite planes around modeset

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Disable/restore sprite planes around mode-set just like we do for the
> primary and cursor planes. Now that we have working sprite clipping,
> this actually works quite decently.
>
> Previosuly we didn't even bother to disable sprites when changing mode,
> which could lead to a corrupted sprite appearing on the screen after a
> modeset (at least on my IVB). Not sure if all hardware generations would
> be so forgiving when enabled sprites end up outside the pipe dimensons.
>
> v2: Disable rather than enable sprites in ironlake_crtc_disable()
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_sprite.c  |  8 ++++++++
>  3 files changed, 38 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 61bee12..e928a54 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3161,6 +3161,28 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
>         }
>  }
>
> +static void intel_enable_planes(struct drm_crtc *crtc)
> +{
> +       struct drm_device *dev = crtc->dev;
> +       enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +       struct intel_plane *intel_plane;
> +
> +       list_for_each_entry(intel_plane, &dev->mode_config.plane_list, base.head)
> +               if (intel_plane->pipe == pipe)
> +                       intel_plane_restore(&intel_plane->base);
> +}
> +
> +static void intel_disable_planes(struct drm_crtc *crtc)
> +{
> +       struct drm_device *dev = crtc->dev;
> +       enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +       struct intel_plane *intel_plane;
> +
> +       list_for_each_entry(intel_plane, &dev->mode_config.plane_list, base.head)
> +               if (intel_plane->pipe == pipe)
> +                       intel_plane_disable(&intel_plane->base);
> +}
> +
>  static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  {
>         struct drm_device *dev = crtc->dev;
> @@ -3216,6 +3238,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>         intel_enable_pipe(dev_priv, pipe,
>                           intel_crtc->config.has_pch_encoder);
>         intel_enable_plane(dev_priv, plane, pipe);
> +       intel_enable_planes(crtc);
>         intel_crtc_update_cursor(crtc, true);
>
>         if (intel_crtc->config.has_pch_encoder)
> @@ -3324,6 +3347,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>         intel_enable_pipe(dev_priv, pipe,
>                           intel_crtc->config.has_pch_encoder);
>         intel_enable_plane(dev_priv, plane, pipe);
> +       intel_enable_planes(crtc);
>         intel_crtc_update_cursor(crtc, true);
>
>         hsw_enable_ips(intel_crtc);
> @@ -3388,6 +3412,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>                 intel_disable_fbc(dev);
>
>         intel_crtc_update_cursor(crtc, false);
> +       intel_disable_planes(crtc);
>         intel_disable_plane(dev_priv, plane, pipe);
>
>         intel_set_pch_fifo_underrun_reporting(dev, pipe, false);
> @@ -3470,6 +3495,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>         hsw_disable_ips(intel_crtc);
>
>         intel_crtc_update_cursor(crtc, false);
> +       intel_disable_planes(crtc);
>         intel_disable_plane(dev_priv, plane, pipe);
>
>         if (intel_crtc->config.has_pch_encoder)
> @@ -3617,6 +3643,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>
>         intel_enable_pipe(dev_priv, pipe, false);
>         intel_enable_plane(dev_priv, plane, pipe);
> +       intel_enable_planes(crtc);
>         intel_crtc_update_cursor(crtc, true);
>
>         intel_update_fbc(dev);
> @@ -3654,6 +3681,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>
>         intel_enable_pipe(dev_priv, pipe, false);
>         intel_enable_plane(dev_priv, plane, pipe);
> +       intel_enable_planes(crtc);
>         intel_crtc_update_cursor(crtc, true);
>         if (IS_G4X(dev))
>                 g4x_fixup_plane(dev_priv, pipe);
> @@ -3706,6 +3734,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>
>         intel_crtc_dpms_overlay(intel_crtc, false);
>         intel_crtc_update_cursor(crtc, false);
> +       intel_disable_planes(crtc);
>         intel_disable_plane(dev_priv, plane, pipe);
>
>         intel_disable_pipe(dev_priv, pipe);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index fdf6303..0355315 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -633,6 +633,7 @@ extern void intel_connector_dpms(struct drm_connector *, int mode);
>  extern bool intel_connector_get_hw_state(struct intel_connector *connector);
>  extern void intel_modeset_check_state(struct drm_device *dev);
>  extern void intel_plane_restore(struct drm_plane *plane);
> +extern void intel_plane_disable(struct drm_plane *plane);
>
>
>  static inline struct intel_encoder *intel_attached_encoder(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 04d38d4..1fa5612 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -957,6 +957,14 @@ void intel_plane_restore(struct drm_plane *plane)
>                            intel_plane->src_w, intel_plane->src_h);
>  }
>
> +void intel_plane_disable(struct drm_plane *plane)
> +{
> +       if (!plane->crtc || !plane->fb)
> +               return;
> +
> +       intel_disable_plane(plane);
> +}
> +
>  static const struct drm_plane_funcs intel_plane_funcs = {
>         .update_plane = intel_update_plane,
>         .disable_plane = intel_disable_plane,
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[PATCH 7/9] drm/i915: Improve assert_planes_disabled()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ever since gen4 primary planes were fixed to pipes.

And for gen2-3, don't check plane B if it doesn't exist.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e928a54..9c89ddf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1102,12 +1102,13 @@ static void assert_plane(struct drm_i915_private *dev_priv,
 static void assert_planes_disabled(struct drm_i915_private *dev_priv,
 				   enum pipe pipe)
 {
+	struct drm_device *dev = dev_priv->dev;
 	int reg, i;
 	u32 val;
 	int cur_pipe;
 
-	/* Planes are fixed to pipes on ILK+ */
-	if (HAS_PCH_SPLIT(dev_priv->dev) || IS_VALLEYVIEW(dev_priv->dev)) {
+	/* Primary planes are fixed to pipes on gen4+ */
+	if (INTEL_INFO(dev)->gen >= 4) {
 		reg = DSPCNTR(pipe);
 		val = I915_READ(reg);
 		WARN((val & DISPLAY_PLANE_ENABLE),
@@ -1117,7 +1118,7 @@ static void assert_planes_disabled(struct drm_i915_private *dev_priv,
 	}
 
 	/* Need to check both planes against the pipe */
-	for (i = 0; i < 2; i++) {
+	for (i = 0; i < INTEL_INFO(dev)->num_pipes; i++) {
 		reg = DSPCNTR(i);
 		val = I915_READ(reg);
 		cur_pipe = (val & DISPPLANE_SEL_PIPE_MASK) >>
-- 
1.8.1.5

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

Re: [PATCH 7/9] drm/i915: Improve assert_planes_disabled()

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Ever since gen4 primary planes were fixed to pipes.
>
> And for gen2-3, don't check plane B if it doesn't exist.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e928a54..9c89ddf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1102,12 +1102,13 @@ static void assert_plane(struct drm_i915_private *dev_priv,
>  static void assert_planes_disabled(struct drm_i915_private *dev_priv,
>                                    enum pipe pipe)
>  {
> +       struct drm_device *dev = dev_priv->dev;
>         int reg, i;
>         u32 val;
>         int cur_pipe;
>
> -       /* Planes are fixed to pipes on ILK+ */
> -       if (HAS_PCH_SPLIT(dev_priv->dev) || IS_VALLEYVIEW(dev_priv->dev)) {
> +       /* Primary planes are fixed to pipes on gen4+ */
> +       if (INTEL_INFO(dev)->gen >= 4) {
>                 reg = DSPCNTR(pipe);
>                 val = I915_READ(reg);
>                 WARN((val & DISPLAY_PLANE_ENABLE),
> @@ -1117,7 +1118,7 @@ static void assert_planes_disabled(struct drm_i915_private *dev_priv,
>         }
>
>         /* Need to check both planes against the pipe */
> -       for (i = 0; i < 2; i++) {
> +       for (i = 0; i < INTEL_INFO(dev)->num_pipes; i++) {
>                 reg = DSPCNTR(i);
>                 val = I915_READ(reg);
>                 cur_pipe = (val & DISPPLANE_SEL_PIPE_MASK) >>
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[PATCH 8/9] drm/i915: Spruce up assert_sprites_disabled()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make assert_sprites_disabled() operational on all platforms where
we currently have sprite support enabled.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9c89ddf..90d02c7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1132,19 +1132,30 @@ static void assert_planes_disabled(struct drm_i915_private *dev_priv,
 static void assert_sprites_disabled(struct drm_i915_private *dev_priv,
 				    enum pipe pipe)
 {
+	struct drm_device *dev = dev_priv->dev;
 	int reg, i;
 	u32 val;
 
-	if (!IS_VALLEYVIEW(dev_priv->dev))
-		return;
-
-	/* Need to check both planes against the pipe */
-	for (i = 0; i < dev_priv->num_plane; i++) {
-		reg = SPCNTR(pipe, i);
+	if (IS_VALLEYVIEW(dev)) {
+		for (i = 0; i < dev_priv->num_plane; i++) {
+			reg = SPCNTR(pipe, i);
+			val = I915_READ(reg);
+			WARN((val & SP_ENABLE),
+			     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
+			     sprite_name(pipe, i), pipe_name(pipe));
+		}
+	} else if (INTEL_INFO(dev)->gen >= 7) {
+		reg = SPRCTL(pipe);
+		val = I915_READ(reg);
+		WARN((val & SPRITE_ENABLE),
+		     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
+		     plane_name(pipe), pipe_name(pipe));
+	} else if (INTEL_INFO(dev)->gen >= 5) {
+		reg = DVSCNTR(pipe);
 		val = I915_READ(reg);
-		WARN((val & SP_ENABLE),
+		WARN((val & DVS_ENABLE),
 		     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
-		     sprite_name(pipe, i), pipe_name(pipe));
+		     plane_name(pipe), pipe_name(pipe));
 	}
 }
 
-- 
1.8.1.5

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

Re: [PATCH 8/9] drm/i915: Spruce up assert_sprites_disabled()

[Rodrigo Vivi]

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make assert_sprites_disabled() operational on all platforms where
> we currently have sprite support enabled.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9c89ddf..90d02c7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1132,19 +1132,30 @@ static void assert_planes_disabled(struct drm_i915_private *dev_priv,
>  static void assert_sprites_disabled(struct drm_i915_private *dev_priv,
>                                     enum pipe pipe)
>  {
> +       struct drm_device *dev = dev_priv->dev;
>         int reg, i;
>         u32 val;
>
> -       if (!IS_VALLEYVIEW(dev_priv->dev))
> -               return;
> -
> -       /* Need to check both planes against the pipe */
> -       for (i = 0; i < dev_priv->num_plane; i++) {
> -               reg = SPCNTR(pipe, i);
> +       if (IS_VALLEYVIEW(dev)) {
> +               for (i = 0; i < dev_priv->num_plane; i++) {
> +                       reg = SPCNTR(pipe, i);
> +                       val = I915_READ(reg);
> +                       WARN((val & SP_ENABLE),
> +                            "sprite %c assertion failure, should be off on pipe %c but is still active\n",
> +                            sprite_name(pipe, i), pipe_name(pipe));
> +               }
> +       } else if (INTEL_INFO(dev)->gen >= 7) {
> +               reg = SPRCTL(pipe);
> +               val = I915_READ(reg);
> +               WARN((val & SPRITE_ENABLE),
> +                    "sprite %c assertion failure, should be off on pipe %c but is still active\n",
> +                    plane_name(pipe), pipe_name(pipe));
> +       } else if (INTEL_INFO(dev)->gen >= 5) {
> +               reg = DVSCNTR(pipe);
>                 val = I915_READ(reg);
> -               WARN((val & SP_ENABLE),
> +               WARN((val & DVS_ENABLE),
>                      "sprite %c assertion failure, should be off on pipe %c but is still active\n",
> -                    sprite_name(pipe, i), pipe_name(pipe));
> +                    plane_name(pipe), pipe_name(pipe));
>         }
>  }
>
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH 8/9] drm/i915: Spruce up assert_sprites_disabled()

[Daniel Vetter]

On Wed, Jun 05, 2013 at 04:39:58PM -0300, Rodrigo Vivi wrote:
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> 
> On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make assert_sprites_disabled() operational on all platforms where
> > we currently have sprite support enabled.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I guess we eventually need some more abstract hw state readout for planes,
similar to what we have on the output routing side. But this is good
enough for now, so merged.
-Daniel

> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++--------
> >  1 file changed, 19 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 9c89ddf..90d02c7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1132,19 +1132,30 @@ static void assert_planes_disabled(struct drm_i915_private *dev_priv,
> >  static void assert_sprites_disabled(struct drm_i915_private *dev_priv,
> >                                     enum pipe pipe)
> >  {
> > +       struct drm_device *dev = dev_priv->dev;
> >         int reg, i;
> >         u32 val;
> >
> > -       if (!IS_VALLEYVIEW(dev_priv->dev))
> > -               return;
> > -
> > -       /* Need to check both planes against the pipe */
> > -       for (i = 0; i < dev_priv->num_plane; i++) {
> > -               reg = SPCNTR(pipe, i);
> > +       if (IS_VALLEYVIEW(dev)) {
> > +               for (i = 0; i < dev_priv->num_plane; i++) {
> > +                       reg = SPCNTR(pipe, i);
> > +                       val = I915_READ(reg);
> > +                       WARN((val & SP_ENABLE),
> > +                            "sprite %c assertion failure, should be off on pipe %c but is still active\n",
> > +                            sprite_name(pipe, i), pipe_name(pipe));
> > +               }
> > +       } else if (INTEL_INFO(dev)->gen >= 7) {
> > +               reg = SPRCTL(pipe);
> > +               val = I915_READ(reg);
> > +               WARN((val & SPRITE_ENABLE),
> > +                    "sprite %c assertion failure, should be off on pipe %c but is still active\n",
> > +                    plane_name(pipe), pipe_name(pipe));
> > +       } else if (INTEL_INFO(dev)->gen >= 5) {
> > +               reg = DVSCNTR(pipe);
> >                 val = I915_READ(reg);
> > -               WARN((val & SP_ENABLE),
> > +               WARN((val & DVS_ENABLE),
> >                      "sprite %c assertion failure, should be off on pipe %c but is still active\n",
> > -                    sprite_name(pipe, i), pipe_name(pipe));
> > +                    plane_name(pipe), pipe_name(pipe));
> >         }
> >  }
> >
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
> _______________________________________________
> 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

[PATCH 9/9] drm/i915: Assert dpll running in intel_crtc_load_lut() on pre-PCH platforms

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 90d02c7..3be69bc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6340,6 +6340,9 @@ void intel_crtc_load_lut(struct drm_crtc *crtc)
 	if (!crtc->enabled || !intel_crtc->active)
 		return;
 
+	if (!HAS_PCH_SPLIT(dev_priv->dev))
+		assert_pll_enabled(dev_priv, pipe);
+
 	/* use legacy palette for Ironlake */
 	if (HAS_PCH_SPLIT(dev))
 		palreg = LGC_PALETTE(pipe);
-- 
1.8.1.5

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

Re: [PATCH 9/9] drm/i915: Assert dpll running in intel_crtc_load_lut() on pre-PCH platforms

[Rodrigo Vivi]

why is this needed?
anyways: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 90d02c7..3be69bc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6340,6 +6340,9 @@ void intel_crtc_load_lut(struct drm_crtc *crtc)
>         if (!crtc->enabled || !intel_crtc->active)
>                 return;
>
> +       if (!HAS_PCH_SPLIT(dev_priv->dev))
> +               assert_pll_enabled(dev_priv, pipe);
> +
>         /* use legacy palette for Ironlake */
>         if (HAS_PCH_SPLIT(dev))
>                 palreg = LGC_PALETTE(pipe);
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH 9/9] drm/i915: Assert dpll running in intel_crtc_load_lut() on pre-PCH platforms

[Ville Syrjälä]

On Wed, Jun 05, 2013 at 04:41:54PM -0300, Rodrigo Vivi wrote:
> why is this needed?

The spec says that on some hardware you need to PLL running before you
can poke at the palette registers. I didn't actually try to anger the
hardware so I'm not really sure what would happen otherwise, but IIRC
Jesse said something about a hard system hang...

> anyways: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> 
> On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 90d02c7..3be69bc 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6340,6 +6340,9 @@ void intel_crtc_load_lut(struct drm_crtc *crtc)
> >         if (!crtc->enabled || !intel_crtc->active)
> >                 return;
> >
> > +       if (!HAS_PCH_SPLIT(dev_priv->dev))
> > +               assert_pll_enabled(dev_priv, pipe);
> > +
> >         /* use legacy palette for Ironlake */
> >         if (HAS_PCH_SPLIT(dev))
> >                 palreg = LGC_PALETTE(pipe);
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 9/9] drm/i915: Assert dpll running in intel_crtc_load_lut() on pre-PCH platforms

[Daniel Vetter]

On Wed, Jun 05, 2013 at 10:58:06PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 05, 2013 at 04:41:54PM -0300, Rodrigo Vivi wrote:
> > why is this needed?
> 
> The spec says that on some hardware you need to PLL running before you
> can poke at the palette registers. I didn't actually try to anger the
> hardware so I'm not really sure what would happen otherwise, but IIRC
> Jesse said something about a hard system hang...

I've added this to the commit message and merged the entire pile. Thanks
for patches&review.
-Daniel

> 
> > anyways: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > 
> > On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 90d02c7..3be69bc 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -6340,6 +6340,9 @@ void intel_crtc_load_lut(struct drm_crtc *crtc)
> > >         if (!crtc->enabled || !intel_crtc->active)
> > >                 return;
> > >
> > > +       if (!HAS_PCH_SPLIT(dev_priv->dev))
> > > +               assert_pll_enabled(dev_priv, pipe);
> > > +
> > >         /* use legacy palette for Ironlake */
> > >         if (HAS_PCH_SPLIT(dev))
> > >                 palreg = LGC_PALETTE(pipe);
> > > --
> > > 1.8.1.5
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> > 
> > -- 
> > Rodrigo Vivi
> > Blog: http://blog.vivi.eng.br
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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

Re: [PATCH 9/9] drm/i915: Assert dpll running in intel_crtc_load_lut() on pre-PCH platforms

[Daniel Vetter]

On Wed, Jun 05, 2013 at 04:41:54PM -0300, Rodrigo Vivi wrote:
> why is this needed?
> anyways: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

An r-b is a formal statement like a s-o-b. From
Documentaion/SubmittingPatches:

	Reviewer's statement of oversight

	By offering my Reviewed-by: tag, I state that:

 	 (a) I have carried out a technical review of this patch to
	     evaluate its appropriateness and readiness for inclusion into
	     the mainline kernel.

	 (b) Any problems, concerns, or questions relating to the patch
	     have been communicated back to the submitter.  I am satisfied
	     with the submitter's response to my comments.

	 (c) While there may be things that could be improved with this
	     submission, I believe that it is, at this time, (1) a
	     worthwhile modification to the kernel, and (2) free of known
	     issues which would argue against its inclusion.

	 (d) While I have reviewed the patch and believe it to be sound, I
	     do not (unless explicitly stated elsewhere) make any
	     warranties or guarantees that it will achieve its stated
	     purpose or function properly in any given situation.

If you're asking "why is this needed?" it does _not_ deserve an r-b on
a pretty fundamental level since it clearly violates c) 1) above.

Please don't just go through the motions when doing review.

Thanks, Daniel

> 
> On Tue, Jun 4, 2013 at 7:49 AM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 90d02c7..3be69bc 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6340,6 +6340,9 @@ void intel_crtc_load_lut(struct drm_crtc *crtc)
> >         if (!crtc->enabled || !intel_crtc->active)
> >                 return;
> >
> > +       if (!HAS_PCH_SPLIT(dev_priv->dev))
> > +               assert_pll_enabled(dev_priv, pipe);
> > +
> >         /* use legacy palette for Ironlake */
> >         if (HAS_PCH_SPLIT(dev))
> >                 palreg = LGC_PALETTE(pipe);
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
> _______________________________________________
> 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