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

[Jason Ekstrand]

On Mon, Jan 23, 2017 at 5:40 PM, Nanley Chery <nanleychery at gmail.com> wrote:

> 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.
>

I don't know how I feel for 17.0.  Obviously, making 17.0 faster is a good
thing.  On the other hand, users will get a 17.1 soon enough and this can
probably wait for that.  I think my preference (mild though it is) would be
to not bother with stable at all


> -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
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170123/84d43ceb/attachment-0001.html>