<div dir="ltr"><div>Oops, forgot to add: </div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Jul 17, 2017 at 9:27 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jul 17, 2017 at 6:34 AM, Topi Pohjolainen <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Once the driver moves to ISL both compressed and uncompressed have the same type. One needs to tell them apart by other means. This can be done by checking the existence of mcs_buf. There is a short period of time within intel_miptree_create() where mcs_buf doesn't exist yet (between calls to intel_miptree_create_layout() and intel_miptree_alloc_mcs()). First compute_msaa_layout() makes the decision if compression is to be used and sets the msaa_layout type. Then based on the type one sets aux_usage and finally decides if mcs_buf is needed. This patch duplicates the logic in compute_msaa_layout() and uses that to make the decision on aux_usage and mcs_buf allocation. Most of the original logic in compute_msaa_layout() will be gone in the following patch leaving only one version. </blockquote><div> </div><div>Not true since you pulled this patch out for early merging. :-) </div><div>That said, I think it's ok to duplicate here. I'm not sure what we're going to want all this to look like in the ISLified future anyway. Probably something like Vulkan where we just call isl_surf_init_mcs and set aux_layout to NONE if it fails. In any case, this doesn't seem to make anything worse. </div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Elsewhere only brw_populate_sampler_prog_key_data() needs to know if compression is used based on the msaa_type. This is now replaced with consideration for number of samples and existence of mcs_buf. All other occurrences consider CMS || UMS which can be represented using single the type of ISL_MSAA_LAYOUT_ARRAY without any tweaks. Signed-off-by: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com" target="_blank">topi.pohjolainen@intel.com</a>> --- src/mesa/drivers/dri/i965/brw_wm.c | 8 +++-- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 43 ++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c index 3a5fcf5485..18056d51d0 100644 --- a/src/mesa/drivers/dri/i965/brw_wm.c +++ b/src/mesa/drivers/dri/i965/brw_wm.c @@ -396,9 +396,11 @@ brw_populate_sampler_prog_key_data(struct gl_context *ctx, /* From gen9 onwards some single sampled buffers can also be * compressed. These don't need ld2dms sampling along with mcs fetch. */ - if (brw->gen >= 7 && - intel_tex->mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS && - intel_tex->mt->num_samples > 1) { + if (intel_tex->mt->aux_usage == ISL_AUX_USAGE_MCS) { + assert(brw->gen >= 7); + assert(intel_tex->mt->num_samples > 1); + assert(intel_tex->mt->mcs_buf); + assert(intel_tex->mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS); key->compressed_multisample_layout_mask |= 1 << s; if (intel_tex->mt->num_samples >= 16) { diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 69a9328d6c..bef544c0ae 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -62,6 +62,45 @@ static bool intel_miptree_alloc_aux(struct brw_context *brw, struct intel_mipmap_tree *mt); +static bool +is_mcs_supported(const struct brw_context *brw, mesa_format format, + uint32_t layout_flags) +{ + /* Prior to Gen7, all MSAA surfaces used IMS layout. */ + if (brw->gen < 7) + return false; + + /* In Gen7, IMS layout is only used for depth and stencil buffers. */ + switch (_mesa_get_format_base_format(format)) { + case GL_DEPTH_COMPONENT: + case GL_STENCIL_INDEX: + case GL_DEPTH_STENCIL: + return false; + default: + /* From the Ivy Bridge PRM, Vol4 Part1 p77 ("MCS Enable"): + * + * This field must be set to 0 for all SINT MSRTs when all RT channels + * are not written + * + * In practice this means that we have to disable MCS for all signed + * integer MSAA buffers. The alternative, to disable MCS only when one + * of the render target channels is disabled, is impractical because it + * would require converting between CMS and UMS MSAA layouts on the fly, + * which is expensive. + */ + if (brw->gen == 7 && _mesa_get_format_datatype(format) == GL_INT) { + return false; + } else if (layout_flags & MIPTREE_LAYOUT_DISABLE_AUX) { + /* We can't use the CMS layout because it uses an aux buffer, the MCS + * buffer. So fallback to UMS, which is identical to CMS without the + * MCS. */ + return false; + } else { + return true; + } + } +} + /** * Determine which MSAA layout should be used by the MSAA surface being * created, based on the chip generation and the surface type. @@ -566,7 +605,9 @@ intel_miptree_choose_aux_usage(struct brw_context *brw, { assert(mt->aux_usage == ISL_AUX_USAGE_NONE); - if (mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS) { + const unsigned no_flags = 0; + if (mt->num_samples > 1 && is_mcs_supported(brw, mt->format, no_flags)) { + assert(mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS); mt->aux_usage = ISL_AUX_USAGE_MCS; } else if (intel_tiling_supports_ccs(brw, mt->tiling) && intel_miptree_supports_ccs(brw, mt)) { -- 2.11.0 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div></div></div> </div></div> </blockquote></div> </div>