From patchwork Wed Sep 25 21:48:05 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 2944721 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id D1B959F288 for ; Wed, 25 Sep 2013 21:48:25 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B6D81202EA for ; Wed, 25 Sep 2013 21:48:24 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id B3A5220221 for ; Wed, 25 Sep 2013 21:48:23 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 97FBBE6273 for ; Wed, 25 Sep 2013 14:48:23 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (server109-228-4-14.live-servers.net [109.228.4.14]) by gabe.freedesktop.org (Postfix) with ESMTP id 534B7E5C1D for ; Wed, 25 Sep 2013 14:48:09 -0700 (PDT) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=87.106.93.118; Received: from fireflyinternet.com (unverified [87.106.93.118]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 129939648-1500050 for ; Wed, 25 Sep 2013 22:48:06 +0100 X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.73.22; Received: from nuc-i3427.alporthouse.com (unverified [78.156.73.22]) by fireflyinternet.com (Firefly Internet (M2)) with ESMTP id 15503583-1500048 for multiple; Wed, 25 Sep 2013 22:48:28 +0100 Received: by nuc-i3427.alporthouse.com (sSMTP sendmail emulation); Wed, 25 Sep 2013 22:48:05 +0100 Date: Wed, 25 Sep 2013 22:48:05 +0100 From: Chris Wilson To: Daniel Vetter Message-ID: <20130925214805.GC17570@nuc-i3427.alporthouse.com> Mail-Followup-To: Chris Wilson , Daniel Vetter , Jesse Barnes , intel-gfx@lists.freedesktop.org References: <1379698123-16249-1-git-send-email-jbarnes@virtuousgeek.org> <1379701772-12818-1-git-send-email-jbarnes@virtuousgeek.org> <20130923180144.GV4531@intel.com> <20130925103833.7ee23acb@jbarnes-desktop> <20130925210020.GR12663@phenom.ffwll.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130925210020.GR12663@phenom.ffwll.local> User-Agent: Mutt/1.5.21 (2010-09-15) X-Originating-IP: 78.156.73.22 X-To-Not-Matched: true Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: add VLV specific clock_get function v3 X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Sep 25, 2013 at 11:00:20PM +0200, Daniel Vetter wrote: > On Wed, Sep 25, 2013 at 10:38:33AM -0700, Jesse Barnes wrote: > > On Mon, 23 Sep 2013 21:01:44 +0300 > > Ville Syrjälä wrote: > > > > > On Fri, Sep 20, 2013 at 11:29:32AM -0700, Jesse Barnes wrote: > > > > Calculation is a little different than other platforms. > > > > > > > > v2: update to use port_clock instead > > > > rebase on top of Ville's changes > > > > v3: update to new port_clock semantics - don't divide by > > > > pixel_multiplier (Ville) > > > > > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=67345 > > > > Signed-off-by: Jesse Barnes > > > > --- > > > > drivers/gpu/drm/i915/intel_display.c | 33 ++++++++++++++++++++++++++++++++- > > > > 1 file changed, 32 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > > index 7eecf37..e5c9c1c 100644 > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > @@ -5048,6 +5048,34 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc, > > > > I915_READ(LVDS) & LVDS_BORDER_ENABLE; > > > > } > > > > > > > > +static void vlv_crtc_clock_get(struct intel_crtc *crtc, > > > > + struct intel_crtc_config *pipe_config) > > > > +{ > > > > + struct drm_device *dev = crtc->base.dev; > > > > + struct drm_i915_private *dev_priv = dev->dev_private; > > > > + int pipe = pipe_config->cpu_transcoder; > > > > + intel_clock_t clock; > > > > + u32 mdiv; > > > > + int refclk = 100000, fastclk, update_rate; > > > > + > > > > + mutex_lock(&dev_priv->dpio_lock); > > > > + mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe)); > > > > + mutex_unlock(&dev_priv->dpio_lock); > > > > + > > > > + clock.m1 = (mdiv >> DPIO_M1DIV_SHIFT) & 7; > > > > + clock.m2 = mdiv & DPIO_M2DIV_MASK; > > > > + clock.n = (mdiv >> DPIO_N_SHIFT) & 0xf; > > > > + clock.p1 = (mdiv >> DPIO_P1_SHIFT) & 7; > > > > + clock.p2 = (mdiv >> DPIO_P2_SHIFT) & 0x1f; > > > > + > > > > + update_rate = refclk / clock.n; > > > > + clock.vco = update_rate * clock.m1 * clock.m2; > > > > + fastclk = clock.vco / clock.p1 / clock.p2; > > > > + clock.dot = (2 * fastclk); > > > > + > > > > + pipe_config->port_clock = clock.dot / 10; > > > > > > Looks like it should get roughly the right answer, but I don't see much > > > point in all the intermediate results. > > > > > > If you want to keep some of them for clarity, then I think this should > > > be enough: > > > > > > clock.vco = refclk * clock.m / clock.n; > > > clock.dot = clock.vco / clock.p; /* fast clock */ > > > pipe_config->port_clock = clock.dot / 5; > > > > > > Although calling the fast clock "dot" is a bit wrong, but I think it's > > > fine here, especially as it matches what I have in mind for > > > vlv_find_best_dpll(). I had to rewrite that sucker a bit to make it > > > compatible with my brain ;) I'll send a patch for that ASAP. > > > > The other advantage of having it all spelled out like this is that it > > matches the freq calculation spreadsheet I sourced it from. > > > > So I'd prefer to keep this as is, if I can have your r-b anyway. :) > > Queued for -next with Ville's irc-r-b, thanks for the patch. > -Daniel Did Jesse post a delta update to this, as #67345 still warns with v3. For the record, this works - can't see why it would make a difference at the moment though (probably some type is too small or loss of precision): diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5741d48..5219fdc 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5083,10 +5083,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc, { struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; + const int refclk = 100000; int pipe = pipe_config->cpu_transcoder; intel_clock_t clock; u32 mdiv; - int refclk = 100000, fastclk, update_rate; mutex_lock(&dev_priv->dpio_lock); mdiv = vlv_dpio_read(dev_priv, pipe, DPIO_DIV(pipe)); @@ -5098,12 +5098,16 @@ static void vlv_crtc_clock_get(struct intel_crtc *crtc, clock.p1 = (mdiv >> DPIO_P1_SHIFT) & 7; clock.p2 = (mdiv >> DPIO_P2_SHIFT) & 0x1f; - update_rate = refclk / clock.n; - clock.vco = update_rate * clock.m1 * clock.m2; - fastclk = clock.vco / clock.p1 / clock.p2; - clock.dot = (2 * fastclk); + clock.vco = refclk * clock.m1 * clock.m2 / clock.n; + clock.dot = 2 * clock.vco / (clock.p1 * clock.p2); pipe_config->port_clock = clock.dot / 10; } -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx