Oops, forgot to add:
Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net>
On Mon, Jul 17, 2017 at 9:27 AM, Jason Ekstrand <ja...@jlekstrand.net>
wrote:
> On Mon, Jul 17, 2017 at 6:34 AM, Topi Pohjolainen <
> topi.pohjolai...@gmail.com> wrote:
>
>> 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.
>>
>
> Not true since you pulled this patch out for early merging. :-)
>
> 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.
>
>
>> 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 <topi.pohjolai...@intel.com>
>> ---
>> 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
>> 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
