[Mesa-dev] [PATCH v3 11/16] anv/cmd_buffer: Move aux_usage assignment up

[Jason Ekstrand]

On Wed, Jun 28, 2017 at 2:14 PM, Nanley Chery <nanleychery at gmail.com> wrote:

> For readability, bring the assignment of CCS closer to the assignment of
> NONE and MCS.
>
> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 62 ++++++++++++++++++------------
> --------
>  1 file changed, 30 insertions(+), 32 deletions(-)
>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 49ad41edbd..1aa79c8e7b 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -253,6 +253,36 @@ color_attachment_compute_aux_usage(struct anv_device
> * device,
>        att_state->input_aux_usage = ISL_AUX_USAGE_MCS;
>        att_state->fast_clear = false;
>        return;
> +   } else if (iview->image->aux_usage == ISL_AUX_USAGE_CCS_E) {
> +      att_state->aux_usage = ISL_AUX_USAGE_CCS_E;
> +      att_state->input_aux_usage = ISL_AUX_USAGE_CCS_E;
>

I'm not sure if this actually improves readability as-is.  The no aux case
and MCS cases are early returns.  Maybe what we want is something like this:

if (aux_surface.isl.size == 0) {
   /* set to none */
   return;
}

switch (aux_usage) {
case MCS:
case CCS_E:
   aux_state->aux_usage = iview->image->aux_usage;
   aux_state->input_aux_usage = iview->image->aux_usage;
   break;

case NONE:
   assert(samples == 1);
   /* stuff below */
   break;

default:
   unreachable();
}

/* Now we determine whether or not we want to fast-clear */

if (samples > 1) {
   perf_debug();
   fast_clear = false;
   return;
}

/* Other fast clear determination. */

Incidentally, it may be cleaner in the long run if we split this into two
functions: compute_ccs_usage and compute_mcs_usage.

Just thoughts BTW.  I'm not 100% sure how to make this the most readable.


> +   } else {
> +      att_state->aux_usage = ISL_AUX_USAGE_CCS_D;
> +      /* From the Sky Lake PRM, RENDER_SURFACE_STATE::
> AuxiliarySurfaceMode:
> +       *
> +       *    "If Number of Multisamples is MULTISAMPLECOUNT_1, AUX_CCS_D
> +       *    setting is only allowed if Surface Format supported for Fast
> +       *    Clear. In addition, if the surface is bound to the sampling
> +       *    engine, Surface Format must be supported for Render Target
> +       *    Compression for surfaces bound to the sampling engine."
> +       *
> +       * In other words, we can only sample from a fast-cleared image if
> it
> +       * also supports color compression.
> +       */
> +      if (isl_format_supports_ccs_e(&device->info, iview->isl.format)) {
> +         /* TODO: Consider using a heuristic to determine if temporarily
> enabling
> +          * CCS_E for this image view would be beneficial.
> +          *
> +          * While fast-clear resolves and partial resolves are fairly
> cheap in the
> +          * case where you render to most of the pixels, full resolves
> are not
> +          * because they potentially involve reading and writing the
> entire
> +          * framebuffer.  If we can't texture with CCS_E, we should leave
> it off and
> +          * limit ourselves to fast clears.
> +          */
> +         att_state->input_aux_usage = ISL_AUX_USAGE_CCS_D;
> +      } else {
> +         att_state->input_aux_usage = ISL_AUX_USAGE_NONE;
> +      }
>     }
>
>     assert(iview->image->aux_surface.isl.usage & ISL_SURF_USAGE_CCS_BIT);
> @@ -315,38 +345,6 @@ color_attachment_compute_aux_usage(struct anv_device
> * device,
>     } else {
>        att_state->fast_clear = false;
>     }
> -
> -   /**
> -    * TODO: Consider using a heuristic to determine if temporarily
> enabling
> -    * CCS_E for this image view would be beneficial.
> -    *
> -    * While fast-clear resolves and partial resolves are fairly cheap in
> the
> -    * case where you render to most of the pixels, full resolves are not
> -    * because they potentially involve reading and writing the entire
> -    * framebuffer.  If we can't texture with CCS_E, we should leave it
> off and
> -    * limit ourselves to fast clears.
> -    */
> -   if (iview->image->aux_usage == ISL_AUX_USAGE_CCS_E) {
> -      att_state->aux_usage = ISL_AUX_USAGE_CCS_E;
> -      att_state->input_aux_usage = ISL_AUX_USAGE_CCS_E;
> -   } else {
> -      att_state->aux_usage = ISL_AUX_USAGE_CCS_D;
> -      /* From the Sky Lake PRM, RENDER_SURFACE_STATE::
> AuxiliarySurfaceMode:
> -       *
> -       *    "If Number of Multisamples is MULTISAMPLECOUNT_1, AUX_CCS_D
> -       *    setting is only allowed if Surface Format supported for Fast
> -       *    Clear. In addition, if the surface is bound to the sampling
> -       *    engine, Surface Format must be supported for Render Target
> -       *    Compression for surfaces bound to the sampling engine."
> -       *
> -       * In other words, we can only sample from a fast-cleared image if
> it
> -       * also supports color compression.
> -       */
> -      if (isl_format_supports_ccs_e(&device->info, iview->isl.format))
> -         att_state->input_aux_usage = ISL_AUX_USAGE_CCS_D;
> -      else
> -         att_state->input_aux_usage = ISL_AUX_USAGE_NONE;
> -   }
>  }
>
>  static bool
> --
> 2.13.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170710/c4148345/attachment-0001.html>