| Message ID | 1383684151-595-4-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 5 Nov 2013 22:42:30 +0200 ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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älä <ville.syrjala@linux.intel.com> > --- > 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/i915_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/i915/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_private *dev_priv) > { > int hpll_freq, vco_freq[] = { 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 = 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 = 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.
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älä <ville.syrjala@linux.intel.com> > > > > 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älä <ville.syrjala@linux.intel.com> > > --- > > 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/i915_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/i915/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_private *dev_priv) > > { > > int hpll_freq, vco_freq[] = { 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 = 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 = 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.
On Wed, Nov 06, 2013 at 10:41:22AM +0200, Ville Syrjälä 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älä <ville.syrjala@linux.intel.com> > > > > > > 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älä <ville.syrjala@linux.intel.com> > > > --- > > > 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/i915_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/i915/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_private *dev_priv) > > > { > > > int hpll_freq, vco_freq[] = { 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 = 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 = 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
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_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/i915/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_private *dev_priv) { int hpll_freq, vco_freq[] = { 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 = 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 = vco_freq[hpll_freq]; + + return dev_priv->hpll_vco; } /* Adjust CDclk dividers to allow high res or save power if possible */