[Mesa-dev] [PATCH 1/2] i965: Rip out system memory backing for buffer objects; just use BOs.
Kenneth Graunke
kenneth at whitecape.org
Mon Jul 9 11:18:44 PDT 2012
In one workload, 45% of CPU time was spent in pwrite(), primarily
uploading large amounts of VBO data. This was in fact the hottest path
in the application, according to sysprof. Removing the system memory
buffer and just storing VBOs in actual BOs cut the application's CPU
usage by about half, and moved pwrite down to about 2.5% of CPU time.
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
src/mesa/drivers/dri/intel/intel_buffer_objects.c | 117 +--------------------
src/mesa/drivers/dri/intel/intel_buffer_objects.h | 3 -
2 files changed, 4 insertions(+), 116 deletions(-)
Here's my version of the same patch. It's quite a bit more invasive,
but it deletes all the code made dead by the change in Eric's patch.
diff --git a/src/mesa/drivers/dri/intel/intel_buffer_objects.c b/src/mesa/drivers/dri/intel/intel_buffer_objects.c
index d2a0709..200fce3 100644
--- a/src/mesa/drivers/dri/intel/intel_buffer_objects.c
+++ b/src/mesa/drivers/dri/intel/intel_buffer_objects.c
@@ -97,8 +97,6 @@ intel_bufferobj_free(struct gl_context * ctx, struct gl_buffer_object *obj)
if (obj->Pointer)
intel_bufferobj_unmap(ctx, obj);
- free(intel_obj->sys_buffer);
-
drm_intel_bo_unreference(intel_obj->buffer);
free(intel_obj);
}
@@ -136,26 +134,7 @@ intel_bufferobj_data(struct gl_context * ctx,
if (intel_obj->buffer != NULL)
release_buffer(intel_obj);
- free(intel_obj->sys_buffer);
- intel_obj->sys_buffer = NULL;
-
if (size != 0) {
- if (usage == GL_DYNAMIC_DRAW
-#ifdef I915
- /* On pre-965, stick VBOs in system memory, as we're always doing
- * swtnl with their contents anyway.
- */
- || target == GL_ARRAY_BUFFER || target == GL_ELEMENT_ARRAY_BUFFER
-#endif
- )
- {
- intel_obj->sys_buffer = malloc(size);
- if (intel_obj->sys_buffer != NULL) {
- if (data != NULL)
- memcpy(intel_obj->sys_buffer, data, size);
- return true;
- }
- }
intel_bufferobj_alloc_buffer(intel, intel_obj);
if (!intel_obj->buffer)
return false;
@@ -189,20 +168,6 @@ intel_bufferobj_subdata(struct gl_context * ctx,
assert(intel_obj);
- /* If we have a single copy in system memory, update that */
- if (intel_obj->sys_buffer) {
- if (intel_obj->source)
- release_buffer(intel_obj);
-
- if (intel_obj->buffer == NULL) {
- memcpy((char *)intel_obj->sys_buffer + offset, data, size);
- return;
- }
-
- free(intel_obj->sys_buffer);
- intel_obj->sys_buffer = NULL;
- }
-
/* Otherwise we need to update the copy in video memory. */
busy =
drm_intel_bo_busy(intel_obj->buffer) ||
@@ -253,14 +218,10 @@ intel_bufferobj_get_subdata(struct gl_context * ctx,
struct intel_context *intel = intel_context(ctx);
assert(intel_obj);
- if (intel_obj->sys_buffer)
- memcpy(data, (char *)intel_obj->sys_buffer + offset, size);
- else {
- if (drm_intel_bo_references(intel->batch.bo, intel_obj->buffer)) {
- intel_batchbuffer_flush(intel);
- }
- drm_intel_bo_get_subdata(intel_obj->buffer, offset, size, data);
+ if (drm_intel_bo_references(intel->batch.bo, intel_obj->buffer)) {
+ intel_batchbuffer_flush(intel);
}
+ drm_intel_bo_get_subdata(intel_obj->buffer, offset, size, data);
}
@@ -298,22 +259,6 @@ intel_bufferobj_map_range(struct gl_context * ctx,
obj->Length = length;
obj->AccessFlags = access;
- if (intel_obj->sys_buffer) {
- const bool read_only =
- (access & (GL_MAP_READ_BIT | GL_MAP_WRITE_BIT)) == GL_MAP_READ_BIT;
-
- if (!read_only && intel_obj->source)
- release_buffer(intel_obj);
-
- if (!intel_obj->buffer || intel_obj->source) {
- obj->Pointer = intel_obj->sys_buffer + offset;
- return obj->Pointer;
- }
-
- free(intel_obj->sys_buffer);
- intel_obj->sys_buffer = NULL;
- }
-
if (intel_obj->buffer == NULL) {
obj->Pointer = NULL;
return NULL;
@@ -426,9 +371,7 @@ intel_bufferobj_unmap(struct gl_context * ctx, struct gl_buffer_object *obj)
assert(intel_obj);
assert(obj->Pointer);
- if (intel_obj->sys_buffer != NULL) {
- /* always keep the mapping around. */
- } else if (intel_obj->range_map_buffer != NULL) {
+ if (intel_obj->range_map_buffer != NULL) {
/* Since we've emitted some blits to buffers that will (likely) be used
* in rendering operations in other cache domains in this batch, emit a
* flush. Once again, we wish for a domain tracker in libdrm to cover
@@ -472,17 +415,6 @@ intel_bufferobj_buffer(struct intel_context *intel,
if (intel_obj->source)
release_buffer(intel_obj);
- if (intel_obj->buffer == NULL) {
- intel_bufferobj_alloc_buffer(intel, intel_obj);
- drm_intel_bo_subdata(intel_obj->buffer,
- 0, intel_obj->Base.Size,
- intel_obj->sys_buffer);
-
- free(intel_obj->sys_buffer);
- intel_obj->sys_buffer = NULL;
- intel_obj->offset = 0;
- }
-
return intel_obj->buffer;
}
@@ -624,13 +556,6 @@ intel_bufferobj_source(struct intel_context *intel,
struct intel_buffer_object *intel_obj,
GLuint align, GLuint *offset)
{
- if (intel_obj->buffer == NULL) {
- intel_upload_data(intel,
- intel_obj->sys_buffer, intel_obj->Base.Size, align,
- &intel_obj->buffer, &intel_obj->offset);
- intel_obj->source = 1;
- }
-
*offset = intel_obj->offset;
return intel_obj->buffer;
}
@@ -651,35 +576,6 @@ intel_bufferobj_copy_subdata(struct gl_context *ctx,
if (size == 0)
return;
- /* If we're in system memory, just map and memcpy. */
- if (intel_src->sys_buffer || intel_dst->sys_buffer) {
- /* The same buffer may be used, but note that regions copied may
- * not overlap.
- */
- if (src == dst) {
- char *ptr = intel_bufferobj_map_range(ctx, 0, dst->Size,
- GL_MAP_READ_BIT |
- GL_MAP_WRITE_BIT,
- dst);
- memmove(ptr + write_offset, ptr + read_offset, size);
- intel_bufferobj_unmap(ctx, dst);
- } else {
- const char *src_ptr;
- char *dst_ptr;
-
- src_ptr = intel_bufferobj_map_range(ctx, 0, src->Size,
- GL_MAP_READ_BIT, src);
- dst_ptr = intel_bufferobj_map_range(ctx, 0, dst->Size,
- GL_MAP_WRITE_BIT, dst);
-
- memcpy(dst_ptr + write_offset, src_ptr + read_offset, size);
-
- intel_bufferobj_unmap(ctx, src);
- intel_bufferobj_unmap(ctx, dst);
- }
- return;
- }
-
/* Otherwise, we have real BOs, so blit them. */
dst_bo = intel_bufferobj_buffer(intel, intel_dst, INTEL_WRITE_PART);
@@ -720,11 +616,6 @@ intel_buffer_object_purgeable(struct gl_context * ctx,
return intel_buffer_purgeable(intel_obj->buffer);
if (option == GL_RELEASED_APPLE) {
- if (intel_obj->sys_buffer != NULL) {
- free(intel_obj->sys_buffer);
- intel_obj->sys_buffer = NULL;
- }
-
return GL_RELEASED_APPLE;
} else {
/* XXX Create the buffer and madvise(MADV_DONTNEED)? */
diff --git a/src/mesa/drivers/dri/intel/intel_buffer_objects.h b/src/mesa/drivers/dri/intel/intel_buffer_objects.h
index 92a4121..11be027 100644
--- a/src/mesa/drivers/dri/intel/intel_buffer_objects.h
+++ b/src/mesa/drivers/dri/intel/intel_buffer_objects.h
@@ -43,9 +43,6 @@ struct intel_buffer_object
drm_intel_bo *buffer; /* the low-level buffer manager's buffer handle */
GLuint offset; /* any offset into that buffer */
- /** System memory buffer data, if not using a BO to store the data. */
- void *sys_buffer;
-
drm_intel_bo *range_map_bo;
void *range_map_buffer;
unsigned int range_map_offset;
--
1.7.10.4
More information about the mesa-dev
mailing list