What is templated DRM code?
It was first discussed in a email about what Gareth had done to bring up the mach64 kernel module.
Not wanting to simply copy-and-paste another version of drv.[ch], context.c, _bufs.s and so on, Gareth did some refactoring along the lines of what him and Rik Faith had discussed a long time ago.
This is very much along the lines of a lot of Mesa code, where there exists a template header file that can be customized with a few defines. At the time, it was done drv.c and context.c, creating drivertmp.h_ and contexttmp.h_ that could be used to build up the core module.
An inspection of mach64drv.c_ on the mach64-0-0-1-branch reveals the following code:
#define DRIVER_AUTHOR "Gareth Hughes"
#define DRIVER_NAME "mach64"
#define DRIVER_DESC "DRM module for the ATI Rage Pro"
#define DRIVER_DATE "20001203"
#define DRIVER_MAJOR 1
#define DRIVER_MINOR 0
#define DRIVER_PATCHLEVEL 0
static drm_ioctl_desc_t mach64_ioctls[] = {
[DRM_IOCTL_NR(DRM_IOCTL_VERSION)] = { mach64_version, 0, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_GET_UNIQUE)] = { drm_getunique, 0, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_GET_MAGIC)] = { drm_getmagic, 0, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_IRQ_BUSID)] = { drm_irq_busid, 0, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_SET_UNIQUE)] = { drm_setunique, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_BLOCK)] = { drm_block, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_UNBLOCK)] = { drm_unblock, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_AUTH_MAGIC)] = { drm_authmagic, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_ADD_MAP)] = { drm_addmap, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_ADD_BUFS)] = { drm_addbufs, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_MARK_BUFS)] = { drm_markbufs, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_INFO_BUFS)] = { drm_infobufs, 1, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_MAP_BUFS)] = { drm_mapbufs, 1, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_FREE_BUFS)] = { drm_freebufs, 1, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_ADD_CTX)] = { mach64_addctx, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_RM_CTX)] = { mach64_rmctx, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_MOD_CTX)] = { mach64_modctx, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_GET_CTX)] = { mach64_getctx, 1, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_SWITCH_CTX)] = { mach64_switchctx, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_NEW_CTX)] = { mach64_newctx, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_RES_CTX)] = { mach64_resctx, 1, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_ADD_DRAW)] = { drm_adddraw, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_RM_DRAW)] = { drm_rmdraw, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_LOCK)] = { mach64_lock, 1, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_UNLOCK)] = { mach64_unlock, 1, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_FINISH)] = { drm_finish, 1, 0 },
#if defined(CONFIG_AGP) || defined(CONFIG_AGP_MODULE)
[DRM_IOCTL_NR(DRM_IOCTL_AGP_ACQUIRE)] = { drm_agp_acquire, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_AGP_RELEASE)] = { drm_agp_release, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_AGP_ENABLE)] = { drm_agp_enable, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_AGP_INFO)] = { drm_agp_info, 1, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_AGP_ALLOC)] = { drm_agp_alloc, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_AGP_FREE)] = { drm_agp_free, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_AGP_BIND)] = { drm_agp_bind, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_AGP_UNBIND)] = { drm_agp_unbind, 1, 1 },
#endif
[DRM_IOCTL_NR(DRM_IOCTL_MACH64_INIT)] = { mach64_dma_init, 1, 1 },
[DRM_IOCTL_NR(DRM_IOCTL_MACH64_CLEAR)] = { mach64_dma_clear, 1, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_MACH64_SWAP)] = { mach64_dma_swap, 1, 0 },
[DRM_IOCTL_NR(DRM_IOCTL_MACH64_IDLE)] = { mach64_dma_idle, 1, 0 },
};
#define DRIVER_IOCTL_COUNT DRM_ARRAY_SIZE( mach64_ioctls )
#define HAVE_CTX_BITMAP 1
#define TAG(x) mach64_##x
#include "driver_tmp.h"
And that's all you need. A trivial amount of code is needed for the context handling:
#define __NO_VERSION__
#include "drmP.h"
#include "mach64_drv.h"
#define TAG(x) mach64_##x
#include "context_tmp.h"
And as far as I can tell, the only thing that's keeping this out of mach64drv.c_ is the NO_VERSION, which is a 2.2 thing and is not used in 2.4 (right?).
To enable all the context bitmap code, we see the
#define HAVE_CTX_BITMAP 1
- To enable things like AGP, MTRR's and DMA management, the author simply needs to define the correct symbols. With less than five minutes of mach64-specific coding, I had a full kernel module that would do everything a basic driver requires -- enough to bring up a software-fallback driver. The above code is all that is needed for the tdfx driver, with appropriate name changes. Indeed, any card that doesn't do kernel-based DMA can have a fully functional DRM module with the above code. DMA-based drivers will need more, of course. The plan is to extend this to basic DMA setup and buffer management, so that the creation of PCI or AGP DMA buffers, installation of IRQs and so on is as trivial as this. What will then be left is the hardware-specific parts of the DRM module that deal with actually programming the card to do things, such as setting state for rendering or kicking off DMA buffers. That is, the interesting stuff.
A couple of points:
- Why was it done like this, and not with C++ features like virtual functions (i.e. why don't I do it in C++)? Because it's the Linux kernel, dammit! No offense to any C++ fan who may be reading this :-) Besides, a lot of the initialization is order-dependent, so inserting or removing blocks of code with #defines is a nice way to achieve the desired result, at least in this situation.
- Much of the core DRM code (like bufs.c, context.c and dma.c) will essentially move into these template headers. I feel that this is a better way to handle the common code. Take context.c as a trivial example -- the i810, mga, tdfx, r128 and mach64 drivers have exactly the same code, with name changes. Take bufs.c as a slightly more interesting example -- some drivers map only AGP buffers, some do both AGP and PCI, some map differently depending on their DMA queue management and so on. Again, rather than cutting and pasting the code from drm_addbufs into my driver, removing the sections I don't need and leaving it at that, I think keeping the core functionality in bufstmp.h_ and allowing this to be customized at compile time is a cleaner and more maintainable solution.
This it has the possibility to make keeping the other OS's code up to date a lot easier.
The current mach64 branch is only using one template in the driver.
Check out the r128 driver from the trunk, for a good example. Notice there are files in there such as r128tritmp.h_. This is a template that gets included in r128tris.c_. What it does basically is consolidate code that is largely reproduced over several functions, so that you set a few macros. For example:
#define IND (R128_TWOSIDE_BIT)
#define TAG(x) x##_twoside
followed by
#include "r128_tritmp.h"
Notice the inline function's name defined in r128tritmp.h_ is the result of the TAG macro, as well the function's content is dependent on what IND value is defined. So essentially the inline function is a template for various functions that have a bit in common. That way you consolidate common code and keep things consistent.
Look at e.g. xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/r128.h though. That's the template architecture at its beauty. Most of the code is shared between the drivers, customized with a few defines. Compare that to the duplication and inconsistency before.