Em Seg, 2018-11-12 às 16:54 +0200, Ville Syrjälä escreveu:
> On Fri, Nov 09, 2018 at 04:23:50PM -0800, Paulo Zanoni wrote:
> > I think I'm probably the one who argued in favor of having separate
> > implementations for both PCHs, but the calculations are actually
> > the
> > same, the clocks are the same and the only difference is that on
> > ICP
> > we write the numerator to the register.
> >
> > I have previously suggested to kill cnp_rawclk() and keep the
> > icp_rawclk() style, but Ville gave some good arguments that what's
> > in
> > this patch may be the better choice.
> >
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_cdclk.c | 37 ++++++++----------------
> > -------------
> > 1 file changed, 8 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> > b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 928671936286..60437675354e 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -2661,36 +2661,17 @@ static int cnp_rawclk(struct
> > drm_i915_private *dev_priv)
> > }
> >
> > rawclk = CNP_RAWCLK_DIV(divider / 1000);
> > - if (fraction)
> > - rawclk |= CNP_RAWCLK_DEN(DIV_ROUND_CLOSEST(1000,
> > - fractio
> > n) - 1);
> > + if (fraction) {
> > + int numerator = 1000;
> >
> > - I915_WRITE(PCH_RAWCLK_FREQ, rawclk);
> > - return divider + fraction;
> > -}
> > -
> > -static int icp_rawclk(struct drm_i915_private *dev_priv)
> > -{
> > - u32 rawclk;
> > - int divider, numerator, denominator, frequency;
> > -
> > - if (I915_READ(SFUSE_STRAP) & SFUSE_STRAP_RAW_FREQUENCY) {
> > - frequency = 24000;
> > - divider = 24;
> > - numerator = 0;
> > - denominator = 0;
> > - } else {
> > - frequency = 19200;
> > - divider = 19;
> > - numerator = 1;
> > - denominator = 4;
> > + rawclk |=
> > CNP_RAWCLK_DEN(DIV_ROUND_CLOSEST(numerator,
> > + fractio
> > n) - 1);
> > + if (HAS_PCH_ICP(dev_priv))
> > + rawclk |= ICP_RAWCLK_NUM(numerator /
> > 1000);
>
> Maybe
> int numerator = 1;
> ...
> DIV_ROUND_CLOSEST(numerator * 1000, ... ?
Will do. I guess I tried to keep the logic of "local variables should
be in KHz and conversion to MHz goes inside the reg-writing macros",
but I don't feel it is better or worse than the suggestion.
>
> The fixed numerator is OK since we only have to support 19.2 for
> now. If in the future we get more frequencies we may want to make
> this more flexible. But not much point in worrying about that now.
>
> Series is
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Thanks a lot. Sorry for the huge delay in the follow-up.
>
> > }
> >
> > - rawclk = CNP_RAWCLK_DIV(divider) |
> > ICP_RAWCLK_NUM(numerator) |
> > - CNP_RAWCLK_DEN(denominator);
> > -
> > I915_WRITE(PCH_RAWCLK_FREQ, rawclk);
> > - return frequency;
> > + return divider + fraction;
> > }
> >
> > static int pch_rawclk(struct drm_i915_private *dev_priv)
> > @@ -2740,9 +2721,7 @@ static int g4x_hrawclk(struct
> > drm_i915_private *dev_priv)
> > */
> > void intel_update_rawclk(struct drm_i915_private *dev_priv)
> > {
> > - if (HAS_PCH_ICP(dev_priv))
> > - dev_priv->rawclk_freq = icp_rawclk(dev_priv);
> > - else if (HAS_PCH_CNP(dev_priv))
> > + if (HAS_PCH_CNP(dev_priv) || HAS_PCH_ICP(dev_priv))
> > dev_priv->rawclk_freq = cnp_rawclk(dev_priv);
> > else if (HAS_PCH_SPLIT(dev_priv))
> > dev_priv->rawclk_freq = pch_rawclk(dev_priv);
> > --
> > 2.14.4
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
