[Mesa-stable] [Mesa-dev] [PATCH] anv/blorp/clear_subpass: Only set surface clear color for fast clears

[Lionel Landwerlin]

On 23/02/17 16:26, Jason Ekstrand wrote:
> On Feb 23, 2017 8:03 AM, "Lionel Landwerlin" 
> <lionel.g.landwerlin at intel.com <mailto:lionel.g.landwerlin at intel.com>> 
> wrote:
>
>     I must be missing something, but I'm struggling to see why this
>     patch change anything.
>     Why does it matter if we write a clear color in SURFACE_STATE?
>     Shouldn't the usage of that color value only depend on whether we
>     enable 3DSTATE_PS.RenderTargetFastClearEnable for instance?
>
>
> Because ISL will assert on us if it's a color the surface state 
> doesn't allow.  In this particular case it was a non-0/1 color which 
> isn't even representable on Broadwell.

Thanks! Makes sense now :)

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

>
>     -
>     Lionel
>
>
>     On 22/02/17 03:31, Jason Ekstrand wrote:
>
>         Not all clear colors are valid.  In particular, on Broadwell and
>         earlier, only 0/1 colors are allowed in surface state.  No CTS
>         tests are
>         affected outright by this because, apparently, the CTS
>         coverage for
>         different clear colors is pretty terrible.  However, when
>         multisample
>         compression is enabled, we do hit it with CTS tests and this
>         commit
>         prevents regressions when enabling MCS on Broadwell and earlier.
>
>         Cc: "13.0 17.0" <mesa-stable at lists.freedesktop.org
>         <mailto:mesa-stable at lists.freedesktop.org>>
>         ---
>           src/intel/vulkan/anv_blorp.c | 5 +++--
>           1 file changed, 3 insertions(+), 2 deletions(-)
>
>         diff --git a/src/intel/vulkan/anv_blorp.c
>         b/src/intel/vulkan/anv_blorp.c
>         index 4e7078b..8db03e4 100644
>         --- a/src/intel/vulkan/anv_blorp.c
>         +++ b/src/intel/vulkan/anv_blorp.c
>         @@ -1198,9 +1198,10 @@ anv_cmd_buffer_clear_subpass(struct
>         anv_cmd_buffer *cmd_buffer)
>                 struct blorp_surf surf;
>                 get_blorp_surf_for_anv_image(image,
>         VK_IMAGE_ASPECT_COLOR_BIT,
>          att_state->aux_usage, &surf);
>         -      surf.clear_color =
>         vk_to_isl_color(att_state->clear_value.color);
>                   if (att_state->fast_clear) {
>         +         surf.clear_color =
>         vk_to_isl_color(att_state->clear_value.color);
>         +
>                    blorp_fast_clear(&batch, &surf, iview->isl.format,
>         iview->isl.base_level,
>         iview->isl.base_array_layer, fb->layers,
>         @@ -1224,7 +1225,7 @@ anv_cmd_buffer_clear_subpass(struct
>         anv_cmd_buffer *cmd_buffer)
>                                render_area.offset.x, render_area.offset.y,
>                                render_area.offset.x +
>         render_area.extent.width,
>                                render_area.offset.y +
>         render_area.extent.height,
>         -                     surf.clear_color, NULL);
>         +  vk_to_isl_color(att_state->clear_value.color), NULL);
>                 }
>                   att_state->pending_clear_aspects = 0;
>
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-stable/attachments/20170223/49e1d0e0/attachment.html>