Messages in this thread | | | Date | Wed, 29 Sep 2004 13:37:59 +0100 | From | Christoph Hellwig <> | Subject | Re: New DRM driver model - gets rid of DRM() macros! |
| |
On Tue, Sep 28, 2004 at 11:54:35AM -0400, Jon Smirl wrote: > I've checked two new directories into DRM CVS for Linux 2.6 - > linux-core, shared-core. This code implements a new model for DRM > where DRM is split into a core piece and personality modules that > share the core. The major reason for doing this is that it allows me > to remove all of the DRM() macros; something that is causing lot's of > complaints from the Linux kernel people.
I gave it a quick look and it looks great so far.
Some ideas that would be nice improvements still (not that some may be inherited from the old drm code, I just looked at the CVS checkout):
- once we have Alan's idea of the graphics core implemented drm_init() should go awaw - drm_probe (and it's call to drm_fill_in_dev) looks a little fishy, what about doing the full ->probe callback in each driver where it can do basic hw setup, dealing with pci and calls back into the drm core for minor number allocation and common structure allocations. This would get rid of the ->preinit and ->postinit hooks. - isn't drm_order just a copy of get_order()? - any chance to use proper kernel-doc comments instead of the bastdized and hard to read version you have currently? - the coding style is a little strange, like spurious whitespaces inside braces, maybe you could run it through scripts/Lindent - care to use linux/lists.h instead of opencoded lists, e.g. in dev->file_last/dev->file_first or dev->vmalist - drm_flush is a noop. a NULL ->flush does the same thing, just easier - dito or ->poll - dito for ->read - why do you use DRM_COPY_FROM_USER_IOCTL in Linux-specific code? - drm__mem_info should be converted to fs/seq_file.c functions - dito for functions in drm_proc.c
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |