[Mesa-stable] [PATCH v2] swr: move msaa resolve to generalized StoreTile

Rowley, Timothy O timothy.o.rowley at intel.com
Thu May 4 19:47:45 UTC 2017


> On Apr 27, 2017, at 6:22 PM, Bruce Cherniak <bruce.cherniak at intel.com> wrote:
> 
> v2: Reword commit message to more closely adhere to community
> guidelines.
> 
> This patch moves msaa resolve down into core/StoreTiles where the
> surface format conversion routines are available.  The previous
> "experimental" resolve was limited to 8-bit unsigned render targets.
> 
> This fixes a number of piglit msaa tests by adding resolve support for
> all the render target formats we support.
> 
> MSAA is still disabled by default, but can be enabled with
> "export SWR_MSAA_MAX_COUNT=4" (1,2,4,8,16 are options)
> The default is 0, which is disabled.
> 
> Because it fixes a number of piglit tests, I kindly request inclusion
> into 17.1 stable.

Could you list the fixed piglit tests, wildcarding if necessary if the list is overly large?

> cc: mesa-stable at lists.freedesktop.org
> ---
> .../drivers/swr/rasterizer/memory/StoreTile.h      | 75 +++++++++++++++++++++
> src/gallium/drivers/swr/swr_context.cpp            | 77 +---------------------
> src/gallium/drivers/swr/swr_screen.cpp             | 10 +--
> 3 files changed, 82 insertions(+), 80 deletions(-)
> 
> diff --git a/src/gallium/drivers/swr/rasterizer/memory/StoreTile.h b/src/gallium/drivers/swr/rasterizer/memory/StoreTile.h
> index ffde574c03..12a5f3d8ce 100644
> --- a/src/gallium/drivers/swr/rasterizer/memory/StoreTile.h
> +++ b/src/gallium/drivers/swr/rasterizer/memory/StoreTile.h
> @@ -1133,6 +1133,64 @@ struct StoreRasterTile
>             }
>         }
>     }
> +
> +    //////////////////////////////////////////////////////////////////////////
> +    /// @brief Resolves an 8x8 raster tile to the resolve destination surface.
> +    /// @param pSrc - Pointer to raster tile.
> +    /// @param pDstSurface - Destination surface state
> +    /// @param x, y - Coordinates to raster tile.
> +    /// @param sampleOffset - Offset between adjacent multisamples
> +    INLINE static void Resolve(
> +        uint8_t *pSrc,
> +        SWR_SURFACE_STATE* pDstSurface,
> +        uint32_t x, uint32_t y, uint32_t sampleOffset, uint32_t renderTargetArrayIndex) // (x, y) pixel coordinate to start of raster tile.
> +    {
> +        uint32_t lodWidth = std::max(pDstSurface->width >> pDstSurface->lod, 1U);
> +        uint32_t lodHeight = std::max(pDstSurface->height >> pDstSurface->lod, 1U);
> +
> +        float oneOverNumSamples = 1.0f / pDstSurface->numSamples;
> +
> +        // For each raster tile pixel (rx, ry)
> +        for (uint32_t ry = 0; ry < KNOB_TILE_Y_DIM; ++ry)
> +        {
> +            for (uint32_t rx = 0; rx < KNOB_TILE_X_DIM; ++rx)
> +            {
> +                // Perform bounds checking.
> +                if (((x + rx) < lodWidth) &&
> +                        ((y + ry) < lodHeight))
> +                {
> +                    // Sum across samples
> +                    float resolveColor[4] = {0};
> +                    for (uint32_t sampleNum = 0; sampleNum < pDstSurface->numSamples; sampleNum++)
> +                    {
> +                        float sampleColor[4] = {0};
> +                        uint8_t *pSampleSrc = pSrc + sampleOffset * sampleNum;
> +                        GetSwizzledSrcColor(pSampleSrc, rx, ry, sampleColor);
> +                        resolveColor[0] += sampleColor[0];
> +                        resolveColor[1] += sampleColor[1];
> +                        resolveColor[2] += sampleColor[2];
> +                        resolveColor[3] += sampleColor[3];
> +                    }
> +
> +                    // Divide by numSamples to average
> +                    resolveColor[0] *= oneOverNumSamples;
> +                    resolveColor[1] *= oneOverNumSamples;
> +                    resolveColor[2] *= oneOverNumSamples;
> +                    resolveColor[3] *= oneOverNumSamples;
> +
> +                    // Use the resolve surface state
> +                    SWR_SURFACE_STATE* pResolveSurface = (SWR_SURFACE_STATE*)pDstSurface->pAuxBaseAddress;
> +                    uint8_t *pDst = (uint8_t*)ComputeSurfaceAddress<false, false>((x + rx), (y + ry),
> +                        pResolveSurface->arrayIndex + renderTargetArrayIndex, pResolveSurface->arrayIndex + renderTargetArrayIndex,
> +                        0, pResolveSurface->lod, pResolveSurface);
> +                    {
> +                        ConvertPixelFromFloat<DstFormat>(pDst, resolveColor);
> +                    }
> +                }
> +            }
> +        }
> +    }
> +
> };
> 
> template<typename TTraits, SWR_FORMAT SrcFormat, SWR_FORMAT DstFormat>
> @@ -2316,6 +2374,9 @@ struct StoreMacroTile
>             pfnStore[sampleNum] = (bForceGeneric || KNOB_USE_GENERIC_STORETILE) ? StoreRasterTile<TTraits, SrcFormat, DstFormat>::Store : OptStoreRasterTile<TTraits, SrcFormat, DstFormat>::Store;
>         }
> 
> +        // Save original for pSrcHotTile resolve.
> +        uint8_t *pResolveSrcHotTile = pSrcHotTile;
> +
>         // Store each raster tile from the hot tile to the destination surface.
>         for(uint32_t row = 0; row < KNOB_MACROTILE_Y_DIM; row += KNOB_TILE_Y_DIM)
>         {
> @@ -2328,6 +2389,20 @@ struct StoreMacroTile
>                 }
>             }
>         }
> +
> +        if (pDstSurface->pAuxBaseAddress)
> +        {
> +            uint32_t sampleOffset = KNOB_TILE_X_DIM * KNOB_TILE_Y_DIM * (FormatTraits<SrcFormat>::bpp / 8);
> +            // Store each raster tile from the hot tile to the destination surface.
> +            for(uint32_t row = 0; row < KNOB_MACROTILE_Y_DIM; row += KNOB_TILE_Y_DIM)
> +            {
> +                for(uint32_t col = 0; col < KNOB_MACROTILE_X_DIM; col += KNOB_TILE_X_DIM)
> +                {
> +                    StoreRasterTile<TTraits, SrcFormat, DstFormat>::Resolve(pResolveSrcHotTile, pDstSurface, (x + col), (y + row), sampleOffset, renderTargetArrayIndex);
> +                    pResolveSrcHotTile += sampleOffset * pDstSurface->numSamples;
> +                }
> +            }
> +        }
>     }
> };
> 
> diff --git a/src/gallium/drivers/swr/swr_context.cpp b/src/gallium/drivers/swr/swr_context.cpp
> index aa5cca8e65..4b7a321e51 100644
> --- a/src/gallium/drivers/swr/swr_context.cpp
> +++ b/src/gallium/drivers/swr/swr_context.cpp
> @@ -267,65 +267,6 @@ swr_resource_copy(struct pipe_context *pipe,
> }
> 
> 
> -/* XXX: This resolve is incomplete and suboptimal. It will be removed once the
> - * pipelined resolve blit works. */
> -void
> -swr_do_msaa_resolve(struct pipe_resource *src_resource,
> -                    struct pipe_resource *dst_resource)
> -{
> -   /* This is a pretty dumb inline resolve.  It only supports 8-bit formats
> -    * (ex RGBA8/BGRA8) - which are most common display formats anyway.
> -    */
> -
> -   /* quick check for 8-bit and number of components */
> -   uint8_t bits_per_component =
> -      util_format_get_component_bits(src_resource->format,
> -            UTIL_FORMAT_COLORSPACE_RGB, 0);
> -
> -   /* Unsupported resolve format */
> -   assert(src_resource->format == dst_resource->format);
> -   assert(bits_per_component == 8);
> -   if ((src_resource->format != dst_resource->format) ||
> -       (bits_per_component != 8)) {
> -      return;
> -   }
> -
> -   uint8_t src_num_comps = util_format_get_nr_components(src_resource->format);
> -
> -   SWR_SURFACE_STATE *src_surface = &swr_resource(src_resource)->swr;
> -   SWR_SURFACE_STATE *dst_surface = &swr_resource(dst_resource)->swr;
> -
> -   uint32_t *src, *dst, offset;
> -   uint32_t num_samples = src_surface->numSamples;
> -   float recip_num_samples = 1.0f / num_samples;
> -   for (uint32_t y = 0; y < src_surface->height; y++) {
> -      for (uint32_t x = 0; x < src_surface->width; x++) {
> -         float r = 0.0f;
> -         float g = 0.0f;
> -         float b = 0.0f;
> -         float a = 0.0f;
> -         for (uint32_t sampleNum = 0;  sampleNum < num_samples; sampleNum++) {
> -            offset = ComputeSurfaceOffset<false>(x, y, 0, 0, sampleNum, 0, src_surface);
> -            src = (uint32_t *) src_surface->pBaseAddress + offset/src_num_comps;
> -            const uint32_t sample = *src;
> -            r += (float)((sample >> 24) & 0xff) / 255.0f * recip_num_samples;
> -            g += (float)((sample >> 16) & 0xff) / 255.0f * recip_num_samples;
> -            b += (float)((sample >>  8) & 0xff) / 255.0f * recip_num_samples;
> -            a += (float)((sample      ) & 0xff) / 255.0f * recip_num_samples;
> -         }
> -         uint32_t result = 0;
> -         result  = ((uint8_t)(r * 255.0f) & 0xff) << 24;
> -         result |= ((uint8_t)(g * 255.0f) & 0xff) << 16;
> -         result |= ((uint8_t)(b * 255.0f) & 0xff) <<  8;
> -         result |= ((uint8_t)(a * 255.0f) & 0xff);
> -         offset = ComputeSurfaceOffset<false>(x, y, 0, 0, 0, 0, src_surface);
> -         dst = (uint32_t *) dst_surface->pBaseAddress + offset/src_num_comps;
> -         *dst = result;
> -      }
> -   }
> -}
> -
> -
> static void
> swr_blit(struct pipe_context *pipe, const struct pipe_blit_info *blit_info)
> {
> @@ -342,28 +283,14 @@ swr_blit(struct pipe_context *pipe, const struct pipe_blit_info *blit_info)
>       debug_printf("swr_blit: color resolve : %d -> %d\n",
>             info.src.resource->nr_samples, info.dst.resource->nr_samples);
> 
> -      /* Because the resolve is being done inline (not pipelined),
> -       * resources need to be stored out of hottiles and the pipeline empty.
> -       *
> -       * Resources are marked unused following fence finish because all
> -       * pipeline operations are complete.  Validation of the blit will mark
> -       * them are read/write again.
> -       */
> +      /* Resolve is done as part of the surface store. */
>       swr_store_dirty_resource(pipe, info.src.resource, SWR_TILE_RESOLVED);
> -      swr_store_dirty_resource(pipe, info.dst.resource, SWR_TILE_RESOLVED);
> -      swr_fence_finish(pipe->screen, NULL, swr_screen(pipe->screen)->flush_fence, 0);
> -      swr_resource_unused(info.src.resource);
> -      swr_resource_unused(info.dst.resource);
> 
>       struct pipe_resource *src_resource = info.src.resource;
>       struct pipe_resource *resolve_target =
>          swr_resource(src_resource)->resolve_target;
> 
> -      /* Inline resolve samples into resolve target resource, then continue
> -       * the blit. */
> -      swr_do_msaa_resolve(src_resource, resolve_target);
> -
> -      /* The resolve target becomes the new source for the blit.  */
> +      /* The resolve target becomes the new source for the blit. */
>       info.src.resource = resolve_target;
>    }
> 
> diff --git a/src/gallium/drivers/swr/swr_screen.cpp b/src/gallium/drivers/swr/swr_screen.cpp
> index 04e613745a..5a36d0211a 100644
> --- a/src/gallium/drivers/swr/swr_screen.cpp
> +++ b/src/gallium/drivers/swr/swr_screen.cpp
> @@ -891,6 +891,10 @@ swr_create_resolve_resource(struct pipe_screen *_screen,
> 
>       /* Attach it to the multisample resource */
>       msaa_res->resolve_target = alt;
> +
> +      /* Hang resolve surface state off the multisample surface state to so
> +       * StoreTiles knows where to resolve the surface. */
> +      msaa_res->swr.pAuxBaseAddress =  (uint8_t *)&swr_resource(alt)->swr;

Not a big fan of this, and that field should probably be renamed at some point, but for ok for now.

>    }
> 
>    return true; /* success */
> @@ -1009,14 +1013,10 @@ swr_flush_frontbuffer(struct pipe_screen *p_screen,
>       SwrEndFrame(swr_context(pipe)->swrContext);
>    }
> 
> -   /* Multisample surfaces need to be resolved before present */
> +   /* Multisample resolved into resolve_target at flush with store_resouce */

resouce -> resource

>    if (pipe && spr->swr.numSamples > 1) {
>       struct pipe_resource *resolve_target = spr->resolve_target;
> 
> -      /* Do an inline surface resolve into the resolve target resource
> -       * XXX: This works, just not optimal. Work on using a pipelined blit. */
> -      swr_do_msaa_resolve(resource, resolve_target);
> -
>       /* Once resolved, copy into display target */
>       SWR_SURFACE_STATE *resolve = &swr_resource(resolve_target)->swr;
> 
> -- 
> 2.11.0
> 
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable



More information about the mesa-stable mailing list