From mboxrd@z Thu Jan 1 00:00:00 1970 From: Damien Lespiau Subject: Re: [PATCH 20/31] drm/i915: simplify the reduced clock handling for pch plls Date: Thu, 13 Jun 2013 12:26:51 +0100 Message-ID: <20130613112651.GK15338@strange.amr.corp.intel.com> References: <1370432073-27634-1-git-send-email-daniel.vetter@ffwll.ch> <1370432073-27634-21-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 mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 873CCE5C5F for ; Thu, 13 Jun 2013 04:26:55 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1370432073-27634-21-git-send-email-daniel.vetter@ffwll.ch> 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: Daniel Vetter Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Wed, Jun 05, 2013 at 01:34:22PM +0200, Daniel Vetter wrote: > Just move the lowfreq_avail logic out of the register writing as a > prep step for the next patch, which will coalesce all the pch pll > enabling into one spot. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index ecf0b1e..fc1b5f7 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5686,7 +5686,10 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > if (encoder->pre_pll_enable) > encoder->pre_pll_enable(encoder); > > - intel_crtc->lowfreq_avail = false; > + if (is_lvds && has_reduced_clock && i915_powersave) > + intel_crtc->lowfreq_avail = true; is_lvds doesn't seem necessary as ironlake_compute_clocks() won't set has_reduced_clock to true if !is_lvds. Doesn't hurt either. > + else > + intel_crtc->lowfreq_avail = false; > > if (intel_crtc->config.has_pch_encoder) { > pll = intel_crtc_to_shared_dpll(intel_crtc); > @@ -5704,12 +5707,10 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > */ > I915_WRITE(PCH_DPLL(pll->id), dpll); > > - if (is_lvds && has_reduced_clock && i915_powersave) { > + if (has_reduced_clock) > I915_WRITE(PCH_FP1(pll->id), fp2); Hum this is not quite the same condition? i915_powersave could be false and we don't want to take that branch? maybe reuse lowfreq_avail? Maybe compute_clocks() could check i915_powersave itself and set has_reduced_clock (or use_reduced_clock) correctly. > - intel_crtc->lowfreq_avail = true; > - } else { > + else > I915_WRITE(PCH_FP1(pll->id), fp); > - } > } > > intel_set_pipe_timings(intel_crtc); > -- > 1.7.11.7 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx