On 07/04/2012 03:21 PM, Paulo Zanoni wrote: > It looks like the check above could go on a separate commit. The first > commit just moves code around, the second one adds the check. This patch attempts to reset cpt_pch_enable to what ironlake_pch_enable was before we added LPT support into it; and make lpt_pch_enable account for LPT checks and such. But yes, I can split it into several commits. > I may have misunderstood something about this code which is not > familiar to me, but instead of limiting everything to pipe 0, can't we > just iterate through all pipes, read DDI_FUNC_CTL(pipe) and see if > there's any other enabled pipe on FDI mode? Yes, the idea in those asserts and checks was that as only 1 pipe can work in FDI mode, and as we only have 1 FDI RX, 1 PCH Transcoder and so on, we simple forced pipe1 to work in this mode, as this way most of older code which uses macros still works. But there is certainly a room for improvements. >> -static void ironlake_pch_enable(struct drm_crtc *crtc) >> +static void cpt_pch_enable(struct drm_crtc *crtc) > > Since this will run on both IBX and CPT, shouldn't we call it > ibx_pch_enable? At least on intel_hdmi.c, the functions are named > after the earliest generation that can run them. Yes, I can rename it to ibx_pch_enable. I'll do those changes and split this one into smaller commits and will resend later. Thanks! Eugeni