Re: [Intel-gfx] [PATCH v3] drm/i915/selftests: add basic selftests for rc6
Quoting Andi Shyti (2019-12-03 12:32:24)
> > > > }
> > > > +
> > > > +static bool test_rc6(struct intel_rc6 *rc6, bool enabled)
> > >
> > > I keep getting confused as to the meaning of the result, forgetting it
> > > changes based on bool enabled.
> > >
> > > Maybe u32 measure_rc6() and leave the pass/fail to the caller?
>
> thinking a bit better... what exactly would I return? what would
> measure_rc6 measure? The "sleeping" function is not precise by
> definition (as you pointed out as well) and it would be out from
> the scope of this function to provide an exact measure of the
> interval count.
>
> The way I would rather do it would be:
>
> u32 measure_rc6(u32 time_in_ms)
> {
> ...
> }
>
> bool test_rc6(bool enable)
> {
> ...
> return enable ^ does_rc6_work(2 * interval);
> }
>
> where measure_rc6 provides the counter in a more precise time
> range and can be also used for other tests, like hysteresis or
> duty cycle tests where I guess time measurement is more critical.
That's how I thought it would look since for the first test,
test_rc6(rc6->enabled) makes sense. But I would like to know the values
of EI, THRESHOLD, slept and measured rc6 to understand failures better.
And when we do get better understanding, the next wave of tests I expect
will be more than simple booleans, but did we get x rc6 cycles. (That
depends on much we keep scratching at the rc6 powersaving surface :)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/selftests: add basic selftests for rc6
> > > }
> > > +
> > > +static bool test_rc6(struct intel_rc6 *rc6, bool enabled)
> >
> > I keep getting confused as to the meaning of the result, forgetting it
> > changes based on bool enabled.
> >
> > Maybe u32 measure_rc6() and leave the pass/fail to the caller?
thinking a bit better... what exactly would I return? what would
measure_rc6 measure? The "sleeping" function is not precise by
definition (as you pointed out as well) and it would be out from
the scope of this function to provide an exact measure of the
interval count.
The way I would rather do it would be:
u32 measure_rc6(u32 time_in_ms)
{
...
}
bool test_rc6(bool enable)
{
...
return enable ^ does_rc6_work(2 * interval);
}
where measure_rc6 provides the counter in a more precise time
range and can be also used for other tests, like hysteresis or
duty cycle tests where I guess time measurement is more critical.
But as for now such function wouldn't be of any use. I could
eventually call 'test_rc6' in a more intuitive way, like
'is_rc6_enabled()'.
Andi
> yes, I was thinking the same, it makes, indeed more sense.
>
> The original idea was just to know whether rc6 works or not.
>
> Thanks,
> Andi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/selftests: add basic selftests for rc6
Hi Chris, > > } > > + > > +static bool test_rc6(struct intel_rc6 *rc6, bool enabled) > > I keep getting confused as to the meaning of the result, forgetting it > changes based on bool enabled. > > Maybe u32 measure_rc6() and leave the pass/fail to the caller? yes, I was thinking the same, it makes, indeed more sense. The original idea was just to know whether rc6 works or not. Thanks, Andi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/selftests: add basic selftests for rc6
Quoting Andi Shyti (2019-12-03 08:53:12)
> Add three basic tests for rc6 power status:
>
> 1. live_rc6_basic - simply checks if rc6 works when it's enabled
>or stops when it's disabled.
>
> 2. live_rc6_threshold - rc6 should not work when the evaluation
>interval is less than the threshold and should work otherwise.
>
> 3. live_rc6_busy - keeps the gpu busy and then goes in idle;
>checks that we don't fall in rc6 when busy and that we do fall
>in rc6 when idling.
>
> The three tests are added as sutest of the bigger live_late_gt_pm
> selftest.
>
> The basic rc6 functionality is tested by checking the reference
> counter within the evaluation interval.
>
> Signed-off-by: Andi Shyti
> Cc: Chris Wilson
> ---
> Hi,
>
> this is the first patche from the pm selftest series. Now it's
> rebased on top of drm-tip.
>
> Changelog:
> * v1 -> v2:
> - some changes from Chris (thank you!).
> * v2 -> v3:
> - rebased on top of the latest drm-tip
> - fixed exiting order in rc6_basic to avoid exiting
> without releasing the pm reference
>
> Andi
>
> drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 3 +
> drivers/gpu/drm/i915/gt/selftest_rc6.c | 190 +++
> drivers/gpu/drm/i915/gt/selftest_rc6.h | 3 +
> 3 files changed, 196 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> index 09ff8e4f88af..84d1a58cfa28 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> @@ -51,6 +51,9 @@ static int live_gt_resume(void *arg)
> int intel_gt_pm_live_selftests(struct drm_i915_private *i915)
> {
> static const struct i915_subtest tests[] = {
> + SUBTEST(live_rc6_basic),
> + SUBTEST(live_rc6_threshold),
> + SUBTEST(live_rc6_busy),
> SUBTEST(live_rc6_manual),
> SUBTEST(live_gt_resume),
> };
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> index f8b7691be4d1..935e721f5479 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rc6.c
> @@ -11,6 +11,7 @@
> #include "selftest_rc6.h"
>
> #include "selftests/i915_random.h"
> +#include "selftests/igt_spinner.h"
>
> int live_rc6_manual(void *arg)
> {
> @@ -202,3 +203,192 @@ int live_rc6_ctx_wa(void *arg)
> kfree(engines);
> return err;
> }
> +
> +static bool test_rc6(struct intel_rc6 *rc6, bool enabled)
I keep getting confused as to the meaning of the result, forgetting it
changes based on bool enabled.
Maybe u32 measure_rc6() and leave the pass/fail to the caller?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
