On Tuesday, May 31, 2016 5:16:22 PM PDT Topi Pohjolainen wrote: > Fixes rendering in Shadow of Mordor with rbc. Application writes > RGBA_UNORM texture filling it with values the application wants to > later on treat as SRGB_ALPHA. > Intel driver enables lossless compression for the buffer by the time > of writing. However, the driver fails to make sure the buffer can be > sampled as something else later on and unfortunately there is > restriction in the hardware for using lossless compression for srgb > formats which looks to extend itself to the sampling engine also. > Requesting srgb to linear conversion on top of compressed buffer > results the color values to be pretty much garbage. > > Fortunately none of tracked benchmarks showed a regression with > this. > > Signed-off-by: Topi Pohjolainen <[email protected]> > CC: Kenneth Graunke <[email protected] > --- > src/mesa/drivers/dri/i965/brw_context.c | 38 > ++++++++++++++++++++++++-- > src/mesa/drivers/dri/i965/gen8_surface_state.c | 13 ++++++++- > 2 files changed, 48 insertions(+), 3 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_context.c > b/src/mesa/drivers/dri/i965/brw_context.c > index 2504dce..652acbf 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.c > +++ b/src/mesa/drivers/dri/i965/brw_context.c > @@ -70,6 +70,7 @@ > #include "tnl/t_pipeline.h" > #include "util/ralloc.h" > #include "util/debug.h" > +#include "isl/isl.h" > > /*************************************** > * Mesa's Driver Functions > @@ -166,6 +167,38 @@ intel_update_framebuffer(struct gl_context *ctx, > fb->DefaultGeometry.NumSamples); > } > > +/* On Gen9 color buffers may be compressed by the hardware (lossless > + * compression). There are, however, format restrictions and care needs to be > + * taken that the sampler engine is capable for re-interpreting a buffer with > + * format different the buffer was originally written with. > + * > + * For example, SRGB formats are not compressible and the sampler engine > isn't > + * capable of treating RGBA_UNORM as SRGB_ALPHA. In such a case the > underlying > + * color buffer needs to be resolved so that the sampling surface can be > + * sampled as non-compressed (i.e., without the auxiliary MCS buffer being > + * set). > + */ > +static bool > +intel_texture_view_requires_resolve(struct brw_context *brw, > + struct intel_texture_object *intel_tex) > +{ > + if (brw->gen < 9 || > + !intel_miptree_is_lossless_compressed(brw, intel_tex->mt)) > + return false; > + > + const uint32_t brw_format = > brw_format_for_mesa_format(intel_tex->_Format); > + > + if (isl_format_supports_lossless_compression(brw->intelScreen->devinfo, > + brw_format)) > + return false; > + > + perf_debug("Incompatible sampling format (%s) for rbc (%s)\n", > + _mesa_get_format_name(intel_tex->_Format), > + _mesa_get_format_name(intel_tex->mt->format)); > + > + return true; > +} > + > static void > intel_update_state(struct gl_context * ctx, GLuint new_state) > { > @@ -198,8 +231,9 @@ intel_update_state(struct gl_context * ctx, GLuint > new_state) > /* Sampling engine understands lossless compression and resolving > * those surfaces should be skipped for performance reasons. > */ > - intel_miptree_resolve_color(brw, tex_obj->mt, > - INTEL_MIPTREE_IGNORE_CCS_E); > + const int flags = intel_texture_view_requires_resolve(brw, tex_obj) ? > + 0 : INTEL_MIPTREE_IGNORE_CCS_E; > + intel_miptree_resolve_color(brw, tex_obj->mt, flags); > brw_render_cache_set_check_flush(brw, tex_obj->mt->bo); > } > > diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c > b/src/mesa/drivers/dri/i965/gen8_surface_state.c > index a3ad108..e7d9306 100644 > --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c > +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c > @@ -40,6 +40,7 @@ > #include "brw_state.h" > #include "brw_defines.h" > #include "brw_wm.h" > +#include "isl/isl.h" > > /** > * Convert an swizzle enumeration (i.e. SWIZZLE_X) to one of the Gen7.5+ > @@ -254,8 +255,18 @@ gen8_emit_texture_surface_state(struct brw_context *brw, > * the color buffer should always have been resolved before it is used as > * a texture so there is no need for it. On Gen9 it will be uploaded when > * the surface is losslessly compressed (CCS_E). > + * However, sampling engine is not capable of re-interpreting the > + * underlying color buffer in non-compressible formats when the surface > + * is configured as compressed. Therefore state upload has made sure the > + * buffer is in resolved state allowing the surface to be configured as > + * as non-compressed. > */ > - if (mt->num_samples <= 1 && aux_mode != GEN9_SURFACE_AUX_MODE_CCS_E) { > + if (mt->num_samples <= 1 && > + (aux_mode != GEN9_SURFACE_AUX_MODE_CCS_E || > + !isl_format_supports_lossless_compression( > + brw->intelScreen->devinfo, format))) { > + assert(!mt->mcs_mt || > + mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_RESOLVED); > aux_mt = NULL; > aux_mode = GEN8_SURFACE_AUX_MODE_NONE; > } >
With Matt and Grazvydas' suggestions fixed up, this series is: Reviewed-by: Kenneth Graunke <[email protected]> Cc: "12.0" <[email protected]>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
