Dnia poniedziałek, 29 czerwca 2009 o 19:33:38 Roland Scheidegger napisał(a):
> On 29.06.2009 19:09, Maciej Cencora wrote:
> > Dnia poniedziałek, 29 czerwca 2009 o 17:52:30 Roland Scheidegger
napisał(a):
> >> On 27.06.2009 23:57, Maciej Cencora wrote:
> >>> Hi,
> >>>
> >>> while playing with r300 driver I've stumbled upon a problem with
> >>> splitting vertexes.
> >>>
> >>> Let's say we get rendering operation where number of indexes in index
> >>> buffer is 80000 and max_index is 20000. We are calling vbo_split_prims
> >>> because number of indexes exceeds hw limit.
> >>> In flush_vertex (vbo_split_inplace.c) function the split->ib is not
> >>> null, so the max_index (20000) won't be changed. In the end the
> >>> draw_prims functions will be called with inappropriate max_index
> >>> number.
> >>>
> >>> I'm seeing this behaviour with UT2004 demo on current r300 driver.
> >>>
> >>> I think the solution would be to always calculate min/max_index numbers
> >>> just like in the !split->ib path but I want to be sure before I commit
> >>> the patch.
> >>>
> >>> Any comments?
> >>
> >> Apart from this problem, I think the limits in the r300 driver set are
> >> maybe not really hw limits. I'm not sure why max_verts is limited at all
> >> (though maybe limited by buffer size?), and max_indices could be bumped
> >> at least for r500. (I always considered it odd that even r200 could
> >> accept 23 bits worth of indices for the INDX_BUFFER command but only 16
> >> bit number of amount of vertices in vertex fetch control, and this
> >> finally seems fixed in r500 - 24 bits possible with
> >> VAP_ALT_NUM_VERTICES.)
> >
> > On <= r300 we are limited by VAP_VF_CNLT_.NUM_VERTICES field size (16
> > bit) for both indices and vertices list. I tried using
> > VAP_ALT_NUM_VERTICES reg on r500 by programming it right before
> > 3D_DRAW_VBUF2 packet, but it always ended in GPU hang. John Bridgman was
> > going to try to dig out some info about it, but no luck so far.
>
> I don't see why that NUM_VERTICES field limits max_verts. This is only
> the number of vertices the chip fetches after all, and it shouldn't
> matter how many vertices are in the buffer.
> BTW there's also a comment in the code that rebase should be done if
> there's more than 8192 / 16384 indices per primitive. I believe though
> the docs are wrong wrt this as it doesn't really make sense as far as I
> can see (says 8192 / 16384 max as per max buffer size, but max buffer
> size is 23bit number of dwords).
>
Does attached patch look sane to you?
Maciej Cencora
From 275d38611a4d506be48514f76d5223f908c4d794 Mon Sep 17 00:00:00 2001
From: Maciej Cencora <m.cenc...@gmail.com>
Date: Mon, 29 Jun 2009 19:50:39 +0200
Subject: [PATCH] r300: fix vertex limits
- don't limit vertex count if we are using indices
- max indices count is 65535 not 65536
- remove some comments that don't apply anymore
- remove unreachable code
---
src/mesa/drivers/dri/r300/r300_draw.c | 10 ++++++----
src/mesa/drivers/dri/r300/r300_render.c | 19 +------------------
2 files changed, 7 insertions(+), 22 deletions(-)
diff --git a/src/mesa/drivers/dri/r300/r300_draw.c b/src/mesa/drivers/dri/r300/r300_draw.c
index 92bb0ee..20fe8db 100644
--- a/src/mesa/drivers/dri/r300/r300_draw.c
+++ b/src/mesa/drivers/dri/r300/r300_draw.c
@@ -442,8 +442,6 @@ static GLboolean r300TryDrawPrims(GLcontext *ctx,
return GL_TRUE;
}
-/* TODO: rebase if number of indices in any of primitives is > 8192 for 32bit indices or 16384 for 16bit indices */
-
static void r300DrawPrims(GLcontext *ctx,
const struct gl_client_array *arrays[],
const struct _mesa_prim *prim,
@@ -455,7 +453,11 @@ static void r300DrawPrims(GLcontext *ctx,
struct split_limits limits;
GLboolean retval;
- limits.max_verts = 65535;
+ if (ib)
+ limits.max_verts = 0xffffffff;
+ else
+ limits.max_verts = 65535;
+
limits.max_indices = 65535;
limits.max_vb_size = 1024*1024;
@@ -463,7 +465,7 @@ static void r300DrawPrims(GLcontext *ctx,
vbo_rebase_prims( ctx, arrays, prim, nr_prims, ib, min_index, max_index, r300DrawPrims );
return;
}
- if ((ib && ib->count > 65536)) {
+ if ((ib && ib->count > 65535)) {
vbo_split_prims (ctx, arrays, prim, nr_prims, ib, min_index, max_index, r300DrawPrims, &limits);
return;
}
diff --git a/src/mesa/drivers/dri/r300/r300_render.c b/src/mesa/drivers/dri/r300/r300_render.c
index bf50b06..36c5ca7 100644
--- a/src/mesa/drivers/dri/r300/r300_render.c
+++ b/src/mesa/drivers/dri/r300/r300_render.c
@@ -359,31 +359,14 @@ void r300RunRenderPrimitive(GLcontext * ctx, int start, int end, int prim)
if (type < 0 || num_verts <= 0)
return;
- /* Make space for at least 64 dwords.
+ /* Make space for at least 128 dwords.
* This is supposed to ensure that we can get all rendering
* commands into a single command buffer.
*/
rcommonEnsureCmdBufSpace(&rmesa->radeon, 128, __FUNCTION__);
if (rmesa->ind_buf.ptr) {
- if (num_verts > 65535) {
- /* not implemented yet */
- WARN_ONCE("Too many elts\n");
- return;
- }
- /* Note: The following is incorrect, but it's the best I can do
- * without a major refactoring of how DMA memory is handled.
- * The problem: Ensuring that both vertex arrays *and* index
- * arrays are at the right position, and then ensuring that
- * the LOAD_VBPNTR, DRAW_INDX and INDX_BUFFER packets are emitted
- * at once.
- *
- * So why is the following incorrect? Well, it seems like
- * allocating the index array might actually evict the vertex
- * arrays. *sigh*
- */
r300EmitElts(ctx, num_verts);
- /* don't pass start if we are split up */
r300EmitAOS(rmesa, rmesa->radeon.tcl.aos_count, 0);
if (rmesa->radeon.radeonScreen->kernel_mm) {
BEGIN_BATCH_NO_AUTOSTATE(2);
--
1.6.0.4
------------------------------------------------------------------------------
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
