Re: [Mesa-dev] [PATCH] svga: refactor draw_vgpu10() function
Looks good to me.
Just one nit pick,
In definition of validate_index_buffer, extra space is required in argument
list.
Reviewed-by: Neha Bhende
Regards,
Neha
Regards,
Neha
From: Brian Paul
Sent: Thursday, March 7, 2019 3:23 PM
To: mesa-dev@lists.freedesktop.org
Cc: Neha Bhende
Subject: [PATCH] svga: refactor draw_vgpu10() function
The draw_vgpu10() function was huge. Move the code for preparing the
vertex buffers and the index buffer into separate functions.
---
src/gallium/drivers/svga/svga_draw.c | 246 ---
1 file changed, 141 insertions(+), 105 deletions(-)
diff --git a/src/gallium/drivers/svga/svga_draw.c
b/src/gallium/drivers/svga/svga_draw.c
index 649bc22..a358170 100644
--- a/src/gallium/drivers/svga/svga_draw.c
+++ b/src/gallium/drivers/svga/svga_draw.c
@@ -507,62 +507,25 @@ vertex_buffers_equal(unsigned count,
}
+/*
+ * Prepare the vertex buffers for a drawing command.
+ */
static enum pipe_error
-draw_vgpu10(struct svga_hwtnl *hwtnl,
-const SVGA3dPrimitiveRange *range,
-unsigned vcount,
-struct pipe_resource *ib,
-unsigned start_instance, unsigned instance_count)
+validate_vertex_buffers(struct svga_hwtnl *hwtnl)
{
struct svga_context *svga = hwtnl->svga;
struct pipe_resource *vbuffers[SVGA3D_INPUTREG_MAX];
struct svga_winsys_surface *vbuffer_handles[SVGA3D_INPUTREG_MAX];
- struct svga_winsys_surface *ib_handle;
const unsigned vbuf_count = hwtnl->cmd.vbuf_count;
int last_vbuf = -1;
- enum pipe_error ret;
unsigned i;
assert(svga_have_vgpu10(svga));
- assert(hwtnl->cmd.prim_count == 0);
-
- /* We need to reemit all the current resource bindings along with the Draw
-* command to be sure that the referenced resources are available for the
-* Draw command, just in case the surfaces associated with the resources
-* are paged out.
-*/
- if (svga->rebind.val) {
- ret = svga_rebind_framebuffer_bindings(svga);
- if (ret != PIPE_OK)
- return ret;
-
- ret = svga_rebind_shaders(svga);
- if (ret != PIPE_OK)
- return ret;
-
- /* Rebind stream output targets */
- ret = svga_rebind_stream_output_targets(svga);
- if (ret != PIPE_OK)
- return ret;
-
- /* No need to explicitly rebind index buffer and vertex buffers here.
- * Even if the same index buffer or vertex buffers are referenced for
this
- * draw and we skip emitting the redundant set command, we will still
- * reference the associated resources.
- */
- }
-
- ret = validate_sampler_resources(svga);
- if (ret != PIPE_OK)
- return ret;
-
- ret = validate_constant_buffers(svga);
- if (ret != PIPE_OK)
- return ret;
/* Get handle for each referenced vertex buffer */
for (i = 0; i < vbuf_count; i++) {
- struct svga_buffer *sbuf =
svga_buffer(hwtnl->cmd.vbufs[i].buffer.resource);
+ struct svga_buffer *sbuf =
+ svga_buffer(hwtnl->cmd.vbufs[i].buffer.resource);
if (sbuf) {
vbuffer_handles[i] = svga_buffer_handle(svga, >b.b,
@@ -584,25 +547,11 @@ draw_vgpu10(struct svga_hwtnl *hwtnl,
vbuffer_handles[i] = NULL;
}
- /* Get handle for the index buffer */
- if (ib) {
- struct svga_buffer *sbuf = svga_buffer(ib);
-
- ib_handle = svga_buffer_handle(svga, ib, PIPE_BIND_INDEX_BUFFER);
- if (!ib_handle)
- return PIPE_ERROR_OUT_OF_MEMORY;
-
- assert(sbuf->key.flags & SVGA3D_SURFACE_BIND_INDEX_BUFFER);
- (void) sbuf; /* silence unused var warning */
- }
- else {
- ib_handle = NULL;
- }
-
/* setup vertex attribute input layout */
if (svga->state.hw_draw.layout_id != hwtnl->cmd.vdecl_layout_id) {
- ret = SVGA3D_vgpu10_SetInputLayout(svga->swc,
- hwtnl->cmd.vdecl_layout_id);
+ enum pipe_error ret =
+ SVGA3D_vgpu10_SetInputLayout(svga->swc,
+ hwtnl->cmd.vdecl_layout_id);
if (ret != PIPE_OK)
return ret;
@@ -658,14 +607,13 @@ draw_vgpu10(struct svga_hwtnl *hwtnl,
* corresponding entries in the device's vertex buffer list.
*/
for (i = 0; i < num_vbuffers; i++) {
- boolean emit;
-
- emit = vertex_buffers_equal(1,
- _attrs[i],
- [i],
-
>state.hw_draw.vbuffer_attrs[i],
- >state.hw_draw.vbuffers[i]);
-
+ boolean emit =
+ vertex_buffers_equal(1,
+ _attrs[i],
+ [i],
+ >state.hw_draw.vbuffer_attrs[i],
+ >state.hw_draw.vbuffers[i]);
+
if
[Mesa-dev] [PATCH] svga: refactor draw_vgpu10() function
The draw_vgpu10() function was huge. Move the code for preparing the
vertex buffers and the index buffer into separate functions.
---
src/gallium/drivers/svga/svga_draw.c | 246 ---
1 file changed, 141 insertions(+), 105 deletions(-)
diff --git a/src/gallium/drivers/svga/svga_draw.c
b/src/gallium/drivers/svga/svga_draw.c
index 649bc22..a358170 100644
--- a/src/gallium/drivers/svga/svga_draw.c
+++ b/src/gallium/drivers/svga/svga_draw.c
@@ -507,62 +507,25 @@ vertex_buffers_equal(unsigned count,
}
+/*
+ * Prepare the vertex buffers for a drawing command.
+ */
static enum pipe_error
-draw_vgpu10(struct svga_hwtnl *hwtnl,
-const SVGA3dPrimitiveRange *range,
-unsigned vcount,
-struct pipe_resource *ib,
-unsigned start_instance, unsigned instance_count)
+validate_vertex_buffers(struct svga_hwtnl *hwtnl)
{
struct svga_context *svga = hwtnl->svga;
struct pipe_resource *vbuffers[SVGA3D_INPUTREG_MAX];
struct svga_winsys_surface *vbuffer_handles[SVGA3D_INPUTREG_MAX];
- struct svga_winsys_surface *ib_handle;
const unsigned vbuf_count = hwtnl->cmd.vbuf_count;
int last_vbuf = -1;
- enum pipe_error ret;
unsigned i;
assert(svga_have_vgpu10(svga));
- assert(hwtnl->cmd.prim_count == 0);
-
- /* We need to reemit all the current resource bindings along with the Draw
-* command to be sure that the referenced resources are available for the
-* Draw command, just in case the surfaces associated with the resources
-* are paged out.
-*/
- if (svga->rebind.val) {
- ret = svga_rebind_framebuffer_bindings(svga);
- if (ret != PIPE_OK)
- return ret;
-
- ret = svga_rebind_shaders(svga);
- if (ret != PIPE_OK)
- return ret;
-
- /* Rebind stream output targets */
- ret = svga_rebind_stream_output_targets(svga);
- if (ret != PIPE_OK)
- return ret;
-
- /* No need to explicitly rebind index buffer and vertex buffers here.
- * Even if the same index buffer or vertex buffers are referenced for
this
- * draw and we skip emitting the redundant set command, we will still
- * reference the associated resources.
- */
- }
-
- ret = validate_sampler_resources(svga);
- if (ret != PIPE_OK)
- return ret;
-
- ret = validate_constant_buffers(svga);
- if (ret != PIPE_OK)
- return ret;
/* Get handle for each referenced vertex buffer */
for (i = 0; i < vbuf_count; i++) {
- struct svga_buffer *sbuf =
svga_buffer(hwtnl->cmd.vbufs[i].buffer.resource);
+ struct svga_buffer *sbuf =
+ svga_buffer(hwtnl->cmd.vbufs[i].buffer.resource);
if (sbuf) {
vbuffer_handles[i] = svga_buffer_handle(svga, >b.b,
@@ -584,25 +547,11 @@ draw_vgpu10(struct svga_hwtnl *hwtnl,
vbuffer_handles[i] = NULL;
}
- /* Get handle for the index buffer */
- if (ib) {
- struct svga_buffer *sbuf = svga_buffer(ib);
-
- ib_handle = svga_buffer_handle(svga, ib, PIPE_BIND_INDEX_BUFFER);
- if (!ib_handle)
- return PIPE_ERROR_OUT_OF_MEMORY;
-
- assert(sbuf->key.flags & SVGA3D_SURFACE_BIND_INDEX_BUFFER);
- (void) sbuf; /* silence unused var warning */
- }
- else {
- ib_handle = NULL;
- }
-
/* setup vertex attribute input layout */
if (svga->state.hw_draw.layout_id != hwtnl->cmd.vdecl_layout_id) {
- ret = SVGA3D_vgpu10_SetInputLayout(svga->swc,
- hwtnl->cmd.vdecl_layout_id);
+ enum pipe_error ret =
+ SVGA3D_vgpu10_SetInputLayout(svga->swc,
+ hwtnl->cmd.vdecl_layout_id);
if (ret != PIPE_OK)
return ret;
@@ -658,14 +607,13 @@ draw_vgpu10(struct svga_hwtnl *hwtnl,
* corresponding entries in the device's vertex buffer list.
*/
for (i = 0; i < num_vbuffers; i++) {
- boolean emit;
-
- emit = vertex_buffers_equal(1,
- _attrs[i],
- [i],
-
>state.hw_draw.vbuffer_attrs[i],
- >state.hw_draw.vbuffers[i]);
-
+ boolean emit =
+ vertex_buffers_equal(1,
+ _attrs[i],
+ [i],
+ >state.hw_draw.vbuffer_attrs[i],
+ >state.hw_draw.vbuffers[i]);
+
if (!emit && i == num_vbuffers-1) {
/* Include the last vertex buffer in the next emit
* if it is different.
@@ -681,10 +629,11 @@ draw_vgpu10(struct svga_hwtnl *hwtnl,
* In this case, there is nothing to send yet.
*/
if (numVBuf) {
-
