Understood. Ultimately, this may not be such a big performance boost,
so if it's not easy to track down the issues, happy to table it.
Before you spend serious time investigating, check if it has any
effect on a benchmark. If not, we can move on - this is purely for
perf, not correctness.
On Mon, Nov 28, 2016 at 8:11 PM, Rowley, Timothy O
<timothy.o.row...@intel.com> wrote:
> 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 <imir...@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 <imir...@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
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
