[Mesa-dev] [PATCH 5/8] i965/upload: Actually comment the upload code.
Kenneth Graunke
kenneth at whitecape.org
Thu Mar 13 01:57:13 PDT 2014
This code literally had zero comments, which made it impenetrable for
basically everybody except the authors.
We still ought to comment the refcounting strategy.
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
src/mesa/drivers/dri/i965/brw_context.h | 24 +++++++
src/mesa/drivers/dri/i965/intel_upload.c | 110 ++++++++++++++++++++++++++++++-
2 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index 734d273..659af92 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1048,10 +1048,34 @@ struct brw_context
bool no_batch_wrap;
struct {
+ /**
+ * A buffer object for holding miscellaneous data.
+ */
drm_intel_bo *bo;
+
+ /**
+ * Offset into "bo" for where to put the next piece of data.
+ *
+ * This is incremented when intel_upload_data or intel_upload_map
+ * ask for space to store data; the data may actually be staged and
+ * not appear in the BO until later.
+ */
GLuint offset;
+
+ /**
+ * The amount of staged data needing to be copied to the BO.
+ */
uint32_t buffer_len;
+
+ /**
+ * Offset into "bo" representing the starting destination address for
+ * the next batched copy.
+ */
uint32_t buffer_offset;
+
+ /**
+ * The CPU-side staging buffer containing data yet to be uploaded.
+ */
char buffer[4096];
} upload;
diff --git a/src/mesa/drivers/dri/i965/intel_upload.c b/src/mesa/drivers/dri/i965/intel_upload.c
index ec3109b..c767b7d 100644
--- a/src/mesa/drivers/dri/i965/intel_upload.c
+++ b/src/mesa/drivers/dri/i965/intel_upload.c
@@ -25,7 +25,16 @@
/**
* @file intel_upload.c
*
- * Batched upload via BOs.
+ * A batched data upload mechanism which can efficiently copy user supplied
+ * data into buffer objects.
+ *
+ * OpenGL allows applications to provide data in user arrays, which may
+ * disappear or change contents at any time. The GPU can't use this memory
+ * directly, so we need to copy it into buffer objects.
+ *
+ * Often times, the amount of data we need to copy is small. On non-LLC
+ * platforms, it's more efficient to stage this data in a CPU-sized buffer
+ * and only copy it to a BO when we have a larger amount of data.
*/
#include "main/imports.h"
@@ -43,6 +52,7 @@
#include "brw_context.h"
+/** The size of brw->upload.bo (the GPU buffer object). */
#define INTEL_UPLOAD_SIZE (64*1024)
/**
@@ -51,6 +61,15 @@
#define ALIGN_NPOT(value, alignment) \
(((value) + (alignment) - 1) / (alignment) * (alignment))
+/**
+ * Finish any outstanding uploads.
+ *
+ * We may have staged data that hasn't yet been copied to the BO; calling
+ * this function forces an immediate copy.
+ *
+ * This is necessary at certain points, such as submitting a batchbuffer
+ * that wants to actually use the data.
+ */
void
intel_upload_finish(struct brw_context *brw)
{
@@ -69,11 +88,22 @@ intel_upload_finish(struct brw_context *brw)
brw->upload.bo = NULL;
}
+/**
+ * Handle running out of space in the BO.
+ *
+ * Called when attempting to upload a new piece of data. If the existing BO
+ * doesn't have enough space, we need to finish uploading any existing staged
+ * data, and allocate a new BO.
+ */
static void
wrap_buffers(struct brw_context *brw, GLuint size)
{
intel_upload_finish(brw);
+ /* Use the larger of size and INTEL_UPLOAD_SIZE. This ensures that the
+ * buffer is large enough to upload any arbitrary piece of data, but will
+ * be reasonably sized even if we wrap when uploading a small bit of data.
+ */
if (size < INTEL_UPLOAD_SIZE)
size = INTEL_UPLOAD_SIZE;
@@ -81,6 +111,16 @@ wrap_buffers(struct brw_context *brw, GLuint size)
brw->upload.offset = 0;
}
+/**
+ * Upload some data (copy from CPU memory to a buffer object).
+ *
+ * @param[in] brw The i965 driver context.
+ * @param[in] ptr The source data to upload.
+ * @param[in] size The number of bytes to upload.
+ * @param[in] align Alignment requirement in bytes.
+ * @param[out] return_bo A pointer to the BO which will contain the data.
+ * @param[out] return_offset Offset into return_bo where the data will be.
+ */
void
intel_upload_data(struct brw_context *brw,
const void *ptr, GLuint size, GLuint align,
@@ -89,6 +129,7 @@ intel_upload_data(struct brw_context *brw,
{
GLuint base, delta;
+ /* Make sure the BO has enough space for this data. */
base = ALIGN_NPOT(brw->upload.offset, align);
if (brw->upload.bo == NULL || base + size > brw->upload.bo->size) {
wrap_buffers(brw, size);
@@ -96,10 +137,19 @@ intel_upload_data(struct brw_context *brw,
}
drm_intel_bo_reference(brw->upload.bo);
+
+ /* The new data will (eventually) end up in brw->upload.bo with an
+ * offset of "base". Return this so the caller can refer to the data.
+ */
*return_bo = brw->upload.bo;
*return_offset = base;
+ /* delta is the number of bytes needed for alignment. */
delta = base - brw->upload.offset;
+
+ /* If there isn't enough space in the staging buffer, go upload the
+ * staged data to free up space.
+ */
if (brw->upload.buffer_len &&
brw->upload.buffer_len + delta + size > sizeof(brw->upload.buffer)) {
drm_intel_bo_subdata(brw->upload.bo,
@@ -110,6 +160,9 @@ intel_upload_data(struct brw_context *brw,
}
if (size < sizeof(brw->upload.buffer)) {
+ /* The new data is small enough to fit in the staging buffer, so
+ * stage it to be copied to the BO later.
+ */
if (brw->upload.buffer_len == 0)
brw->upload.buffer_offset = base;
else
@@ -118,25 +171,53 @@ intel_upload_data(struct brw_context *brw,
memcpy(brw->upload.buffer + brw->upload.buffer_len, ptr, size);
brw->upload.buffer_len += size;
} else {
+ /* The new data is too large to fit in the (empty) staging buffer,
+ * which means it wouldn't benefit from batched uploading anyway.
+ *
+ * Just upload it directly.
+ */
drm_intel_bo_subdata(brw->upload.bo, base, size, ptr);
}
brw->upload.offset = base + size;
}
+/**
+ * Provide a pointer to a region of memory which will be uploaded.
+ *
+ * Sometimes callers want to rearrange the source data when uploading.
+ * While they could allocate their own temporary memory, rearrange the data,
+ * and then call intel_upload_data, this would result in double staging.
+ *
+ * Instead, using intel_upload_map reserves space in the staging buffer
+ * (or a temporary allocation if it isn't large enough), and allows direct
+ * access to that. This results in a single staging operation.
+ *
+ * @param brw The i965 driver context.
+ * @param size The number of bytes to be uploaded.
+ * @param align Alignment requirement in bytes.
+ *
+ * It is an (unchecked) error to mismatch calls to map/unmap.
+ */
void *
intel_upload_map(struct brw_context *brw, GLuint size, GLuint align)
{
GLuint base, delta;
char *ptr;
+ /* Make sure the BO has enough space for this data. */
base = ALIGN_NPOT(brw->upload.offset, align);
if (brw->upload.bo == NULL || base + size > brw->upload.bo->size) {
wrap_buffers(brw, size);
base = 0;
}
+ /* delta is the number of bytes needed for alignment. */
delta = base - brw->upload.offset;
+
+ /* If there isn't enough space in the staging buffer, go upload the
+ * staged data to free up space.
+ */
if (brw->upload.buffer_len &&
brw->upload.buffer_len + delta + size > sizeof(brw->upload.buffer)) {
drm_intel_bo_subdata(brw->upload.bo,
@@ -147,6 +228,9 @@ intel_upload_map(struct brw_context *brw, GLuint size, GLuint align)
}
if (size <= sizeof(brw->upload.buffer)) {
+ /* The new data is small enough to fit in the staging buffer, so
+ * stage it to be copied to the BO later.
+ */
if (brw->upload.buffer_len == 0)
brw->upload.buffer_offset = base;
else
@@ -155,12 +239,27 @@ intel_upload_map(struct brw_context *brw, GLuint size, GLuint align)
ptr = brw->upload.buffer + brw->upload.buffer_len;
brw->upload.buffer_len += size;
} else {
+ /* The new data is too large to fit into the default staging buffer,
+ * even when empty. Allocate a larger, one-shot staging buffer.
+ */
ptr = malloc(size);
}
return ptr;
}
+/**
+ * Complete the upload/staging operation started by intel_upload_map().
+ *
+ * @param[in] brw The i965 driver context.
+ * @param[in] ptr The pointer returned by intel_upload_map().
+ * @param[in] size Must match the value given to intel_upload_map().
+ * @param[in] align Must match the value given to intel_upload_map().
+ * @param[out] return_bo A pointer to the BO which will contain the data.
+ * @param[out] return_offset Offset into return_bo where the data will be.
+ *
+ * It is an (unchecked) error to mismatch calls to map/unmap.
+ */
void
intel_upload_unmap(struct brw_context *brw,
const void *ptr, GLuint size, GLuint align,
@@ -171,11 +270,20 @@ intel_upload_unmap(struct brw_context *brw,
base = ALIGN_NPOT(brw->upload.offset, align);
if (size > sizeof(brw->upload.buffer)) {
+ /* The data was too large to fit in the default staging buffer, so we
+ * allocated a one-shot temporary staging buffer in intel_upload_map().
+ *
+ * Immediately copy it to the BO and free the one-shot staging buffer.
+ */
drm_intel_bo_subdata(brw->upload.bo, base, size, ptr);
free((void*)ptr);
}
drm_intel_bo_reference(brw->upload.bo);
+
+ /* The new data will (eventually) end up in brw->upload.bo with an
+ * offset of "base". Return this so the caller can refer to the data.
+ */
*return_bo = brw->upload.bo;
*return_offset = base;
--
1.9.0
More information about the mesa-dev
mailing list