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

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

[Imre Deak]

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>
---
 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;
 		}
 
 		if (units == 0)
-- 
2.1.4

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

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

[Rodrigo Vivi]

On Mon, May 18, 2015 at 8:46 AM, Imre Deak <imre.deak@intel.com> wrote:
> The divider value to convert from CZ clock rate to ms needs a +1
> adjustment on VLV just like on CHV.

On CHV this was an special case for 320MHz, on VLV we have only one
freq possible or it is global?
The spec I have here doesn't show different freqs here on VLV as we have on CHV.

> This matches both the spec and

Anyway, even on CHV I couldn't find where spec mentions this. Could
you please point me or share your spec in pvt so I can give  a rv-b
here.

> the accuracy test by pm_rc6_residency.
>
> Testcase: igt/pm_rc6_residency
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  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;
>                 }
>
>                 if (units == 0)
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Imre Deak]

On ma, 2015-05-18 at 09:13 -0700, Rodrigo Vivi wrote:
> On Mon, May 18, 2015 at 8:46 AM, Imre Deak <imre.deak@intel.com> wrote:
> > The divider value to convert from CZ clock rate to ms needs a +1
> > adjustment on VLV just like on CHV.
> 
> On CHV this was an special case for 320MHz, on VLV we have only one
> freq possible or it is global?
> The spec I have here doesn't show different freqs here on VLV as we have on CHV.

The CZ clock rate on VLV can be any of 200, 266 and 300 MHz. In all
these cases we need to have the +1 adjustment.

> > This matches both the spec and
> 
> Anyway, even on CHV I couldn't find where spec mentions this. Could
> you please point me or share your spec in pvt so I can give  a rv-b
> here.

Ok, sent it to you.

> > the accuracy test by pm_rc6_residency.
> >
> > Testcase: igt/pm_rc6_residency
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  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;
> >                 }
> >
> >                 if (units == 0)
> > --
> > 2.1.4
> >
> > _______________________________________________
> > 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

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

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6429
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  234/234              234/234
ILK                                  262/262              262/262
SNB                 -1              282/282              281/282
IVB                                  300/300              300/300
BYT                                  254/254              254/254
BDW                                  275/275              275/275
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp      DMESG_WARN(7)PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Imre Deak]

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;
>  		}
>  
>  		if (units == 0)


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

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

[Rodrigo Vivi]

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;

>>               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>

-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Imre Deak]

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

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

[Imre Deak]

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@intel.com>
---
 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);
 		}
 
-		if (units == 0)
-			units = DIV_ROUND_UP_ULL(30ULL * bias,
-						 (u64)czcount_30ns);
-
 		if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
 			units <<= 8;
 
-- 
2.1.4

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

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

[Rodrigo Vivi]

On Mon, Jun 1, 2015 at 12:32 AM, Imre Deak <imre.deak@intel.com> 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@intel.com>
> ---
>  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?
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! ;)

More organized than I had suggested, thanks.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

>                 }
>
> -               if (units == 0)
> -                       units = DIV_ROUND_UP_ULL(30ULL * bias,
> -                                                (u64)czcount_30ns);
> -
>                 if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
>                         units <<= 8;
>
> --
> 2.1.4
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6512
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  303/303              303/303
SNB                 -1              315/315              314/315
IVB                                  343/343              343/343
BYT                                  287/287              287/287
BDW                 -1              321/321              320/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp      PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
*BDW  igt@gem_flink@bad-flink      PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_display.c:#assert_plane[i915]()@WARNING:.* at .* assert_plane
assertion_failure@assertion failure
WARNING:at_drivers/gpu/drm/drm_irq.c:#drm_wait_one_vblank[drm]()@WARNING:.* at .* drm_wait_one_vblank+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Imre Deak]

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@intel.com> 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@intel.com>
> > ---
> >  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@intel.com>

Thanks.

Forgot to add:
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76877

> >                 }
> >
> > -               if (units == 0)
> > -                       units = DIV_ROUND_UP_ULL(30ULL * bias,
> > -                                                (u64)czcount_30ns);
> > -
> >                 if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
> >                         units <<= 8;
> >
> > --
> > 2.1.4
> >
> 
> 
> 


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

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

[Daniel Vetter]

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@intel.com> 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@intel.com>
> > > ---
> > >  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@intel.com>
> 
> 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx