[Intel-gfx] [PATCH 2/2] drm/i915/fbc: Allow on unfenced surfaces, for recent gen

[Daniel Vetter]

On Wed, Aug 24, 2016 at 7:54 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Mon, Aug 22, 2016 at 09:39:17PM -0300, Paulo Zanoni wrote:
>> 2016-08-18 5:21 GMT-03:00 Chris Wilson <chris at chris-wilson.co.uk>:
>> > Only fbc1 is tied to using a fence. Later iterations of fbc are more
>> > flexible and allow operation on unfenced frontbuffers.
>> >
>> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> > Cc: Daniel Vetter <daniel.vetter at intel.com>
>> > Cc: "Zanoni, Paulo R" <paulo.r.zanoni at intel.com>
>>
>> Hi
>>
>> I see this patch was applied. Now, on my Skylake machine, if I boot
>> with i915.enable_fbc=1 I'll get FIFO underruns under fbcon. Just
>> booting already gives me a FIFO underrun message, and then if I "sudo
>> systemctl stop lightdm" I'll get a constantly-blinking screen.
>>
>> Of course, applying the patch that disables FBC after a FIFO underrun
>> will help stopping the ever-blinking fbcon, but I think we should try
>> to avoid the underruns in the places we know we can, and leave the
>> "disable FBC on FIFO underruns" just for the cases we're not expecting.
>>
>> Also, please remember that I mentioned there are FBC workarounds for
>> untiled that we still don't implement (although when I re-read my
>> email it may sound like I suggested the workarounds are for non-GTT
>> tracking). IMHO this argument alone should
>> have prevented this patch from being merged...
>>
>> Based on that, can we please revert this patch? I'm afraid some people
>> would consider these underruns as blockers to enabling FBC, so it's
>> probably better to enable FBC only on X tiled for now, and leave this
>> for when we know how to prevent the underrun (possibly by implementing
>> the missing WAs).
>>
>>
>> (I'm sorry if you got this message twice, but the mail servers are a
>> little crazy these days and I didn't receive my copy, so I'm sending
>> it again).
>
> Yeah, mailman was on vacation a bit the last few days due to a ddos
> probably. +1 from me for just reverting if this is causing troubles.
> Also, patch doesn't seem to have a Testcase: line, was the
> kms_frontbuffer_tracking test not extended to cover this new use-case? In
> that case definitely revert, since failed to pass testing requirements.

Original patch also wasn't acked by Paulo, which it pretty much has to
be as an fbc patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch