On Thu, Nov 21, 2013 at 06:33:21PM +0200, Ville Syrjälä wrote: > On Thu, Nov 21, 2013 at 11:49:47AM +0000, Chris Wilson wrote: > > On Thu, Nov 21, 2013 at 01:14:10PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > As per the SNB and HSW PM guides, we should enable FBC render/blitter > > > tracking only during batches targetting the front buffer. > > > > > > On SNB we must also update the FBC render tracking address whenever it > > > changes. And since the register in question is stored in the context, > > > we need to make sure we reload it with correct data after context > > > switches. > > > > > > On IVB/HSW we use the render nuke mechanism, so no render tracking > > > address updates are needed. Hoever on the blitter side we need to > > > enable the blitter tracking like on SNB, and in addition we need > > > to issue the cache clean messages, which we already did. > > > > > > v2: Introduce intel_fb_obj_has_fbc() > > > Fix crtc locking around crtc->fb access > > > Drop a hunk that was included by accident in v1 > > > Set fbc_address_dirty=false not true after emitting the LRI > > > v3: Now that fbc hangs on to the fb intel_fb_obj_has_fbc() doesn't > > > need to upset lockdep anymore > > > v4: Use |= instead of = to update fbc_address_dirty > > > > Ah, should we do the same for fbc_dirty? In the past we could dispatch a > > batchbuffer but fail to add the request (and so fail to flush the > > rendering/fbc). We currently preallocate the request so that failure > > path is history, but we will more than likely be caught out again in the > > future. > > I guess we could do it for fbc_dirty as well, but I don't think that > should actually change anything. Either we're rendering to the FBC > scanout buffer, or we're not. The scenario I worry about here is the missing flush after the rendering. It has been possible for us to lose it (under memory pressure, other constaints etc) and to issue a catch-up flush on the next batch. Without the or, we'd lose the FBC flush. It is just a potential issue for the future. > I did start pondering if I should actually move the fbc_address to live > under the context once that's where it actually belongs. If we'd track > it per-context we might be able to avoid emitting the LRI for every > context switch. > > > > > Like pc8, I'd like for a device (or crtc if you must) property whether > > or not indirect rendering is preferred. > > > > Other than that and the bikeshed to kill the redundant fbc_obj local > > variable and pack the dirty bits, it looks good to me. > > Actually I just fired it up for real on SNB and it failed. The problem > is that we end up doing the invalidate before the context switch. So > we've not yet forced fbc_address_dirty=true due to the context switch > when we do the invalidate. The most straightforward fix would be to > simply move i915_gem_execbuffer_move_to_gpu() to be called after > i915_switch_context(). I did that on my machine and now it passes my > context related FBC tests. But I do wonder if the order of these > operations was chose for a reason, and whether the reordering might > cause other problems. Ben, opinion on whether the ordering was just convenience? -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
