On Wed, Sep 12, 2012 at 06:00:58PM -0300, Paulo Zanoni wrote: > Hi > > 2012/9/6 Daniel Vetter <daniel.vetter at ffwll.ch>: > > By using the new pre_enabel/post_disable functions. > > s/enabel/enable > > > > > To ensure that we only frob the cpu edp pll while the pipe is off add > > the relevant asserts. Thanks to the new output state staging, this is > > now really easy. > > This patch does more than it says. It creates the new pre/post > enable/disable functions, but it also replaces the dpms connector > function with the default intel_connector_dpms (because after removing > the edp enable/disable code from the dpms function, it will look > almost exactly like intel_connector_dpms). Ideally this should be 2 > patches: first do the pre/post enable/disable dance, then switch the > specialized dpms with the generic one. The main reason to split this > in 2 patches is to make it easier to reviewers understand what's going > on, so they can review faster without trying to discover why you > switched the dpms function. But since you've already got a reviewer, > you should at least write about the dpms change in the commit message. > > With the typo fixed and at least a small sentence on the commit > message explaining the replacement of intel_dp_dpms with > intel_connector_dpms: > Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com> I've added a paragraph to explain why we can now simplify the dpms code, too. Thanks for the review, patches 1&2 applied to dinq. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
