* [PATCH] drm/i915/vlv: fix RC6 residency time calculation
@ 2015-05-18 15:46 Imre Deak
2015-05-18 16:13 ` Rodrigo Vivi
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Imre Deak @ 2015-05-18 15:46 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/i915/vlv: fix RC6 residency time calculation 2015-05-18 15:46 [PATCH] drm/i915/vlv: fix RC6 residency time calculation Imre Deak @ 2015-05-18 16:13 ` Rodrigo Vivi 2015-05-18 16:24 ` Imre Deak 2015-05-19 19:41 ` shuang.he ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Rodrigo Vivi @ 2015-05-18 16:13 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: fix RC6 residency time calculation 2015-05-18 16:13 ` Rodrigo Vivi @ 2015-05-18 16:24 ` Imre Deak 0 siblings, 0 replies; 12+ messages in thread From: Imre Deak @ 2015-05-18 16:24 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: fix RC6 residency time calculation 2015-05-18 15:46 [PATCH] drm/i915/vlv: fix RC6 residency time calculation Imre Deak 2015-05-18 16:13 ` Rodrigo Vivi @ 2015-05-19 19:41 ` shuang.he 2015-05-29 12:56 ` Imre Deak 2015-06-01 7:32 ` [PATCH v2] " Imre Deak 3 siblings, 0 replies; 12+ messages in thread From: shuang.he @ 2015-05-19 19:41 UTC (permalink / raw) To: shuang.he, lei.a.liu, intel-gfx, imre.deak 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: fix RC6 residency time calculation 2015-05-18 15:46 [PATCH] drm/i915/vlv: fix RC6 residency time calculation Imre Deak 2015-05-18 16:13 ` Rodrigo Vivi 2015-05-19 19:41 ` shuang.he @ 2015-05-29 12:56 ` Imre Deak 2015-05-29 20:22 ` Rodrigo Vivi 2015-06-01 7:32 ` [PATCH v2] " Imre Deak 3 siblings, 1 reply; 12+ messages in thread From: Imre Deak @ 2015-05-29 12:56 UTC (permalink / raw) To: intel-gfx; +Cc: Rodrigo Vivi 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: fix RC6 residency time calculation 2015-05-29 12:56 ` Imre Deak @ 2015-05-29 20:22 ` Rodrigo Vivi 2015-06-01 6:29 ` Imre Deak 0 siblings, 1 reply; 12+ messages in thread From: Rodrigo Vivi @ 2015-05-29 20:22 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx, 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vlv: fix RC6 residency time calculation 2015-05-29 20:22 ` Rodrigo Vivi @ 2015-06-01 6:29 ` Imre Deak 0 siblings, 0 replies; 12+ messages in thread From: Imre Deak @ 2015-06-01 6:29 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: intel-gfx, Rodrigo Vivi 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] drm/i915/vlv: fix RC6 residency time calculation 2015-05-18 15:46 [PATCH] drm/i915/vlv: fix RC6 residency time calculation Imre Deak ` (2 preceding siblings ...) 2015-05-29 12:56 ` Imre Deak @ 2015-06-01 7:32 ` Imre Deak 2015-06-01 19:01 ` Rodrigo Vivi 2015-06-01 19:44 ` shuang.he 3 siblings, 2 replies; 12+ messages in thread From: Imre Deak @ 2015-06-01 7:32 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915/vlv: fix RC6 residency time calculation 2015-06-01 7:32 ` [PATCH v2] " Imre Deak @ 2015-06-01 19:01 ` Rodrigo Vivi 2015-06-02 9:37 ` Imre Deak 2015-06-01 19:44 ` shuang.he 1 sibling, 1 reply; 12+ messages in thread From: Rodrigo Vivi @ 2015-06-01 19:01 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915/vlv: fix RC6 residency time calculation 2015-06-01 19:01 ` Rodrigo Vivi @ 2015-06-02 9:37 ` Imre Deak 2015-06-15 9:57 ` Daniel Vetter 0 siblings, 1 reply; 12+ messages in thread From: Imre Deak @ 2015-06-02 9:37 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915/vlv: fix RC6 residency time calculation 2015-06-02 9:37 ` Imre Deak @ 2015-06-15 9:57 ` Daniel Vetter 0 siblings, 0 replies; 12+ messages in thread From: Daniel Vetter @ 2015-06-15 9:57 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915/vlv: fix RC6 residency time calculation 2015-06-01 7:32 ` [PATCH v2] " Imre Deak 2015-06-01 19:01 ` Rodrigo Vivi @ 2015-06-01 19:44 ` shuang.he 1 sibling, 0 replies; 12+ messages in thread From: shuang.he @ 2015-06-01 19:44 UTC (permalink / raw) To: shuang.he, lei.a.liu, intel-gfx, imre.deak 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-06-15 9:54 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-18 15:46 [PATCH] drm/i915/vlv: fix RC6 residency time calculation Imre Deak 2015-05-18 16:13 ` Rodrigo Vivi 2015-05-18 16:24 ` Imre Deak 2015-05-19 19:41 ` shuang.he 2015-05-29 12:56 ` Imre Deak 2015-05-29 20:22 ` Rodrigo Vivi 2015-06-01 6:29 ` Imre Deak 2015-06-01 7:32 ` [PATCH v2] " Imre Deak 2015-06-01 19:01 ` Rodrigo Vivi 2015-06-02 9:37 ` Imre Deak 2015-06-15 9:57 ` Daniel Vetter 2015-06-01 19:44 ` shuang.he
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox