Today I am sending out the last of the patches that I intend to commit to the texmem-0-0-1 branch. After these patches are committed and Leif is satisfied with the state of the r128 driver, I will freeze the branch and start the merge process. Hopefully I will be able to start the merge by the end of the week (28-Feb-2003).
There are three patch sets here. I will try to describe the important points of each. They are:
1. Rework of x86 & SSE assembly templates for R100 & R200
code-gen.
2. Changes to GLX extension support in libGL.so.
3. Support for vertical-retrace related support in MGA, R100,
& R200 drivers.
========================= x86 & SSE Code-Gen Rework =========================
Quite some time ago I noticed that there were several sets of functions in radeon_vtxtmp_x86.S that did exactly the same thing. Specifically, _x86_Normal3f and _x86_Color3f did the same thing. Both functions moved three floating point values from one location to another. However, the two functions were coded differently!
The first change that I made was use a single function in radeon_vtxtmp_x86.S for both GL functions. After some discussion on dri-devel, these generic functions were named _x86_Attribute2f, _x86_Attribute2fv, _x86_Attribute3f, and _x86_Attribute3fv. The routines that generate functions from the stubs were modified to use the generic routines radeon_makeX86Attribute2f, etc.
The second major change was to create SSE optimized versions of most of the assembly templates. Most of the SSE functions are based on generic versions, like the regular X86 routines.
The third change was applied to the assembly stubs and the C version. The MultiTexCoord[12]f[v] functions would subtract GL_TEXTURE0 from target and mask the result with 0x01. For all valid values of target, the upper 27 bits will be 0x84C0. Therefore, the subtract is unnecessary. The compiler cannot know this, so I removed the subtraction.
A couple of people that have seen this patch didn't care for that change. It is a little ugly. However, there is a comment in the code that explains why the subtract is not there and why the code is valid.
Both the R100 driver and the R200 driver use the same assembly template file. Both drivers also use nearly identical *_vtxfmt_{x86,sse}.c files. In the future it would be nice to optimized versions for PowerPC. I also wrote and tested SSE and X86 versions of radeon_Color4f_ub and radeon_Color4fv_ub. I did not integrate either function. It would take a bit more work to add these into the code generators. If anyone is interested, I can send functions to the list.
Most of this code is pretty generic. It should be possible to move most, if not all, of it to lib/GL/mesa/src/common. No drivers other than radeon and r200 will use it now, but in the future other drivers may.
===================== GLX Extension Support =====================
A long time ago I started looking into adding support to the DRI for some additional GLX extensions. I did this for several reasons.
- As the texmem-0-0-2 branch matures, several extensions such as GLX_SGIX_fbconfig and GLX_SGIX_pbuffer will be added. - There are several other extensions, such as GLX_SGI_make_current_read that can be implemented now with little work. It is desirable to implement these extensions on a per-driver basis. - Each vendor that ships binary-only drivers supports additional GLX extensions. Because of this, these vendors need to ship their own libGL.so. This is not good for anyone.
Some work had already been done to allow drivers to expose additional GLX extensions. This interface was used by the R200 driver to export a couple extensions. The problem with this interface is that it put all the burden of exporting the extension in each driver. It also didn't provide an easy way for multiple drivers to share common code. Because of this I created a new interface that works much like the _mesa_enable_extension interface. This code lives in lib/GL/glx/glxextensions.[ch].
There are five functions in this interface, and each is described below. Drivers should not directly call these functions. Instead, the driver should call glXGetProcAddress to get a pointer to each function. The driver cannot assume that every libGL.so will export these functions. Drivers should use the new driCompareGLXAPIVersion function to determine if the libGL.so supports the 20021207 (yes, that's how long I've had this patch floating around) GLX API.
The functions __glXEnableExtension and __glXDisableExtension take a name of an extension and either enable or disable the extension. This mirrors the functionality of _mesa_enable_extension and _mesa_disable_extension. The function __glXAddExtension is used to add a new extension string and default state to the internal extension table. The state of the extension can later be changed via __glXEnableExtension or __glXDisableExtension.
The function __glXExtensionBitIsEnabled is used to determine if one of the compiled-in extensions is enabled. Each extension has a bit defined in glxextensions.h. If a driver (or the libGL.so GLX code) wants to determine if GLX_SGIX_pbuffer is enabled, it would make this call:
pbuffer_enabled = __glXExtensionBitIsEnabled(SGIX_pbuffer_bit);
The final function is __glXGetClientExtensions. This function returns the string of all the client-side enabled GLX extensions.
IMPORTANT!
I didn't change it in this patch, but I believe that the current implementation of glXQueryExtensionsString is wrong. I haven't been able to find any "official" GLX documentation to confirm this, but the "GLX Extension Availability" section of http://oss.sgi.com/projects/ogl-sample/registry/doc/rules.html says:
"glXQueryExtensionsString...intersects the returned string with
the set of extensions it can support and then appends any
client-side only extensions to the list."Our implementation of does not do this. Depending on what the consensus is, I believe an additional function __glXGetClientOnlyExtensions should be added that returns the list of client-side extensions, and __glXGetClientExtensions should be modified to not return those extension strings. glXQueryExtensionsString should then be modified to append the string returned by __glXGetClientOnlyExtensions to the extension string. The catch is that some extensions are only supported for direct contexts (i.e., GLX_OML_sync_control). Is it possible for
glXQueryExtensionsString to know if the app has a direct context?
The remainder of the changes to the GLX code are to support a few similar GLX extensions. In a couple of messages that I posted to the
list earlier, I described these changes.
http://www.mail-archive.com/[EMAIL PROTECTED]/msg07562.html http://www.mail-archive.com/[EMAIL PROTECTED]/msg08469.html
Instead of re-hashing the changes, I'm going to point out a couple of things that should perhaps be done differently. I'd really like for people to look at those previous threads and the code, and send any design / implementation comments to the list.
There are a few values that I put in __DRIdrawablePrivateRec (in lib/GL/dri/dri_util.h), that might be better implemented as a function that fills in a small structure. These are swap_count, swap_ust, swap_missed_count, and swap_missed_usage. All drivers that use a synchronous wait-and-swap approach can easily implement this values in global, client-side storage. This makes it very easy to implement the functions in the driver-independent DRI code, and this is the reason I put these values in the __DRIdrawablePrivateRec.
In the future we will want to have drivers that have an asynchronous buffer swap routine. This is required to implement a couple very useful GLX extensions. One way to do this is to have a DRM call that queues a buffer swap. Another way would be to have the DRM send a signal to the client-driver when the vsync-count reaches the correct value for the swap. If the work is done in the kernel, it may not be trivial to directly update these values in the __DRIdrawablePrivateRec.
My thinking is to create a new structure like the following and replace the data fields in the __DRIdrawablePrivateRec with a single function that fills in the structure with the current values.
typedef struct {
/*
** Number of swapBuffers operations that have been *completed*.
*/
unsigned swap_count; /*
** Unadjusted system time of the last buffer swap. This is the time
** when the swap completed, not the time when swapBuffers was
** called.
*/
int64_t swap_ust; /*
** Number of swap operations that occurred after the swap deadline.
** That is if a swap happens more than swap_interval frames after
** the previous swap, it has missed its deadline. If swap_interval
** is 0, then the swap deadline is 1 frame after the previous swap.
*/
unsigned swap_missed_count; /*
** Amount of time used by the last swap that missed its deadline.
** This is calculated as (__glXGetUST() - swap_ust) / (swap_interval
** * time_for_single_vrefresh)). If the actual value of
** swap_interval is 0, then 1 is used instead. If swap_missed_count
** is non-zero, this should be greater-than 1.0.
*/
float swap_missed_usage;
} driSwapCountInfoRec;I would be willing to help IHVs support these changes in any way that I can. Requiring users to install a new libGL.so just sucks. :( There are several roadblocks to removing this requirement, and I would love to help remove them!
======================= Driver Specific Changes =======================
I made just a few changes to some of the drivers. I moved the WaitForVBlank code to a common/vblank.[ch]. I did this because the WaitForVBlank code in the radeon, r200, and mga drivers was virtually identical. This also made it easier to implement the various new wait policies for the new GLX extensions.
The patch for mga_irq.c is not intended to be committed for the final merge. There was also some discussion about this earlier.
http://www.mail-archive.com/[EMAIL PROTECTED]/msg08442.html
glx_and_codegen_patch.tar.bz2
Description: Binary data
