From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/5] drm/i915: clean up the cpu edp pll special case Date: Thu, 13 Sep 2012 17:23:15 +0200 Message-ID: <20120913152315.GD5693@phenom.ffwll.local> References: <1346962544-7439-1-git-send-email-daniel.vetter@ffwll.ch> <1346962544-7439-3-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f41.google.com (mail-wg0-f41.google.com [74.125.82.41]) by gabe.freedesktop.org (Postfix) with ESMTP id 069B4A0E13 for ; Thu, 13 Sep 2012 08:22:41 -0700 (PDT) Received: by wgbds1 with SMTP id ds1so380711wgb.0 for ; Thu, 13 Sep 2012 08:22:41 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Paulo Zanoni Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Wed, Sep 12, 2012 at 06:00:58PM -0300, Paulo Zanoni wrote: > Hi > > 2012/9/6 Daniel Vetter : > > 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 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