From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 3/4] drm/i915: Store HPLL frequency in dev_priv on VLV Date: Wed, 6 Nov 2013 11:02:48 +0100 Message-ID: <20131106100248.GD14082@phenom.ffwll.local> References: <1383684151-595-1-git-send-email-ville.syrjala@linux.intel.com> <1383684151-595-4-git-send-email-ville.syrjala@linux.intel.com> <20131105130258.7c6749eb@jbarnes-desktop> <20131106084122.GI13047@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ea0-f178.google.com (mail-ea0-f178.google.com [209.85.215.178]) by gabe.freedesktop.org (Postfix) with ESMTP id C295E132112 for ; Wed, 6 Nov 2013 02:02:21 -0800 (PST) Received: by mail-ea0-f178.google.com with SMTP id a10so1742089eae.9 for ; Wed, 06 Nov 2013 02:02:20 -0800 (PST) Content-Disposition: inline In-Reply-To: <20131106084122.GI13047@intel.com> 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: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Nov 06, 2013 at 10:41:22AM +0200, Ville Syrj=E4l=E4 wrote: > On Tue, Nov 05, 2013 at 01:02:58PM -0800, Jesse Barnes wrote: > > On Tue, 5 Nov 2013 22:42:30 +0200 > > ville.syrjala@linux.intel.com wrote: > > = > > > From: Ville Syrj=E4l=E4 > > > = > > > Keep the HPLL frequencey in dev_priv on VLV instead of reading > > > it from CCK every time it's needed. > > > = > > > Signed-off-by: Ville Syrj=E4l=E4 > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > > drivers/gpu/drm/i915/intel_display.c | 7 ++++++- > > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > = > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i= 915_drv.h > > > index 4bae871..dd40925 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -1360,7 +1360,7 @@ typedef struct drm_i915_private { > > > int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */ > > > int num_fence_regs; /* 8 on pre-965, 16 otherwise */ > > > = > > > - unsigned int fsb_freq, mem_freq, is_ddr3; > > > + unsigned int fsb_freq, mem_freq, is_ddr3, hpll_vco; > > > = > > > /** > > > * wq - Driver workqueue for GEM. > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i= 915/intel_display.c > > > index 48f4990..f97e895 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -3898,13 +3898,18 @@ int valleyview_get_vco(struct drm_i915_privat= e *dev_priv) > > > { > > > int hpll_freq, vco_freq[] =3D { 800, 1600, 2000, 2400 }; > > > = > > > + if (dev_priv->hpll_vco) > > > + return dev_priv->hpll_vco; > > > + > > > /* Obtain SKU information */ > > > mutex_lock(&dev_priv->dpio_lock); > > > hpll_freq =3D vlv_cck_read(dev_priv, CCK_FUSE_REG) & > > > CCK_FUSE_HPLL_FREQ_MASK; > > > mutex_unlock(&dev_priv->dpio_lock); > > > = > > > - return vco_freq[hpll_freq]; > > > + dev_priv->hpll_vco =3D vco_freq[hpll_freq]; > > > + > > > + return dev_priv->hpll_vco; > > > } > > > = > > > /* Adjust CDclk dividers to allow high res or save power if possible= */ > > = > > I'd just move this to init_clock_gating or something, then use > > dev_priv->hpll_vco everywhere, rather than this conditional lazy > > initialization. > = > The problem was the we need it at gmbus init time, and we do that very > early. So I couldn't figure out a nice place to stick it, and so > I ended up doing the lazy thing. > = > I suspect the best thing to do would be to move gmbus init to happen > later, alongside other modeset setup. But I'm feeling a bit lazy and > don't want to tackle that task right now. > = > Anyways, since we don't want patch 4/4, I think for now we can just > drop this one as well. The places where we call it currently aren't > really that frequent or performance sensitive. init ordering hell strikes again. If we opt for a fixed order (the lazy ordering seems to not be too evil though in this case) I'd like to have a big WARN in there in case we get it wrong. -Daniel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch