[Mesa-dev] [PATCH 4/4] swr: [rasterizer core] use ClearTile helper to store fast clears

[Rowley, Timothy O]

This patch is showing some regressions on internal testing.  As we talked about on irc, it appears to be a combination of crashes (probably missing table entries) and possibly wrong clear values.  Will need to back to you later about the errors, but for now we need to hold off on this patch.

-Tim

> On Nov 19, 2016, at 9:48 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> 
> No point in clearing the hot tile and then storing that - may as well
> just store the clear color to the surface directly. Use the helper that
> already exists for this purpose.
> 
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
> 
> My theory is that this is going to be a very modest perf improvement. Instead
> of first clearing the hot tile and then storing it, we store the clear color
> directly.
> 
> It does bring up a rare case where a tile might be cleared, stored, and then
> re-used with the same buffer. In that case, the former logic would avoid the
> load while the new logic will end up reloading the clear color/etc. There was
> a grand total of one piglit that was hit by this:
> 
>  fbo-attachments-blit-scaled-linear
> 
> (and that is the reason that we have to set the hottile to INVALID rather than
> the post state when storing.)
> 
> src/gallium/drivers/swr/rasterizer/core/backend.cpp | 17 ++++++++++-------
> src/gallium/drivers/swr/rasterizer/core/tilemgr.cpp | 15 +++++----------
> 2 files changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/src/gallium/drivers/swr/rasterizer/core/backend.cpp b/src/gallium/drivers/swr/rasterizer/core/backend.cpp
> index c45c0a7..ff08233 100644
> --- a/src/gallium/drivers/swr/rasterizer/core/backend.cpp
> +++ b/src/gallium/drivers/swr/rasterizer/core/backend.cpp
> @@ -358,16 +358,19 @@ void ProcessStoreTileBE(DRAW_CONTEXT *pDC, uint32_t workerId, uint32_t macroTile
>     HOTTILE *pHotTile = pContext->pHotTileMgr->GetHotTileNoLoad(pContext, pDC, macroTile, attachment, false);
>     if (pHotTile)
>     {
> -        // clear if clear is pending (i.e., not rendered to), then mark as dirty for store.
> +        // clear the surface directly
>         if (pHotTile->state == HOTTILE_CLEAR)
>         {
> -            PFN_CLEAR_TILES pfnClearTiles = sClearTilesTable[srcFormat];
> -            SWR_ASSERT(pfnClearTiles != nullptr);
> -
> -            pfnClearTiles(pDC, attachment, macroTile, pHotTile->renderTargetArrayIndex, pHotTile->clearData, pDesc->rect);
> +            pContext->pfnClearTile(GetPrivateState(pDC), attachment,
> +                x * KNOB_MACROTILE_X_DIM, y * KNOB_MACROTILE_Y_DIM,
> +                pHotTile->renderTargetArrayIndex,
> +                (const float *)pHotTile->clearData);
> +
> +            // Since the state is effectively uninitialized, make sure that we
> +            // reload any data.
> +            pHotTile->state = HOTTILE_INVALID;
>         }
> -
> -        if (pHotTile->state == HOTTILE_DIRTY || pDesc->postStoreTileState == (SWR_TILE_STATE)HOTTILE_DIRTY)
> +        else if (pHotTile->state == HOTTILE_DIRTY || pDesc->postStoreTileState == (SWR_TILE_STATE)HOTTILE_DIRTY)
>         {
>             int32_t destX = KNOB_MACROTILE_X_DIM * x;
>             int32_t destY = KNOB_MACROTILE_Y_DIM * y;
> diff --git a/src/gallium/drivers/swr/rasterizer/core/tilemgr.cpp b/src/gallium/drivers/swr/rasterizer/core/tilemgr.cpp
> index f398667..a4a6152 100644
> --- a/src/gallium/drivers/swr/rasterizer/core/tilemgr.cpp
> +++ b/src/gallium/drivers/swr/rasterizer/core/tilemgr.cpp
> @@ -151,17 +151,12 @@ HOTTILE* HotTileMgr::GetHotTile(SWR_CONTEXT* pContext, DRAW_CONTEXT* pDC, uint32
> 
>             if (hotTile.state == HOTTILE_CLEAR)
>             {
> -                if (attachment == SWR_ATTACHMENT_STENCIL)
> -                    ClearStencilHotTile(&hotTile);
> -                else if (attachment == SWR_ATTACHMENT_DEPTH)
> -                    ClearDepthHotTile(&hotTile);
> -                else
> -                    ClearColorHotTile(&hotTile);
> -
> -                hotTile.state = HOTTILE_DIRTY;
> +                pContext->pfnClearTile(GetPrivateState(pDC), attachment,
> +                    x * KNOB_MACROTILE_X_DIM, y * KNOB_MACROTILE_Y_DIM,
> +                    hotTile.renderTargetArrayIndex,
> +                    (const float *)hotTile.clearData);
>             }
> -
> -            if (hotTile.state == HOTTILE_DIRTY)
> +            else if (hotTile.state == HOTTILE_DIRTY)
>             {
>                 pContext->pfnStoreTile(GetPrivateState(pDC), format, attachment,
>                     x * KNOB_MACROTILE_X_DIM, y * KNOB_MACROTILE_Y_DIM, hotTile.renderTargetArrayIndex, hotTile.pBuffer);
> -- 
> 2.7.3
>