* [PATCH igt 1/2] kms_frontbuffer_tracking: don't fail fbc-farfromfence on SKL
2016-02-16 20:47 [PATCH] drm/i915/fbc: enable FBC by default on HSW and BDW Paulo Zanoni
@ 2016-02-16 20:47 ` Paulo Zanoni
2016-02-16 20:47 ` [PATCH igt 2/2] kms_frontbuffer_tracking: add basic subtest Paulo Zanoni
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Paulo Zanoni @ 2016-02-16 20:47 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
SKL doesn't do the same dspaddr_offset magic as the older gens, so FBC
may not be enabled on fbc-farfromfence. This is not a bug since FBC is
just disabled, and this is the expected case of the current Kernel, so
let's not fail the test.
If/once we land the proper infrastructure to fix this in the Kernel we
may revert this commit.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_frontbuffer_tracking.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 9090a74..4d40e24 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -2822,9 +2822,10 @@ static void farfromfence_subtest(const struct test_mode *t)
struct modeset_params *params = pick_params(t);
struct draw_pattern_info *pattern = &pattern1;
struct fb_region *target;
- int max_height;
+ int max_height, assertions = 0;
+ int gen = intel_gen(intel_get_drm_devid(drm.fd));
- switch (intel_gen(intel_get_drm_devid(drm.fd))) {
+ switch (gen) {
case 2:
max_height = 2048;
break;
@@ -2836,6 +2837,11 @@ static void farfromfence_subtest(const struct test_mode *t)
break;
}
+ /* Gen 9 doesn't do the same dspaddr_offset magic as the older
+ * gens, so FBC may not be enabled there. */
+ if (gen >= 9)
+ assertions |= DONT_ASSERT_FEATURE_STATUS;
+
prepare_subtest(t, pattern);
target = pick_target(t, params);
@@ -2848,13 +2854,14 @@ static void farfromfence_subtest(const struct test_mode *t)
params->fb.x = 0;
params->fb.y = max_height - params->mode->vdisplay;
set_mode_for_params(params);
- do_assertions(0);
+ do_assertions(assertions);
for (r = 0; r < pattern->n_rects; r++) {
draw_rect(pattern, target, t->method, r);
update_wanted_crc(t, &pattern->crcs[t->format][r]);
+
/* GTT draws disable PSR. */
- do_assertions(ASSERT_PSR_DISABLED);
+ do_assertions(assertions | ASSERT_PSR_DISABLED);
}
igt_remove_fb(drm.fd, &tall_fb);
--
2.7.0
_______________________________________________
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* [PATCH igt 2/2] kms_frontbuffer_tracking: add basic subtest
2016-02-16 20:47 [PATCH] drm/i915/fbc: enable FBC by default on HSW and BDW Paulo Zanoni
2016-02-16 20:47 ` [PATCH igt 1/2] kms_frontbuffer_tracking: don't fail fbc-farfromfence on SKL Paulo Zanoni
@ 2016-02-16 20:47 ` Paulo Zanoni
2016-02-17 8:40 ` ✗ Fi.CI.BAT: warning for drm/i915/fbc: enable FBC by default on HSW and BDW Patchwork
2016-02-17 16:23 ` [PATCH] " Daniel Vetter
3 siblings, 0 replies; 6+ messages in thread
From: Paulo Zanoni @ 2016-02-16 20:47 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
This test executes one of each of the very basic operations with
whatever features are enabled by default in the Kernel, without
checking their statuses: we only care about the CRCs. Although it's
completely redundant, it will be ran by the CI so we can spot
regressions both in the specific features and in the frontbuffer
tracking subsystem.
We're not checking sink CRCs since they're still unreliable and we
don't want false negatives in the CI.
It takes 5 seconds to run this test on my machines.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_frontbuffer_tracking.c | 83 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 80 insertions(+), 3 deletions(-)
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 4d40e24..26e12d0 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -97,12 +97,15 @@ struct test_mode {
FBS_COUNT,
} fbs;
- /* Which features are we going to test now? This is a mask! */
+ /* Which features are we going to test now? This is a mask!
+ * FEATURE_DEFAULT is a special value which instruct the test to just
+ * keep what's already enabled by default in the Kernel. */
enum {
FEATURE_NONE = 0,
FEATURE_FBC = 1,
FEATURE_PSR = 2,
FEATURE_COUNT = 4,
+ FEATURE_DEFAULT = 4,
} feature;
/* Possible pixel formats. We just use FORMAT_DEFAULT for most tests and
@@ -1103,8 +1106,11 @@ static void unset_all_crtcs(void)
}
}
-static void disable_features(void)
+static void disable_features(const struct test_mode *t)
{
+ if (t->feature == FEATURE_DEFAULT)
+ return;
+
fbc_disable();
psr_disable();
}
@@ -1727,6 +1733,9 @@ static void set_sprite_for_test(const struct test_mode *t,
static void enable_features_for_test(const struct test_mode *t)
{
+ if (t->feature == FEATURE_DEFAULT)
+ return;
+
if (t->feature & FEATURE_FBC)
fbc_enable();
if (t->feature & FEATURE_PSR)
@@ -1806,7 +1815,7 @@ static void prepare_subtest_data(const struct test_mode *t,
stop_busy_thread();
- disable_features();
+ disable_features(t);
set_crtc_fbs(t);
if (t->screen == SCREEN_OFFSCREEN)
@@ -3063,6 +3072,64 @@ static void tilingchange_subtest(const struct test_mode *t)
}
}
+/*
+ * basic - do some basic operations regardless of which features are enabled
+ *
+ * METHOD
+ * This subtest does page flips and draw operations and checks the CRCs of the
+ * results. The big difference between this and the others is that here we
+ * don't enable/disable any features such as FBC or PSR: we go with whatever
+ * the Kernel has enabled by default for us. This subtest only does things
+ * that are exercised by the other subtests and in a less exhaustive way: it's
+ * completely redundant. On the other hand, it is very quick and was created
+ * with the CI system in mind: it's a quick way to detect regressions, so if
+ * it fails, then we can run the other subtests to find out why.
+ *
+ * EXPECTED RESULTS
+ * Passed CRC assertions.
+ *
+ * FAILURES
+ * If you get a failure here, you should run the more specific draw and flip
+ * subtests of each feature in order to discover what exactly is failing and
+ * why.
+ *
+ * TODO: do sink CRC assertions in case sink_crc.supported. Only do this after
+ * our sink CRC code gets 100% reliable, in order to avoid CI false negatives.
+ */
+static void basic_subtest(const struct test_mode *t)
+{
+ struct draw_pattern_info *pattern = &pattern1;
+ struct modeset_params *params = pick_params(t);
+ enum igt_draw_method method;
+ struct igt_fb *fb1, fb2;
+ int r;
+ int assertions = DONT_ASSERT_FEATURE_STATUS;
+
+ prepare_subtest(t, pattern);
+
+ create_fb(t->format, params->fb.fb->width, params->fb.fb->height,
+ LOCAL_I915_FORMAT_MOD_X_TILED, t->plane, &fb2);
+ fb1 = params->fb.fb;
+
+ for (r = 0, method = 0; method < IGT_DRAW_METHOD_COUNT; method++, r++) {
+ if (r == pattern->n_rects) {
+ params->fb.fb = (params->fb.fb == fb1) ? &fb2 : fb1;
+
+ fill_fb_region(¶ms->fb, COLOR_PRIM_BG);
+ update_wanted_crc(t, &blue_crcs[t->format].crc);
+
+ page_flip_for_params(params, t->flip);
+ do_assertions(assertions);
+
+ r = 0;
+ }
+
+ draw_rect(pattern, ¶ms->fb, method, r);
+ update_wanted_crc(t, &pattern->crcs[t->format][r]);
+ do_assertions(assertions);
+ }
+}
+
static int opt_handler(int option, int option_index, void *data)
{
switch (option) {
@@ -3466,6 +3533,16 @@ if (0)
suspend_subtest(&t);
TEST_MODE_ITER_END
+ t.pipes = PIPE_SINGLE;
+ t.screen = SCREEN_PRIM;
+ t.plane = PLANE_PRI;
+ t.fbs = FBS_INDIVIDUAL;
+ t.feature = FEATURE_DEFAULT;
+ t.format = FORMAT_DEFAULT;
+ t.flip = FLIP_PAGEFLIP;
+ igt_subtest("basic")
+ basic_subtest(&t);
+
igt_fixture
teardown_environment();
--
2.7.0
_______________________________________________
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: [PATCH] drm/i915/fbc: enable FBC by default on HSW and BDW
2016-02-16 20:47 [PATCH] drm/i915/fbc: enable FBC by default on HSW and BDW Paulo Zanoni
` (2 preceding siblings ...)
2016-02-17 8:40 ` ✗ Fi.CI.BAT: warning for drm/i915/fbc: enable FBC by default on HSW and BDW Patchwork
@ 2016-02-17 16:23 ` Daniel Vetter
3 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2016-02-17 16:23 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Tue, Feb 16, 2016 at 06:47:21PM -0200, Paulo Zanoni wrote:
> These platforms should be fine now.
>
> FBC can allow very significant power savings for screen-on idle
> systems, but it is worth mentioning that a lot of people won't get
> significant power savings by enabling this feature because they may
> have something else preventing the system from getting into the
> deepest sleep states. Examples may include a hungry wifi device or a
> max_performance SATA link power management policy. You can check your
> PC state residencies on the powertop "Idle stats" tab. I recommend
> trying to run "sudo powertop --auto-tune" and then seeing if the
> residencies improve.
>
> Oh, and in case you - the person reading this commit message - found
> this commit through git bisect, please do the following:
> - Check your dmesg and see if there are error messages mentioning
> underruns around the time your problem started happening.
> - Download intel-gpu-tools, compile it, and run:
> $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 | tee fbc.txt
> Then send us the fbc.txt file, especially if you get a failure.
> This will really maximize your chances of getting the bug fixed
> quickly.
> - Try to find a reliable way to reproduce the problem, and tell us.
> - Boot with drm.debug=0xe, reproduce the problem, then send us the
> dmesg file.
>
> v2: Don't enable by default on SKL.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_fbc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 3614a95..0f0492f 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -823,13 +823,15 @@ static bool intel_fbc_can_choose(struct intel_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> struct intel_fbc *fbc = &dev_priv->fbc;
> + bool enable_by_default = IS_HASWELL(dev_priv) ||
> + IS_BROADWELL(dev_priv);
>
> if (intel_vgpu_active(dev_priv->dev)) {
> fbc->no_fbc_reason = "VGPU is active";
> return false;
> }
>
> - if (i915.enable_fbc < 0) {
> + if (i915.enable_fbc < 0 && !enable_by_default) {
> fbc->no_fbc_reason = "disabled per chip default";
> return false;
> }
> --
> 2.7.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
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