On Wed, Oct 23, 2013 at 12:58 PM, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > I'm thinking the crtc->base.enabled check is actually pointless. > AFAICS we should never get here with crtc->base.enabled==false and > crtc->active==true Hm yeah. I was kinda shooting for the minimal thing here. > 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. The current logic (after this patch) is to - always clear the bit when both pipes are off. - always set if if we either enable pipe B or C and we'd need it to allow the other pipe to be lit up. If pipe B uses 4 lanes then we can't light pipe C up anymore, so we don't set the bit. There is a potential issue though with all pipes disabled with dpms off now. I think conceptually I need to precompute the desired state of the bifurcate bit in the compute config stage and then only set it here. > 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 pipe > B's output. The fix would be to call ivybridge_update_fdi_bc_bifurcation() > only when has_pch_encoder==true. has_pch_encoder should only be true if we have fdi_lanes > 0. So we shouldn't actually enable this stuff (and iirc I've tested the edp on pipe C with pipe B using 4 lanes). The actual configuration checks all happen in the pipe configuration computation stage. I'll rework the patch a bit (but that'll take a while). Thanks for the review, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html