[Mesa-dev] [PATCH 27/32] i965: Change resolve flags to enum
Ben Widawsky
ben at bwidawsk.net
Thu Jan 5 01:58:46 UTC 2017
On 17-01-04 10:00:59, Topi Pohjolainen Topi Pohjolainen wrote:
>On Mon, Jan 02, 2017 at 06:37:18PM -0800, Ben Widawsky wrote:
>> In the foreseeable future it doesn't seem to make sense to have multiple
>> resolve flags. What does make sense is to have the caller give an
>> indication to the lower layers what it things should be done for
>> resolve. The enum change distinguishes this binary selection.
>>
>> v2: Make setting the hint more concise (Topi)
>>
>> Cc: Topi Pohjolainen <topi.pohjolainen at gmail.com>
>> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>> Acked-by: Daniel Stone <daniels at collabora.com>
>> ---
>> src/mesa/drivers/dri/i965/brw_blorp.c | 8 ++++----
>> src/mesa/drivers/dri/i965/brw_context.c | 13 +++++++------
>> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 12 ++++++------
>> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 13 ++++++++-----
>> 4 files changed, 25 insertions(+), 21 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
>> index 8d58616f59..d02920be57 100644
>> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>> @@ -209,12 +209,12 @@ blorp_surf_for_miptree(struct brw_context *brw,
>> surf->aux_usage = ISL_AUX_USAGE_NONE;
>> }
>> } else if (!(safe_aux_usage & (1 << surf->aux_usage))) {
>> - uint32_t flags = 0;
>> - if (safe_aux_usage & (1 << ISL_AUX_USAGE_CCS_E))
>> - flags |= INTEL_MIPTREE_IGNORE_CCS_E;
>> + const enum intel_resolve_hint hint =
>> + safe_aux_usage & (1 << ISL_AUX_USAGE_CCS_E) ?
>> + INTEL_RESOLVE_HINT_IGNORE_CCS_E : 0;
>>
>> intel_miptree_resolve_color(brw, mt,
>> - *level, start_layer, num_layers, flags);
>> + *level, start_layer, num_layers, hint);
>>
>> assert(!intel_miptree_has_color_unresolved(mt, *level, 1,
>> start_layer, num_layers));
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
>> index ce01605826..ffcefe799d 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.c
>> +++ b/src/mesa/drivers/dri/i965/brw_context.c
>> @@ -261,9 +261,10 @@ 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.
>> */
>> - const int flags = intel_texture_view_requires_resolve(brw, tex_obj) ?
>> - 0 : INTEL_MIPTREE_IGNORE_CCS_E;
>> - intel_miptree_all_slices_resolve_color(brw, tex_obj->mt, flags);
>> + const enum intel_resolve_hint hint =
>> + intel_texture_view_requires_resolve(brw, tex_obj) ? 0 :
> ^
>extra space. You could also put it to the line below with the else-branch...
>
Fixed the space. I don't get the part about putting it with the else-branch.
>> + INTEL_RESOLVE_HINT_IGNORE_CCS_E;
>> + intel_miptree_all_slices_resolve_color(brw, tex_obj->mt, hint);
>> brw_render_cache_set_check_flush(brw, tex_obj->mt->bo);
>>
>> if (tex_obj->base.StencilSampling ||
>> @@ -316,9 +317,9 @@ intel_update_state(struct gl_context * ctx, GLuint new_state)
>> intel_renderbuffer(fb->_ColorDrawBuffers[i]);
>>
>> if (irb &&
>> - intel_miptree_resolve_color(
>> - brw, irb->mt, irb->mt_level, irb->mt_layer, irb->layer_count,
>> - INTEL_MIPTREE_IGNORE_CCS_E))
>> + intel_miptree_resolve_color(brw, irb->mt, irb->mt_level,
>> + irb->mt_layer, irb->layer_count,
>> + INTEL_RESOLVE_HINT_IGNORE_CCS_E))
>> brw_render_cache_set_check_flush(brw, irb->mt->bo);
>> }
>> }
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 5057d517a6..26029df09a 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -2321,7 +2321,7 @@ intel_miptree_used_for_rendering(const struct brw_context *brw,
>> static bool
>> intel_miptree_needs_color_resolve(const struct brw_context *brw,
>> const struct intel_mipmap_tree *mt,
>> - int flags)
>> + enum intel_resolve_hint hint)
>> {
>> if (mt->aux_disable & INTEL_AUX_DISABLE_CCS)
>> return false;
>> @@ -2333,7 +2333,7 @@ intel_miptree_needs_color_resolve(const struct brw_context *brw,
>> * surfaces called "lossless compressed". These don't need to be always
>> * resolved.
>> */
>> - if ((flags & INTEL_MIPTREE_IGNORE_CCS_E) && is_lossless_compressed)
>> + if ((hint == INTEL_RESOLVE_HINT_IGNORE_CCS_E) && is_lossless_compressed)
>
>Should we keep on "and-ing"? If we add any parallel hints in the future this
>won't work anymore. Otherwise:
>
The point of the change was that a hint is a hint and there are no parallel
hints. If you feel strongly that there will be such hints, then I can forego the
enum, but my proposal is let's try this for now and see how it goes.
>Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>
Thanks :-)
>> return false;
>>
>> /* Fast color clear resolves only make sense for non-MSAA buffers. */
>> @@ -2347,11 +2347,11 @@ bool
>> intel_miptree_resolve_color(struct brw_context *brw,
>> struct intel_mipmap_tree *mt, unsigned level,
>> unsigned start_layer, unsigned num_layers,
>> - int flags)
>> + enum intel_resolve_hint hint)
>> {
>> intel_miptree_check_color_resolve(brw, mt, level, start_layer);
>>
>> - if (!intel_miptree_needs_color_resolve(brw, mt, flags))
>> + if (!intel_miptree_needs_color_resolve(brw, mt, hint))
>> return false;
>>
>> /* Arrayed fast clear is only supported for gen8+. */
>> @@ -2380,9 +2380,9 @@ intel_miptree_resolve_color(struct brw_context *brw,
>> void
>> intel_miptree_all_slices_resolve_color(struct brw_context *brw,
>> struct intel_mipmap_tree *mt,
>> - int flags)
>> + enum intel_resolve_hint hint)
>> {
>> - if (!intel_miptree_needs_color_resolve(brw, mt, flags))
>> + if (!intel_miptree_needs_color_resolve(brw, mt, hint))
>> return;
>>
>> foreach_list_typed_safe(struct intel_resolve_map, map, link,
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> index 1f1166a12b..d5f2689edd 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>> @@ -946,21 +946,24 @@ intel_miptree_used_for_rendering(const struct brw_context *brw,
>> * Flag values telling color resolve pass which special types of buffers
>> * can be ignored.
>> *
>> - * INTEL_MIPTREE_IGNORE_CCS_E: Lossless compressed (single-sample
>> - * compression scheme since gen9)
>> + * INTEL_RESOLVE_HINT_IGNORE_CCS_E: Lossless compressed (single-sample
>> + * compression scheme since gen9)
>> */
>> -#define INTEL_MIPTREE_IGNORE_CCS_E (1 << 0)
>> +enum intel_resolve_hint {
>> + INTEL_RESOLVE_HINT_NO_HINT = 0,
>> + INTEL_RESOLVE_HINT_IGNORE_CCS_E
>> +};
>>
>> bool
>> intel_miptree_resolve_color(struct brw_context *brw,
>> struct intel_mipmap_tree *mt, unsigned level,
>> unsigned start_layer, unsigned num_layers,
>> - int flags);
>> + enum intel_resolve_hint);
>>
>> void
>> intel_miptree_all_slices_resolve_color(struct brw_context *brw,
>> struct intel_mipmap_tree *mt,
>> - int flags);
>> + enum intel_resolve_hint hint);
>>
>> void
>> intel_miptree_make_shareable(struct brw_context *brw,
>> --
>> 2.11.0
>>
More information about the mesa-dev
mailing list