Bug 7994

Summary: r128 incorrectly renders some meshes, and sometimes causes lockups
Product: Mesa Reporter: Slava Gorbunov <nococu>
Component: Drivers/DRI/r128Assignee: Xorg Project Team <xorg-team>
Status: RESOLVED FIXED QA Contact:
Severity: major    
Priority: high CC: alexdeucher, chrissalch, slava, sustmidown
Version: unspecified   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: Screenshot of Blender window
Another Blender screenshot, here 3 vertices (out of 8) are shifted.

Description Slava Gorbunov 2006-08-25 03:18:11 UTC
For example, screensaver "superquadrics" (from xscreensaver) renders
incorrectly: some vertices are shifted to the left edge of the window.
Continuous running of screensaver causes lockup (or only sometimes causes? I
have observed this at least once, but I don't want to test this right now).
Replacing lib/modules/dri/r128_dri.so with the same file from Xorg-6.8.1 fixes
this problem.

The problem also exists in xorg-6.9.0

Output while running screensaver with broken r128_dri.so:
$ LIBGL_DEBUG=verbose /usr/libexec/xscreensaver/superquadrics 
libGL: XF86DRIGetClientDriverName: 4.0.1 r128 (screen 0)
libGL: OpenDriver: trying /usr/X11R6/lib/modules/dri/r128_dri.so
drmOpenByBusid: Searching for BusID pci:0000:01:00.0
drmOpenDevice: node name is /dev/dri/card0
drmOpenDevice: open result is 4, (OK)
drmOpenByBusid: drmOpenMinor returns 4
drmOpenByBusid: drmGetBusid reports pci:0000:01:00.0
libGL warning: 3D driver claims to not support visual 0x24
libGL warning: 3D driver claims to not support visual 0x28
libGL warning: 3D driver claims to not support visual 0x2c
libGL warning: 3D driver claims to not support visual 0x30
libGL error: 
Can't open configuration file /etc/drirc: No such file or directory.
libGL error: 
Can't open configuration file /home/slava/.drirc: No such file or directory.

Output while running screensaver with r128_dri.so from xorg-6.8.1 (correct
rendering):
$ LIBGL_DEBUG=verbose /usr/libexec/xscreensaver/superquadrics 
libGL: XF86DRIGetClientDriverName: 4.0.1 r128 (screen 0)
libGL: OpenDriver: trying /usr/X11R6/lib/modules/dri/r128_dri.so
drmOpenByBusid: Searching for BusID pci:0000:01:00.0
drmOpenDevice: node name is /dev/dri/card0
drmOpenDevice: open result is 4, (OK)
drmOpenByBusid: drmOpenMinor returns 4
drmOpenByBusid: drmGetBusid reports pci:0000:01:00.0
libGL warning: 3D driver claims to not support visual 0x24
libGL warning: 3D driver claims to not support visual 0x28
libGL warning: 3D driver claims to not support visual 0x2c
libGL warning: 3D driver claims to not support visual 0x30
libGL error: 
Can't open configuration file /etc/drirc: No such file or directory.
libGL error: 
Can't open configuration file /home/slava/.drirc: No such file or directory.
Comment 1 Alex Deucher 2006-08-26 11:34:00 UTC
this is DRI/Mesa related.
Comment 2 Slava Gorbunov 2006-12-10 07:40:53 UTC
Xorg-7.0. Superquadrics screensaver keeps rendering incorrectly, as described
above. Additional issue: blender-2.42a even doesn't start up, outputting the
following:

$ LIBGL_DEBUG=verbose blender
guessing 'blender' == '/usr/bin/blender'
Compiled with Python version 2.4.
Checking for installed Python... got it!
libGL: XF86DRIGetClientDriverName: 4.0.4 r128 (screen 0)
libGL: OpenDriver: trying /usr/lib/xorg/modules/dri/r128_dri.so
drmOpenByBusid: Searching for BusID pci:0000:01:00.0
drmOpenDevice: node name is /dev/dri/card0
drmOpenDevice: open result is 4, (OK)
drmOpenByBusid: drmOpenMinor returns 4
drmOpenByBusid: drmGetBusid reports pci:0000:01:00.0
libGL error: 
Can't open configuration file /etc/drirc: No such file or directory.
libGL error: 
Can't open configuration file /home/slava/.drirc: No such file or directory.
Error: Rage 128 timed out... exiting

There is also two messages in syslog:

Dec 10 18:35:19 fpfe [drm:r128_cce_depth] *ERROR* r128_cce_depth called without
lock held
Dec 10 18:35:19 fpfe [drm:r128_cce_idle] *ERROR* r128_cce_idle called without
lock held

With r128_dri from xorg-6.8.1 everything works correctly.
Comment 3 Michel Dänzer 2006-12-11 07:12:20 UTC
(In reply to comment #2)
> 
> Dec 10 18:35:19 fpfe [drm:r128_cce_depth] *ERROR* r128_cce_depth called without
> lock held

Not sure about the other problems, but this should be fixed as of Mesa 6.5.1.
Comment 4 Slava Gorbunov 2007-01-24 11:16:54 UTC
Created attachment 8495 [details]
Screenshot of Blender window

(In reply to comment #3)
> (In reply to comment #2)
> > 
> > Dec 10 18:35:19 fpfe [drm:r128_cce_depth] *ERROR* r128_cce_depth called without
> > lock held
> 
> Not sure about the other problems, but this should be fixed as of Mesa 6.5.1.
Yes, this bug is fixed (MESA-6.5.2) (for example, Blender now starts without errors). But shifted vertices are still present. Look at the attached screenshot (it is Blender window, just after starting). You can see that some vertices are 
shifted to the left edge of area.
Comment 5 Slava Gorbunov 2007-01-24 11:19:06 UTC
Created attachment 8496 [details]
Another Blender screenshot, here 3 vertices (out of 8) are shifted.
Comment 6 Slava Gorbunov 2007-01-27 08:01:40 UTC
Today I have accidently caused locking up of the X server. I opened a project in Blender, performed zooming in/out several times, then X server locked up (no response to keyboard/mouse clicking, but mouse cursor moves correctly). Alt-sysrq key combinations work, too (so the whole system appears to keep working normally, only X server is affected). The following message was written to the syslog:

[drm:drm_lock_take] *ERROR* 3 holds heavyweight lock

I am using linux-2.6.19.2 on x86, mesa-6.5.2, xorg-server-1.2.0, xf86-video-ati-6.6.3
Comment 7 Miroslav Šustek 2007-02-07 12:37:37 UTC
(In reply to comment #6)

> [drm:drm_lock_take] *ERROR* 3 holds heavyweight lock

Look at: http://bugs.freedesktop.org/show_bug.cgi?id=9379
Comment 8 Miroslav Šustek 2007-02-07 13:15:16 UTC
(In reply to comment #5)
> Another Blender screenshot, here 3 vertices (out of 8) are shifted.

Yes I can confirm it.
I think it is bug somewhere in r128/dri.

(mesa ver. 6.5.2 (maybe some gentoo patches :-, ))
I found out that this deformation is caused in ´src/mesa/tnl_dd/t_dd_tritmp.h´:
line 484:
    VERT_SET_RGBA( v[0], vbcolor[e0] );

this macro is defined in ´src/mesa/drivers/dri/r128/r128_tris.c´:
line 177:
#define VERT_SET_RGBA( v, c )                                   \
do {                                                            \
   r128_color_t *color = (r128_color_t *)&((v)->ui[coloroffset]);       \
   UNCLAMPED_FLOAT_TO_UBYTE(color->red, (c)[0]);                \
   UNCLAMPED_FLOAT_TO_UBYTE(color->green, (c)[1]);              \
   UNCLAMPED_FLOAT_TO_UBYTE(color->blue, (c)[2]);               \
   UNCLAMPED_FLOAT_TO_UBYTE(color->alpha, (c)[3]);              \
} while (0)

This isn´n anything unusual. But the ´color´ pointer points to same position where the ´x´ coord of vertex ´v´ is stored (v->v.x or v->vt.x). So this colors (unsigned byte) are written to x coordinate (float) and this is the mismatch.
I think the 4th (last one) (3th if counting from zero) vertex of QUAD is rewritten by color data because it thinks that there are only 3 vertices...
If you change the ´coloroffset´ (inc by 4 floats (x,y,z,w)) an another vertex (probably the next) will be damaged.

I spend hours finding where is the mistake, haven´t found it yet.
Maybe someone from DRI-developers could find it... (please :) )
Comment 9 Miroslav Šustek 2007-02-07 15:17:28 UTC
One more hint (what i´ve seen in blender):
  - only vertices of QUADs are ´damaged´
  - only QUADs which are rotated invisible-side-forward are damaged (eg. backside of cube)

I know that r128 is very old HW, but if anybody know (or can fast find out) where is the problem please help. Thanks a lot.
Comment 10 Chris Salch 2007-03-14 21:39:00 UTC
I may have a fix for this.  After a little digging in the r128_tris.c, it would appear that the coloroffset value is being set improperly in lines 607-612:

   /* EMIT_ATTR's must be in order as they tell t_vertex.c how to
    * build up a hardware vertex.
    */
   if (RENDERINPUTS_TEST_RANGE( index_bitset, _TNL_FIRST_TEX, _TNL_LAST_TEX ))
      EMIT_ATTR( _TNL_ATTRIB_POS, EMIT_4F_VIEWPORT, R128_CCE_VC_FRMT_RHW, 16 );
   else
      EMIT_ATTR( _TNL_ATTRIB_POS, EMIT_3F_VIEWPORT, 0, 12 );

   rmesa->coloroffset = offset;

The macro EMIT_ATTR increments offset by the last argument.  If I'm readin this right, this value is either 16 bytes or 12 bytes depending on wether the driver is using (x,y,z,w) or (x,y,z).  In t_dd_vertex.h the r128_color_t->ui is defined as an GLuint array, which should step in four byte intervals.  That would mean that the coloroffset value would be pushed out 64 or 48 bytes, depending on the type of coordinates used, rather than the 12 or 16 implied by the above code.

So, a possible patch is to replace the line:

   rmesa->coloroffset = offset;

with
   rmesa->coloroffset = offset;
   rmesa->coloroffset >>= 2; 

which the code dealing with specoffset appears to use latter.
Some cursory tesitng on my part, (blender, glxgears, the antInspect screen saver) appears to prove this out.  Unfortunately, I don't know enough about the internals of the r128 card or the dri to definatively say if what I've got here is actaully the problem or simply a fluke and this change clobbers something.

Can anyone check verify what I'm seeing?


(In reply to comment #8)
> (In reply to comment #5)
> > Another Blender screenshot, here 3 vertices (out of 8) are shifted.
> 
> Yes I can confirm it.
> I think it is bug somewhere in r128/dri.
> 
> (mesa ver. 6.5.2 (maybe some gentoo patches :-, ))
> I found out that this deformation is caused in
> ´src/mesa/tnl_dd/t_dd_tritmp.h´:
> line 484:
>     VERT_SET_RGBA( v[0], vbcolor[e0] );
> 
> this macro is defined in ´src/mesa/drivers/dri/r128/r128_tris.c´:
> line 177:
> #define VERT_SET_RGBA( v, c )                                   \
> do {                                                            \
>    r128_color_t *color = (r128_color_t *)&((v)->ui[coloroffset]);       \
>    UNCLAMPED_FLOAT_TO_UBYTE(color->red, (c)[0]);                \
>    UNCLAMPED_FLOAT_TO_UBYTE(color->green, (c)[1]);              \
>    UNCLAMPED_FLOAT_TO_UBYTE(color->blue, (c)[2]);               \
>    UNCLAMPED_FLOAT_TO_UBYTE(color->alpha, (c)[3]);              \
> } while (0)
> 
> This isn´n anything unusual. But the ´color´ pointer points to same position
> where the ´x´ coord of vertex ´v´ is stored (v->v.x or v->vt.x). So this
> colors (unsigned byte) are written to x coordinate (float) and this is the
> mismatch.
> I think the 4th (last one) (3th if counting from zero) vertex of QUAD is
> rewritten by color data because it thinks that there are only 3 vertices...
> If you change the ´coloroffset´ (inc by 4 floats (x,y,z,w)) an another vertex
> (probably the next) will be damaged.
> 
> I spend hours finding where is the mistake, haven´t found it yet.
> Maybe someone from DRI-developers could find it... (please :) )
> 

Comment 11 Miroslav Šustek 2007-03-20 06:46:13 UTC
Great work.
I think thats it.
I would be changed simply to this:

if (RENDERINPUTS_TEST_RANGE( index_bitset, _TNL_FIRST_TEX, _TNL_LAST_TEX ))
       EMIT_ATTR( _TNL_ATTRIB_POS, EMIT_4F_VIEWPORT, R128_CCE_VC_FRMT_RHW, 4 );
    else
       EMIT_ATTR( _TNL_ATTRIB_POS, EMIT_3F_VIEWPORT, 0, 3 );

I counted byte instead of 4 bytes (GLuint or GLfloat) and the same mistake possibly did author of this (with 16 and 12) code...
It´s ´unvisible´ error :)
Thanks

(In reply to comment #10)
> I may have a fix for this.  After a little digging in the r128_tris.c, it would
> appear that the coloroffset value is being set improperly in lines 607-612:
> 
>    /* EMIT_ATTR's must be in order as they tell t_vertex.c how to
>     * build up a hardware vertex.
>     */
>    if (RENDERINPUTS_TEST_RANGE( index_bitset, _TNL_FIRST_TEX, _TNL_LAST_TEX ))
>       EMIT_ATTR( _TNL_ATTRIB_POS, EMIT_4F_VIEWPORT, R128_CCE_VC_FRMT_RHW, 16 );
>    else
>       EMIT_ATTR( _TNL_ATTRIB_POS, EMIT_3F_VIEWPORT, 0, 12 );
> 
>    rmesa->coloroffset = offset;
> 
> The macro EMIT_ATTR increments offset by the last argument.  If I'm readin this
> right, this value is either 16 bytes or 12 bytes depending on wether the driver
> is using (x,y,z,w) or (x,y,z).  In t_dd_vertex.h the r128_color_t->ui is
> defined as an GLuint array, which should step in four byte intervals.  That
> would mean that the coloroffset value would be pushed out 64 or 48 bytes,
> depending on the type of coordinates used, rather than the 12 or 16 implied by
> the above code.
> 
> So, a possible patch is to replace the line:
> 
>    rmesa->coloroffset = offset;
> 
> with
>    rmesa->coloroffset = offset;
>    rmesa->coloroffset >>= 2; 
> 
> which the code dealing with specoffset appears to use latter.
> Some cursory tesitng on my part, (blender, glxgears, the antInspect screen
> saver) appears to prove this out.  Unfortunately, I don't know enough about the
> internals of the r128 card or the dri to definatively say if what I've got here
> is actaully the problem or simply a fluke and this change clobbers something.
> 
> Can anyone check verify what I'm seeing?
Comment 12 Alex Deucher 2007-03-26 19:42:53 UTC
committed:
25f21b5331d27225b1f6b7aaf2c9bf3f32764d91

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.