Re: [PATCH 1/5] drm/i915/vlv: Move czclk to intel_pm

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Mar 04, 2019 at 06:57:15PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 01, 2019 at 04:49:31PM -0800, José Roberto de Souza wrote:
> > Moving VLV/CHV/BYT czclk to intel_pm as it is a core clock used as
> > base by several other GPU blocks including GT.
> > 
> > BSpec: 14370
> > 
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 12 ------------
> >  drivers/gpu/drm/i915/intel_pm.c      | 10 ++++++++++
> >  2 files changed, 10 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7c5e84ef5171..91a8ee611b12 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -180,17 +180,6 @@ int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> >  				 dev_priv->hpll_freq);
> >  }
> >  
> > -static void intel_update_czclk(struct drm_i915_private *dev_priv)
> > -{
> > -	if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
> > -		return;
> > -
> > -	dev_priv->czclk_freq = vlv_get_cck_clock_hpll(dev_priv, "czclk",
> > -						      CCK_CZ_CLOCK_CONTROL);
> > -
> > -	DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
> > -}
> > -
> >  static inline u32 /* units of 100MHz */
> >  intel_fdi_link_freq(struct drm_i915_private *dev_priv,
> >  		    const struct intel_crtc_state *pipe_config)
> > @@ -15533,7 +15522,6 @@ int intel_modeset_init(struct drm_device *dev)
> >  	intel_shared_dpll_init(dev);
> >  	intel_update_fdi_pll_freq(dev_priv);
> >  
> > -	intel_update_czclk(dev_priv);
> >  	intel_modeset_init_hw(dev);
> 
> Is is here because intel_modeset_init_hw() needs it.

No, actually it doesn't. Dang, I'm starting to forget the details. It
is needed by modeset code when it comes time to actually reprogam
cdclk.

> 
> >  
> >  	intel_hdcp_component_init(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 9c97a95c1816..cd363fa47cbc 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -7622,6 +7622,14 @@ static void vlv_init_gpll_ref_freq(struct drm_i915_private *dev_priv)
> >  			 dev_priv->gt_pm.rps.gpll_ref_freq);
> >  }
> >  
> > +static void vlv_update_czclk(struct drm_i915_private *dev_priv)
> > +{
> > +	dev_priv->czclk_freq = vlv_get_cck_clock_hpll(dev_priv, "czclk",
> > +						      CCK_CZ_CLOCK_CONTROL);
> > +
> > +	DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
> > +}
> > +
> >  static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
> >  {
> >  	struct intel_rps *rps = &dev_priv->gt_pm.rps;
> > @@ -7629,6 +7637,7 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
> >  
> >  	valleyview_setup_pctx(dev_priv);
> >  
> > +	vlv_update_czclk(dev_priv);
> 
> This is far too late AFAICS. Also putting it into gt/gem specific code
> is equally wrong as having it in the modeset code.

So this last argument still holds. We should put it into some common
code. Maybe we need some kind of clock_init() thing.

> 
> >  	vlv_init_gpll_ref_freq(dev_priv);
> >  
> >  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> > @@ -7675,6 +7684,7 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
> >  
> >  	cherryview_setup_pctx(dev_priv);
> >  
> > +	vlv_update_czclk(dev_priv);
> >  	vlv_init_gpll_ref_freq(dev_priv);
> >  
> >  	mutex_lock(&dev_priv->sb_lock);
> > -- 
> > 2.21.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx