<div class="gmail_quote">On Mon, Feb 14, 2011 at 6:47 PM, José Fonseca <<a href="mailto:jfonseca@vmware.com">jfonseca@vmware.com</a>> wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> <div class="im">On Sun, 2011-02-13 at 23:58 -0800, Dave Airlie wrote: > >> if(buf->base.base.size < size) > >> return 0; > >> > >> @@ -242,13 +240,10 @@ pb_cache_is_buffer_compat(struct pb_cache_buffer > >> *buf, > >> if(!pb_check_usage(desc->usage, buf->base.base.usage)) > >> return 0; > >> > >> - map = pb_map(buf->buffer, PB_USAGE_DONTBLOCK, NULL); > >> - if (!map) { > >> - return -1; > >> - } > >> + if (buf->mgr->base.is_buffer_busy) > >> + if (buf->mgr->base.is_buffer_busy(&buf->mgr->base, buf->buffer)) > >> + return -1; > > > > Oops, this is wrong. I will locally replace any occurences of > > "buf->mgr->base(.)" with "buf->mgr->provider(->)", which is how it was meant > > to be, but the idea remains the same. Please review. </div>Marek, I don't understand what you want to do here: you removed the pb_map, but you left the pb_unmap, and what will happen if is_buffer_busy is not defined? </blockquote><div> I didn't leave the pb_unmap call, it was removed too, I just cut it off in my second email, since it wasn't relevant to the typo. Sorry about that. So there's only one way: is_buffer_busy. </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> <div class="im"> > > I actually suggested this originally, but Jose I think preferred using > the dontblock to the buffer mapping. </div>I'd prefer that there is one way of doing this, but I didn't/don't feel strong about this. IMO, having two ways, PB_USAGE_DONTBLOCK and is_buffer_busy, is not cleaner that just PB_USAGE_DONTBLOCK, even if is_buffer_busy is conceptually cleaner. </blockquote><div> The thing is mapping a buffer just to know whether it's being used is unnecessary, and the mapping itself may be slower than a simple is_busy query. </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> Marek, Would adding an inline function, pb_is_buffer_busy, that calls pb_map(PB_USAGE_DONTBLOCK)+pb_unmap inside work for you? Another way, would be to add is_buffer_busy and have the default implementation to do pb_map/pb_unmap. </blockquote><div> I can add a piece of code that uses pb_map/pb_unmap if the is_buffer_busy hook is not set, so that the original behavior is preserved. Would that be ok with you? Here's a new patch: pb_bufmgr_cache: add is_buffer_busy hook and use it instead of non-blocking map This is cleaner and implementing the hook is optional. diff --git a/src/gallium/auxiliary/pipebuffer/pb_bufmgr.h b/src/gallium/auxiliary/pipebuffer/pb_bufm index 2ef0216..960068c 100644 --- a/src/gallium/auxiliary/pipebuffer/pb_bufmgr.h +++ b/src/gallium/auxiliary/pipebuffer/pb_bufmgr.h @@ -82,6 +82,10 @@ struct pb_manager */ void (*flush)( struct pb_manager *mgr ); + + boolean + (*is_buffer_busy)( struct pb_manager *mgr, + struct pb_buffer *buf ); }; diff --git a/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c b/src/gallium/auxiliary/pipebuffer/p index a6eb403..25accef 100644 --- a/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c +++ b/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c @@ -227,8 +227,6 @@ pb_cache_is_buffer_compat(struct pb_cache_buffer *buf, pb_size size, const struct pb_desc *desc) { - void *map; - if(buf->base.base.size < size) return 0; @@ -242,13 +240,18 @@ pb_cache_is_buffer_compat(struct pb_cache_buffer *buf, if(!pb_check_usage(desc->usage, buf->base.base.usage)) return 0; - map = pb_map(buf->buffer, PB_USAGE_DONTBLOCK, NULL); - if (!map) { - return -1; + if (buf->mgr->provider->is_buffer_busy) { + if (buf->mgr->provider->is_buffer_busy(buf->mgr->provider, buf->buffer)) + return -1; + } else { + void *ptr = pb_map(buf->buffer, PB_USAGE_DONTBLOCK, NULL); + + if (!ptr) + return -1; + + pb_unmap(buf->buffer); } - pb_unmap(buf->buffer); - return 1; } Marek </div></div>