On Mon, Mar 09, 2015 at 11:33:57AM +0200, Ander Conselvan De Oliveira wrote: > On Mon, 2015-03-09 at 11:24 +0200, Jani Nikula wrote: > > On Mon, 09 Mar 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx> wrote: > > > When enabling pipe C, the check for the number of lanes pipe B uses was > > > ignored in case pipe B wasn't active. This would allow pipe C to be > > > configured while pipe B is in DPMS off state even if it used more than 2 > > > lanes. Making pipe B active again while pipe C was also active would > > > then fail. > > > > Seems like a good catch. Broken when, or since forever? Cc: stable? > > Bugzillas? > > I had to touch this code in the last patch series I submitted, and I > raised a concern that this might do the wrong thing. Daniel suggested a > tried the test case I described above, which indeed does fail. I haven't > done the actual history digging until a minute ago, which turns out > quite interesting. The exact opposite of this patch was done in the > following patch: > > commit 1fbc0d789d12fec313c91912fc11733fdfbab863 > Author: Daniel Vetter <daniel.vetter@xxxxxxxx> > Date: Tue Oct 29 12:04:08 2013 +0100 > > drm/i915: Fix the PPT fdi lane bifurcate state handling on ivb > > I'm not sure how much has changed since then, or if the comments on that > commit's message are still relevant. Particularly, if the unifying of > mode set and dpms on code was ever done, and if it has any effect here. Indeed your patch would break things again I think for the case. What we essentially want to know is whether we've had to do a modeset change on a given pipe that might require us to update the bifurcate state. We abuse intel->active as a cheap way to tell since for the case we're interested in we have crtc->base.enabled == true and crtc->active == false. That's the case where the pipe will be enabled again, but has been shut down to change the mode. With atomic we need to probably look at crtc_state->mode_changed. As an interim solution we have the same information available in the modeset_pipes bitmask. I think replacing the check for intel_crtc->active with !(modeset_pipes & (1<<intel_crtc->pipe)) should work. The way to reproduce the original bug should be: - Light up pipe B with something requiring more than 2 lanes. - Do an immediate modeset on pipe B to a mode requiring at most 2 lanes. Not that SNA/X always does an interim modeset to everything off, you'd need to code up an igt I think. Or vt-switch between X servers with different modes. Without the intel_crtc->active check we won't set the bifurcate bit. - Try to light up pipe C and go boom on the bifurcate consistency checks. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
