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.
--Jason On Thu, Sep 14, 2017 at 3:54 AM, Iago Toral Quiroga <ito...@igalia.com> wrote: > Jason, Ken: this is what I came up based on your findings that Ken > reported in https://bugs.freedesktop.org/show_bug.cgi?id=102611. > > 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 > 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
