* [Intel-gfx] [PATCH] drm/i915/gt: Fix up clock frequency
@ 2020-04-26 22:29 Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2020-04-26 22:29 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson
The bspec lists both the clock frequency and the effective interval. The
interval corresponds to observed behaviour, so adjust the frequency to
match.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
index 852a7d731b3b..999079686846 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
@@ -7,9 +7,9 @@
#include "intel_gt.h"
#include "intel_gt_clock_utils.h"
+#define MHZ_12 12000000 /* 12MHz (24MHz/2), 83.333ns */
+#define MHZ_12_5 12500000 /* 12.5MHz (25MHz/2), 80ns */
#define MHZ_19_2 19200000 /* 19.2MHz, 52.083ns */
-#define MHZ_24 24000000 /* 24MHz, 83.333ns */
-#define MHZ_25 25000000 /* 25MHz, 80ns */
static u32 read_clock_frequency(const struct intel_gt *gt)
{
@@ -21,19 +21,19 @@ static u32 read_clock_frequency(const struct intel_gt *gt)
config >>= GEN11_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT;
switch (config) {
- case 0: return MHZ_24;
+ case 0: return MHZ_12;
case 1:
case 2: return MHZ_19_2;
default:
- case 3: return MHZ_25;
+ case 3: return MHZ_12_5;
}
} else if (INTEL_GEN(gt->i915) >= 9) {
if (IS_GEN9_LP(gt->i915))
return MHZ_19_2;
else
- return MHZ_24;
+ return MHZ_12;
} else {
- return MHZ_25;
+ return MHZ_12_5;
}
}
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/gt: Fix up clock frequency
@ 2020-04-27 12:02 Chris Wilson
2020-04-27 12:15 ` Mika Kuoppala
2020-04-27 12:50 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gt: Fix up clock frequency (rev2) Patchwork
0 siblings, 2 replies; 6+ messages in thread
From: Chris Wilson @ 2020-04-27 12:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Chris Wilson
The bspec lists both the clock frequency and the effective interval. The
interval corresponds to observed behaviour, so adjust the frequency to
match.
v2: Mika rightfully asked if we could measure the clock frequency from a
selftest.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
.../gpu/drm/i915/gt/intel_gt_clock_utils.c | 12 +-
drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 1 +
drivers/gpu/drm/i915/gt/selftest_rps.c | 125 ++++++++++++++++++
drivers/gpu/drm/i915/gt/selftest_rps.h | 1 +
4 files changed, 133 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
index 852a7d731b3b..999079686846 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
@@ -7,9 +7,9 @@
#include "intel_gt.h"
#include "intel_gt_clock_utils.h"
+#define MHZ_12 12000000 /* 12MHz (24MHz/2), 83.333ns */
+#define MHZ_12_5 12500000 /* 12.5MHz (25MHz/2), 80ns */
#define MHZ_19_2 19200000 /* 19.2MHz, 52.083ns */
-#define MHZ_24 24000000 /* 24MHz, 83.333ns */
-#define MHZ_25 25000000 /* 25MHz, 80ns */
static u32 read_clock_frequency(const struct intel_gt *gt)
{
@@ -21,19 +21,19 @@ static u32 read_clock_frequency(const struct intel_gt *gt)
config >>= GEN11_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT;
switch (config) {
- case 0: return MHZ_24;
+ case 0: return MHZ_12;
case 1:
case 2: return MHZ_19_2;
default:
- case 3: return MHZ_25;
+ case 3: return MHZ_12_5;
}
} else if (INTEL_GEN(gt->i915) >= 9) {
if (IS_GEN9_LP(gt->i915))
return MHZ_19_2;
else
- return MHZ_24;
+ return MHZ_12;
} else {
- return MHZ_25;
+ return MHZ_12_5;
}
}
diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
index e02fdec58826..242181a5214c 100644
--- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
@@ -53,6 +53,7 @@ int intel_gt_pm_live_selftests(struct drm_i915_private *i915)
{
static const struct i915_subtest tests[] = {
SUBTEST(live_rc6_manual),
+ SUBTEST(live_rps_clock_interval),
SUBTEST(live_rps_control),
SUBTEST(live_rps_frequency_cs),
SUBTEST(live_rps_frequency_srm),
diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
index e13cbcb82825..fe92c55572c1 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rps.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
@@ -208,6 +208,131 @@ static void show_pstate_limits(struct intel_rps *rps)
}
}
+int live_rps_clock_interval(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_rps *rps = >->rps;
+ void (*saved_work)(struct work_struct *wrk);
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ struct igt_spinner spin;
+ int err = 0;
+
+ if (!rps->enabled)
+ return 0;
+
+ intel_gt_check_clock_frequency(gt);
+
+ if (igt_spinner_init(&spin, gt))
+ return -ENOMEM;
+
+ intel_gt_pm_wait_for_idle(gt);
+ saved_work = rps->work.func;
+ rps->work.func = dummy_rps_work;
+
+ intel_gt_pm_get(gt);
+ intel_rps_disable(>->rps);
+
+ for_each_engine(engine, gt, id) {
+ unsigned long saved_heartbeat;
+ struct i915_request *rq;
+ u32 cycles, expected;
+ ktime_t dt;
+ u64 time;
+
+ if (!intel_engine_can_store_dword(engine))
+ continue;
+
+ saved_heartbeat = engine_heartbeat_disable(engine);
+
+ rq = igt_spinner_create_request(&spin,
+ engine->kernel_context,
+ MI_NOOP);
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ break;
+ }
+
+ i915_request_add(rq);
+
+ if (!igt_wait_for_spinner(&spin, rq)) {
+ pr_err("%s: RPS spinner did not start\n",
+ engine->name);
+ igt_spinner_end(&spin);
+ engine_heartbeat_enable(engine, saved_heartbeat);
+ intel_gt_set_wedged(engine->gt);
+ err = -EIO;
+ break;
+ }
+
+ intel_uncore_write(gt->uncore, GEN6_RP_CUR_UP_EI, 0);
+
+ /* Set the evaluation interval to infinity! */
+ intel_uncore_write(gt->uncore,
+ GEN6_RP_UP_EI, 0xffffffff);
+ intel_uncore_write(gt->uncore,
+ GEN6_RP_UP_THRESHOLD, 0xffffffff);
+
+ intel_uncore_write(gt->uncore, GEN6_RP_CONTROL,
+ GEN6_RP_ENABLE | GEN6_RP_UP_BUSY_AVG);
+
+ if (wait_for(intel_uncore_read(gt->uncore, GEN6_RP_CUR_UP_EI),
+ 10)) {
+ pr_err("%s: rps evalution interval not ticking\n",
+ engine->name);
+ igt_spinner_end(&spin);
+ engine_heartbeat_enable(engine, saved_heartbeat);
+ err = -ENODEV;
+ break;
+ }
+
+ dt = ktime_get();
+ cycles = -intel_uncore_read(gt->uncore, GEN6_RP_CUR_UP_EI);
+ usleep_range(1000, 2000);
+ dt = ktime_sub(ktime_get(), dt);
+ cycles += intel_uncore_read(gt->uncore, GEN6_RP_CUR_UP_EI);
+
+ intel_uncore_write(gt->uncore, GEN6_RP_CONTROL, 0);
+
+ igt_spinner_end(&spin);
+ engine_heartbeat_enable(engine, saved_heartbeat);
+
+ time = intel_gt_pm_interval_to_ns(gt, cycles);
+ expected = intel_gt_ns_to_pm_interval(gt, ktime_to_ns(dt));
+ pr_info("%s: rps counted %d C0 cycles [%lldns] in %lldns [%d cycles], using GT clock frequency of %uKHz\n",
+ engine->name, cycles, time, ktime_to_ns(dt), expected,
+ gt->clock_frequency / 1000);
+
+ if (10 * time < 9 * ktime_to_ns(dt) ||
+ 10 * time > 11 * ktime_to_ns(dt)) {
+ pr_err("%s: rps clock time does not match walltime!\n",
+ engine->name);
+ err = -EINVAL;
+ }
+
+ if (10 * expected < 9 * cycles || 10 * expected > 11 * cycles) {
+ pr_err("%s: walltime does not match rps clock ticks!\n",
+ engine->name);
+ err = -EINVAL;
+ }
+
+ if (igt_flush_test(gt->i915))
+ err = -EIO;
+
+ break; /* once is enough */
+ }
+
+ intel_rps_enable(>->rps);
+ intel_gt_pm_put(gt);
+
+ igt_spinner_fini(&spin);
+
+ intel_gt_pm_wait_for_idle(gt);
+ rps->work.func = saved_work;
+
+ return err;
+}
+
int live_rps_control(void *arg)
{
struct intel_gt *gt = arg;
diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.h b/drivers/gpu/drm/i915/gt/selftest_rps.h
index 76c4b19553e6..6e82a631cfa1 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rps.h
+++ b/drivers/gpu/drm/i915/gt/selftest_rps.h
@@ -7,6 +7,7 @@
#define SELFTEST_RPS_H
int live_rps_control(void *arg);
+int live_rps_clock_interval(void *arg);
int live_rps_frequency_cs(void *arg);
int live_rps_frequency_srm(void *arg);
int live_rps_power(void *arg);
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: Fix up clock frequency
2020-04-27 12:02 [Intel-gfx] [PATCH] drm/i915/gt: Fix up clock frequency Chris Wilson
@ 2020-04-27 12:15 ` Mika Kuoppala
2020-04-27 12:28 ` Chris Wilson
2020-04-27 12:50 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gt: Fix up clock frequency (rev2) Patchwork
1 sibling, 1 reply; 6+ messages in thread
From: Mika Kuoppala @ 2020-04-27 12:15 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Chris Wilson
Chris Wilson <chris@chris-wilson.co.uk> writes:
> The bspec lists both the clock frequency and the effective interval. The
> interval corresponds to observed behaviour, so adjust the frequency to
> match.
>
> v2: Mika rightfully asked if we could measure the clock frequency from a
> selftest.
This kind of delivery times might impact on demand :P
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> .../gpu/drm/i915/gt/intel_gt_clock_utils.c | 12 +-
> drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 1 +
> drivers/gpu/drm/i915/gt/selftest_rps.c | 125 ++++++++++++++++++
> drivers/gpu/drm/i915/gt/selftest_rps.h | 1 +
> 4 files changed, 133 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
> index 852a7d731b3b..999079686846 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
> @@ -7,9 +7,9 @@
> #include "intel_gt.h"
> #include "intel_gt_clock_utils.h"
>
> +#define MHZ_12 12000000 /* 12MHz (24MHz/2), 83.333ns */
> +#define MHZ_12_5 12500000 /* 12.5MHz (25MHz/2), 80ns */
> #define MHZ_19_2 19200000 /* 19.2MHz, 52.083ns */
> -#define MHZ_24 24000000 /* 24MHz, 83.333ns */
> -#define MHZ_25 25000000 /* 25MHz, 80ns */
>
> static u32 read_clock_frequency(const struct intel_gt *gt)
> {
> @@ -21,19 +21,19 @@ static u32 read_clock_frequency(const struct intel_gt *gt)
> config >>= GEN11_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT;
>
> switch (config) {
> - case 0: return MHZ_24;
> + case 0: return MHZ_12;
> case 1:
> case 2: return MHZ_19_2;
> default:
> - case 3: return MHZ_25;
> + case 3: return MHZ_12_5;
> }
> } else if (INTEL_GEN(gt->i915) >= 9) {
> if (IS_GEN9_LP(gt->i915))
> return MHZ_19_2;
> else
> - return MHZ_24;
> + return MHZ_12;
> } else {
> - return MHZ_25;
> + return MHZ_12_5;
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> index e02fdec58826..242181a5214c 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> @@ -53,6 +53,7 @@ int intel_gt_pm_live_selftests(struct drm_i915_private *i915)
> {
> static const struct i915_subtest tests[] = {
> SUBTEST(live_rc6_manual),
> + SUBTEST(live_rps_clock_interval),
> SUBTEST(live_rps_control),
> SUBTEST(live_rps_frequency_cs),
> SUBTEST(live_rps_frequency_srm),
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> index e13cbcb82825..fe92c55572c1 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> @@ -208,6 +208,131 @@ static void show_pstate_limits(struct intel_rps *rps)
> }
> }
>
> +int live_rps_clock_interval(void *arg)
> +{
> + struct intel_gt *gt = arg;
> + struct intel_rps *rps = >->rps;
> + void (*saved_work)(struct work_struct *wrk);
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> + struct igt_spinner spin;
> + int err = 0;
> +
> + if (!rps->enabled)
> + return 0;
> +
> + intel_gt_check_clock_frequency(gt);
> +
> + if (igt_spinner_init(&spin, gt))
> + return -ENOMEM;
> +
> + intel_gt_pm_wait_for_idle(gt);
> + saved_work = rps->work.func;
> + rps->work.func = dummy_rps_work;
> +
> + intel_gt_pm_get(gt);
> + intel_rps_disable(>->rps);
> +
> + for_each_engine(engine, gt, id) {
> + unsigned long saved_heartbeat;
> + struct i915_request *rq;
> + u32 cycles, expected;
> + ktime_t dt;
> + u64 time;
> +
> + if (!intel_engine_can_store_dword(engine))
> + continue;
> +
> + saved_heartbeat = engine_heartbeat_disable(engine);
> +
> + rq = igt_spinner_create_request(&spin,
> + engine->kernel_context,
> + MI_NOOP);
> + if (IS_ERR(rq)) {
> + err = PTR_ERR(rq);
> + break;
> + }
> +
> + i915_request_add(rq);
> +
> + if (!igt_wait_for_spinner(&spin, rq)) {
> + pr_err("%s: RPS spinner did not start\n",
> + engine->name);
> + igt_spinner_end(&spin);
> + engine_heartbeat_enable(engine, saved_heartbeat);
> + intel_gt_set_wedged(engine->gt);
> + err = -EIO;
> + break;
> + }
> +
> + intel_uncore_write(gt->uncore, GEN6_RP_CUR_UP_EI, 0);
> +
> + /* Set the evaluation interval to infinity! */
> + intel_uncore_write(gt->uncore,
> + GEN6_RP_UP_EI, 0xffffffff);
> + intel_uncore_write(gt->uncore,
> + GEN6_RP_UP_THRESHOLD, 0xffffffff);
To keep it constant during the calculation?
> +
> + intel_uncore_write(gt->uncore, GEN6_RP_CONTROL,
> + GEN6_RP_ENABLE | GEN6_RP_UP_BUSY_AVG);
> +
> + if (wait_for(intel_uncore_read(gt->uncore, GEN6_RP_CUR_UP_EI),
> + 10)) {
> + pr_err("%s: rps evalution interval not ticking\n",
> + engine->name);
> + igt_spinner_end(&spin);
> + engine_heartbeat_enable(engine, saved_heartbeat);
> + err = -ENODEV;
> + break;
> + }
> +
> + dt = ktime_get();
> + cycles = -intel_uncore_read(gt->uncore, GEN6_RP_CUR_UP_EI);
Hmm you have not zeroed the cycles on the first run, but does it matter
is then the question.
-Mika
> + usleep_range(1000, 2000);
> + dt = ktime_sub(ktime_get(), dt);
> + cycles += intel_uncore_read(gt->uncore, GEN6_RP_CUR_UP_EI);
> +
> + intel_uncore_write(gt->uncore, GEN6_RP_CONTROL, 0);
> +
> + igt_spinner_end(&spin);
> + engine_heartbeat_enable(engine, saved_heartbeat);
> +
> + time = intel_gt_pm_interval_to_ns(gt, cycles);
> + expected = intel_gt_ns_to_pm_interval(gt, ktime_to_ns(dt));
> + pr_info("%s: rps counted %d C0 cycles [%lldns] in %lldns [%d cycles], using GT clock frequency of %uKHz\n",
> + engine->name, cycles, time, ktime_to_ns(dt), expected,
> + gt->clock_frequency / 1000);
> +
> + if (10 * time < 9 * ktime_to_ns(dt) ||
> + 10 * time > 11 * ktime_to_ns(dt)) {
> + pr_err("%s: rps clock time does not match walltime!\n",
> + engine->name);
> + err = -EINVAL;
> + }
> +
> + if (10 * expected < 9 * cycles || 10 * expected > 11 * cycles) {
> + pr_err("%s: walltime does not match rps clock ticks!\n",
> + engine->name);
> + err = -EINVAL;
> + }
> +
> + if (igt_flush_test(gt->i915))
> + err = -EIO;
> +
> + break; /* once is enough */
> + }
> +
> + intel_rps_enable(>->rps);
> + intel_gt_pm_put(gt);
> +
> + igt_spinner_fini(&spin);
> +
> + intel_gt_pm_wait_for_idle(gt);
> + rps->work.func = saved_work;
> +
> + return err;
> +}
> +
> int live_rps_control(void *arg)
> {
> struct intel_gt *gt = arg;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.h b/drivers/gpu/drm/i915/gt/selftest_rps.h
> index 76c4b19553e6..6e82a631cfa1 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.h
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.h
> @@ -7,6 +7,7 @@
> #define SELFTEST_RPS_H
>
> int live_rps_control(void *arg);
> +int live_rps_clock_interval(void *arg);
> int live_rps_frequency_cs(void *arg);
> int live_rps_frequency_srm(void *arg);
> int live_rps_power(void *arg);
> --
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: Fix up clock frequency
2020-04-27 12:15 ` Mika Kuoppala
@ 2020-04-27 12:28 ` Chris Wilson
2020-04-27 12:34 ` Mika Kuoppala
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2020-04-27 12:28 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2020-04-27 13:15:23)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > The bspec lists both the clock frequency and the effective interval. The
> > interval corresponds to observed behaviour, so adjust the frequency to
> > match.
> >
> > v2: Mika rightfully asked if we could measure the clock frequency from a
> > selftest.
>
> This kind of delivery times might impact on demand :P
>
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> > .../gpu/drm/i915/gt/intel_gt_clock_utils.c | 12 +-
> > drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 1 +
> > drivers/gpu/drm/i915/gt/selftest_rps.c | 125 ++++++++++++++++++
> > drivers/gpu/drm/i915/gt/selftest_rps.h | 1 +
> > 4 files changed, 133 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
> > index 852a7d731b3b..999079686846 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
> > @@ -7,9 +7,9 @@
> > #include "intel_gt.h"
> > #include "intel_gt_clock_utils.h"
> >
> > +#define MHZ_12 12000000 /* 12MHz (24MHz/2), 83.333ns */
> > +#define MHZ_12_5 12500000 /* 12.5MHz (25MHz/2), 80ns */
> > #define MHZ_19_2 19200000 /* 19.2MHz, 52.083ns */
> > -#define MHZ_24 24000000 /* 24MHz, 83.333ns */
> > -#define MHZ_25 25000000 /* 25MHz, 80ns */
> >
> > static u32 read_clock_frequency(const struct intel_gt *gt)
> > {
> > @@ -21,19 +21,19 @@ static u32 read_clock_frequency(const struct intel_gt *gt)
> > config >>= GEN11_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT;
> >
> > switch (config) {
> > - case 0: return MHZ_24;
> > + case 0: return MHZ_12;
> > case 1:
> > case 2: return MHZ_19_2;
> > default:
> > - case 3: return MHZ_25;
> > + case 3: return MHZ_12_5;
> > }
> > } else if (INTEL_GEN(gt->i915) >= 9) {
> > if (IS_GEN9_LP(gt->i915))
> > return MHZ_19_2;
> > else
> > - return MHZ_24;
> > + return MHZ_12;
> > } else {
> > - return MHZ_25;
> > + return MHZ_12_5;
> > }
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> > index e02fdec58826..242181a5214c 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> > @@ -53,6 +53,7 @@ int intel_gt_pm_live_selftests(struct drm_i915_private *i915)
> > {
> > static const struct i915_subtest tests[] = {
> > SUBTEST(live_rc6_manual),
> > + SUBTEST(live_rps_clock_interval),
> > SUBTEST(live_rps_control),
> > SUBTEST(live_rps_frequency_cs),
> > SUBTEST(live_rps_frequency_srm),
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> > index e13cbcb82825..fe92c55572c1 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> > @@ -208,6 +208,131 @@ static void show_pstate_limits(struct intel_rps *rps)
> > }
> > }
> >
> > +int live_rps_clock_interval(void *arg)
> > +{
> > + struct intel_gt *gt = arg;
> > + struct intel_rps *rps = >->rps;
> > + void (*saved_work)(struct work_struct *wrk);
> > + struct intel_engine_cs *engine;
> > + enum intel_engine_id id;
> > + struct igt_spinner spin;
> > + int err = 0;
> > +
> > + if (!rps->enabled)
> > + return 0;
> > +
> > + intel_gt_check_clock_frequency(gt);
> > +
> > + if (igt_spinner_init(&spin, gt))
> > + return -ENOMEM;
> > +
> > + intel_gt_pm_wait_for_idle(gt);
> > + saved_work = rps->work.func;
> > + rps->work.func = dummy_rps_work;
> > +
> > + intel_gt_pm_get(gt);
> > + intel_rps_disable(>->rps);
> > +
> > + for_each_engine(engine, gt, id) {
> > + unsigned long saved_heartbeat;
> > + struct i915_request *rq;
> > + u32 cycles, expected;
> > + ktime_t dt;
> > + u64 time;
> > +
> > + if (!intel_engine_can_store_dword(engine))
> > + continue;
> > +
> > + saved_heartbeat = engine_heartbeat_disable(engine);
> > +
> > + rq = igt_spinner_create_request(&spin,
> > + engine->kernel_context,
> > + MI_NOOP);
> > + if (IS_ERR(rq)) {
> > + err = PTR_ERR(rq);
> > + break;
> > + }
> > +
> > + i915_request_add(rq);
> > +
> > + if (!igt_wait_for_spinner(&spin, rq)) {
> > + pr_err("%s: RPS spinner did not start\n",
> > + engine->name);
> > + igt_spinner_end(&spin);
> > + engine_heartbeat_enable(engine, saved_heartbeat);
> > + intel_gt_set_wedged(engine->gt);
> > + err = -EIO;
> > + break;
> > + }
> > +
> > + intel_uncore_write(gt->uncore, GEN6_RP_CUR_UP_EI, 0);
> > +
> > + /* Set the evaluation interval to infinity! */
> > + intel_uncore_write(gt->uncore,
> > + GEN6_RP_UP_EI, 0xffffffff);
> > + intel_uncore_write(gt->uncore,
> > + GEN6_RP_UP_THRESHOLD, 0xffffffff);
>
> To keep it constant during the calculation?
Yeah, it shouldn't conceptually make a difference. I'm just worried in
case the interrupt and the EI stopped when it crosses threshold, as
opposed to evaluating the counters strictly at the end of the EI.
> > +
> > + intel_uncore_write(gt->uncore, GEN6_RP_CONTROL,
> > + GEN6_RP_ENABLE | GEN6_RP_UP_BUSY_AVG);
> > +
> > + if (wait_for(intel_uncore_read(gt->uncore, GEN6_RP_CUR_UP_EI),
> > + 10)) {
> > + pr_err("%s: rps evalution interval not ticking\n",
> > + engine->name);
> > + igt_spinner_end(&spin);
> > + engine_heartbeat_enable(engine, saved_heartbeat);
> > + err = -ENODEV;
> > + break;
> > + }
> > +
> > + dt = ktime_get();
> > + cycles = -intel_uncore_read(gt->uncore, GEN6_RP_CUR_UP_EI);
>
> Hmm you have not zeroed the cycles on the first run, but does it matter
> is then the question.
cycles = -X,
not
cycles -= X
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: Fix up clock frequency
2020-04-27 12:28 ` Chris Wilson
@ 2020-04-27 12:34 ` Mika Kuoppala
0 siblings, 0 replies; 6+ messages in thread
From: Mika Kuoppala @ 2020-04-27 12:34 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> Quoting Mika Kuoppala (2020-04-27 13:15:23)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>>
>> > The bspec lists both the clock frequency and the effective interval. The
>> > interval corresponds to observed behaviour, so adjust the frequency to
>> > match.
>> >
>> > v2: Mika rightfully asked if we could measure the clock frequency from a
>> > selftest.
>>
>> This kind of delivery times might impact on demand :P
>>
>> >
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> > ---
>> > .../gpu/drm/i915/gt/intel_gt_clock_utils.c | 12 +-
>> > drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 1 +
>> > drivers/gpu/drm/i915/gt/selftest_rps.c | 125 ++++++++++++++++++
>> > drivers/gpu/drm/i915/gt/selftest_rps.h | 1 +
>> > 4 files changed, 133 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
>> > index 852a7d731b3b..999079686846 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
>> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
>> > @@ -7,9 +7,9 @@
>> > #include "intel_gt.h"
>> > #include "intel_gt_clock_utils.h"
>> >
>> > +#define MHZ_12 12000000 /* 12MHz (24MHz/2), 83.333ns */
>> > +#define MHZ_12_5 12500000 /* 12.5MHz (25MHz/2), 80ns */
>> > #define MHZ_19_2 19200000 /* 19.2MHz, 52.083ns */
>> > -#define MHZ_24 24000000 /* 24MHz, 83.333ns */
>> > -#define MHZ_25 25000000 /* 25MHz, 80ns */
>> >
>> > static u32 read_clock_frequency(const struct intel_gt *gt)
>> > {
>> > @@ -21,19 +21,19 @@ static u32 read_clock_frequency(const struct intel_gt *gt)
>> > config >>= GEN11_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT;
>> >
>> > switch (config) {
>> > - case 0: return MHZ_24;
>> > + case 0: return MHZ_12;
>> > case 1:
>> > case 2: return MHZ_19_2;
>> > default:
>> > - case 3: return MHZ_25;
>> > + case 3: return MHZ_12_5;
>> > }
>> > } else if (INTEL_GEN(gt->i915) >= 9) {
>> > if (IS_GEN9_LP(gt->i915))
>> > return MHZ_19_2;
>> > else
>> > - return MHZ_24;
>> > + return MHZ_12;
>> > } else {
>> > - return MHZ_25;
>> > + return MHZ_12_5;
>> > }
>> > }
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
>> > index e02fdec58826..242181a5214c 100644
>> > --- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
>> > +++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
>> > @@ -53,6 +53,7 @@ int intel_gt_pm_live_selftests(struct drm_i915_private *i915)
>> > {
>> > static const struct i915_subtest tests[] = {
>> > SUBTEST(live_rc6_manual),
>> > + SUBTEST(live_rps_clock_interval),
>> > SUBTEST(live_rps_control),
>> > SUBTEST(live_rps_frequency_cs),
>> > SUBTEST(live_rps_frequency_srm),
>> > diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
>> > index e13cbcb82825..fe92c55572c1 100644
>> > --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
>> > +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
>> > @@ -208,6 +208,131 @@ static void show_pstate_limits(struct intel_rps *rps)
>> > }
>> > }
>> >
>> > +int live_rps_clock_interval(void *arg)
>> > +{
>> > + struct intel_gt *gt = arg;
>> > + struct intel_rps *rps = >->rps;
>> > + void (*saved_work)(struct work_struct *wrk);
>> > + struct intel_engine_cs *engine;
>> > + enum intel_engine_id id;
>> > + struct igt_spinner spin;
>> > + int err = 0;
>> > +
>> > + if (!rps->enabled)
>> > + return 0;
>> > +
>> > + intel_gt_check_clock_frequency(gt);
>> > +
>> > + if (igt_spinner_init(&spin, gt))
>> > + return -ENOMEM;
>> > +
>> > + intel_gt_pm_wait_for_idle(gt);
>> > + saved_work = rps->work.func;
>> > + rps->work.func = dummy_rps_work;
>> > +
>> > + intel_gt_pm_get(gt);
>> > + intel_rps_disable(>->rps);
>> > +
>> > + for_each_engine(engine, gt, id) {
>> > + unsigned long saved_heartbeat;
>> > + struct i915_request *rq;
>> > + u32 cycles, expected;
>> > + ktime_t dt;
>> > + u64 time;
>> > +
>> > + if (!intel_engine_can_store_dword(engine))
>> > + continue;
>> > +
>> > + saved_heartbeat = engine_heartbeat_disable(engine);
>> > +
>> > + rq = igt_spinner_create_request(&spin,
>> > + engine->kernel_context,
>> > + MI_NOOP);
>> > + if (IS_ERR(rq)) {
>> > + err = PTR_ERR(rq);
>> > + break;
>> > + }
>> > +
>> > + i915_request_add(rq);
>> > +
>> > + if (!igt_wait_for_spinner(&spin, rq)) {
>> > + pr_err("%s: RPS spinner did not start\n",
>> > + engine->name);
>> > + igt_spinner_end(&spin);
>> > + engine_heartbeat_enable(engine, saved_heartbeat);
>> > + intel_gt_set_wedged(engine->gt);
>> > + err = -EIO;
>> > + break;
>> > + }
>> > +
>> > + intel_uncore_write(gt->uncore, GEN6_RP_CUR_UP_EI, 0);
>> > +
>> > + /* Set the evaluation interval to infinity! */
>> > + intel_uncore_write(gt->uncore,
>> > + GEN6_RP_UP_EI, 0xffffffff);
>> > + intel_uncore_write(gt->uncore,
>> > + GEN6_RP_UP_THRESHOLD, 0xffffffff);
>>
>> To keep it constant during the calculation?
>
> Yeah, it shouldn't conceptually make a difference. I'm just worried in
> case the interrupt and the EI stopped when it crosses threshold, as
> opposed to evaluating the counters strictly at the end of the EI.
>
>> > +
>> > + intel_uncore_write(gt->uncore, GEN6_RP_CONTROL,
>> > + GEN6_RP_ENABLE | GEN6_RP_UP_BUSY_AVG);
>> > +
>> > + if (wait_for(intel_uncore_read(gt->uncore, GEN6_RP_CUR_UP_EI),
>> > + 10)) {
>> > + pr_err("%s: rps evalution interval not ticking\n",
>> > + engine->name);
>> > + igt_spinner_end(&spin);
>> > + engine_heartbeat_enable(engine, saved_heartbeat);
>> > + err = -ENODEV;
>> > + break;
>> > + }
>> > +
>> > + dt = ktime_get();
>> > + cycles = -intel_uncore_read(gt->uncore, GEN6_RP_CUR_UP_EI);
>>
>> Hmm you have not zeroed the cycles on the first run, but does it matter
>> is then the question.
>
> cycles = -X,
> not
> cycles -= X
Ah indeed. A bit of trickery but tricks are useful.
For the bspec, I didn't find a note about the actual intervals.
But evidence dictates.
Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gt: Fix up clock frequency (rev2)
2020-04-27 12:02 [Intel-gfx] [PATCH] drm/i915/gt: Fix up clock frequency Chris Wilson
2020-04-27 12:15 ` Mika Kuoppala
@ 2020-04-27 12:50 ` Patchwork
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2020-04-27 12:50 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/gt: Fix up clock frequency (rev2)
URL : https://patchwork.freedesktop.org/series/76512/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_8372 -> Patchwork_17476
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_17476 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_17476, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17476/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_17476:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live@gt_pm:
- fi-icl-y: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8372/fi-icl-y/igt@i915_selftest@live@gt_pm.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17476/fi-icl-y/igt@i915_selftest@live@gt_pm.html
- fi-icl-u2: [PASS][3] -> [INCOMPLETE][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8372/fi-icl-u2/igt@i915_selftest@live@gt_pm.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17476/fi-icl-u2/igt@i915_selftest@live@gt_pm.html
#### Warnings ####
* igt@i915_selftest@live@gt_pm:
- fi-tgl-y: [DMESG-FAIL][5] ([i915#1759]) -> [INCOMPLETE][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8372/fi-tgl-y/igt@i915_selftest@live@gt_pm.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17476/fi-tgl-y/igt@i915_selftest@live@gt_pm.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@i915_selftest@live@gt_pm:
- {fi-tgl-dsi}: [DMESG-FAIL][7] ([i915#1759]) -> [INCOMPLETE][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8372/fi-tgl-dsi/igt@i915_selftest@live@gt_pm.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17476/fi-tgl-dsi/igt@i915_selftest@live@gt_pm.html
- {fi-tgl-u}: [DMESG-FAIL][9] ([i915#1759]) -> [INCOMPLETE][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8372/fi-tgl-u/igt@i915_selftest@live@gt_pm.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17476/fi-tgl-u/igt@i915_selftest@live@gt_pm.html
Known issues
------------
Here are the changes found in Patchwork_17476 that come from known issues:
### IGT changes ###
#### Possible fixes ####
* igt@i915_selftest@live@gt_pm:
- fi-cfl-8109u: [DMESG-FAIL][11] ([i915#1791]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8372/fi-cfl-8109u/igt@i915_selftest@live@gt_pm.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17476/fi-cfl-8109u/igt@i915_selftest@live@gt_pm.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#1759]: https://gitlab.freedesktop.org/drm/intel/issues/1759
[i915#1791]: https://gitlab.freedesktop.org/drm/intel/issues/1791
Participating hosts (49 -> 43)
------------------------------
Additional (1): fi-kbl-7560u
Missing (7): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-kbl-7500u fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_8372 -> Patchwork_17476
CI-20190529: 20190529
CI_DRM_8372: 82c11b773e7d77543950ff67b2561cd984032d14 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5612: c8dc1fd926a550308b971ca7d83fe0a927a38152 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_17476: b1c6c060d6ebf4ad9110ae3b160733afb5c83413 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
b1c6c060d6eb drm/i915/gt: Fix up clock frequency
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17476/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-27 12:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-27 12:02 [Intel-gfx] [PATCH] drm/i915/gt: Fix up clock frequency Chris Wilson
2020-04-27 12:15 ` Mika Kuoppala
2020-04-27 12:28 ` Chris Wilson
2020-04-27 12:34 ` Mika Kuoppala
2020-04-27 12:50 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gt: Fix up clock frequency (rev2) Patchwork
-- strict thread matches above, loose matches on Subject: below --
2020-04-26 22:29 [Intel-gfx] [PATCH] drm/i915/gt: Fix up clock frequency Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.