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

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

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@linux.intel.com wrote:
> >>> From: Ville Syrjälä <ville.syrjala@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@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.

> 
> 
> >>>    }
> >>>    
> >>>    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@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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