On Tue, Sep 18, 2012 at 10:18 PM, Paulo Zanoni <przanoni at gmail.com> wrote: > 2012/9/12 Daniel Vetter <daniel at ffwll.ch>: >> On Wed, Sep 12, 2012 at 10:06:35AM -0300, Paulo Zanoni wrote: >>> From: Paulo Zanoni <paulo.r.zanoni at intel.com> > [ ... ] >>> + if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) { >> >> Since the plan is to move all the hsw_crtc_stuff out into it's own >> function I'd prefer a !HAS_PCH_LPT check here. > > I don't agree with the LPT check. Let me give some more details about > my plan so you can understand why I did this. > > My plan was to add even more (HAS_PCH_IBX || HAS_PCH_CPT) checks to > other places of this function and copy them all to the > haswell_crtc_mode_set version since I'm still not sure we'll never > ever have HSW with something older than LPT. After forking the Haswell > version, the plan was to add a WARN(!(HAS_PCH_IBX || HAS_PCH_CPT)) to > ironlake_crtc_mode_set and then remove the checks form it (leaving the > checks on haswell_crtc_mode_set untouched and prepared for the 42 new > PCHs they plan to ship after LPT). This code under the check was made > specifically for IBX/CPT (and PPT) and only tested on it, so I guess > the correct check is check for IBX/CPT. > > So, may I keep the IBX/CPT check? Should I change the plan instead? Makes sense, so I'm ok with keeping these checks and code-blocks in the haswell code. We can rip them out once haswell ships and we know whether the hw guys want to ship 42 different pch models or not. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch