Re: [PATCH 03/11] drm/i915: Move vlv cdclk code to .get_display_clock_speed()

Daniel Vetter
2014-07-07T09:16:55Z

On Wed, Jun 25, 2014 at 11:47:50AM -0700, Jesse Barnes wrote:
> On Fri, 13 Jun 2014 13:37:49 +0300
> ville.syrjala@linux.intel.com wrote:
>
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We have a standard hook for reading out the current cdclk. Move the VLV
> > code from valleyview_cur_cdclk() to .get_display_clock_speed().
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 33 +++++++++++++--------------------
> > drivers/gpu/drm/i915/intel_drv.h | 1 -
> > drivers/gpu/drm/i915/intel_pm.c | 2 +-
> > 3 files changed, 14 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 36562b5..61d7ea2 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4449,7 +4449,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > u32 val, cmd;
> >
> > - WARN_ON(valleyview_cur_cdclk(dev_priv) != dev_priv->vlv_cdclk_freq);
> > + WARN_ON(dev_priv->display.get_display_clock_speed(dev) != dev_priv->vlv_cdclk_freq);
> > dev_priv->vlv_cdclk_freq = cdclk;
> >
> > if (cdclk >= 320000) /* jump to highest voltage for 400MHz too */
> > @@ -4506,24 +4506,6 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
> > intel_i2c_reset(dev);
> > }
> >
> > -int valleyview_cur_cdclk(struct drm_i915_private *dev_priv)
> > -{
> > - int cur_cdclk, vco;
> > - int divider;
> > -
> > - vco = valleyview_get_vco(dev_priv);
> > -
> > - mutex_lock(&dev_priv->dpio_lock);
> > - divider = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> > - mutex_unlock(&dev_priv->dpio_lock);
> > -
> > - divider &= DISPLAY_FREQUENCY_VALUES;
> > -
> > - cur_cdclk = DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> > -
> > - return cur_cdclk;
> > -}
> > -
> > static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
> > int max_pixclk)
> > {
> > @@ -5228,7 +5210,18 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >
> > static int valleyview_get_display_clock_speed(struct drm_device *dev)
> > {
> > - return 400000; /* FIXME */
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + int vco = valleyview_get_vco(dev_priv);
> > + u32 val;
> > + int divider;
> > +
> > + mutex_lock(&dev_priv->dpio_lock);
> > + val = vlv_cck_read(dev_priv, CCK_DISPLAY_CLOCK_CONTROL);
> > + mutex_unlock(&dev_priv->dpio_lock);
> > +
> > + divider = val & DISPLAY_FREQUENCY_VALUES;
> > +
> > + return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> > }
> >
> > static int i945_get_display_clock_speed(struct drm_device *dev)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 78d4124..65ce0bb 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -717,7 +717,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> > const char *intel_output_name(int output);
> > bool intel_has_pending_fb_unpin(struct drm_device *dev);
> > int intel_pch_rawclk(struct drm_device *dev);
> > -int valleyview_cur_cdclk(struct drm_i915_private *dev_priv);
> > void intel_mark_busy(struct drm_device *dev);
> > void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
> > struct intel_engine_cs *ring);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index a9ddf74..67f019c1 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5535,7 +5535,7 @@ static void valleyview_init_clock_gating(struct drm_device *dev)
> > }
> > DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
> >
> > - dev_priv->vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
> > + dev_priv->vlv_cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> > DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz",
> > dev_priv->vlv_cdclk_freq);
> >
>
> Looks like we only really use that on < gen4 but so it seems harmless.

We have the same (or at least very similar) "can we dot this dotclock with
the given display clock, if not boost" kind of logic so imo fits into the
gen3 heritage of the vlv display block ...
-Daniel

>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>
> --
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch