[Intel-gfx] [PATCH 2/4] drm/i915: Fix chv GPU freq<->opcode conversions

[Ville Syrjälä]

On Tue, Nov 18, 2014 at 08:03:15PM +0530, Deepak S wrote:
> 
> On Monday 17 November 2014 06:11 PM, Ville Syrjälä wrote:
> > On Tue, Nov 18, 2014 at 05:59:07PM +0530, Deepak S wrote:
> >> On Monday 17 November 2014 05:05 PM, Ville Syrjälä wrote:
> >>> On Tue, Nov 18, 2014 at 02:38:25PM +0530, Deepak S wrote:
> >>>> On Tuesday 11 November 2014 02:25 AM, ville.syrjala at linux.intel.com wrote:
> >>>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >>>>>
> >>>>> Currently we miscalculate the GPU frequency on chv. This causes us to
> >>>>> report the GPU frequency as half of what it really is. Drop the extra
> >>>>> factor of 2 from the calculations to get the correct answer.
> >>>>>
> >>>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >>>>> ---
> >>>>>     drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> >>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>>>> index 03fbb45..74e4293 100644
> >>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
> >>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >>>>> @@ -7329,7 +7329,7 @@ static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val)
> >>>>>     	if (div < 0)
> >>>>>     		return div;
> >>>>>     
> >>>>> -	return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div) / 2;
> >>>>> +	return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div);
> >>>> I think CHV is 2 X cck, shouldn't we report the cck freq and not 2 *cck?
> >>> Hmm. Once again the docs are extremely unclear. Dropping the 2x factor
> >>> gives me the same numbers that the tables in the docs have. But then
> >>> the spreadsheet calls it a "2x clock" in some places, which does suggest
> >>> it might get further divided down by 2.
> >>>
> >>> Oh, now I did find a somewhat clear note in the clock HAS:
> >>> "The dedicated GPLL (Graphics PLL) sends a 2GHz gfx clock to GenLC,
> >>> which gets divided inside the GenLC block to derive a 1GHz Gfx fast clock."
> >>>
> >>> So based on that the original code does make more sense.
> >>>
> >> Do we need to mention in comment about 2 * GFX clock?
> > Yeah, a comment would probably be a good idea. Could avoid some confusion
> > in the future if someone else looks at this code.
> 
> Will you add the comment and submit a patch?

Actually I'm no longer sure about the 2x :(

I also see the following note in the clock HAS:
GT_GFCLK: This is the full rate clock, its max freq is 2000MHz.

Which is sort of telling me that there's no /2 for gfclk.

I guess it doesn't really matter what we report as long as we
consistently do the conversion the the same way both ways. But it
would be nice if we would end up reporting the frequency of the same
clock that BDW reports. Unfortunately the BDW docs I have are even
more useless in this regard.

> 
> >>
> >>>>>     }
> >>>>>     
> >>>>>     static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
> >>>>> @@ -7341,7 +7341,7 @@ static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
> >>>>>     		return mul;
> >>>>>     
> >>>>>     	/* CHV needs even values */
> >>>>> -	return DIV_ROUND_CLOSEST(val * 2 * mul, czclk_freq) * 2;
> >>>>> +	return DIV_ROUND_CLOSEST(val * mul, czclk_freq) * 2;
> >>>>>     }
> >>>>>     
> >>>>>     int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val)
> >>>> _______________________________________________
> >>>> Intel-gfx mailing list
> >>>> Intel-gfx at lists.freedesktop.org
> >>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC