Re: New DRM driver model - gets rid of DRM() macros!

[Christoph Hellwig]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/