Bug 768 - r128 t_vertex.c conversion
Summary: r128 t_vertex.c conversion
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/r128 (show other bugs)
Version: unspecified
Hardware: x86 (IA32) FreeBSD
: high enhancement
Assignee: Eric Anholt
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-06-20 13:32 UTC by Eric Anholt
Modified: 2004-10-01 01:34 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
r128-t_vertex.diff (30.48 KB, patch)
2004-06-20 13:33 UTC, Eric Anholt
Details | Splinter Review
r128-t_vertex-3.diff (42.57 KB, patch)
2004-06-21 01:18 UTC, Eric Anholt
Details | Splinter Review
r128-t_vertex-4.diff (44.92 KB, patch)
2004-06-21 22:34 UTC, Eric Anholt
Details | Splinter Review

Description Eric Anholt 2004-06-20 13:32:16 UTC
Attached is a snapshot of my t_vertex.c conversion for r128 to be tested.  It
would be nice if we could use t_dd_triemit.h, but there's the endianness
difference in there.  I managed to hang the card both with and without the
changes iirc, so r128 could stand some good testing.  I only tested with some
real basic apps.
Comment 1 Eric Anholt 2004-06-20 13:33:16 UTC
Created attachment 387 [details] [review]
r128-t_vertex.diff
Comment 2 Paul Mackerras 2004-06-20 15:12:48 UTC
I'm just getting to know the r128 code, but this bit of the patch
looks a bit wrong:

+#define CTX_ARG sisContextPtr smesa
+#define CTX_ARG2 smesa
+#define GET_VERTEX_DWORDS() smesa->vertex_size
+#define ALLOC_VERTS( n, size ) sisAllocDmaLow( smesa, n * size * sizeof(int) )
+#undef LOCAL_VARS
+#define LOCAL_VARS                                             \
+   sisContextPtr smesa = SIS_CONTEXT(ctx);                     \
+   const char *vertptr = smesa->verts;
+#define VERT(x) (sisVertex *)(vertptr + (x * vertsize * sizeof(int)))
+#define VERTEX sisVertex 
+#undef TAG
+#define TAG(x) sis_##x

Shouldn't you s/sis/r128/ in here?  If not then I (for one :) would appreciate
a big comment telling me why that code uses sis rather than r128.
Comment 3 Eric Anholt 2004-06-21 01:08:45 UTC
Okay, so ajax was bugging me to put my diff up like I said I would, and I
thought I hadn't messed things up since the point that they were basically
working, so I just test-compiled again and threw it up here.  Here's a new
version, with t_dd_triemit.h fixed up for r128 and being actually used, the fix
for bug 755, and some attempts at fixing something new I noticed: tuxracer's
colors are wacky.

Other issue I noticed: Lack of SpanRenderStart/SpanRenderFinish around the
fallback primitives.

t_dd_triemit.h notes: Do other cards need the CPU_TO_LE32?  If not, why is r128
special?  Should it be a required define (to 0 or 1, like other HAVE_*)?

Need to work on other code for a while, though.  Review welcome, but this is
quite unfinished I guess.
Comment 4 Eric Anholt 2004-06-21 01:18:31 UTC
Created attachment 395 [details] [review]
r128-t_vertex-3.diff
Comment 5 Eric Anholt 2004-06-21 22:34:47 UTC
Created attachment 403 [details] [review]
r128-t_vertex-4.diff

New version fixing diffuse color emitting, emitting texcoord correctly for st1
&& !st0, using EMIT_PAD for fog/spec when unnecessary, and using better
color/spec offsets I think.

Still explodes if you hit a glut menu in multiarb or texcyl, though.
Comment 6 Eric Anholt 2004-07-15 21:44:33 UTC
Use a slightly more permanent email address, and the one I use in changelogs.
Comment 7 Eric Anholt 2004-10-01 18:34:44 UTC
Fixed the bug, and committed.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.