[Intel-gfx] [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C

[Daniel Vetter]

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 at intel.com> 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 at ffwll.ch>
> 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