From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [Intel-gfx] [PATCH] drm/i915: Sanitize the PPT fdi lane bifurcate state on ivb Date: Wed, 23 Oct 2013 13:58:26 +0300 Message-ID: <20131023105826.GH13047@intel.com> References: <1382445473-17558-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <1382445473-17558-1-git-send-email-daniel.vetter@ffwll.ch> Sender: stable-owner@vger.kernel.org To: Daniel Vetter Cc: Intel Graphics Development , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Oct 22, 2013 at 02:37:53PM +0200, Daniel Vetter wrote: > We expect this bit to be always set when possible, but some BIOSes ar= e > lazy and don't do this. The result is a pile of WARNs and unhappy fdi > link training code ... >=20 > v2: It's actually the inverse: The BIOS sets this bit when it's not > strictly needed. This should be cleaned up in the > global_modeset_resources callback, but we've failed to look at the > active bit. Which means this won't fire (and so clean up BIOS state) > when enabling pipe B or C for the first time. >=20 > v3: Wrap lines. >=20 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3D70507 > Tested-by: Jan-Michael Brummer > Cc: stable@vger.kernel.org > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i= 915/intel_display.c > index cfe9e709..3569db6 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2421,9 +2421,10 @@ static void intel_fdi_normal_train(struct drm_= crtc *crtc) > FDI_FE_ERRC_ENABLE); > } > =20 > -static bool pipe_has_enabled_pch(struct intel_crtc *intel_crtc) > +static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > { > - return intel_crtc->base.enabled && intel_crtc->config.has_pch_encod= er; > + return crtc->base.enabled && crtc->active && > + crtc->config.has_pch_encoder; I'm thinking the crtc->base.enabled check is actually pointless. AFAICS we should never get here with crtc->base.enabled=3D=3Dfalse and crtc->active=3D=3Dtrue. We anyway re-enable the bifurcation bit when we do the mode_set. Actually that in itself could be a maybe a bug. We'd turn off the bifurcation bit when both pipes B and C are disabled based on prepare_pipes, but we only do the mode_set based on modeset_pipes. But currently we are saved by the fact that those two bitmasks are the same. Another potential bug I found is that we always set the bifurcation bit in mode_set, even if we're not using FDI. So if we have eg. pipe B enabled w/ FDI using 4 lanes, and then we enable pipe C w/ EDP, we'd still flip the bifurcation bit in pipe C's mode_set and destroy pi= pe B's output. The fix would be to call ivybridge_update_fdi_bc_bifurcatio= n() only when has_pch_encoder=3D=3Dtrue. > } > =20 > static void ivb_modeset_global_resources(struct drm_device *dev) > --=20 > 1.8.4.rc3 >=20 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx --=20 Ville Syrj=E4l=E4 Intel OTC