[4/3] drm/i915: pass intel_crtc as argument for intel_enable_pipe

Commit Message

Paulo Zanoni Jan. 17, 2014, 3:51 p.m. UTC

From: Paulo Zanoni <[e-mail]>

We want to remove those 3 boolean arguments. This is the first step.
The "pipe" passed as the argument is always intel_crtc->pipe.

Also adjust the function documentation.

Signed-off-by: Paulo Zanoni <[e-mail]>
---
 drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

Comments

Lespiau, Damien Feb. 10, 2014, 2:17 p.m. UTC | #1

On Fri, Jan 17, 2014 at 01:51:09PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <[e-mail]>
> 
> We want to remove those 3 boolean arguments. This is the first step.
> The "pipe" passed as the argument is always intel_crtc->pipe.
> 
> Also adjust the function documentation.
> 
> Signed-off-by: Paulo Zanoni <[e-mail]>

Reviewed-by: Damien Lespiau <[e-mail]>

Daniel Vetter Feb. 10, 2014, 5:23 p.m. UTC | #2

On Mon, Feb 10, 2014 at 02:17:03PM +0000, Damien Lespiau wrote:
> On Fri, Jan 17, 2014 at 01:51:09PM -0200, Paulo Zanoni wrote:
> > From: Paulo Zanoni <[e-mail]>
> > 
> > We want to remove those 3 boolean arguments. This is the first step.
> > The "pipe" passed as the argument is always intel_crtc->pipe.
> > 
> > Also adjust the function documentation.
> > 
> > Signed-off-by: Paulo Zanoni <[e-mail]>
> 
> Reviewed-by: Damien Lespiau <[e-mail]>

Ok, I've pulled in the entire series, but a bunch of things changed so had
to resolve some (minor) conflicts. Please double-check that I didn't botch
things up too badly.

Thanks, Daniel

Paulo Zanoni Feb. 11, 2014, 3:23 p.m. UTC | #3

Hi

2014-02-10 15:23 GMT-02:00 Daniel Vetter <[e-mail]>:
> On Mon, Feb 10, 2014 at 02:17:03PM +0000, Damien Lespiau wrote:
>> On Fri, Jan 17, 2014 at 01:51:09PM -0200, Paulo Zanoni wrote:
>> > From: Paulo Zanoni <[e-mail]>
>> >
>> > We want to remove those 3 boolean arguments. This is the first step.
>> > The "pipe" passed as the argument is always intel_crtc->pipe.
>> >
>> > Also adjust the function documentation.
>> >
>> > Signed-off-by: Paulo Zanoni <[e-mail]>
>>
>> Reviewed-by: Damien Lespiau <[e-mail]>
>
> Ok, I've pulled in the entire series, but a bunch of things changed so had
> to resolve some (minor) conflicts. Please double-check that I didn't botch
> things up too badly.

You forgot to apply patch 2, and this is probably the reason why every
subsequent patch gave you a conflict.

You also applied patch 3 twice: once for Ironlake and once for
Haswell. You shouldn't change the Ironlake function.

Do you plan to rebase or do I need to submit patches on top?

IMHO if a series starts getting messy to apply, I think you should
probably just ask the author to rebase and resend the final stuff.
Maybe with this we would be able to reduce the amount of bad merges,
which is becoming a very common problem, at least for my patches.

Thanks,
Paulo

>
> Thanks, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> [phone] - http://blog.ffwll.ch

Daniel Vetter Feb. 11, 2014, 3:44 p.m. UTC | #4

On Tue, Feb 11, 2014 at 4:23 PM, Paulo Zanoni <[e-mail]> wrote:
>
> 2014-02-10 15:23 GMT-02:00 Daniel Vetter <[e-mail]>:
>> On Mon, Feb 10, 2014 at 02:17:03PM +0000, Damien Lespiau wrote:
>>> On Fri, Jan 17, 2014 at 01:51:09PM -0200, Paulo Zanoni wrote:
>>> > From: Paulo Zanoni <[e-mail]>
>>> >
>>> > We want to remove those 3 boolean arguments. This is the first step.
>>> > The "pipe" passed as the argument is always intel_crtc->pipe.
>>> >
>>> > Also adjust the function documentation.
>>> >
>>> > Signed-off-by: Paulo Zanoni <[e-mail]>
>>>
>>> Reviewed-by: Damien Lespiau <[e-mail]>
>>
>> Ok, I've pulled in the entire series, but a bunch of things changed so had
>> to resolve some (minor) conflicts. Please double-check that I didn't botch
>> things up too badly.
>
> You forgot to apply patch 2, and this is probably the reason why every
> subsequent patch gave you a conflict.

The conflicts where actually with one of Ville's patches to move the
plane enabling around. I've fixed those up but apparently then missed
the other conflict hidden underneath those.

> You also applied patch 3 twice: once for Ironlake and once for
> Haswell. You shouldn't change the Ironlake function.
>
> Do you plan to rebase or do I need to submit patches on top?

I've applied the missing patched and dropped the ironlake patch of the
double-merged one. So if the new tree looks ok no need to resend
anything.

> IMHO if a series starts getting messy to apply, I think you should
> probably just ask the author to rebase and resend the final stuff.
> Maybe with this we would be able to reduce the amount of bad merges,
> which is becoming a very common problem, at least for my patches.

The problem is that small conflicts are really common, both because I
want people to submit against drm-intel-nightly (so that I can do the
backmerging and branch shuffling correctly) and because of our
development speed. I don't think me asking for rebases in all these
cases is the better option. I tend to poke people to double-check when
I screw things up, but I guess it's just been bad luck that recently
the conflict fallout has always hit your patches :(

Cheers, Daniel

Paulo Zanoni Feb. 11, 2014, 5:09 p.m. UTC | #5

2014-02-11 13:44 GMT-02:00 Daniel Vetter <[e-mail]>:
> On Tue, Feb 11, 2014 at 4:23 PM, Paulo Zanoni <[e-mail]> wrote:
>>
>> 2014-02-10 15:23 GMT-02:00 Daniel Vetter <[e-mail]>:
>>> On Mon, Feb 10, 2014 at 02:17:03PM +0000, Damien Lespiau wrote:
>>>> On Fri, Jan 17, 2014 at 01:51:09PM -0200, Paulo Zanoni wrote:
>>>> > From: Paulo Zanoni <[e-mail]>
>>>> >
>>>> > We want to remove those 3 boolean arguments. This is the first step.
>>>> > The "pipe" passed as the argument is always intel_crtc->pipe.
>>>> >
>>>> > Also adjust the function documentation.
>>>> >
>>>> > Signed-off-by: Paulo Zanoni <[e-mail]>
>>>>
>>>> Reviewed-by: Damien Lespiau <[e-mail]>
>>>
>>> Ok, I've pulled in the entire series, but a bunch of things changed so had
>>> to resolve some (minor) conflicts. Please double-check that I didn't botch
>>> things up too badly.
>>
>> You forgot to apply patch 2, and this is probably the reason why every
>> subsequent patch gave you a conflict.
>
> The conflicts where actually with one of Ville's patches to move the
> plane enabling around. I've fixed those up but apparently then missed
> the other conflict hidden underneath those.
>
>> You also applied patch 3 twice: once for Ironlake and once for
>> Haswell. You shouldn't change the Ironlake function.
>>
>> Do you plan to rebase or do I need to submit patches on top?
>
> I've applied the missing patched and dropped the ironlake patch of the
> double-merged one. So if the new tree looks ok no need to resend
> anything.
>
>> IMHO if a series starts getting messy to apply, I think you should
>> probably just ask the author to rebase and resend the final stuff.
>> Maybe with this we would be able to reduce the amount of bad merges,
>> which is becoming a very common problem, at least for my patches.
>
> The problem is that small conflicts are really common, both because I
> want people to submit against drm-intel-nightly (so that I can do the
> backmerging and branch shuffling correctly) and because of our
> development speed. I don't think me asking for rebases in all these
> cases is the better option. I tend to poke people to double-check when
> I screw things up, but I guess it's just been bad luck that recently
> the conflict fallout has always hit your patches :(
>
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> [phone] - http://blog.ffwll.ch

Paulo Zanoni Feb. 11, 2014, 5:20 p.m. UTC | #6

2014-02-11 15:09 GMT-02:00 Paulo Zanoni <[e-mail]>:
> 2014-02-11 13:44 GMT-02:00 Daniel Vetter <[e-mail]>:
>> On Tue, Feb 11, 2014 at 4:23 PM, Paulo Zanoni <[e-mail]> wrote:
>>>
>>> 2014-02-10 15:23 GMT-02:00 Daniel Vetter <[e-mail]>:
>>>> On Mon, Feb 10, 2014 at 02:17:03PM +0000, Damien Lespiau wrote:
>>>>> On Fri, Jan 17, 2014 at 01:51:09PM -0200, Paulo Zanoni wrote:
>>>>> > From: Paulo Zanoni <[e-mail]>
>>>>> >
>>>>> > We want to remove those 3 boolean arguments. This is the first step.
>>>>> > The "pipe" passed as the argument is always intel_crtc->pipe.
>>>>> >
>>>>> > Also adjust the function documentation.
>>>>> >
>>>>> > Signed-off-by: Paulo Zanoni <[e-mail]>
>>>>>
>>>>> Reviewed-by: Damien Lespiau <[e-mail]>
>>>>
>>>> Ok, I've pulled in the entire series, but a bunch of things changed so had
>>>> to resolve some (minor) conflicts. Please double-check that I didn't botch
>>>> things up too badly.
>>>
>>> You forgot to apply patch 2, and this is probably the reason why every
>>> subsequent patch gave you a conflict.
>>
>> The conflicts where actually with one of Ville's patches to move the
>> plane enabling around. I've fixed those up but apparently then missed
>> the other conflict hidden underneath those.
>>
>>> You also applied patch 3 twice: once for Ironlake and once for
>>> Haswell. You shouldn't change the Ironlake function.
>>>
>>> Do you plan to rebase or do I need to submit patches on top?
>>
>> I've applied the missing patched and dropped the ironlake patch of the
>> double-merged one. So if the new tree looks ok no need to resend
>> anything.

Doesn't look ok yet.

So previously I had "[PATCH 2/8] drm/i915: don't wait for vblank after
enabling pipe on HSW", which removes a wait_for_vblank on HSW.

Then on "[PATCH 7/8] drm/i915: remove wait_for_vblank argument form
intel_enable_pipe" we just change the function parameters without
changing the function behavior.

With this, if we bisect something to patch 2 we know the problem is
that we stopped waiting for a vblank, and if we bisect to patch 7 we
know the problem is something else.

But since you just skipped patch 2, patch 7 is now more than just a
coding style change: it actually does what patch 2 was supposed to do.
So in a way, we can say patch 2 is not really necessary, but it was
written weeks before patch 7, and patch 7 should be just a result of
the review comments.

And the version of "drm/i915: don't wait for vblank after enabling
pipe on HSW" which you just committed as a last patch instead of
second patch (the one that changes the argument to
intel_crtc_update_cursor) is just plain wrong. That needs to be
reverted. So either we add the original patch 2 at the right place, or
we completely discard it...

I know it's common to change the patch ordering when applying to our
trees, but it can be quite dangerous...

Thanks,
Paulo

>>
>>> IMHO if a series starts getting messy to apply, I think you should
>>> probably just ask the author to rebase and resend the final stuff.
>>> Maybe with this we would be able to reduce the amount of bad merges,
>>> which is becoming a very common problem, at least for my patches.
>>
>> The problem is that small conflicts are really common, both because I
>> want people to submit against drm-intel-nightly (so that I can do the
>> backmerging and branch shuffling correctly) and because of our
>> development speed. I don't think me asking for rebases in all these
>> cases is the better option. I tend to poke people to double-check when
>> I screw things up, but I guess it's just been bad luck that recently
>> the conflict fallout has always hit your patches :(
>>
>> Cheers, Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> [phone] - http://blog.ffwll.ch
>
>
>
> --
> Paulo Zanoni

Daniel Vetter Feb. 11, 2014, 9:54 p.m. UTC | #7

On Tue, Feb 11, 2014 at 6:20 PM, Paulo Zanoni <[e-mail]> wrote:
> 2014-02-11 15:09 GMT-02:00 Paulo Zanoni <[e-mail]>:
>> 2014-02-11 13:44 GMT-02:00 Daniel Vetter <[e-mail]>:
>>> On Tue, Feb 11, 2014 at 4:23 PM, Paulo Zanoni <[e-mail]> wrote:
>>>>
>>>> 2014-02-10 15:23 GMT-02:00 Daniel Vetter <[e-mail]>:
>>>>> On Mon, Feb 10, 2014 at 02:17:03PM +0000, Damien Lespiau wrote:
>>>>>> On Fri, Jan 17, 2014 at 01:51:09PM -0200, Paulo Zanoni wrote:
>>>>>> > From: Paulo Zanoni <[e-mail]>
>>>>>> >
>>>>>> > We want to remove those 3 boolean arguments. This is the first step.
>>>>>> > The "pipe" passed as the argument is always intel_crtc->pipe.
>>>>>> >
>>>>>> > Also adjust the function documentation.
>>>>>> >
>>>>>> > Signed-off-by: Paulo Zanoni <[e-mail]>
>>>>>>
>>>>>> Reviewed-by: Damien Lespiau <[e-mail]>
>>>>>
>>>>> Ok, I've pulled in the entire series, but a bunch of things changed so had
>>>>> to resolve some (minor) conflicts. Please double-check that I didn't botch
>>>>> things up too badly.
>>>>
>>>> You forgot to apply patch 2, and this is probably the reason why every
>>>> subsequent patch gave you a conflict.
>>>
>>> The conflicts where actually with one of Ville's patches to move the
>>> plane enabling around. I've fixed those up but apparently then missed
>>> the other conflict hidden underneath those.
>>>
>>>> You also applied patch 3 twice: once for Ironlake and once for
>>>> Haswell. You shouldn't change the Ironlake function.
>>>>
>>>> Do you plan to rebase or do I need to submit patches on top?
>>>
>>> I've applied the missing patched and dropped the ironlake patch of the
>>> double-merged one. So if the new tree looks ok no need to resend
>>> anything.
>
> Doesn't look ok yet.

Oh dear ... I've tried again. Not sure whether I should have though :(
-Daniel

> So previously I had "[PATCH 2/8] drm/i915: don't wait for vblank after
> enabling pipe on HSW", which removes a wait_for_vblank on HSW.
>
> Then on "[PATCH 7/8] drm/i915: remove wait_for_vblank argument form
> intel_enable_pipe" we just change the function parameters without
> changing the function behavior.
>
> With this, if we bisect something to patch 2 we know the problem is
> that we stopped waiting for a vblank, and if we bisect to patch 7 we
> know the problem is something else.
>
> But since you just skipped patch 2, patch 7 is now more than just a
> coding style change: it actually does what patch 2 was supposed to do.
> So in a way, we can say patch 2 is not really necessary, but it was
> written weeks before patch 7, and patch 7 should be just a result of
> the review comments.
>
> And the version of "drm/i915: don't wait for vblank after enabling
> pipe on HSW" which you just committed as a last patch instead of
> second patch (the one that changes the argument to
> intel_crtc_update_cursor) is just plain wrong. That needs to be
> reverted. So either we add the original patch 2 at the right place, or
> we completely discard it...
>
> I know it's common to change the patch ordering when applying to our
> trees, but it can be quite dangerous...
>
> Thanks,
> Paulo
>
>>>
>>>> IMHO if a series starts getting messy to apply, I think you should
>>>> probably just ask the author to rebase and resend the final stuff.
>>>> Maybe with this we would be able to reduce the amount of bad merges,
>>>> which is becoming a very common problem, at least for my patches.
>>>
>>> The problem is that small conflicts are really common, both because I
>>> want people to submit against drm-intel-nightly (so that I can do the
>>> backmerging and branch shuffling correctly) and because of our
>>> development speed. I don't think me asking for rebases in all these
>>> cases is the better option. I tend to poke people to double-check when
>>> I screw things up, but I guess it's just been bad luck that recently
>>> the conflict fallout has always hit your patches :(
>>>
>>> Cheers, Daniel
>>> --
>>> Daniel Vetter
>>> Software Engineer, Intel Corporation
>>> [phone] - http://blog.ffwll.ch
>>
>>
>>
>> --
>> Paulo Zanoni
>
>
>
> --
> Paulo Zanoni

Paulo Zanoni Feb. 12, 2014, 3:56 p.m. UTC | #8

2014-02-11 19:54 GMT-02:00 Daniel Vetter <[e-mail]>:
> On Tue, Feb 11, 2014 at 6:20 PM, Paulo Zanoni <[e-mail]> wrote:
>> 2014-02-11 15:09 GMT-02:00 Paulo Zanoni <[e-mail]>:
>>> 2014-02-11 13:44 GMT-02:00 Daniel Vetter <[e-mail]>:
>>>> On Tue, Feb 11, 2014 at 4:23 PM, Paulo Zanoni <[e-mail]> wrote:
>>>>>
>>>>> 2014-02-10 15:23 GMT-02:00 Daniel Vetter <[e-mail]>:
>>>>>> On Mon, Feb 10, 2014 at 02:17:03PM +0000, Damien Lespiau wrote:
>>>>>>> On Fri, Jan 17, 2014 at 01:51:09PM -0200, Paulo Zanoni wrote:
>>>>>>> > From: Paulo Zanoni <[e-mail]>
>>>>>>> >
>>>>>>> > We want to remove those 3 boolean arguments. This is the first step.
>>>>>>> > The "pipe" passed as the argument is always intel_crtc->pipe.
>>>>>>> >
>>>>>>> > Also adjust the function documentation.
>>>>>>> >
>>>>>>> > Signed-off-by: Paulo Zanoni <[e-mail]>
>>>>>>>
>>>>>>> Reviewed-by: Damien Lespiau <[e-mail]>
>>>>>>
>>>>>> Ok, I've pulled in the entire series, but a bunch of things changed so had
>>>>>> to resolve some (minor) conflicts. Please double-check that I didn't botch
>>>>>> things up too badly.
>>>>>
>>>>> You forgot to apply patch 2, and this is probably the reason why every
>>>>> subsequent patch gave you a conflict.
>>>>
>>>> The conflicts where actually with one of Ville's patches to move the
>>>> plane enabling around. I've fixed those up but apparently then missed
>>>> the other conflict hidden underneath those.
>>>>
>>>>> You also applied patch 3 twice: once for Ironlake and once for
>>>>> Haswell. You shouldn't change the Ironlake function.
>>>>>
>>>>> Do you plan to rebase or do I need to submit patches on top?
>>>>
>>>> I've applied the missing patched and dropped the ironlake patch of the
>>>> double-merged one. So if the new tree looks ok no need to resend
>>>> anything.
>>
>> Doesn't look ok yet.
>
> Oh dear ... I've tried again. Not sure whether I should have though :(

Now it looks correct :)

> -Daniel
>
>> So previously I had "[PATCH 2/8] drm/i915: don't wait for vblank after
>> enabling pipe on HSW", which removes a wait_for_vblank on HSW.
>>
>> Then on "[PATCH 7/8] drm/i915: remove wait_for_vblank argument form
>> intel_enable_pipe" we just change the function parameters without
>> changing the function behavior.
>>
>> With this, if we bisect something to patch 2 we know the problem is
>> that we stopped waiting for a vblank, and if we bisect to patch 7 we
>> know the problem is something else.
>>
>> But since you just skipped patch 2, patch 7 is now more than just a
>> coding style change: it actually does what patch 2 was supposed to do.
>> So in a way, we can say patch 2 is not really necessary, but it was
>> written weeks before patch 7, and patch 7 should be just a result of
>> the review comments.
>>
>> And the version of "drm/i915: don't wait for vblank after enabling
>> pipe on HSW" which you just committed as a last patch instead of
>> second patch (the one that changes the argument to
>> intel_crtc_update_cursor) is just plain wrong. That needs to be
>> reverted. So either we add the original patch 2 at the right place, or
>> we completely discard it...
>>
>> I know it's common to change the patch ordering when applying to our
>> trees, but it can be quite dangerous...
>>
>> Thanks,
>> Paulo
>>
>>>>
>>>>> IMHO if a series starts getting messy to apply, I think you should
>>>>> probably just ask the author to rebase and resend the final stuff.
>>>>> Maybe with this we would be able to reduce the amount of bad merges,
>>>>> which is becoming a very common problem, at least for my patches.
>>>>
>>>> The problem is that small conflicts are really common, both because I
>>>> want people to submit against drm-intel-nightly (so that I can do the
>>>> backmerging and branch shuffling correctly) and because of our
>>>> development speed. I don't think me asking for rebases in all these
>>>> cases is the better option. I tend to poke people to double-check when
>>>> I screw things up, but I guess it's just been bad luck that recently
>>>> the conflict fallout has always hit your patches :(
>>>>
>>>> Cheers, Daniel
>>>> --
>>>> Daniel Vetter
>>>> Software Engineer, Intel Corporation
>>>> [phone] - http://blog.ffwll.ch
>>>
>>>
>>>
>>> --
>>> Paulo Zanoni
>>
>>
>>
>> --
>> Paulo Zanoni
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> [phone] - http://blog.ffwll.ch

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 332aa2d..e5fabba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1744,21 +1744,20 @@  static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
 
 /**
  * intel_enable_pipe - enable a pipe, asserting requirements
- * @dev_priv: i915 private structure
- * @pipe: pipe to enable
+ * @crtc: crtc responsible for the pipe
  * @pch_port: on ILK+, is this pipe driving a PCH port or not
+ * @dsi: output type is DSI
+ * @wait_for_vblank: whether we should for a vblank or not after enabling it
  *
- * Enable @pipe, making sure that various hardware specific requirements
+ * Enable @crtc's pipe, making sure that various hardware specific requirements
  * are met, if applicable, e.g. PLL enabled, LVDS pairs enabled, etc.
- *
- * @pipe should be %PIPE_A or %PIPE_B.
- *
- * Will wait until the pipe is actually running (i.e. first vblank) before
- * returning.
  */
-static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
+static void intel_enable_pipe(struct intel_crtc *crtc,
 			      bool pch_port, bool dsi, bool wait_for_vblank)
 {
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe = crtc->pipe;
 	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
 								      pipe);
 	enum pipe pch_transcoder;
@@ -3565,8 +3564,8 @@  static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc_load_lut(crtc);
 
 	intel_update_watermarks(crtc);
-	intel_enable_pipe(dev_priv, pipe,
-			  intel_crtc->config.has_pch_encoder, false, true);
+	intel_enable_pipe(intel_crtc, intel_crtc->config.has_pch_encoder, false,
+			  true);
 	intel_enable_primary_plane(dev_priv, plane, pipe);
 	intel_enable_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
@@ -3711,8 +3710,8 @@  static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_ddi_enable_transcoder_func(crtc);
 
 	intel_update_watermarks(crtc);
-	intel_enable_pipe(dev_priv, pipe,
-			  intel_crtc->config.has_pch_encoder, false, false);
+	intel_enable_pipe(intel_crtc, intel_crtc->config.has_pch_encoder, false,
+			  false);
 
 	if (intel_crtc->config.has_pch_encoder)
 		lpt_pch_enable(crtc);
@@ -4137,7 +4136,7 @@  static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc_load_lut(crtc);
 
 	intel_update_watermarks(crtc);
-	intel_enable_pipe(dev_priv, pipe, false, is_dsi, true);
+	intel_enable_pipe(intel_crtc, false, is_dsi, true);
 	intel_enable_primary_plane(dev_priv, plane, pipe);
 	intel_enable_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
@@ -4175,7 +4174,7 @@  static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc_load_lut(crtc);
 
 	intel_update_watermarks(crtc);
-	intel_enable_pipe(dev_priv, pipe, false, false, true);
+	intel_enable_pipe(intel_crtc, false, false, true);
 	intel_enable_primary_plane(dev_priv, plane, pipe);
 	intel_enable_planes(crtc);
 	/* The fixup needs to happen before cursor is enabled */