Re: [PATCH] drm/i915/vlv: fix RC6 residency time calculation

[Imre Deak <imre.deak@intel.com>]

On pe, 2015-05-29 at 13:22 -0700, Rodrigo Vivi wrote:
> On Fri, May 29, 2015 at 5:56 AM, Imre Deak <imre.deak@intel.com> wrote:
> > On ma, 2015-05-18 at 18:46 +0300, Imre Deak wrote:
> >> The divider value to convert from CZ clock rate to ms needs a +1
> >> adjustment on VLV just like on CHV. This matches both the spec and
> >> the accuracy test by pm_rc6_residency.
> >>
> >> Testcase: igt/pm_rc6_residency
> >> Signed-off-by: Imre Deak <imre.deak@intel.com>
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76877
> >
> > Rodrigo, could you review this?
> >
> > Thanks,
> > Imre
> >
> >> ---
> >>  drivers/gpu/drm/i915/i915_sysfs.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> >> index 2476268..aa99efc 100644
> >> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> >> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> >> @@ -76,6 +76,8 @@ static u32 calc_residency(struct drm_device *dev, const u32 reg)
> >>                               /* chv counts are one less */
> >>                               czcount_30ns += 1;
> >>                       }
> >> +             } else {
> >> +                     czcount_30ns += 1;
> 
> Thanks for spec and explanation...
> Reading code again I believe czcount_30ns += 1; shouldn't be
> duplicated in middle of elses here.
> 
> It would be better to do something like:
> units = 0;
> div = 1000000ULL;
> 
> if (IS_CHERRYVIEW(dev) && czcount_30ns == 1) {
>     /* Special case for 320Mhz on CHV */
>      div = 10000000ULL;
>      units = 3125ULL;
> }
> 
> /* counts are one less */
> czcount_30ns += 1;

This would increment also for the special case, but I agree we should
simplify this. I'll follow up with v2.

> 
> >>               if (units == 0)
> >
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> But with my bikeshed or not let's move fwd because I hold this for to
> long already:
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx