2014-11-24 13:54 GMT-02:00 Daniel Vetter <daniel.vetter@xxxxxxxx>: > Nothing in Bspec seems to indicate that we actually needs this, and it > looks like can't work since by this point the pipe is off and so > vblanks won't really happen any more. > > Note that Bspec mentions that it takes a vblank for this bit to > change, but _only_ when enabling. > > Dropping this code quenches an annoying backtrace introduced by the > more anal checking since > > commit 51e31d49c89055299e34b8f44d13f70e19aaaad1 > Author: Daniel Vetter <daniel.vetter@xxxxxxxx> > Date: Mon Sep 15 12:36:02 2014 +0200 > > drm/i915: Use generic vblank wait > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86095 > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_dp.c | 17 +---------------- > 1 file changed, 1 insertion(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 46731da407c0..63fcdbf90652 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3514,8 +3514,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) > enum port port = intel_dig_port->port; > struct drm_device *dev = intel_dig_port->base.base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *intel_crtc = > - to_intel_crtc(intel_dig_port->base.base.crtc); > uint32_t DP = intel_dp->DP; > > if (WARN_ON(HAS_DDI(dev))) > @@ -3540,8 +3538,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) > > if (HAS_PCH_IBX(dev) && > I915_READ(intel_dp->output_reg) & DP_PIPEB_SELECT) { > - struct drm_crtc *crtc = intel_dig_port->base.base.crtc; > - > /* Hardware workaround: leaving our transcoder select > * set to transcoder B while it's off will prevent the > * corresponding HDMI output on transcoder A. > @@ -3552,18 +3548,7 @@ intel_dp_link_down(struct intel_dp *intel_dp) > */ > DP &= ~DP_PIPEB_SELECT; > I915_WRITE(intel_dp->output_reg, DP); > - > - /* Changes to enable or select take place the vblank > - * after being written. > - */ > - if (WARN_ON(crtc == NULL)) { > - /* We should never try to disable a port without a crtc > - * attached. For paranoia keep the code around for a > - * bit. */ > - POSTING_READ(intel_dp->output_reg); > - msleep(50); > - } else > - intel_wait_for_vblank(dev, intel_crtc->pipe); What I can guess is that we have the vblank wait here because the DP_PORT_EN bit is still enabled at this point. It would make some sense to have it if the pipe were not off... So removing the waits looks sane: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> But when I read the spec, it makes me think that maybe doing the I915_WRITE above is also wrong, since the port is still enabled. Maybe we should only clear bit 30 in the same write as the one that clears bit 31? > + POSTING_READ(intel_dp->output_reg); > } > > DP &= ~DP_AUDIO_OUTPUT_ENABLE; > -- > 2.1.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
