[Mesa-dev] [PATCH 1/2] anv/cmd_buffer: Don't temporarily enable CCS_E within a render pass

[Nanley Chery]

On Mon, Jan 23, 2017 at 05:30:22PM -0800, Jason Ekstrand wrote:
> On Mon, Jan 23, 2017 at 4:55 PM, Nanley Chery <nanleychery at gmail.com> wrote:
> 
> > Compressing a render target and decompressing it in the same
> > single-subpass render pass may waste bandwidth. While this may be
> > beneficial in some circumstances, it does not help in all.
> >
> > Cc: "13.0 17.0" <mesa-stable at lists.freedesktop.org>
> >
> 
> This doesn't really fix a bug...  I guess you can consider it a perf bug
> but it's not going to cause apps to fail.  I think I'm ok with pulling it
> back to 17.0 but let's leave 13.0 alone.
> 
> 

Sounds good to me. I retract the "13.0" from this patch and the
following (I plan to officially do so in future patch revisions). As a
disclaimer, I personally don't mind retracting the nomination for "17.0"
if it's an issue.

-Nanley

> > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > ---
> >  src/intel/vulkan/genX_cmd_buffer.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > b/src/intel/vulkan/genX_cmd_buffer.c
> > index a22fb2b6fb..9cde6896bb 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -265,8 +265,10 @@ color_attachment_compute_aux_usage(struct anv_device
> > *device,
> >        att_state->fast_clear = false;
> >     }
> >
> > -   if (isl_format_supports_lossless_compression(&device->info,
> > -                                                iview->isl.format)) {
> > +   /* TODO: Consider using a heuristic to determine if temporarily
> > enabling
> > +    * CCS_E for this image view would be beneficial.
> > +    */
> >
> 
> Maybe we should update this comment a bit:
> 
> 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.
> 
> Also... This doesn't do quite what you think it does.  It shuts off fast
> clears entirely on Sky Lake if we can't texture from CCS_E.  We need to add
> some code above to do
> 
> if (GEN_GEN >= 9 && !isl_format_supports_lossless_compression(...))
>    att_state->fast_clear = false;
> 
> And then remove the GEN_GEN >= 9 case below.  Maybe we want to do the code
> shuffling as a refactor patch and put this patch on top of it?
> 
> 
> > +   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 if (att_state->fast_clear) {
> > --
> > 2.11.0
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >