Re: DRM code reorganization

[Keith Whitwell]

Ian Romanick wrote:

> I think this is the right place to start.  A couple of these look easier 
> to get rid of than others.  __HAVE_MTRR and __HAVE_AGP are enabled in 
> every driver except ffb.  It should be easy enough to get rid of them. 
> It looks like __HAVE_RELEASE, __HAVE_DMA_READY, __HAVE_DMA_FLUSH, 
> __HAVE_DMA_QUIESCENT, and __HAVE_MULTIPLE_DMA_QUEUES (which looks broken 
> anyway) should also be low-hanging fruit.

We've actually managed to do a fair bit of cleanup already - if you look at 
the gamma driver, there's a lot of stuff in there which used to be shared but 
ifdef'ed out between all the drivers.  The __HAVE_MULTIPLE_DMA_QUEUES macro is 
a remnant of this, but I think you'll break gamma when you try & remove it.

It used to be the case that 50% of the #if hoo-hah was just to try & keep the 
gamma driver working.  I don't know how true this is anymore, though.

> If we get that far, I think the next step would be to replace the 
> DRIVER_* macros with a table of function pointers that would get passed 
> around.  Since I doubt any of those uses are performance critical, that 
> should be fine.
>
> Then we can start looking at data structure refactoring.
>
> >  > >If this kind of abuse wasn't so widespread, abstracting this code
> >  > >out into shared sections and driver specific parts would be a lot
> >  > >simpler. Sadly, this is the tip of the iceberg.
> >  >  > I think it comes down to the fact that the original DRM 
> > developers  > wanted templates.  C doesn't have them, so they did the 
> > "next best" thing.
> >
> > I vaguelly recall the code at one point not looking quite 'so bad',
> > it just grew and grew into this monster.  I'm sure it was done originally
> > with the best of intentions, but it seems someone along the line got
> > a bit carried away.
>
>
> There was a point when a *lot* of the device-dependent code was still in 
> the OS-dependent directories.  This is how the i810 and i830 drivers 
> still are.  I think as more of the code got moved into the 
> OS-independent directory, it got less pleasant to read.

Not a great deal changed as drivers got moved to shared/ -- things like 
copy_from_user() got replaced by DRM_COPY_FROM_USER(), etc, but that's about 
as far as it went.  The template abstractions haven't really changed a great 
deal with the introduction of freebsd support.  If anything, code has been 
simplified by moving the gamma-specific code out of the shared templates and 
into gamma_* files.

Keith


-
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/