[Intel-gfx] [PATCH RESEND] drm: i915: Preserve old FBC status for update without new planes

[Chris Wilson]

Quoting Gabriel Krisman Bertazi (2017-06-01 16:36:08)
> If the atomic commit doesn't include any new plane, there is no need to
> choose a new CRTC for FBC, and the intel_fbc_choose_crtc() will bail out
> early.  Although, if the FBC setup failed in the previous commit, if the
> current commit doesn't include new plane update, it tries to overwrite
> no_fbc_reason to "no suitable CRTC for FBC".  For that scenario, it is
> better that we simply keep the old message in-place to make debugging
> easier.
> 
> A scenario where this happens is with the
> igt at kms_frontbuffer_tracking@fbc-suspend testcase when executed on a
> Haswell system with not enough stolen memory.  When enabling the CRTC,
> the FBC setup will be correctly initialized to a specific CRTC, but
> won't be enabled, since there is not enough memory.  The testcase will
> then enable CRC checking, which requires a quirk for Haswell, which
> issues a new atomic commit that doesn't update the planes.  Since that
> update doesn't include any new planes (and the FBC wasn't enabled),
> intel_fbc_choose_crtc() will not find any suitable CRTC, but update the
> error message, hiding the lack of memory information, which is the
> actual cause of the initialization failure.  As a result, this causes
> that test, which should skip, to fail on Haswell.
> 
> Changes since v1:
>   - Update commit message (Manasi)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100020
> Fixes: f7e9b004b8a3 ("drm/i915/fbc: inline intel_fbc_can_choose()")

I just don't see the test case as being a good reason to claim the
kernel behaviour is broken. The kernel may report any of the reasons as
the one that caused FBC to be disabled (they are all valid, it is only
the order in which we test, or the order in which we set the reason that
determines the "reason" reported via debugfs). The test itself looks to
be at fault for rejecting a valid FBC failure and claiming it to be a
test failure. The test is far too fragile and dependent upon assumptions
about the kernel internals.
-Chris