On 12/24/2013 03:47 PM, Eric Anholt wrote:
> We definitely want to fall through to the unsynchronized map case, instead
> of wasting bandwidth on a copy. Prevents a -43.2407% +/- 1.06113% (n=49)
> performance regression on aa10perf when teaching glamor to provide the
> GL_INVALIDATE_RANGE_BIT information.
>
> This is a performance fix, which I usually wouldn't cherry-pick to stable.
> But this was really was just a bug in the code, its presence would
> discourage developers from giving us the best information they can, and I
> think we've got fairly high confidence in the unsynchronized map path
> already.
>
> Cc: 10.0 9.2 <mesa-sta...@lists.freedesktop.org>
> ---
> src/mesa/drivers/dri/i965/intel_buffer_objects.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Yep, obvious improvement. I would've sent this out earlier, but never
found an application affected by it. Glad you found one.
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
I won't object to it landing on stable...it seems like an obvious small
fix. Though, I suppose it could expose bugs in broken applications.
Seems unlikely though, so it should be fine.
A few comments about potential follow-ups...
> diff --git a/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> b/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> index cab805a..84bc29d 100644
> --- a/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> +++ b/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> @@ -357,7 +357,8 @@ intel_bufferobj_map_range(struct gl_context * ctx,
> * BO, and we'll copy what they put in there out at unmap or
> * FlushRange time.
> */
> - if ((access & GL_MAP_INVALIDATE_RANGE_BIT) &&
> + if (!(access & GL_MAP_UNSYNCHRONIZED_BIT) &&
> + (access & GL_MAP_INVALIDATE_RANGE_BIT) &&
> drm_intel_bo_busy(intel_obj->buffer)) {
> if (access & GL_MAP_FLUSH_EXPLICIT_BIT) {
> intel_obj->range_map_buffer = malloc(length);
>
This function is getting pretty awful to read.
The INVALIDATE_RANGE block here looks fairly redundant with the
INVALIDATE_BUFFER block, above. Both call drm_intel_bo_busy, which
isn't that cheap. It seems like the two blocks could be
combined...check references, check busy, either chuck the whole buffer
or arrange for a subrange blit BO...
If you don't have time, I can always try and give it a shot.
--Ken
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
