This patch fixes two issues in nv40_miptree_layout. First, pt->width0 is used, which is the size of the whole texture, while width, which is the size of the mipmap level, should be used. Second, the current code does not 64-byte align the pitch of swizzled textures. However, on my NV40 this causes a pgraph error regarding the pitch register (and sometimes a system lockup too), which is fixed by this patch. I'm not sure how small mipmaps could have worked with the previous code. Also the offset code below may need some review. And furthermore, wide_pitch is set for any kind of texture usage, so maybe it should be made unconditional (what's the point of allocating a texture that the GPU can't use in any way?). diff --git a/src/gallium/drivers/nv40/nv40_miptree.c b/src/gallium/drivers/nv40/nv40_miptree.c index b974e68..9f54187 100644 --- a/src/gallium/drivers/nv40/nv40_miptree.c +++ b/src/gallium/drivers/nv40/nv40_miptree.c @@ -31,8 +31,8 @@ nv40_miptree_layout(struct nv40_miptree *mt) } for (l = 0; l <= pt->last_level; l++) { - if (wide_pitch && (pt->tex_usage & NOUVEAU_TEXTURE_USAGE_LINEAR)) - mt->level[l].pitch = align(util_format_get_stride(pt->format, pt->width0), 64); + if (wide_pitch) + mt->level[l].pitch = align(util_format_get_stride(pt->format, width), 64); else mt->level[l].pitch = util_format_get_stride(pt->format, width);
On Fri, Dec 25, 2009 at 7:59 PM, Luca Barbieri <luca at luca-barbieri.com> wrote:> This patch fixes two issues in nv40_miptree_layout. > > First, pt->width0 is used, which is the size of the whole texture, > while width, which is the size of the mipmap level, should be used. > > Second, the current code does not 64-byte align the pitch of swizzled > textures. However, on my NV40 this causes a pgraph error regarding the > pitch register (and sometimes a system lockup too), which is fixed by > this patch. > I'm not sure how small mipmaps could have worked with the previous code. > > Also the offset code below may need some review. > And furthermore, wide_pitch is set for any kind of texture usage, so > maybe it should be made unconditional (what's the point of allocating > a texture that the GPU can't use in any way?). > > diff --git a/src/gallium/drivers/nv40/nv40_miptree.c > b/src/gallium/drivers/nv40/nv40_miptree.c > index b974e68..9f54187 100644 > --- a/src/gallium/drivers/nv40/nv40_miptree.c > +++ b/src/gallium/drivers/nv40/nv40_miptree.c > @@ -31,8 +31,8 @@ nv40_miptree_layout(struct nv40_miptree *mt) > ? ? ? ?} > > ? ? ? ?for (l = 0; l <= pt->last_level; l++) { > - ? ? ? ? ? ? ? if (wide_pitch && (pt->tex_usage & NOUVEAU_TEXTURE_USAGE_LINEAR)) > - ? ? ? ? ? ? ? ? ? ? ? mt->level[l].pitch = align(util_format_get_stride(pt->format, > pt->width0), 64); > + ? ? ? ? ? ? ? if (wide_pitch) > + ? ? ? ? ? ? ? ? ? ? ? mt->level[l].pitch = align(util_format_get_stride(pt->format, width), 64); > ? ? ? ? ? ? ? ?else > ? ? ? ? ? ? ? ? ? ? ? ?mt->level[l].pitch = util_format_get_stride(pt->format, width);Hi, Width0 is actually the correct value as far as I know. All mip levels of a linear texture are padded out to the pitch of the base level so they can be used by the 3D engine. I haven't run into the 64-byte pitch error for swizzled textures myself, if you can reproduce it and have a patch for it it would be appreciated. The wide_pitch flag is set for textures the 3D engine will sample from or render to, but for temporary textures that are used to do swizzled texture transfers the 2D engine doesn't care (or will complain, I don't remember), so it saves a bit of memory.
You are right. The patch is wrong. Both changes fix my program, but do break OpenGL (e.g. redbook/mipmap). I managed to reproduce the problem with perf/genmipmap. When run, it causes several instances of one of these 3 errors (using swizzled textures): [12949.125732] [drm] nouveau 0000:01:00.0: PGRAPH_ERROR - nSource: DATA_ERROR, nStatus: BAD_ARGUMENT [12949.125738] [drm] nouveau 0000:01:00.0: PGRAPH_ERROR - Ch 3/7 Class 0x4097 Mthd 0x020c Data 0x00000000:0x00000008 [12949.214750] [drm] nouveau 0000:01:00.0: PGRAPH_ERROR - nSource: DATA_ERROR, nStatus: BAD_ARGUMENT [12949.214757] [drm] nouveau 0000:01:00.0: PGRAPH_ERROR - Ch 3/7 Class 0x4097 Mthd 0x020c Data 0x00000000:0x00000010 [12951.752081] [drm] nouveau 0000:01:00.0: PGRAPH_ERROR - nSource: DATA_ERROR, nStatus: BAD_ARGUMENT [12951.752088] [drm] nouveau 0000:01:00.0: PGRAPH_ERROR - Ch 3/7 Class 0x4097 Mthd 0x020c Data 0x00000000:0x00000020 It seems they are due to PGRAPH not liking an 8/16/32 pitch. In my program I got these as well and narrowed it down to doing mipmap generation on the small levels that have pitch set that way. This patch does make them go away but breaks progs/mipmap (both changes are wrong). Apparently the miptree layout is correct, but the lowest mipmap levels cannot be rendered to with the current code (that however tries to, resulting in the errors), possibly due to an hardware limitation. I guess a possible solution could be to modify nv40_miptree_surface_new to allocate temporary surfaces for pitch <64 levels (i.e. 8x, 4x and 1x mipmap levels for RGBA) and then do a copy with the 2D engine in nv40_miptree_surface_del. Alternatively, for square Pot textures, it may be possible to map the 8x8, 4x4, 2x2 and 1x1 mipmaps as a single 16x16 pitch 64 swizzled texture in which they should appear as rectangles, and then restrict drawing to the rectangle by adjusting the viewport (and finding a way to make bypass work too). Not sure if this works and whether it can be generalized to non-square-pot textures. How does the nVidia driver implement glGenerateMipmap?
Apparently Analagous Threads
- [PATCH] nv04-nv40: Rewrite and unify miptree and transfer code
- [PATCH] nv04-nv40: Rewrite and unify miptree and transfer code (v2)
- [PATCH] gallium/nouveau: use pre-calculated stride for resource_get_handle
- [PATCH] Print NOUVEAU_NO_SWIZZLE and NOUVEAU_NO_TRANSFER messages only once
- [Bug 20130] New: GT200: nv50 chipset 0xa0 lockup