On 24.03.2017 13:11, Julien Isorce wrote:
1: Ack, thx. 2: u_upload_alloc can fail, i.e. set output param *ptr to NULL, for 2 reasons: alloc fails or map fails. For both there are already a fprintf/stderr, i.e., in radeon_create_bo and radeon_bo_do_map . It looks to me that in src/gallium/drivers/ it is a common usage to just avoid to crash by doing a silent check. But it defers the fprintf to where the error comes from, i.e. libdrm calls.
Okay, that makes sense.
Also I think that if drmCommandWriteRead returns -ENOMEM it should be translated to _mesa_error_no_memory(__func__); at some point. But it seems it is not possible to use _mesa_error_no_memory in mesa/src/gallium/winsys (though it is possible from gallium/state_trackers/glx). And it would require to be in the gl context's thread to call it.
Unfortunately, that doesn't work because the driver could be called from another state tracker (video APIs, nine, opencl).
Although, we could add a way for the state tracker to register an out-of-memory callback function, similar to the debug callback.
Also, I remember thinking at one point that in si_state_draw.c, it might be helpful to just flag out-of-memory conditions in a context-wide bool which is checked once before the actual draw commands are written. This would reduce the need to bubble up error conditions.
Cheers, Nicolai
Regards Julien On 24 March 2017 at 11:19, Nicolai Hähnle <nhaeh...@gmail.com <mailto:nhaeh...@gmail.com>> wrote: I generally like the patches in the series, thanks for that. Two points though: 1. The order of patches in series is usually general code before driver code, i.e. the st/cb_bitmap should come first. 2. I don't like having silent errors, as that could be confusing. In places where the error isn't propagated to the application (as it really should be...), I think we should have an fprintf to stderr. Cheers, Nicolai On 24.03.2017 12:08, Julien Isorce wrote: Like done in si_state_draw.c::si_draw_vbo Signed-off-by: Julien Isorce <jiso...@oblong.com <mailto:jiso...@oblong.com>> --- src/gallium/drivers/r600/r600_state_common.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c index 6f8279f..cedeb74 100644 --- a/src/gallium/drivers/r600/r600_state_common.c +++ b/src/gallium/drivers/r600/r600_state_common.c @@ -1746,6 +1746,10 @@ static void r600_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info u_upload_alloc(ctx->stream_uploader, start, count * 2, 256, &out_offset, &out_buffer, &ptr); + if (unlikely(!ptr)) { + pipe_resource_reference(&ib.buffer, NULL); + return; + } util_shorten_ubyte_elts_to_userptr( &rctx->b.b, &ib, 0, 0, ib.offset + start, count, ptr); -- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte.
-- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
