On Tue, Jun 02, 2015 at 12:37:13PM +0300, Imre Deak wrote:
> On ma, 2015-06-01 at 12:01 -0700, Rodrigo Vivi wrote:
> > On Mon, Jun 1, 2015 at 12:32 AM, Imre Deak <imre.deak@xxxxxxxxx> 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.
> > >
> > > v2:
> > > - simplify logic checking for the CHV 320MHz special case (Rodrigo)
> > >
> > > Testcase: igt/pm_rc6_residency
> > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/i915_sysfs.c | 22 +++++++---------------
> > > 1 file changed, 7 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> > > index 2476268..55bd04c 100644
> > > --- a/drivers/gpu/drm/i915/i915_sysfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> > > @@ -64,24 +64,16 @@ static u32 calc_residency(struct drm_device *dev, const u32 reg)
> > > goto out;
> > > }
> > >
> > > - units = 0;
> > > - div = 1000000ULL;
> > > -
> > > - if (IS_CHERRYVIEW(dev)) {
> > > + if (IS_CHERRYVIEW(dev) && czcount_30ns == 1) {
> > > /* Special case for 320Mhz */
> > > - if (czcount_30ns == 1) {
> > > - div = 10000000ULL;
> > > - units = 3125ULL;
> > > - } else {
> > > - /* chv counts are one less */
> > > - czcount_30ns += 1;
> > > - }
> > > + div = 10000000ULL;
> > > + units = 3125ULL;
> > > + } else {
> > > + czcount_30ns += 1;
> > > + div = 1000000ULL;
> > > + units = DIV_ROUND_UP_ULL(30ULL * bias, czcount_30ns);
> > Is (u64) cast unnecessary?
>
> Yes. Here the only reason for it would be overflow, but that's not
> possible.
>
> > But reading like this now I wonder if we couldn't just pass
> > czcount_30ns+1 instead of the increment...
> > But if we don't need the cast let's please just ignore this bikeshed
> > and let's move fwd! ;)
>
> Yes, I think we could do more cleanup in this function as a follow-up.
> For example we may still loose precision in the current way, would be
> better to calculate the result directly from the reference clock rate
> (CZ clock in case of VLV/CHV).
>
> > More organized than I had suggested, thanks.
> >
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>
> Thanks.
>
> Forgot to add:
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76877
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
