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@xxxxxxxxx>
> > Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
