On Mon, 16 Feb 2015, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: > On Wed, 07 Jan 2015, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: >> 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 51e31d49c89055299e34b8f44d13f70e19aaaad1 >> Author: Daniel Vetter <daniel.vetter@xxxxxxxx> >> Date: Mon Sep 15 12:36:02 2014 +0200 >> >> drm/i915: Use generic vblank wait# Please enter the commit message for your changes. Lines starting >> >> 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@xxxxxxxxxxxxxxx> >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> Acked-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > Should this be forwarded to stable 3.19? https://bugs.freedesktop.org/show_bug.cgi?id=89108 and probably http://mid.gmane.org/20150131211609.GA3710@yulia-desktop BR, Jani. > > BR, > Jani. > > >> --- >> drivers/gpu/drm/i915/intel_display.c | 42 ++++++++++++++++++------------------ >> 1 file changed, 21 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index a1dbe747a372..e224820ea5a4 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -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); >> } >> >> @@ -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); >> @@ -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); >> >> @@ -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); >> @@ -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. */ >> @@ -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); >> >> /* >> @@ -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); >> -- >> 2.1.3 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
