[Intel-gfx] [PATCH igt 3/4] tests/kms_fbc_crc: run even if FBC is disabled by default

[Daniel Vetter]

On Mon, Jun 15, 2015 at 11:08:27AM -0300, Paulo Zanoni wrote:
> 2015-06-15 9:11 GMT-03:00 Daniel Vetter <daniel at ffwll.ch>:
> > On Thu, Jun 04, 2015 at 11:31:05AM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >>
> >> We may not be perfect, but if we don't even test, we will probably
> >> only get worse over time.
> >>
> >> The function called makes sure we restore whatever was the original
> >> FBC parameter when we exit the test, so this should not affect the
> >> other tests.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >> ---
> >>  tests/kms_fbc_crc.c | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> >> index 37221ac..d964224 100644
> >> --- a/tests/kms_fbc_crc.c
> >> +++ b/tests/kms_fbc_crc.c
> >> @@ -559,10 +559,10 @@ igt_main
> >>               igt_assert_lt(0, fread(buf, 1, sizeof(buf), status));
> >>               fclose(status);
> >>               buf[sizeof(buf) - 1] = '\0';
> >> -             igt_require_f(!strstr(buf, "unsupported on this chipset") &&
> >> -                           !strstr(buf, "disabled per module param") &&
> >> -                           !strstr(buf, "disabled per chip default"),
> >> -                           "FBC not supported/enabled\n");
> >> +             igt_require_f(!strstr(buf, "unsupported on this chipset"),
> >> +                           "FBC not supported\n");
> >> +
> >> +             igt_set_module_param_int("enable_fbc", 1);
> >
> > This is risky since on older chips fbc is known to pretty much insta-kill
> > the box. Imo we should have a gen6+ here at least (that's roughly the
> > point where the hw workarounds start to sound less scary).
> 
> Well, we can try and then keep an eye on bugzilla, watching for the
> possible insta-kill bug reports, can't we? At least to me, this was
> not known as instal-kill since it's not documented anywhere and we
> don't have bug reports. Also, the move to the frontbuffer tracking
> infrastructure could maybe have solved the insta-kill problem.

Afaik fbc kills the chip on gen2 reliable when enabling, and iirc the same
on gen3. Not sure about gen4, but on ilk the w/a list in bspec is scary
enough to make grown-ups scream. Art recommended to just not bother, since
some of the required w/a (which we don't implement) force the chip into an
ill-defined state to provoke some kind of soft-reset. It's really all
fairly horrible.

> > Trying to enable this on older platforms just isn't worth it imo.
> 
> If we're not even going to try this, shouldn't we remove all the
> Kernel code support for FBC on these platforms? This is something I
> wanted to discuss at some point, but now you gave us a nice excuse :).
> Maybe it's better to just not have the code at all vs have a code that
> (maybe) instakills the machine and we're probably not going to fix
> anytime soon. I don't know, just an idea.

Yeah it might be time to nuke the old fbc code entirely.

> Btw, those patches are already on IGT.  Thomas seemed to be OK with
> the ideas, and after one week on the mailing list I pushed them.

Yeah didn't notice that yet. I'll follow up with a quick patch just for
paranoia.
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch