On 23 May 2013 09:57, Eric Anholt <e...@anholt.net> wrote: > Paul Berry <stereotype...@gmail.com> writes: > > > With the advent of fast color clears, it will no longer be safe for > > the driver to access the data stored in a miptree with impunity. For > > example, sometimes a resolve will need to be performed first (to > > ensure that deferred writes due to a fast clear are performed before > > the buffer is accessed). Other times, fast clear will need to be > > disabled for the miptree, so that its contents can be safely shared > > with an entity that Mesa can't synchronize with easily. > > > > To prepare for that, this patch renames intel_mipmap_tree::region to > > intel_mipmap_tree::region_private and creates an accessor function, > > intel_miptree_get_region(). At the moment, the accessor function > > simply returns region_private. Later in the patch series, this > > function will be expanded to take appropriate actions to maintain the > > proper fast color clear state. > > > > As much as possible, I've tried to restrict the functions which > > directly access region_private to low-level miptree functions > > (e.g. miptree initialization functions), so that it will be easy to > > verify that those functions access the miptree contents safely. > > I don't like this change. I think we should be explicitly resolving at > the right points, like in patch 10. In this patch, the places I see > that look like they could trigger a resolve from ACCESS_RENDER would all > break the GPU state, so you have to have things resolved before. This > means that these intel_miptree_get_region() functions just freak me out > when I see them in some code -- "oh crap, would we resolve here? that > would be bad... oh, looks like we prevent that over in this codepath > over here." > > Once the places that should absolutely never resolve get removed, > there's hardly anything left in this patch. It also goes against the > work I've done to kill the region struct. >
We (me, Ken, Eric, and Chad) just had an in-person discussion about this, and came up with a new plan: 1. Eric will send out some patches that funnel all of the blitting operations* through a new intel_miptree_blit() function (whose arguments are miptrees rather than regions) 2. Once those land, I'll rework my series so that it does the resolve (and other state updating) inside intel_miptree_blit(). Hopefully that will allow us to drop this patch. *Technically there is one blitting operation that can't go through intel_miptree_blit() (intelEmitImmediateColorExpandBlit(), which is used to accelerate glBitmap()). We'll put a resolve hook in there as a special case.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
