[Intel-gfx] [PATCH 2/5] drm/i915: clean up the cpu edp pll special case
Daniel Vetter
daniel at ffwll.ch
Thu Sep 13 17:23:15 CEST 2012
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
More information about the Intel-gfx
mailing list