[PATCH 1/7] drm/i915: move debug output back to the right place

[PATCH 1/7] drm/i915: move debug output back to the right place

[Daniel Vetter]

When adding the pipe config computation step I've accidentally moved
this a bit away. Which momentarily confused me since the pipe config
step rejected some modesetting operations I expected and so left me
looking in vain for that debug output.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1b2c744..f60493b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7841,6 +7841,9 @@ int intel_set_mode(struct drm_crtc *crtc,
 	intel_modeset_affected_pipes(crtc, &modeset_pipes,
 				     &prepare_pipes, &disable_pipes);
 
+	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
+		      modeset_pipes, prepare_pipes, disable_pipes);
+
 	*saved_hwmode = crtc->hwmode;
 	*saved_mode = crtc->mode;
 
@@ -7859,9 +7862,6 @@ int intel_set_mode(struct drm_crtc *crtc,
 		}
 	}
 
-	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
-		      modeset_pipes, prepare_pipes, disable_pipes);
-
 	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
 		intel_crtc_disable(&intel_crtc->base);
 
-- 
1.7.10.4

[PATCH 2/7] drm/i915: Fixup Oops in the pipe config computation

[Daniel Vetter]

Yet again our current confusion between doing the modeset globally,
but only having the new parameters for one crtc at a time.

This time things blew up when restoring modes in the switchless resume
code - intel_modeset_affected_pipes figured out that pipe 2 should
be restored, but since pipe 1 was disabled there was no mode nor fb
when trying to restore the first crtc.

Hilarity ensued and broke resume on my i945gme machine since the
pipe_config_set_bpp added in

commit 4e53c2e010e531b4a014692199e978482d471c7e
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Mar 27 00:44:58 2013 +0100

    drm/i915: precompute pipe bpp before touching the hw

fell over the lack of an fb.

Fix this mess by now by justing shunting all the cool new global
modeset logic in intel_modeset_affected_pipes.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f60493b..58c6bb6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7629,6 +7629,10 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
 	/* ... and mask these out. */
 	*modeset_pipes &= ~(*disable_pipes);
 	*prepare_pipes &= ~(*disable_pipes);
+
+	/* HACK: We don't (yet) fully support global modesets. */
+	*modeset_pipes &= 1 << intel_crtc->pipe;
+	*prepare_pipes &= 1 << intel_crtc->pipe;
 }
 
 static bool intel_crtc_in_use(struct drm_crtc *crtc)
-- 
1.7.10.4

Re: [PATCH 2/7] drm/i915: Fixup Oops in the pipe config computation

[Daniel Vetter]

On Thu, Apr 11, 2013 at 04:29:05PM +0200, Daniel Vetter wrote:
> Yet again our current confusion between doing the modeset globally,
> but only having the new parameters for one crtc at a time.
> 
> This time things blew up when restoring modes in the switchless resume
> code - intel_modeset_affected_pipes figured out that pipe 2 should
> be restored, but since pipe 1 was disabled there was no mode nor fb
> when trying to restore the first crtc.
> 
> Hilarity ensued and broke resume on my i945gme machine since the
> pipe_config_set_bpp added in
> 
> commit 4e53c2e010e531b4a014692199e978482d471c7e
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Mar 27 00:44:58 2013 +0100
> 
>     drm/i915: precompute pipe bpp before touching the hw
> 
> fell over the lack of an fb.
> 
> Fix this mess by now by justing shunting all the cool new global
> modeset logic in intel_modeset_affected_pipes.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Potentially the same bug reported against 3.8.1:

Bugzill: https://bugzilla.redhat.com/show_bug.cgi?id=917725
Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_display.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f60493b..58c6bb6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7629,6 +7629,10 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
>  	/* ... and mask these out. */
>  	*modeset_pipes &= ~(*disable_pipes);
>  	*prepare_pipes &= ~(*disable_pipes);
> +
> +	/* HACK: We don't (yet) fully support global modesets. */
> +	*modeset_pipes &= 1 << intel_crtc->pipe;
> +	*prepare_pipes &= 1 << intel_crtc->pipe;
>  }
>  
>  static bool intel_crtc_in_use(struct drm_crtc *crtc)
> -- 
> 1.7.10.4
> 

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

Re: [PATCH 2/7] drm/i915: Fixup Oops in the pipe config computation

[Daniel Vetter]

On Thu, Apr 11, 2013 at 11:09:00PM +0200, Daniel Vetter wrote:
> On Thu, Apr 11, 2013 at 04:29:05PM +0200, Daniel Vetter wrote:
> > Yet again our current confusion between doing the modeset globally,
> > but only having the new parameters for one crtc at a time.
> > 
> > This time things blew up when restoring modes in the switchless resume
> > code - intel_modeset_affected_pipes figured out that pipe 2 should
> > be restored, but since pipe 1 was disabled there was no mode nor fb
> > when trying to restore the first crtc.
> > 
> > Hilarity ensued and broke resume on my i945gme machine since the
> > pipe_config_set_bpp added in
> > 
> > commit 4e53c2e010e531b4a014692199e978482d471c7e
> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Date:   Wed Mar 27 00:44:58 2013 +0100
> > 
> >     drm/i915: precompute pipe bpp before touching the hw
> > 
> > fell over the lack of an fb.
> > 
> > Fix this mess by now by justing shunting all the cool new global
> > modeset logic in intel_modeset_affected_pipes.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Potentially the same bug reported against 3.8.1:
> 
> Bugzill: https://bugzilla.redhat.com/show_bug.cgi?id=917725
> Cc: stable@vger.kernel.org

References: http://www.mail-archive.com/stable@vger.kernel.org/msg38084.html
Tested-by: Richard Cochran <richardcochran@gmail.com>


> > ---
> >  drivers/gpu/drm/i915/intel_display.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f60493b..58c6bb6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7629,6 +7629,10 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
> >  	/* ... and mask these out. */
> >  	*modeset_pipes &= ~(*disable_pipes);
> >  	*prepare_pipes &= ~(*disable_pipes);
> > +
> > +	/* HACK: We don't (yet) fully support global modesets. */
> > +	*modeset_pipes &= 1 << intel_crtc->pipe;
> > +	*prepare_pipes &= 1 << intel_crtc->pipe;
> >  }
> >  
> >  static bool intel_crtc_in_use(struct drm_crtc *crtc)
> > -- 
> > 1.7.10.4
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

Re: [PATCH 2/7] drm/i915: Fixup Oops in the pipe config computation

[Chris Wilson]

On Thu, Apr 11, 2013 at 04:29:05PM +0200, Daniel Vetter wrote:
> Yet again our current confusion between doing the modeset globally,
> but only having the new parameters for one crtc at a time.
> 
> This time things blew up when restoring modes in the switchless resume
> code - intel_modeset_affected_pipes figured out that pipe 2 should
> be restored, but since pipe 1 was disabled there was no mode nor fb
> when trying to restore the first crtc.
> 
> Hilarity ensued and broke resume on my i945gme machine since the
> pipe_config_set_bpp added in

I can see how the hack works, but I'm not clear on how we got into the
situation of trying to enable multiple pipes in the first place. Do you
mind expanding upon the failure conditions a bit more?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/7] drm/i915: Fixup Oops in the pipe config computation

[Daniel Vetter]

On Fri, Apr 12, 2013 at 11:46 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, Apr 11, 2013 at 04:29:05PM +0200, Daniel Vetter wrote:
>> Yet again our current confusion between doing the modeset globally,
>> but only having the new parameters for one crtc at a time.
>>
>> This time things blew up when restoring modes in the switchless resume
>> code - intel_modeset_affected_pipes figured out that pipe 2 should
>> be restored, but since pipe 1 was disabled there was no mode nor fb
>> when trying to restore the first crtc.
>>
>> Hilarity ensued and broke resume on my i945gme machine since the
>> pipe_config_set_bpp added in
>
> I can see how the hack works, but I'm not clear on how we got into the
> situation of trying to enable multiple pipes in the first place. Do you
> mind expanding upon the failure conditions a bit more?

Yeah, that's worth spilling a few words ;-)

So the issue is that intel_set_mode essentially already does a global
modeset: intel_modeset_affected_pipes compares the current state with
where we want to go to (which is carefully set up by
intel_crtc_set_config) and then goes through the modeset sequence for
any crtc which needs updating.

Now the issue is that the actual interface with the remaining code
still only works on one crtc, and so we only pass in one fb and one
mode. In intel_set_mode we also only compute one intel_crtc_config
(which should be the one for the crtc we're doing a modeset on).

The reason for that mismatch is twofold:
- We want to eventually do all modeset as global state changes, so
it's just infrastructure prep.
- But even the old semantics can change more than one crtc when you
e.g. move a connector from crtc A to crtc B, then both crtc A and B
need to be updated. Usually that means one pipe is disabled and the
other enabled. This is also the reason why the hack doesn't touch the
disable_pipes mask.

Now hilarity ensued in our kms config restore paths when we actually
try to do a modeset on all crtcs: If the first crtc should be off and
the second should be on, then the call on the first crtc will notice
that the 2nd one should be switched on and so tries to compute the
pipe_config. But due to a lack of passed-in fb (crtc 1 should be off
after all) it only results in tears.

This case is ridiculously easy to hit on gen2/3 where the lvds output
is restricted to pipe B ;-) Also, before the pipe_config bpp rework
gen2/3 didn't care really about the fb->depth and so just stumbled a
bit, but never fell over completely. But apparently Ajax also managed
to blow up pch platforms, probably with some randomized configs, and
pch platforms trip up over the lack of an fb even in the old code.

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

[PATCH] drm/i915: Fixup Oops in the pipe config computation

[Daniel Vetter]

Yet again our current confusion between doing the modeset globally,
but only having the new parameters for one crtc at a time.

So that intel_set_mode essentially already does a global modeset:
intel_modeset_affected_pipes compares the current state with where we
want to go to (which is carefully set up by intel_crtc_set_config) and
then goes through the modeset sequence for any crtc which needs
updating.

Now the issue is that the actual interface with the remaining code
still only works on one crtc, and so we only pass in one fb and one
mode. In intel_set_mode we also only compute one intel_crtc_config
(which should be the one for the crtc we're doing a modeset on).

The reason for that mismatch is twofold:
- We want to eventually do all modeset as global state changes, so
it's just infrastructure prep.
- But even the old semantics can change more than one crtc when you
e.g. move a connector from crtc A to crtc B, then both crtc A and B
need to be updated. Usually that means one pipe is disabled and the
other enabled. This is also the reason why the hack doesn't touch the
disable_pipes mask.

Now hilarity ensued in our kms config restore paths when we actually
try to do a modeset on all crtcs: If the first crtc should be off and
the second should be on, then the call on the first crtc will notice
that the 2nd one should be switched on and so tries to compute the
pipe_config. But due to a lack of passed-in fb (crtc 1 should be off
after all) it only results in tears.

This case is ridiculously easy to hit on gen2/3 where the lvds output
is restricted to pipe B. Note that before the pipe_config bpp rework
gen2/3 didn't care really about the fb->depth, so this is a regression
brought to light with

commit 4e53c2e010e531b4a014692199e978482d471c7e
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Mar 27 00:44:58 2013 +0100

    drm/i915: precompute pipe bpp before touching the hw

But apparently Ajax also managed to blow up pch platforms, probably
with some randomized configs, and pch platforms trip up over the lack
of an fb even in the old code. So this actually goes back to the first
introduction of the new modeset restore code in

commit 45e2b5f640b3766da3eda48f6c35f088155c06f3
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Fri Nov 23 18:16:34 2012 +0100

    drm/i915: force restore on lid open

Fix this mess by now by justing shunting all the cool new global
modeset logic in intel_modeset_affected_pipes.

v2: Improve commit message and clean up all the comments in
intel_modeset_affected_pipes - since the introduction of the modeset
restore code they've been a bit outdated.

Bugzill: https://bugzilla.redhat.com/show_bug.cgi?id=917725
Cc: stable@vger.kernel.org
References: http://www.mail-archive.com/stable@vger.kernel.org/msg38084.html
Tested-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1b2c744..010115c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7613,22 +7613,25 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
 	if (crtc->enabled)
 		*prepare_pipes |= 1 << intel_crtc->pipe;
 
-	/* We only support modeset on one single crtc, hence we need to do that
-	 * only for the passed in crtc iff we change anything else than just
-	 * disable crtcs.
-	 *
-	 * This is actually not true, to be fully compatible with the old crtc
-	 * helper we automatically disable _any_ output (i.e. doesn't need to be
-	 * connected to the crtc we're modesetting on) if it's disconnected.
-	 * Which is a rather nutty api (since changed the output configuration
-	 * without userspace's explicit request can lead to confusion), but
-	 * alas. Hence we currently need to modeset on all pipes we prepare. */
+	/*
+	 * For simplicity do a full modeset on any pipe where the output routing
+	 * changed. We could be more clever, but that would require us to be
+	 * more careful with calling the relevant encoder->mode_set functions.
+	 */
 	if (*prepare_pipes)
 		*modeset_pipes = *prepare_pipes;
 
 	/* ... and mask these out. */
 	*modeset_pipes &= ~(*disable_pipes);
 	*prepare_pipes &= ~(*disable_pipes);
+
+	/*
+	 * HACK: We don't (yet) fully support global modesets. intel_set_config
+	 * obies this rule, but the modeset restore mode of
+	 * intel_modeset_setup_hw_state does not.
+	 */
+	*modeset_pipes &= 1 << intel_crtc->pipe;
+	*prepare_pipes &= 1 << intel_crtc->pipe;
 }
 
 static bool intel_crtc_in_use(struct drm_crtc *crtc)
-- 
1.7.10.4

Re: [PATCH] drm/i915: Fixup Oops in the pipe config computation

[Chris Wilson]

On Fri, Apr 12, 2013 at 06:48:43PM +0200, Daniel Vetter wrote:
> Yet again our current confusion between doing the modeset globally,
> but only having the new parameters for one crtc at a time.
> 
> So that intel_set_mode essentially already does a global modeset:
> intel_modeset_affected_pipes compares the current state with where we
> want to go to (which is carefully set up by intel_crtc_set_config) and
> then goes through the modeset sequence for any crtc which needs
> updating.
> 
> Now the issue is that the actual interface with the remaining code
> still only works on one crtc, and so we only pass in one fb and one
> mode. In intel_set_mode we also only compute one intel_crtc_config
> (which should be the one for the crtc we're doing a modeset on).
> 
> The reason for that mismatch is twofold:
> - We want to eventually do all modeset as global state changes, so
> it's just infrastructure prep.
> - But even the old semantics can change more than one crtc when you
> e.g. move a connector from crtc A to crtc B, then both crtc A and B
> need to be updated. Usually that means one pipe is disabled and the
> other enabled. This is also the reason why the hack doesn't touch the
> disable_pipes mask.
> 
> Now hilarity ensued in our kms config restore paths when we actually
> try to do a modeset on all crtcs: If the first crtc should be off and
> the second should be on, then the call on the first crtc will notice
> that the 2nd one should be switched on and so tries to compute the
> pipe_config. But due to a lack of passed-in fb (crtc 1 should be off
> after all) it only results in tears.
> 
> This case is ridiculously easy to hit on gen2/3 where the lvds output
> is restricted to pipe B. Note that before the pipe_config bpp rework
> gen2/3 didn't care really about the fb->depth, so this is a regression
> brought to light with
> 
> commit 4e53c2e010e531b4a014692199e978482d471c7e
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Mar 27 00:44:58 2013 +0100
> 
>     drm/i915: precompute pipe bpp before touching the hw
> 
> But apparently Ajax also managed to blow up pch platforms, probably
> with some randomized configs, and pch platforms trip up over the lack
> of an fb even in the old code. So this actually goes back to the first
> introduction of the new modeset restore code in
> 
> commit 45e2b5f640b3766da3eda48f6c35f088155c06f3
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Fri Nov 23 18:16:34 2012 +0100
> 
>     drm/i915: force restore on lid open
> 
> Fix this mess by now by justing shunting all the cool new global
> modeset logic in intel_modeset_affected_pipes.
> 
> v2: Improve commit message and clean up all the comments in
> intel_modeset_affected_pipes - since the introduction of the modeset
> restore code they've been a bit outdated.
> 
> Bugzill: https://bugzilla.redhat.com/show_bug.cgi?id=917725
^ Bugzilla
> Cc: stable@vger.kernel.org
> References: http://www.mail-archive.com/stable@vger.kernel.org/msg38084.html
> Tested-by: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'm happier with that and with reducing the amount of confusion from the
comments,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 3/7] drm/i915: Fixup pfit disabling for gen2/3

[Daniel Vetter]

The recent rework of the pfit handling didn't take into account that
the panel fitter is fixed to pipe B:

commit 24a1f16de97c4cf0029d9acd04be06db32208726
Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Date:   Fri Feb 8 16:35:37 2013 +0200

    drm/i915: disable shared panel fitter for pipe

Fix this up by properly computing the pipe the pfit is on. Also
extract the logic into its own function, add a debug assert to check
that the pipe is off (mostly just documentation) and add some debug
output.

If pipe A was disabled after pipe B was set up, the panel fitter will
be disabled. Now most userspace doesn't do modesets in this order,
which is why I couldn't ever reproduce this and why it took me so long
to figure out.

We really need hw state readout and check support for the pannel
fitter ...

Reported-by: Hans de Bruin <jmdebruin@xmsnet.nl>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Hans de Bruin <jmdebruin@xmsnet.nl>
References: http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/19049
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |   28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 58c6bb6..38b8a77 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3623,6 +3623,26 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 		encoder->enable(encoder);
 }
 
+static void i9xx_pfit_disable(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe;
+	uint32_t pctl = I915_READ(PFIT_CONTROL);
+
+	assert_pipe_disabled(dev_priv, crtc->pipe);
+
+	if (INTEL_INFO(dev)->gen >= 4)
+		pipe = (pctl & PFIT_PIPE_MASK) >> PFIT_PIPE_SHIFT;
+	else
+		pipe = PIPE_B;
+
+	if (pipe == crtc->pipe) {
+		DRM_DEBUG_DRIVER("disabling pfit, current: 0x%08x\n", pctl);
+		I915_WRITE(PFIT_CONTROL, 0);
+	}
+}
+
 static void i9xx_crtc_disable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -3631,8 +3651,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
-	u32 pctl;
-
 
 	if (!intel_crtc->active)
 		return;
@@ -3652,11 +3670,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	intel_disable_plane(dev_priv, plane, pipe);
 	intel_disable_pipe(dev_priv, pipe);
 
-	/* Disable pannel fitter if it is on this pipe. */
-	pctl = I915_READ(PFIT_CONTROL);
-	if ((pctl & PFIT_ENABLE) &&
-	    ((pctl & PFIT_PIPE_MASK) >> PFIT_PIPE_SHIFT) == pipe)
-		I915_WRITE(PFIT_CONTROL, 0);
+	i9xx_pfit_disable(intel_crtc);
 
 	intel_disable_pll(dev_priv, pipe);
 
-- 
1.7.10.4

Re: [PATCH 3/7] drm/i915: Fixup pfit disabling for gen2/3

[Chris Wilson]

On Thu, Apr 11, 2013 at 04:29:06PM +0200, Daniel Vetter wrote:
> The recent rework of the pfit handling didn't take into account that
> the panel fitter is fixed to pipe B:
> 
> commit 24a1f16de97c4cf0029d9acd04be06db32208726
> Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Date:   Fri Feb 8 16:35:37 2013 +0200
> 
>     drm/i915: disable shared panel fitter for pipe
> 
> Fix this up by properly computing the pipe the pfit is on. Also
> extract the logic into its own function, add a debug assert to check
> that the pipe is off (mostly just documentation) and add some debug
> output.
> 
> If pipe A was disabled after pipe B was set up, the panel fitter will
> be disabled. Now most userspace doesn't do modesets in this order,
> which is why I couldn't ever reproduce this and why it took me so long
> to figure out.
> 
> We really need hw state readout and check support for the pannel
> fitter ...
> 
> Reported-by: Hans de Bruin <jmdebruin@xmsnet.nl>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Hans de Bruin <jmdebruin@xmsnet.nl>
> References: http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/19049
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 4/7] drm/i915: don't enable the plane too early in i9xx_crtc_mode_set

[Daniel Vetter]

This is horrible lore and we should be able to get rid of it now
that the lvds/pfit handling code actually does the right thing.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 38b8a77..4f9f846 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4632,8 +4632,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 
 	i9xx_set_pipeconf(intel_crtc);
 
-	intel_enable_pipe(dev_priv, pipe, false);
-
 	intel_wait_for_vblank(dev, pipe);
 
 	I915_WRITE(DSPCNTR(plane), dspcntr);
-- 
1.7.10.4

Re: [PATCH 4/7] drm/i915: don't enable the plane too early in i9xx_crtc_mode_set

[Chris Wilson]

On Thu, Apr 11, 2013 at 04:29:07PM +0200, Daniel Vetter wrote:
> This is horrible lore and we should be able to get rid of it now
> that the lvds/pfit handling code actually does the right thing.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'd prefer 4 & 5 squashed, specially as 5 will then introduce a WARN,
but
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 5/7] drm/i915: drop redundant vblank waits

[Daniel Vetter]

Just blows through 50ms for naught, since the pipe is off.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4f9f846..e91e01c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4632,8 +4632,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 
 	i9xx_set_pipeconf(intel_crtc);
 
-	intel_wait_for_vblank(dev, pipe);
-
 	I915_WRITE(DSPCNTR(plane), dspcntr);
 	POSTING_READ(DSPCNTR(plane));
 
@@ -5605,8 +5603,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	ironlake_set_pipeconf(crtc, adjusted_mode, dither);
 
-	intel_wait_for_vblank(dev, pipe);
-
 	/* Set up the display plane register */
 	I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
 	POSTING_READ(DSPCNTR(plane));
-- 
1.7.10.4

Re: [PATCH 5/7] drm/i915: drop redundant vblank waits

[Paulo Zanoni]

Hi

2013/4/11 Daniel Vetter <daniel.vetter@ffwll.ch>:
> Just blows through 50ms for naught, since the pipe is off.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Looks correct, but can you also please add some WARNs in case the pipe
is actually on? Check haswell_crtc_mode_set for examples:

- WARN_ON(I915_READ(PIPECONF(intel_crtc->cpu_transcoder)) &
(PIPECONF_ENABLE | I965_PIPECONF_ACTIVE));
- WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);


> ---
>  drivers/gpu/drm/i915/intel_display.c |    4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4f9f846..e91e01c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4632,8 +4632,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>
>         i9xx_set_pipeconf(intel_crtc);
>
> -       intel_wait_for_vblank(dev, pipe);
> -
>         I915_WRITE(DSPCNTR(plane), dspcntr);
>         POSTING_READ(DSPCNTR(plane));
>
> @@ -5605,8 +5603,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>
>         ironlake_set_pipeconf(crtc, adjusted_mode, dither);
>
> -       intel_wait_for_vblank(dev, pipe);
> -
>         /* Set up the display plane register */
>         I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
>         POSTING_READ(DSPCNTR(plane));
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



--
Paulo Zanoni

Re: [PATCH 5/7] drm/i915: drop redundant vblank waits

[Daniel Vetter]

On Thu, Apr 11, 2013 at 02:47:05PM -0300, Paulo Zanoni wrote:
> Hi
> 
> 2013/4/11 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > Just blows through 50ms for naught, since the pipe is off.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Looks correct, but can you also please add some WARNs in case the pipe
> is actually on? Check haswell_crtc_mode_set for examples:
> 
> - WARN_ON(I915_READ(PIPECONF(intel_crtc->cpu_transcoder)) &
> (PIPECONF_ENABLE | I965_PIPECONF_ACTIVE));
> - WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);

We have _tons_ of assert_pipe_disabled in the enable/disable sequence for
i9xx now. So I think we're covered.

On that topic: Can't we use the same macros for the Haswell code? Would
make things a bit more readable imo ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 5/7] drm/i915: drop redundant vblank waits

[Chris Wilson]

On Thu, Apr 11, 2013 at 04:29:08PM +0200, Daniel Vetter wrote:
> Just blows through 50ms for naught, since the pipe is off.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 6/7] drm/i915: add pipe asserts for the crtc enable sequence

[Daniel Vetter]

The i9xx modeset sequence is currently pretty fishy, so tight it all
up with some good assert-sprinkling.

We already have good coverage on the disable side, but the enable side
is spotty (since until recently it was wrong).

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e91e01c..0941159 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1379,6 +1379,8 @@ static void intel_enable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	int reg;
 	u32 val;
 
+	assert_pipe_disabled(dev_priv, pipe);
+
 	/* No really, not for ILK+ */
 	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev) && dev_priv->info->gen >= 5);
 
@@ -1740,6 +1742,9 @@ static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
 	int reg;
 	u32 val;
 
+	assert_planes_disabled(dev_priv, pipe);
+	assert_sprites_disabled(dev_priv, pipe);
+
 	if (HAS_PCH_LPT(dev_priv->dev))
 		pch_transcoder = TRANSCODER_A;
 	else
-- 
1.7.10.4

Re: [PATCH 6/7] drm/i915: add pipe asserts for the crtc enable sequence

[Chris Wilson]

On Thu, Apr 11, 2013 at 04:29:09PM +0200, Daniel Vetter wrote:
> The i9xx modeset sequence is currently pretty fishy, so tight it all
> up with some good assert-sprinkling.
> 
> We already have good coverage on the disable side, but the enable side
> is spotty (since until recently it was wrong).
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 7/7] drm/i915: add i9xx pfit pipe asserts

[Daniel Vetter]

We can only enable the pfit if the pipe. Ensure that this is obied
with a neat assert.

Also check whether the pfit is off before enabling it - if not we've
lost track of things somewhere since the pfit is only ever used by the
lvds output.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_lvds.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index ca2d903..1ff981f 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -159,6 +159,9 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
 	if (HAS_PCH_SPLIT(dev) || !enc->pfit_control)
 		return;
 
+	WARN_ON(I915_READ(PFIT_CONTROL) & PFIT_ENABLE);
+	assert_pipe_disabled(dev_priv, to_intel_crtc(encoder->base.crtc)->pipe);
+
 	/*
 	 * Enable automatic panel scaling so that non-native modes
 	 * fill the screen.  The panel fitter should only be
-- 
1.7.10.4

Re: [PATCH 7/7] drm/i915: add i9xx pfit pipe asserts

[Ville Syrjälä]

On Thu, Apr 11, 2013 at 04:29:10PM +0200, Daniel Vetter wrote:
> We can only enable the pfit if the pipe. Ensure that this is obied
                                         ^

"is disabled" or something?

> with a neat assert.
> 
> Also check whether the pfit is off before enabling it - if not we've
> lost track of things somewhere since the pfit is only ever used by the
> lvds output.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_lvds.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index ca2d903..1ff981f 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -159,6 +159,9 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
>  	if (HAS_PCH_SPLIT(dev) || !enc->pfit_control)
>  		return;
>  
> +	WARN_ON(I915_READ(PFIT_CONTROL) & PFIT_ENABLE);
> +	assert_pipe_disabled(dev_priv, to_intel_crtc(encoder->base.crtc)->pipe);
> +
>  	/*
>  	 * Enable automatic panel scaling so that non-native modes
>  	 * fill the screen.  The panel fitter should only be
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 7/7] drm/i915: add i9xx pfit pipe asserts

[Daniel Vetter]

On Thu, Apr 11, 2013 at 6:37 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Apr 11, 2013 at 04:29:10PM +0200, Daniel Vetter wrote:
>> We can only enable the pfit if the pipe. Ensure that this is obied
>                                          ^
>
> "is disabled" or something?

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

Re: [PATCH 7/7] drm/i915: add i9xx pfit pipe asserts

[Jani Nikula]

On Thu, 11 Apr 2013, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Apr 11, 2013 at 04:29:10PM +0200, Daniel Vetter wrote:
>> We can only enable the pfit if the pipe. Ensure that this is obied
                                                                ^^^^^
"obeyed" or something?

Jani.

>                                          ^
>
> "is disabled" or something?
>
>> with a neat assert.
>> 
>> Also check whether the pfit is off before enabling it - if not we've
>> lost track of things somewhere since the pfit is only ever used by the
>> lvds output.
>> 
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>  drivers/gpu/drm/i915/intel_lvds.c |    3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
>> index ca2d903..1ff981f 100644
>> --- a/drivers/gpu/drm/i915/intel_lvds.c
>> +++ b/drivers/gpu/drm/i915/intel_lvds.c
>> @@ -159,6 +159,9 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
>>  	if (HAS_PCH_SPLIT(dev) || !enc->pfit_control)
>>  		return;
>>  
>> +	WARN_ON(I915_READ(PFIT_CONTROL) & PFIT_ENABLE);
>> +	assert_pipe_disabled(dev_priv, to_intel_crtc(encoder->base.crtc)->pipe);
>> +
>>  	/*
>>  	 * Enable automatic panel scaling so that non-native modes
>>  	 * fill the screen.  The panel fitter should only be
>> -- 
>> 1.7.10.4
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915: add i9xx pfit pipe asserts

[Chris Wilson]

On Thu, Apr 11, 2013 at 04:29:10PM +0200, Daniel Vetter wrote:
> We can only enable the pfit if the pipe. Ensure that this is obied
> with a neat assert.
> 
> Also check whether the pfit is off before enabling it - if not we've
> lost track of things somewhere since the pfit is only ever used by the
> lvds output.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Ignoring the pedants picking over the changelog,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 7/7] drm/i915: add i9xx pfit pipe asserts

[Daniel Vetter]

On Fri, Apr 12, 2013 at 03:02:32PM +0100, Chris Wilson wrote:
> On Thu, Apr 11, 2013 at 04:29:10PM +0200, Daniel Vetter wrote:
> > We can only enable the pfit if the pipe. Ensure that this is obied
> > with a neat assert.
> > 
> > Also check whether the pfit is off before enabling it - if not we've
> > lost track of things somewhere since the pfit is only ever used by the
> > lvds output.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Ignoring the pedants picking over the changelog,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Slurped in all the patches, thanks for reviewing them. Due to lack of disk
space not yet pushed out though.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915: move debug output back to the right place

[Daniel Vetter]

When adding the pipe config computation step I've accidentally moved
this a bit away. Which momentarily confused me since the pipe config
step rejected some modesetting operations I expected and so left me
looking in vain for that debug output.

v2: Move the debug output into the right function to prevent this from
happening again.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1b2c744..95f92ba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7629,6 +7629,9 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
 	/* ... and mask these out. */
 	*modeset_pipes &= ~(*disable_pipes);
 	*prepare_pipes &= ~(*disable_pipes);
+
+	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
+		      modeset_pipes, prepare_pipes, disable_pipes);
 }
 
 static bool intel_crtc_in_use(struct drm_crtc *crtc)
@@ -7859,9 +7862,6 @@ int intel_set_mode(struct drm_crtc *crtc,
 		}
 	}
 
-	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
-		      modeset_pipes, prepare_pipes, disable_pipes);
-
 	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
 		intel_crtc_disable(&intel_crtc->base);
 
-- 
1.7.10.4

Re: [PATCH] drm/i915: move debug output back to the right place

[Ville Syrjälä]

On Thu, Apr 11, 2013 at 04:39:53PM +0200, Daniel Vetter wrote:
> When adding the pipe config computation step I've accidentally moved
> this a bit away. Which momentarily confused me since the pipe config
> step rejected some modesetting operations I expected and so left me
> looking in vain for that debug output.
> 
> v2: Move the debug output into the right function to prevent this from
> happening again.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1b2c744..95f92ba 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7629,6 +7629,9 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
>  	/* ... and mask these out. */
>  	*modeset_pipes &= ~(*disable_pipes);
>  	*prepare_pipes &= ~(*disable_pipes);
> +
> +	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
> +		      modeset_pipes, prepare_pipes, disable_pipes);

*modeset_pipes, *prepare_pipes, *disable_pipes);

>  }
>  
>  static bool intel_crtc_in_use(struct drm_crtc *crtc)
> @@ -7859,9 +7862,6 @@ int intel_set_mode(struct drm_crtc *crtc,
>  		}
>  	}
>  
> -	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
> -		      modeset_pipes, prepare_pipes, disable_pipes);
> -
>  	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
>  		intel_crtc_disable(&intel_crtc->base);
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH] drm/i915: move debug output back to the right place

[Daniel Vetter]

When adding the pipe config computation step I've accidentally moved
this a bit away. Which momentarily confused me since the pipe config
step rejected some modesetting operations I expected and so left me
looking in vain for that debug output.

v2: Move the debug output into the right function to prevent this from
happening again.

v3: Make it compile (Ville). Also reorder the patch so that the two
bugfixes are first.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ea7ded1..47c540e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7647,6 +7647,9 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
 	/* HACK: We don't (yet) fully support global modesets. */
 	*modeset_pipes &= 1 << intel_crtc->pipe;
 	*prepare_pipes &= 1 << intel_crtc->pipe;
+
+	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
+		      *modeset_pipes, *prepare_pipes, *disable_pipes);
 }
 
 static bool intel_crtc_in_use(struct drm_crtc *crtc)
@@ -7877,9 +7880,6 @@ int intel_set_mode(struct drm_crtc *crtc,
 		}
 	}
 
-	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
-		      modeset_pipes, prepare_pipes, disable_pipes);
-
 	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
 		intel_crtc_disable(&intel_crtc->base);
 
-- 
1.7.10.4