[Mesa-dev] [PATCH 02/12] intel: Create intel_miptree_get_region() to prepare for fast color clear.

[Paul Berry]

On 23 May 2013 09:57, Eric Anholt <eric at anholt.net> wrote:

> Paul Berry <stereotype441 at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130523/49aed333/attachment.html>