On Wed, 2017-03-15 at 10:47 +0200, Pohjolainen, Topi wrote: > On Tue, Mar 14, 2017 at 02:15:51PM +0100, Iago Toral wrote: > > > > BTW, in case it is useful, I have the series available here, > > including > > fixes to review feedback obtained so far: > > > > https://github.com/Igalia/mesa/tree/vk-oom > I recorded comments in patches 2, 3, 4, 5, 7, 9, 10, 21 and 22. In > case of 4 > the question was if we should squash it with 14 - I don't have strong > preference, either way is fine. And I think we agree on the difficult > bits. > Patch 10 I'd like to see how it looks revised, the rest should be > trivial > enough to fix locally.
Thanks a lot for all the review effort Topi, the version with your feedback is more thorough and robust. I have just sent a v3 of patch 10 for review. If you want to have a look at the final version of any of the other patches you can use this branch from my github repo: https://github.com/Igalia/mesa/tree/vk-oom Just a couple of things worth pointing out: 1. I made the change where we make the upload_shader() hook return a bool a separate commit (that is patch #2). 2. Ignore the patch "anv: ensure that we don't ever try to adjust relocations more than once" (which is not reviewed), that should not be pushed as the specs for that are under discussion, I only kept it in the branch because the tests under development by Khronos currently require this. Iago > Otherwise the series is: > > Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com> > > > > > > > Iago > > > > On Fri, 2017-03-10 at 13:38 +0100, Iago Toral Quiroga wrote: > > > > > > I think this series addresses all the issues raised for v1, > > > except > > > making similar changes to anv_cmd_buffer_alloc_dynamic_state and > > > anv_cmd_buffer_alloc_blorp_binding_table which Jason suggested we > > > should > > > probably do as well, and that I'd like to cover separately from > > > this. > > > The main change from v1 is that we now track errors in anv_batch, > > > which > > > allows us to track and report errors that occur during batch > > > emissions as > > > well. > > > > > > As discussed during v1, I have dropped the patch > > > "anv: ensure that we don't ever try to adjust relocations more > > > than > > > once", since > > > Jason pointed out that the issue this was trying to address was > > > under > > > discussion > > > in Khronos. I have also pointed that discussion to the test > > > developers so that > > > the tests are updated accordingly when a final decision is made. > > > > > > Besides addressing review feedback, this series incorporates a > > > bunch > > > of > > > additional improvements including better reporting of errors for > > > pipelines > > > (we can do this now because we track out of memory errors in > > > batches) > > > and a few other fixes, mostly crash guards that I had missed > > > before > > > and > > > a few other minor things. > > > > > > I have also changed a bit the order of some patches in the series > > > so > > > related fixes are closer to each other. > > > > > > Iago Toral Quiroga (24): > > > anv: remove unnecessary function prototype. > > > anv: do not try to ref/unref NULL shaders > > > anv/blorp: return early if we failed to create the shader > > > binary > > > anv/cmd_buffer: report errors in vkBeginCommandBuffer() > > > anv/cmd_buffer: add a status field to anv_batch > > > anv: add anv_batch_set_error() and anv_batch_has_error() > > > helpers > > > anv: handle allocation failure in anv_batch_emit_batch() > > > anv: handle allocation failure in anv_batch_emit_dwords() > > > anv: avoid crashes when failing to allocate batches > > > anv: handle failures when growing reloc lists > > > anv/cmd_buffer: report tracked errors in vkEndCommandBuffer() > > > anv/cmd_buffer: skip vkCmdNextSubpass() for broken command > > > buffers > > > anv/cmd_buffer: skip vkCmdEndRenderPass() for broken command > > > buffers > > > anv/cmd_buffer: handle allocation errors during > > > vkCmdBeginRenderPass() > > > anv/cmd_buffer: handle out of memory during vkCmdPushConstants > > > anv: handle memory allocation errors during queue submissions > > > anv/cmd_buffer: skip vkCmdDraw*() on broken command buffers > > > anv/cmd_buffer: skip vkCmdDispatch() on broken command buffers > > > anv/cmd_buffer: skip vkCmdExecuteCommands() on broken command > > > buffers > > > anv/device: assert that commands submitted to a queue are not > > > bogus > > > anv/blorp: make anv_cmd_buffer_alloc_blorp_binding_table() > > > return a > > > VkResult > > > anv: handle errors while allocating new binding table blocks > > > anv: handle errors in emit_binding_table() and emit_samplers() > > > anv: improve error reporting when creating pipelines > > > > > > src/intel/blorp/blorp_genX_exec.h | 25 +++++--- > > > src/intel/vulkan/anv_batch_chain.c | 62 +++++++++++++------ > > > src/intel/vulkan/anv_blorp.c | 38 +++++++----- > > > src/intel/vulkan/anv_cmd_buffer.c | 19 +++++- > > > src/intel/vulkan/anv_device.c | 1 + > > > src/intel/vulkan/anv_pipeline.c | 1 + > > > src/intel/vulkan/anv_pipeline_cache.c | 5 +- > > > src/intel/vulkan/anv_private.h | 56 +++++++++++++---- > > > src/intel/vulkan/genX_blorp_exec.c | 18 +++--- > > > src/intel/vulkan/genX_cmd_buffer.c | 113 > > > +++++++++++++++++++++++++--------- > > > src/intel/vulkan/genX_pipeline.c | 9 ++- > > > 11 files changed, 253 insertions(+), 94 deletions(-) > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
