<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jan 22, 2018 at 12:17 AM, Pohjolainen, Topi <<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"><div class="HOEnZb"><div class="h5">On Fri, Jan 19, 2018 at 03:47:26PM -0800, Jason Ekstrand wrote: > This moves it to being based on layout_to_aux_usage instead of being > hard-coded based on bits of a priori knowledge of how transitions > interact with layouts. This conceptually simplifies things because > we're now using layout_to_aux_usage and layout_supports_fast_clear to > make resolve decisions so changes to those functions will do what one > expects. > > This fixes a potential bug with window system integration on gen9+ where > we wouldn't do a resolve when transitioning to the PRESENT_SRC layout > because we just assume that everything that handles CCS_E can handle it > all the time. When handing a CCS_E image off to the window system, we > may need to do a full resolve if the window system does not support the > CCS_E modifier. The only reason why this hasn't been a problem yet is > because we don't support modifiers in Vulkan WSI and so we always get X > tiling which implies no CCS on gen9+. > > v2 (Jason Ekstrand): > - Make a few more things const > - Use the anv_fast_clear_support enum > > Reviewed-by: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> > --- > src/intel/vulkan/genX_cmd_buffer.c | 56 ++++++++++++++++++++++++++++++-------- > 1 file changed, 44 insertions(+), 12 deletions(-) > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c > index 6a6d8b2..fd27463 100644 > --- a/src/intel/vulkan/genX_cmd_buffer.c > +++ b/src/intel/vulkan/genX_cmd_buffer.c > @@ -593,6 +593,7 @@ transition_color_buffer(struct anv_cmd_buffer *cmd_buffer, > VkImageLayout initial_layout, > VkImageLayout final_layout) > { > + const struct gen_device_info *devinfo = &cmd_buffer->device->info; > /* Validate the inputs. */ > assert(cmd_buffer); > assert(image && image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV); > @@ -733,17 +734,51 @@ transition_color_buffer(struct anv_cmd_buffer *cmd_buffer, > VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, > final_layout); > } > - } else if (initial_layout != VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL) { > - /* Resolves are only necessary if the subresource may contain blocks > - * fast-cleared to values unsupported in other layouts. This only occurs > - * if the initial layout is COLOR_ATTACHMENT_OPTIMAL. > - */ > - return; > - } else if (image->samples > 1) { > - /* MCS buffers don't need resolving. */ > return; > } > > + /* If initial aux usage is NONE, there is nothing to resolve */ > + const enum isl_aux_usage initial_aux_usage = > + anv_layout_to_aux_usage(devinfo, image, aspect, initial_layout); > + if (initial_aux_usage == ISL_AUX_USAGE_NONE) > + return; > + > + enum isl_aux_op resolve_op = ISL_AUX_OP_NONE; > + > + /* If the initial layout supports more fast clear than the final layout > + * then we need at least a partial resolve. > + */ > + const enum anv_fast_clear_type initial_fast_clear = > + anv_layout_to_fast_clear_type(devinfo, image, aspect, initial_layout); > + const enum anv_fast_clear_type final_fast_clear = > + anv_layout_to_fast_clear_type(devinfo, image, aspect, final_layout); > + if (final_fast_clear < initial_fast_clear) > + resolve_op = ISL_AUX_OP_PARTIAL_RESOLVE; > + > + const enum isl_aux_usage final_aux_usage = > + anv_layout_to_aux_usage(devinfo, image, aspect, final_layout); > + if (initial_aux_usage == ISL_AUX_USAGE_CCS_E && > + final_aux_usage != ISL_AUX_USAGE_CCS_E) > + resolve_op = ISL_AUX_OP_FULL_RESOLVE; > + > + /* CCS_D only supports full resolves and BLORP will assert on us if we try > + * to do a partial resolve on a CCS_D surface. > + */ > + if (resolve_op == ISL_AUX_OP_PARTIAL_RESOLVE && > + initial_aux_usage == ISL_AUX_USAGE_CCS_D) > + resolve_op = ISL_AUX_OP_FULL_RESOLVE; > + > + if (resolve_op == ISL_AUX_OP_NONE) > + return; > + > + /* Even though the above code can theoretically handle multiple resolve > + * types such as CCS_D -> CCS_E, the predication code below can't. We only > + * really handle a couple of cases. > + */ > + assert(initial_aux_usage == ISL_AUX_USAGE_NONE || </div></div>Earlier there is explicit early return for "initial_aux_usage == ISL_AUX_USAGE_NONE". Just checking if you really meant to assert it here. </blockquote><div> </div><div>I don't know. I'm trying to make the point with the assert of what the code can handle. I can't do this assert any earlier because we don't have final_aux_usage until we get here. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > + final_aux_usage == ISL_AUX_USAGE_NONE || > + initial_aux_usage == final_aux_usage); > + > /* Perform a resolve to synchronize data between the main and aux buffer. > * Before we begin, we must satisfy the cache flushing requirement specified > * in the Sky Lake PRM Vol. 7, "MCS Buffer for Render Target(s)": > @@ -774,10 +809,7 @@ transition_color_buffer(struct anv_cmd_buffer *cmd_buffer, > genX(load_needs_resolve_predicate)(cmd_buffer, image, aspect, level); > > anv_image_ccs_op(cmd_buffer, image, aspect, level, > - base_layer, layer_count, > - image->planes[plane].aux_usage == ISL_AUX_USAGE_CCS_E ? > - ISL_AUX_OP_PARTIAL_RESOLVE : ISL_AUX_OP_FULL_RESOLVE, > - true); > + base_layer, layer_count, resolve_op, true); > > genX(set_image_needs_resolve)(cmd_buffer, image, aspect, level, false); > } > -- > 2.5.0.400.gff86faf > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">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>