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 <nanleych...@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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
