| Message ID | 1431963961-24349-1-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 > > >
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 '*'
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)
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>
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> >
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)
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(+)