Re: [PATCH 2/5] drm/i915: clean up the cpu edp pll special case

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Sep 12, 2012 at 06:00:58PM -0300, Paulo Zanoni wrote:
> Hi
> 
> 2012/9/6 Daniel Vetter <daniel.vetter@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@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