Op 27-06-18 om 16:14 schreef Ville Syrjälä: > On Wed, Jun 27, 2018 at 01:45:04PM +0200, Maarten Lankhorst wrote: >> Op 26-06-18 om 19:59 schreef Ville Syrjälä: >>> On Mon, Jun 25, 2018 at 06:37:58PM +0200, Maarten Lankhorst wrote: >>>> The only time we should start FBC is when we have waited a vblank >>>> after the atomic update. >>> What about front buffer tracking? Is this going to leave FBC permanently >>> disabled unless there are flips/plane updates? >> No, see intel_fbc_flush. If there's a race with frontbuffer tracking and page flip, >> we will not enable FBC in intel_fbc_flush(), but in that case we would enable it from >> intel_fbc_post_update(). > I guess what I haven't quite figured out is why the pre_plane_update() > even leaves fbc logically enabled. I would think we should just disable > fbc there if anything important is going to change, and then re-enable > at post_plane_update() after reallocating the cfb. Indeed. >> Or the other way around, intel_fbc_post_update won't enable FBC if the fb is dirty, but >> intel_fbc_flush() afterwards will. >>> I think there are a few cases we need to consider: >>> 1. plane update which doesn't need fbc disable >>> 2. plane update which needs to disable fbc but can re-enable it after the flip >>> has happended (eg. need to reallocate the cfb due to plane size/format change) >>> 3. plane update which needs to disable fbc and can't re-enable it (eg. the new >>> plane state no longer fbc compatible) >>> 4. front buffer invalidate + flush >>> >>> HW nuke will handle case 1. Case 2 could do the fbc re-enable after the >>> post-flip vblank wait. Case 3 would ideally let us move FBC to another >>> plane (thinking of pre-HSW hardware here). And case 4 must re-enable fbc >>> after the flush has happened. >> I don't see how this code will break any case. :) > OK, so I guess you just remove the delayed activation at > flush/post_plane_update. So now it'll enable it immediately. > > Hmm, and if we just get a flush without the invalidate then we're going > to just use the nuke msg register anyway. So I suppose this shouldn't > cause much additional fbc on<->off ping pong. > > Actually, are we now effecitively forcing a synchronous vblank wait > in pre_plane_update for every frame via the hw_deactivate() polling > for fbc to stop? AFAICS with the re-enable now being immediate we're > going to have to disable fbc every single time. I think the correct > fix for that would involve a) not disabling fbc around flips when the > hw flip nuke will suffice, and b) convert the "wait for fbc to disable" > thing into a post_plane_update time assert (and verify whether the hw > is actually happy with disabling both fbc and the plane at the same > time). Could we not block in hw_deactivate then? But only <gen5 is affected, not sure how much we still care about i8xx fbc since it's disabled by default. I think doing it in pre_plane_update is fine? FBC is only enabled on bdw or later. It won't stall there. ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
