[Intel-gfx] [PATCH] drm: i915: Preserve old FBC status if update with no new planes

[Gabriel Krisman Bertazi]

Manasi Navare <manasi.d.navare at intel.com> writes:

Hi Manasi,

> So the purpose of this patch is to avoid overwriting the no_fbc_reason
> field during atomic_check in case there is no plane update so that
> it retains the actual failure message from previous atomic commit operation
> failure where it failed to enable fbc in intel_fbc_can_enable() during
> the post plane update right?

yes, correct.

> On Mon, May 15, 2017 at 09:33:04PM -0300, Gabriel Krisman Bertazi wrote:
>> 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 beforehand for whatever reason,
>> we don't bail early, but we change the no_fbc_reason to "no suitable
>> CRTC for FBC", which simply hides the real reason why the FBC wasn't
>
> I think this can be reworded a bit like " Although, if the FBC setup failed
> in the previous commit, if the current commit doesnt include new plane update,
> it tries to overwrite no_fbc_reason to "no suitable CRTC for FBC".
>
>
>> initialized.  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 to fail on Haswell.
>
> So the problem here is just a wrong error message.
> How does a wrong error message cause the IGT test to fail?

igt is prepared to skip the test on boxes where there isn't enough
stolen memory, but since we overwrite that message, the test will
execute and fail.  We discussed earlier on the list about adding a new
check to igt for the "no suitable CRTC for FBC" message, but that could
end up hiding other real error conditions.

-- 
Gabriel Krisman Bertazi