[Mesa-dev] [PATCH 00/14] Enable sRGB-encoded fast-clears on CannonLake

[Nanley Chery]

On Tue, Apr 03, 2018 at 03:50:38PM -0700, Jason Ekstrand wrote:
> I think I've reviewed all the ones that make significant functional
> changes.  The exception is the patch that makes us not do a fast depth
> clear.  I think what you did is probably better but I haven't thought about
> it enough to be sure.
> 

I can drop this patch. Although I think it's an improvement, I should
probably find some benchmarks to substantiate the claim.

> I'm not sure what I think about the last several that mostly move code
> around.  On the one hand, it kind-of unifies the "should we fast-clear"
> question into intel_mipmap_tree.c.  On the other hand, it pulls it away
> from the function that actually does the clear spreads the code out more.
> Multiple times in the past, I've wanted to pull the guts of the color
> clearing code out of brw_blorp.c and into brw_clear.c and just have
> brw_clear_miptree and brw_ccs/mcs_clear_miptree helpers.  Then all of those
> decisions would be together in brw_clear.c.  In any case, I'll think on it
> more.
> 

I understand your concern. There are multiple ways to go about hiding
::fast_clear_color and the one I went for (with super-powered setters)
does have the issues you bring up. I'll send out a v2 that accomplishes
hiding it in a less invasive manner. Please, let me know which version
you prefer (if either).

-Nanley

> --Jason
> 
> 
> On Fri, Mar 30, 2018 at 11:12 AM, Nanley Chery <nanleychery at gmail.com>
> wrote:
> 
> > Starting with CannonLake, the sampler no longer decodes the surface
> > state clear color when using an sRGB-formatted texture. This change
> > requires that our driver perform this decode in software instead. We
> > accounted for this change initially by disabling fast-clears when sRGB
> > encode was enabled. This series implements the software decode and
> > re-enables sRGB-encoded fast-clears.
> >
> > The software decode is performed through a new getter for the miptree
> > clear color. To keep the miptree API balanced and to discourage its
> > users from accessing the clear color field directly, we add a getter for
> > the depth clear value and change the existing setters so that the user
> > no longer needs to know the current clear color to perform an efficient
> > clear operation.
> >
> > Two piglit tests have been modified to test that the linearization of
> > the clear color occurs (when appropriate) for shader texture() calls and
> > on framebuffer blit sources. The modification patches can be found here:
> > https://lists.freedesktop.org/archives/piglit/2018-March/023996.html
> >
> > Jason Ekstrand (1):
> >   util/srgb: Add a float sRGB -> linear helper
> >
> > Nanley Chery (13):
> >   i965: Use the brw_context for the clear color and value setters
> >   i965/miptree: Move the clear color and value setter implementations
> >   i965: Make the miptree clear color setter take a gl_color_union
> >   i965/miptree: Add and use a getter for the clear color
> >   i965/miptree: Extend the sRGB-blending WA to future platforms
> >   i965/meta_util: Re-enable sRGB-encoded fast-clears on CNL
> >   i965: Add and use a getter for depth miptree clear values
> >   i965: Allow failure when setting the depth clear value
> >   i965/brw_clear: Don't resolve to change the depth clear value
> >   i965/brw_clear: Delete redundant code
> >   i965/blorp: Also skip the fast clear if the clear color differs
> >   i965/miptree: Allow failure when setting the clear color
> >   i965/blorp: Delete redundant code
> >
> >  src/mesa/drivers/dri/i965/brw_blorp.c            |  60 +++------
> >  src/mesa/drivers/dri/i965/brw_clear.c            |  45 +------
> >  src/mesa/drivers/dri/i965/brw_meta_util.c        |  11 --
> >  src/mesa/drivers/dri/i965/brw_misc_state.c       |  15 ---
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |   4 +-
> >  src/mesa/drivers/dri/i965/gen6_depth_state.c     |   4 +-
> >  src/mesa/drivers/dri/i965/gen7_misc_state.c      |   3 +-
> >  src/mesa/drivers/dri/i965/gen8_depth_state.c     |   3 +-
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c    | 165
> > ++++++++++++++++++++++-
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h    |  53 ++++----
> >  src/util/format_srgb.h                           |  14 ++
> >  11 files changed, 233 insertions(+), 144 deletions(-)
> >
> > --
> > 2.16.2
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >