Skip to content

Commit

Permalink
drm/i915: Push vblank enable/disable past encoder->enable/disable
Browse files Browse the repository at this point in the history
It is platform/output depenedent when exactly the pipe will start
running. Sometimes we just need the (cpu) pipe enabled, in other cases
the pch transcoder is enough and in yet other cases the (DP) port is
sending the frame start signal.

In a perfect world we'd put the drm_crtc_vblank_on call exactly where
the pipe starts running, but due to cloning and similar things this
will get messy. And the current approach of picking the most
conservative place for all combinations also doesn't work since that
results in legit vblank waits (in encoder->enable hooks, e.g. the 2
vblank waits for sdvo) failing.

Completely going back to the old world before

commit 51e31d4
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon Sep 15 12:36:02 2014 +0200

    drm/i915: Use generic vblank wait

isn't great either since screaming when the vblank wait work because
the pipe is off is kinda nice.

Pick a compromise and move the drm_crtc_vblank_on right before the
encoder->enable call. This is a lie on some outputs/platforms, but
after the ->enable callback the pipe is guaranteed to run everywhere.
So not that bad really. Suggested by Ville.

v2: Same treatment for drm_crtc_vblank_off and encoder->disable: I've
missed the ibx pipe B select w/a, which also has a vblank wait in the
disable function (while the pipe is obviously still running).

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
  • Loading branch information
Daniel Vetter committed Jan 7, 2015
1 parent 676fa57 commit f9b61ff
Showing 1 changed file with 21 additions and 21 deletions.
42 changes: 21 additions & 21 deletions drivers/gpu/drm/i915/intel_display.c
Original file line number Diff line number Diff line change
Expand Up @@ -4301,15 +4301,15 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc->config.has_pch_encoder)
ironlake_pch_enable(crtc);

assert_vblank_disabled(crtc);
drm_crtc_vblank_on(crtc);

for_each_encoder_on_crtc(dev, crtc, encoder)
encoder->enable(encoder);

if (HAS_PCH_CPT(dev))
cpt_verify_modeset(dev, intel_crtc->pipe);

assert_vblank_disabled(crtc);
drm_crtc_vblank_on(crtc);

intel_crtc_enable_planes(crtc);
}

Expand Down Expand Up @@ -4421,14 +4421,14 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc->config.dp_encoder_is_mst)
intel_ddi_set_vc_payload_alloc(crtc, true);

assert_vblank_disabled(crtc);
drm_crtc_vblank_on(crtc);

for_each_encoder_on_crtc(dev, crtc, encoder) {
encoder->enable(encoder);
intel_opregion_notify_encoder(encoder, true);
}

assert_vblank_disabled(crtc);
drm_crtc_vblank_on(crtc);

/* If we change the relative order between pipe/planes enabling, we need
* to change the workaround. */
haswell_mode_set_planes_workaround(intel_crtc);
Expand Down Expand Up @@ -4479,12 +4479,12 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)

intel_crtc_disable_planes(crtc);

drm_crtc_vblank_off(crtc);
assert_vblank_disabled(crtc);

for_each_encoder_on_crtc(dev, crtc, encoder)
encoder->disable(encoder);

drm_crtc_vblank_off(crtc);
assert_vblank_disabled(crtc);

if (intel_crtc->config.has_pch_encoder)
intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);

Expand Down Expand Up @@ -4544,14 +4544,14 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)

intel_crtc_disable_planes(crtc);

drm_crtc_vblank_off(crtc);
assert_vblank_disabled(crtc);

for_each_encoder_on_crtc(dev, crtc, encoder) {
intel_opregion_notify_encoder(encoder, false);
encoder->disable(encoder);
}

drm_crtc_vblank_off(crtc);
assert_vblank_disabled(crtc);

if (intel_crtc->config.has_pch_encoder)
intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
false);
Expand Down Expand Up @@ -5021,12 +5021,12 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
intel_update_watermarks(crtc);
intel_enable_pipe(intel_crtc);

for_each_encoder_on_crtc(dev, crtc, encoder)
encoder->enable(encoder);

assert_vblank_disabled(crtc);
drm_crtc_vblank_on(crtc);

for_each_encoder_on_crtc(dev, crtc, encoder)
encoder->enable(encoder);

intel_crtc_enable_planes(crtc);

/* Underruns don't raise interrupts, so check manually. */
Expand Down Expand Up @@ -5082,12 +5082,12 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
intel_update_watermarks(crtc);
intel_enable_pipe(intel_crtc);

for_each_encoder_on_crtc(dev, crtc, encoder)
encoder->enable(encoder);

assert_vblank_disabled(crtc);
drm_crtc_vblank_on(crtc);

for_each_encoder_on_crtc(dev, crtc, encoder)
encoder->enable(encoder);

intel_crtc_enable_planes(crtc);

/*
Expand Down Expand Up @@ -5159,12 +5159,12 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
*/
intel_wait_for_vblank(dev, pipe);

drm_crtc_vblank_off(crtc);
assert_vblank_disabled(crtc);

for_each_encoder_on_crtc(dev, crtc, encoder)
encoder->disable(encoder);

drm_crtc_vblank_off(crtc);
assert_vblank_disabled(crtc);

intel_disable_pipe(intel_crtc);

i9xx_pfit_disable(intel_crtc);
Expand Down

0 comments on commit f9b61ff

Please sign in to comment.