<div dir="ltr"><div>I read through the series and, while I think it will fix the issue in 90% of cases, I don't think it's quite the right solution. There's a *lot* of subtlety here and we need to tread carefully. I think the better thing to do would be to whack the new flag in two places: Whenever we change the fast clear value (brw_blorp.c and brw_clear.c for depth) and whenever intel_miptree_set_aux_state actually changes something. There are several things which look at aux_state and it would be better to flag on that changing just to make sure we get them all. </div>--Jason <div> </div></div><div class="gmail_extra"> <div class="gmail_quote">On Thu, Sep 14, 2017 at 3:54 AM, Iago Toral Quiroga <<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Jason, Ken: this is what I came up based on your findings that Ken reported in <a href="https://bugs.freedesktop.org/show_bug.cgi?id=102611" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=102611</a>. Ken mentioned that he is not completely certain that we need to flag dirty state every time we update the surfaces and that maybe flagging only when we go from/to AUX_USAGE_NONE might suffice. I am not sure, but to me it sounds safer to flag when we create the surfaces, since at that point we know we are going to use them and in that case we need the new surface states emitted, at least until we find specific cases where we don't need this and we can drop the flag for them. Let me know if you think a different approach is better though. The last two patches should probably be squashed. The last patch signals dirty AUX state when we drop aux surfaces. I think this is in necessary too since when we upload new renderbuffers we also consider the case where we don't have aux, but I decided to split it because we have not been signaling anything for dropped aux surfaces before. Iago Toral Quiroga (3): i965: rename BRW_NEW_FAST_CLEAR_COLOR to BRW_NEW_AUX_STATE i965: emit BRW_NEW_AUX_STATE when we allocate aux surfaces i965: flag BRW_NEW_AUX_STATE if we drop the aux buffer src/mesa/drivers/dri/i965/brw_blorp.c | 2 +- src/mesa/drivers/dri/i965/brw_context.h | 4 ++-- src/mesa/drivers/dri/i965/brw_gs_surface_state.c | 2 +- src/mesa/drivers/dri/i965/brw_state_upload.c | 2 +- src/mesa/drivers/dri/i965/brw_tcs_surface_state.c | 2 +- src/mesa/drivers/dri/i965/brw_tes_surface_state.c | 2 +- src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 2 +- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 ++++++------ src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 +++++++ 9 files changed, 21 insertions(+), 14 deletions(-) -- 2.11.0 _______________________________________________ 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>